Fix mounter cache duplication on dup #2690
Conversation
duplication should not affect the duplicated objects path
| expect(new_event).not_to be @event | ||
| expect(new_event.image.model).to be new_event | ||
| expect(@event.save).to be_truthy | ||
| expect(@event.image.current_path).to eq public_path("uploads/event/image/#{@event.id}/test.jpeg") |
There was a problem hiding this comment.
Currently, current_path returns "uploads/event/image/test.jpeg" after the second save.
There was a problem hiding this comment.
Also, expect(File.exist?(public_path("uploads/event/image/test.jpeg"))).to be_truthty passes, so the files is saved to this path.
|
@mshibuya |
rajyan
changed the title
lib/carrierwave/orm/activerecord.rb
Outdated
| @@ -43,6 +43,7 @@ def reload(*) | |||
| # Reset cached mounter on record dup | |||
| def initialize_dup(other) | |||
| old_uploaders = _mounter(:"#{column}").uploaders | |||
| @_mounters = @_mounters.dup | |||
There was a problem hiding this comment.
Thank you for the PR! Your fix works, but let me propose a slight modification...
As was discussed in #1962 (comment), this will be executed for every mount_uploader / mount_uploaders, not just once for a ActiveRecord instance. If we move this to CarrierWave::Mount::Extension like
--- a/lib/carrierwave/mount.rb
+++ b/lib/carrierwave/mount.rb
@@ -428,6 +428,11 @@ module CarrierWave
private
+ def initialize_dup(other)
+ @_mounters = @_mounters.dup
+ super
+ end
+
def _mounter(column)
# We cannot memoize in frozen objects :(
return Mounter.build(self, column) if frozen?I think we can resolve that mismatch. What do you think?
There was a problem hiding this comment.
Thanks for your suggestion! This definitely looks like a better solution.
|
Thank you so much 👍 |
Added a failing test for #2689
duplication should not affect the duplicated objects path.
Updated
And now fixes #2689