Skip to content

Commit

Permalink
Fix dup with multiple mounters or not cached
Browse files Browse the repository at this point in the history
- Don't call `each` if `@_mounters` is unitialized or nil. It will be
  nil for calls to later mounters, or if not referenced or saved.
- Don't interpolate `column` when it should be a local variable.
- Don't define global functions in specs. Make them local to the block.
- Aggregate failures in specs.
- Cover more missing scenarios.

[Fixes #1962]
  • Loading branch information
BrianHawley committed Jan 4, 2023
1 parent 9fcd397 commit e6e0628
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 21 deletions.
6 changes: 3 additions & 3 deletions lib/carrierwave/orm/activerecord.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,11 @@ def reload(*)
# Reset cached mounter on record dup
def initialize_dup(other)
old_mounters = @_mounters
old_mounters = @_mounters if instance_variable_defined?(:@_mounters)
@_mounters = nil
super
old_mounters.each do |column, mounter|
_mounter(:#{column}).cache(mounter.uploaders)
old_mounters&.each do |column, mounter|
_mounter(column).cache(mounter.uploaders)
end
end
RUBY
Expand Down
78 changes: 60 additions & 18 deletions spec/orm/activerecord_spec.rb
Original file line number Diff line number Diff line change
@@ -1,26 +1,26 @@
require 'spec_helper'
require 'support/activerecord'

def create_table(name)
ActiveRecord::Base.connection.create_table(name, force: true) do |t|
t.column :image, :string
t.column :images, :json
t.column :textfile, :string
t.column :textfiles, :json
t.column :foo, :string
describe CarrierWave::ActiveRecord, :aggregate_failures do
def create_table(name)
ActiveRecord::Base.connection.create_table(name, force: true) do |t|
t.column :image, :string
t.column :images, :json
t.column :textfile, :string
t.column :textfiles, :json
t.column :foo, :string
end
end
end

def drop_table(name)
ActiveRecord::Base.connection.drop_table(name)
end
def drop_table(name)
ActiveRecord::Base.connection.drop_table(name)
end

def reset_class(class_name)
Object.send(:remove_const, class_name) rescue nil
Object.const_set(class_name, Class.new(ActiveRecord::Base))
end
def reset_class(class_name)
Object.send(:remove_const, class_name) rescue nil
Object.const_set(class_name, Class.new(ActiveRecord::Base))
end

describe CarrierWave::ActiveRecord do
before(:all) { create_table("events") }
after(:all) { drop_table("events") }

Expand Down Expand Up @@ -1731,14 +1731,56 @@ def reload(*)
@event.save
new_event = @event.dup

expect(new_event.image).to be_cached
expect(new_event.image).to be_a(@uploader) & be_cached
expect(new_event.image.filename).to eq("test.jpeg")
end

it "appropriately removes the model reference from the new models uploader" do
@event.save
new_event = @event.dup

expect(new_event.image.model).not_to eq @event
expect(new_event).not_to be @event
expect(new_event.image.model).to be new_event
end

it "works when the mounters haven't been initialized" do
expect(@event.instance_variable_defined?(:@_mounters)).to eq false
expect(@event.dup).to be_a Event
end

context "with more than one mount" do
before do
@uploader1 = Class.new(CarrierWave::Uploader::Base)
Event.mount_uploaders(:images, @uploader1)
end

it "caches the existing files into the new model" do
@event.image = stub_file('test.jpeg')
@event.images = [stub_file('test.jpg')]
@event.save
new_event = @event.dup

expect(new_event.image).to be_cached & be_a(@uploader)
expect(new_event.image.filename).to eq("test.jpeg")
expect(new_event.images).to all(be_cached & be_a(@uploader1))
expect(new_event.images.map(&:filename)).to eq ["test.jpg"]
end

it "appropriately removes the model references from the new models uploaders" do
@event.save
new_event = @event.dup

expect(new_event).not_to be @event
expect(new_event.image.model).to be new_event
end

it "works when the mounters haven't been initialized" do
expect(@event.instance_variable_defined?(:@_mounters)).to eq false
new_event = @event.dup

expect(new_event).to be_a Event
expect(new_event).not_to be @event
end
end

context 'when #initialize_dup is overridden in the model' do
Expand Down

0 comments on commit e6e0628

Please sign in to comment.