Skip to content

Commit

Permalink
Don't leak derivatives versions compat setting
Browse files Browse the repository at this point in the history
When we extend Shrine::Plugins::Derivatives::AttacherMethods with
additional behavior, that gets applied to all other uploaders that have
loaded the derivatives plugin, not just the one that enabled versions
compatibility. So, we instead extend only the attacher class of the
current uploader.
  • Loading branch information
janko committed Jul 6, 2023
1 parent 58117ef commit ffd87e8
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
## HEAD

* `derivatives` – Don't leak `versions_compatibility: true` setting into other uploaders (@janko)

* `remove_attachment` – Fix passing boolean values being broken in Ruby 3.2 (@janko)

* `model` – When duplicating a record, make the duplicated attacher reference the duplicated record (@janko)
Expand Down
8 changes: 4 additions & 4 deletions lib/shrine/plugins/derivatives.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,18 @@ module Derivatives
}.inspect}"
end

def self.load_dependencies(uploader, versions_compatibility: false, **)
def self.load_dependencies(uploader, **)
uploader.plugin :default_url

AttacherMethods.prepend(VersionsCompatibility) if versions_compatibility
end

def self.configure(uploader, log_subscriber: LOG_SUBSCRIBER, **opts)
def self.configure(uploader, log_subscriber: LOG_SUBSCRIBER, versions_compatibility: false, **opts)
uploader.opts[:derivatives] ||= { processors: {}, processor_settings: {}, storage: proc { store_key }, mutex: true }
uploader.opts[:derivatives].merge!(opts)

# instrumentation plugin integration
uploader.subscribe(:derivatives, &log_subscriber) if uploader.respond_to?(:subscribe)

uploader::Attacher.include(VersionsCompatibility) if versions_compatibility
end

module AttachmentMethods
Expand Down
6 changes: 6 additions & 0 deletions test/plugin/derivatives_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1413,6 +1413,12 @@
assert_equal Hash.new, @attacher.derivatives
end
end

it "doesn't affect other uploaders" do
@attacher = attacher { plugin :derivatives }

refute_includes @attacher.class.ancestors, Shrine::Plugins::Derivatives::VersionsCompatibility
end
end

it "can be marshalled without mutex" do
Expand Down

0 comments on commit ffd87e8

Please sign in to comment.