DRAFT toward fixing after_post_process is run even if validations fail

lib/paperclip/attachment.rb

@@ -499,12 +499,15 @@ 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?
-            post_process_styles(*style_args)
+      catch(:cancel_outer_callbacks) do
+        instance.run_paperclip_callbacks(:post_process) do
+          inner_result = instance.run_paperclip_callbacks(:"#{name}_post_process") do
+            if !@options[:check_validity_before_processing] || !instance.errors.any?
+              post_process_styles(*style_args)
+            end
+            true
           end
+          throw :cancel_outer_callbacks unless inner_result
         end
       end
     end

lib/paperclip/callbacks.rb

@@ -7,7 +7,7 @@ def self.included(base)
 
     module Defining
       def define_paperclip_callbacks(*callbacks)
-        define_callbacks(*[callbacks, { terminator: hasta_la_vista_baby }].flatten)
+        define_callbacks(*[callbacks, { terminator: hasta_la_vista_baby, skip_after_callbacks_if_terminated: true }].flatten)
         callbacks.each do |callback|
           eval <<-end_callbacks
             def before_#{callback}(*args, &blk)

spec/paperclip/attachment_processing_spec.rb

@@ -44,6 +44,37 @@
 
       attachment.assign(invalid_assignment)
     end
+
+    context "with custom post-processing" do
+      before do
+        Dummy.class_eval do
+          validates_attachment_content_type :avatar, content_type: "image/png"
+          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
+      end
+      ## FALSE POSITIVE: This passes even before our change in callbacks.rb,
+      # even though we know it had a problem with this. This passes
+      # even if we don't trigger a validation error, we haven't succesfully
+      # set up our callbacks at all somehow.
+      it 'does not run custom post-processing if validation fails' do
+        file = File.new(fixture_file("5k.png"))
+
+        instance = Dummy.new
+        attachment = instance.avatar
+
+        attachment.expects(:do_after_avatar).never
+        attachment.expects(:do_after_all).never
+
+        attachment.assign(file)
+      end
+    end
   end
 
   context 'using validates_attachment' do

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