diff --git a/lib/paperclip/attachment.rb b/lib/paperclip/attachment.rb index 65c26ddc7..f2e7d672a 100644 --- a/lib/paperclip/attachment.rb +++ b/lib/paperclip/attachment.rb @@ -109,8 +109,6 @@ def assign(uploaded_file) nil else assign_attributes - post_process_file - reset_file_if_original_reprocessed end else nil @@ -238,6 +236,8 @@ def dirty? # Saves the file, if there are no errors. If there are, it flushes them to # the instance's errors and returns false, cancelling the save. def save + post_process_file + reset_file_if_original_reprocessed flush_deletes unless @options[:keep_old_files] process = only_process if process.any? && !process.include?(:original) diff --git a/spec/paperclip/attachment_processing_spec.rb b/spec/paperclip/attachment_processing_spec.rb index f0231440f..e90c0c0f3 100644 --- a/spec/paperclip/attachment_processing_spec.rb +++ b/spec/paperclip/attachment_processing_spec.rb @@ -4,34 +4,47 @@ before { rebuild_class } context 'using validates_attachment_content_type' do - it 'processes attachments given a valid assignment' do + it "doesn't process attachments on assignment" do file = File.new(fixture_file("5k.png")) Dummy.validates_attachment_content_type :avatar, content_type: "image/png" instance = Dummy.new attachment = instance.avatar - attachment.expects(:post_process_styles) + attachment.expects(:post_process_styles).never attachment.assign(file) end - it 'does not process attachments given an invalid assignment with :not' do + it 'processes attachments given the object is valid' do + file = File.new(fixture_file("5k.png")) + Dummy.validates_attachment_content_type :avatar, content_type: "image/png" + instance = Dummy.new + attachment = instance.avatar + attachment.assign(file) + attachment.expects(:post_process_styles) + + attachment.save + end + + it 'does not process attachments given an invalid object with :not' do file = File.new(fixture_file("5k.png")) Dummy.validates_attachment_content_type :avatar, not: "image/png" instance = Dummy.new attachment = instance.avatar + attachment.assign(file) attachment.expects(:post_process_styles).never - attachment.assign(file) + attachment.save end - it 'does not process attachments given an invalid assignment with :content_type' do + it 'does not process attachments given an invalid object with :content_type' do file = File.new(fixture_file("5k.png")) Dummy.validates_attachment_content_type :avatar, content_type: "image/tiff" instance = Dummy.new attachment = instance.avatar + attachment.assign(file) attachment.expects(:post_process_styles).never - attachment.assign(file) + attachment.save end it 'allows what would be an invalid assignment when validation :if clause returns false' do @@ -39,14 +52,15 @@ Dummy.validates_attachment_content_type :avatar, content_type: "image/tiff", if: lambda{false} instance = Dummy.new attachment = instance.avatar + attachment.assign(invalid_assignment) attachment.expects(:post_process_styles) - attachment.assign(invalid_assignment) + attachment.save end end context 'using validates_attachment' do - it 'processes attachments given a valid assignment' do + it "doesn't process attachments on assignment" do file = File.new(fixture_file("5k.png")) Dummy.validates_attachment :avatar, content_type: {content_type: "image/png"} instance = Dummy.new @@ -56,24 +70,37 @@ attachment.assign(file) end - it 'does not process attachments given an invalid assignment with :not' do + it 'processes attachments given a valid object' do + file = File.new(fixture_file("5k.png")) + Dummy.validates_attachment :avatar, content_type: {content_type: "image/png"} + instance = Dummy.new + attachment = instance.avatar + attachment.assign(file) + attachment.expects(:post_process_styles) + + attachment.save + end + + it 'does not process attachments given an invalid object with :not' do file = File.new(fixture_file("5k.png")) Dummy.validates_attachment :avatar, content_type: {not: "image/png"} instance = Dummy.new attachment = instance.avatar + attachment.assign(file) attachment.expects(:post_process_styles).never - attachment.assign(file) + attachment.save end - it 'does not process attachments given an invalid assignment with :content_type' do + it 'does not process attachments given an invalid object with :content_type' do file = File.new(fixture_file("5k.png")) Dummy.validates_attachment :avatar, content_type: {content_type: "image/tiff"} instance = Dummy.new attachment = instance.avatar + attachment.assign(file) attachment.expects(:post_process_styles).never - attachment.assign(file) + attachment.save end end end diff --git a/spec/paperclip/attachment_spec.rb b/spec/paperclip/attachment_spec.rb index 76d7e9fb4..d766e7162 100644 --- a/spec/paperclip/attachment_spec.rb +++ b/spec/paperclip/attachment_spec.rb @@ -711,9 +711,10 @@ class Paperclip::Test < Paperclip::Processor; end Paperclip::Thumbnail.stubs(:make).returns(@file) end - context "when assigned" do + context "when saved" do it "calls #make on all specified processors" do @dummy.avatar = @file + @dummy.save expect(Paperclip::Thumbnail).to have_received(:make) expect(Paperclip::Test).to have_received(:make) @@ -729,12 +730,14 @@ class Paperclip::Test < Paperclip::Processor; end }) @dummy.avatar = @file + @dummy.save expect(Paperclip::Thumbnail).to have_received(:make).with(anything, expected_params, anything) end it "calls #make with attachment passed as third argument" do @dummy.avatar = @file + @dummy.save expect(Paperclip::Test).to have_received(:make).with(anything, anything, @dummy.avatar) end @@ -745,6 +748,42 @@ class Paperclip::Test < Paperclip::Processor; end @dummy.avatar = @file end end + + context "when assigned" do + it "doesn't call #make on all specified processors" do + @dummy.avatar = @file + + expect(Paperclip::Thumbnail).not_to have_received(:make) + expect(Paperclip::Test).not_to have_received(:make) + end + + it "doesn't call #make with the right parameters passed as second argument" do + expected_params = @style_params[:once].merge({ + style: :once, + processors: [:thumbnail, :test], + whiny: true, + convert_options: "", + source_file_options: "" + }) + + @dummy.avatar = @file + + expect(Paperclip::Thumbnail).not_to have_received(:make).with(anything, expected_params, anything) + end + + it "doesn't call #make with attachment passed as third argument" do + @dummy.avatar = @file + + expect(Paperclip::Test).not_to have_received(:make).with(anything, anything, @dummy.avatar) + end + + it "doesn't call #make and unlinks intermediary files afterward" do + # TODO: Fix this test? + @dummy.avatar.expects(:unlink_files).with([@file, @file]) + + @dummy.avatar = @file + end + end end context "An attachment with a processor that returns original file" do @@ -758,8 +797,18 @@ def make; @file; end @dummy = Dummy.new end + context "when saved" do + it "calls #make and doesn't unlink the original file" do + @dummy.avatar.expects(:unlink_files).with([]) + + @dummy.avatar = @file + @dummy.save + end + end + context "when assigned" do - it "#calls #make and doesn't unlink the original file" do + it "doesn't call #make and doesn't unlink the original file" do + # TODO: Fix this test? @dummy.avatar.expects(:unlink_files).with([]) @dummy.avatar = @file @@ -822,6 +871,7 @@ def make; @file; end end context "Assigning an attachment with post_process hooks" do + # NOTE: Investigate the before_post_process hooks, as they may be intended to run on assignment before do rebuild_class styles: { something: "100x100#" } Dummy.class_eval do @@ -841,13 +891,23 @@ def do_after_all; end @attachment = @dummy.avatar end - it "calls the defined callbacks when assigned" do + it "calls the defined callbacks when saved" do @dummy.expects(:do_before_avatar).with() @dummy.expects(:do_after_avatar).with() @dummy.expects(:do_before_all).with() @dummy.expects(:do_after_all).with() Paperclip::Thumbnail.expects(:make).returns(@file) @dummy.avatar = @file + @dummy.save + end + + it "doesn't call the defined callbacks when assigned" do + @dummy.expects(:do_before_avatar).never + @dummy.expects(:do_after_avatar).never + @dummy.expects(:do_before_all).never + @dummy.expects(:do_after_all).never + Paperclip::Thumbnail.expects(:make).never + @dummy.avatar = @file end it "does not cancel the processing if a before_post_process returns nil" do @@ -857,6 +917,7 @@ def do_after_all; end @dummy.expects(:do_after_all).with() Paperclip::Thumbnail.expects(:make).returns(@file) @dummy.avatar = @file + @dummy.save end it "cancels the processing if a before_post_process returns false" do @@ -866,6 +927,7 @@ def do_after_all; end @dummy.expects(:do_after_all) Paperclip::Thumbnail.expects(:make).never @dummy.avatar = @file + @dummy.save end it "cancels the processing if a before_avatar_post_process returns false" do @@ -875,6 +937,7 @@ def do_after_all; end @dummy.expects(:do_after_all) Paperclip::Thumbnail.expects(:make).never @dummy.avatar = @file + @dummy.save end end