diff --git a/lib/paperclip/attachment.rb b/lib/paperclip/attachment.rb index 773ad24aa..e4d35c713 100644 --- a/lib/paperclip/attachment.rb +++ b/lib/paperclip/attachment.rb @@ -50,7 +50,7 @@ def self.default_options # +url+ - a relative URL of the attachment. This is interpolated using +interpolator+ # +path+ - where on the filesystem to store the attachment. This is interpolated using +interpolator+ # +styles+ - a hash of options for processing the attachment. See +has_attached_file+ for the details - # +only_process+ - style args to be run through the post-processor. This defaults to the empty list (which is + # +only_process+ - style args to be run through the post-processor. This defaults to the empty list (which is # a special case that indicates all styles should be processed) # +default_url+ - a URL for the missing image # +default_style+ - the style to use when an argument is not specified e.g. #url, #path @@ -499,11 +499,18 @@ def process_options(options_type, style) #:nodoc: def post_process(*style_args) #:nodoc: return if @queued_for_write[:original].nil? - - instance.run_paperclip_callbacks(:post_process) do - instance.run_paperclip_callbacks(:"#{name}_post_process") do - if !@options[:check_validity_before_processing] || !instance.errors.any? + catch(:cancel_all_after_callbacks) do + instance.run_paperclip_callbacks(:post_process) do + inner_result = instance.run_paperclip_callbacks(:"#{name}_post_process") do + # paperclip validations set instance.errors at this point... + if @options[:check_validity_before_processing] && instance.errors.any? + throw :cancel_all_after_callbacks + end post_process_styles(*style_args) + true + end + unless inner_result + throw :cancel_all_after_callbacks end end end diff --git a/lib/paperclip/callbacks.rb b/lib/paperclip/callbacks.rb index 53a436857..6c55eab5b 100644 --- a/lib/paperclip/callbacks.rb +++ b/lib/paperclip/callbacks.rb @@ -7,7 +7,11 @@ def self.included(base) module Defining def define_paperclip_callbacks(*callbacks) - define_callbacks(*[callbacks, { terminator: hasta_la_vista_baby }].flatten) + options = { + terminator: hasta_la_vista_baby, + skip_after_callbacks_if_terminated: true, + } + define_callbacks(*[callbacks, options].flatten) callbacks.each do |callback| eval <<-end_callbacks def before_#{callback}(*args, &blk) diff --git a/spec/paperclip/attachment_callbacks_spec.rb b/spec/paperclip/attachment_callbacks_spec.rb new file mode 100644 index 000000000..2fc3e2949 --- /dev/null +++ b/spec/paperclip/attachment_callbacks_spec.rb @@ -0,0 +1,56 @@ +require 'spec_helper' + +describe Paperclip::Attachment do + context "callbacks" do + context "with content_type validation" do + def rebuild(required_content_type) + rebuild_class styles: { something: "100x100#" } + Dummy.class_eval do + validates_attachment_content_type :avatar, content_type: [required_content_type] + before_avatar_post_process :do_before_avatar + after_avatar_post_process :do_after_avatar + before_post_process :do_before_all + after_post_process :do_after_all + def do_before_avatar ; end + def do_after_avatar; end + def do_before_all; end + def do_after_all; end + end + @dummy = Dummy.new + end + + context "that passes" do + let!(:required_content_type) { "image/png" } + before { rebuild(required_content_type) } + let(:fake_file) { StringIO.new(".").tap { |s| s.stubs(:to_tempfile).returns(s) } } + + it "calls all callbacks when assigned" 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(fake_file) + @dummy.avatar = File.new(fixture_file("5k.png")) + end + end + + context "that fails" do + let!(:required_content_type) { "image/jpeg" } + before { rebuild(required_content_type) } + + it "does not call after callbacks when assigned" do + # before callbacks ARE still called at present + @dummy.expects(:do_before_all).with() + @dummy.expects(:do_before_avatar).with() + + # But after_* are not + @dummy.expects(:do_after_avatar).never + @dummy.expects(:do_after_all).never + Paperclip::Thumbnail.expects(:make).never + + @dummy.avatar = File.new(fixture_file("5k.png")) + end + end + end + end +end diff --git a/spec/paperclip/attachment_spec.rb b/spec/paperclip/attachment_spec.rb index c95cd5a5a..9c8b9bf2f 100644 --- a/spec/paperclip/attachment_spec.rb +++ b/spec/paperclip/attachment_spec.rb @@ -847,16 +847,16 @@ def do_after_all; end @dummy.expects(:do_before_avatar).never @dummy.expects(:do_after_avatar).never @dummy.expects(:do_before_all).with().returns(false) - @dummy.expects(:do_after_all) + @dummy.expects(:do_after_all).never Paperclip::Thumbnail.expects(:make).never @dummy.avatar = @file end it "cancels the processing if a before_avatar_post_process returns false" do @dummy.expects(:do_before_avatar).with().returns(false) - @dummy.expects(:do_after_avatar) + @dummy.expects(:do_after_avatar).never @dummy.expects(:do_before_all).with().returns(true) - @dummy.expects(:do_after_all) + @dummy.expects(:do_after_all).never Paperclip::Thumbnail.expects(:make).never @dummy.avatar = @file end