Merge pull request #10034 from benofsky/fix_skipping_object_callback_filters

Fixes skipping object callback filters

activesupport/CHANGELOG.md

@@ -14,6 +14,9 @@
 
     *Charles Jones*
 
+*   Fix skipping of filters defined by objects in `ActiveSupport::Callbacks::Callback`.
+
+    *Ben McRedmond*
 
 ## Rails 4.0.0.beta1 (February 25, 2013) ##
 

activesupport/lib/active_support/callbacks.rb

@@ -131,6 +131,7 @@ module ActiveSupport
       end
 
       def matches?(_kind, _filter)
+        _filter = _method_name_for_object_filter(_kind, _filter) if @_is_object_filter
         @kind == _kind && @filter == _filter
       end
 
@@ -234,6 +235,14 @@ module ActiveSupport
         @compiled_options = conditions.flatten.join(" && ")
       end
 
+      def _method_name_for_object_filter(kind, filter)
+        class_name = filter.kind_of?(Class) ? filter.to_s : filter.class.to_s
+        class_name.gsub!(/<|>|#/, '')
+        class_name.gsub!(/\/|:/, "_")
+
+        "_callback_#{kind}_#{class_name}"
+      end
+
       # Filters support:
       #
       #   Arrays::  Used in conditions. This is used to specify
@@ -255,6 +264,8 @@ module ActiveSupport
       #     a method is created that calls the before_foo method
       #     on the object.
       def _compile_filter(filter)
+        @_is_object_filter = false
+
         case filter
         when Array
           filter.map {|f| _compile_filter(f)}
@@ -269,7 +280,8 @@ module ActiveSupport
 
           method_name << (filter.arity == 1 ? "(self)" : " self, Proc.new ")
         else
-          method_name = "_callback_#{@kind}_#{next_id}"
+          method_name = _method_name_for_object_filter(kind, filter)
+          @_is_object_filter = true
           @klass.send(:define_method, "#{method_name}_object") { filter }
 
           _normalize_legacy_filter(kind, filter)

activesupport/test/callbacks_test.rb

@@ -66,6 +66,16 @@ module CallbacksTest
     end
   end
 
+  class CallbackClass
+    def self.before(model)
+      model.history << [:before_save, :class]
+    end
+    
+    def self.after(model)
+      model.history << [:after_save, :class]
+    end
+  end
+
   class Person < Record
     [:before_save, :after_save].each do |callback_method|
       callback_method_sym = callback_method.to_sym
@@ -73,6 +83,7 @@ module CallbacksTest
       send(callback_method, callback_string(callback_method_sym))
       send(callback_method, callback_proc(callback_method_sym))
       send(callback_method, callback_object(callback_method_sym.to_s.gsub(/_save/, '')))
+      send(callback_method, CallbackClass)
       send(callback_method) { |model| model.history << [callback_method_sym, :block] }
     end
 
@@ -86,6 +97,7 @@ module CallbacksTest
     skip_callback :save, :after, :before_save_method, :unless => :yes
     skip_callback :save, :after, :before_save_method, :if => :no
     skip_callback :save, :before, :before_save_method, :unless => :no
+    skip_callback :save, :before, CallbackClass , :if => :yes
     def yes; true; end
     def no; false; end
   end
@@ -430,6 +442,7 @@ module CallbacksTest
         [:before_save, :object],
         [:before_save, :block],
         [:after_save, :block],
+        [:after_save, :class],
         [:after_save, :object],
         [:after_save, :proc],
         [:after_save, :string],
@@ -449,8 +462,10 @@ module CallbacksTest
         [:before_save, :string],
         [:before_save, :proc],
         [:before_save, :object],
+        [:before_save, :class],
         [:before_save, :block],
         [:after_save, :block],
+        [:after_save, :class],
         [:after_save, :object],
         [:after_save, :proc],
         [:after_save, :string],
@@ -715,8 +730,10 @@ module CallbacksTest
         [:before_save, :string],
         [:before_save, :proc],
         [:before_save, :object],
+        [:before_save, :class],
         [:before_save, :block],
         [:after_save, :block],
+        [:after_save, :class],
         [:after_save, :object],
         [:after_save, :proc],
         [:after_save, :string],