From d5ac941ddc3de7ad1aaff80ed67aa04fb626a263 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron@rubyonrails.org>
Date: Tue, 2 Mar 2021 17:20:35 -0800
Subject: [PATCH] Remove special case filtering for Procs.

I'm writing this patch for two purposes:

1. I want to reduce the number of times `object_id` is called.  Calling
   `object_id` can have negative impacts on performance in Ruby 2.7+, so
   it would be nice to stop calling it.

2. I'm not sure why we're treating lambdas specially here.  It looks
   like we wanted to prevent people from skipping callbacks that were
   defined with a lambda, but I think that is silly.  If the user has a
   reference to a lambda, and they want to skip it, we should let them.

I think this cleans up some code, helps with performance, and is a more
intuitive interface.
---
 actionpack/test/controller/filters_test.rb    |  2 +-
 activesupport/lib/active_support/callbacks.rb | 15 +--------------
 activesupport/test/callbacks_test.rb          |  9 ---------
 activesupport/test/test_case_test.rb          |  8 ++++----
 4 files changed, 6 insertions(+), 28 deletions(-)

diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb
index c26e09d8ac..b19d1488de 100644
--- a/actionpack/test/controller/filters_test.rb
+++ b/actionpack/test/controller/filters_test.rb
@@ -12,7 +12,7 @@ class ActionController::Base
 
     def before_actions
       filters = _process_action_callbacks.select { |c| c.kind == :before }
-      filters.map!(&:raw_filter)
+      filters.map!(&:filter)
     end
   end
 end
diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb
index a86af00527..a65ff5cf72 100644
--- a/activesupport/lib/active_support/callbacks.rb
+++ b/activesupport/lib/active_support/callbacks.rb
@@ -289,21 +289,17 @@ module ActiveSupport
         end
 
         attr_accessor :kind, :name
-        attr_reader :chain_config
+        attr_reader :chain_config, :filter
 
         def initialize(name, filter, kind, options, chain_config)
           @chain_config = chain_config
           @name    = name
           @kind    = kind
           @filter  = filter
-          @key     = compute_identifier filter
           @if      = check_conditionals(options[:if])
           @unless  = check_conditionals(options[:unless])
         end
 
-        def filter; @key; end
-        def raw_filter; @filter; end
-
         def merge_conditional_options(chain, if_option:, unless_option:)
           options = {
             if: @if.dup,
@@ -367,15 +363,6 @@ module ActiveSupport
             conditionals.freeze
           end
 
-          def compute_identifier(filter)
-            case filter
-            when ::Proc
-              filter.object_id
-            else
-              filter
-            end
-          end
-
           def conditions_lambdas
             @if.map { |c| CallTemplate.build(c, self).make_lambda } +
               @unless.map { |c| CallTemplate.build(c, self).inverted_lambda }
diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb
index 499698c1a3..a1cba3ce45 100644
--- a/activesupport/test/callbacks_test.rb
+++ b/activesupport/test/callbacks_test.rb
@@ -1129,15 +1129,6 @@ module CallbacksTest
       }
     end
 
-    def test_skip_lambda # raises error
-      calls = []
-      callback = ->(o) { calls << o }
-      klass = build_class(callback)
-      assert_raises(ArgumentError) { klass.skip callback }
-      klass.new.run
-      assert_equal 10, calls.length
-    end
-
     def test_skip_symbol # removes all
       calls = []
       klass = build_class(:bar)
diff --git a/activesupport/test/test_case_test.rb b/activesupport/test/test_case_test.rb
index ac2923a313..e6342e8382 100644
--- a/activesupport/test/test_case_test.rb
+++ b/activesupport/test/test_case_test.rb
@@ -325,9 +325,9 @@ class SetupAndTeardownTest < ActiveSupport::TestCase
   teardown :foo, :sentinel
 
   def test_inherited_setup_callbacks
-    assert_equal [:reset_callback_record, :foo], self.class._setup_callbacks.map(&:raw_filter)
+    assert_equal [:reset_callback_record, :foo], self.class._setup_callbacks.map(&:filter)
     assert_equal [:foo], @called_back
-    assert_equal [:foo, :sentinel], self.class._teardown_callbacks.map(&:raw_filter)
+    assert_equal [:foo, :sentinel], self.class._teardown_callbacks.map(&:filter)
   end
 
   def setup
@@ -355,9 +355,9 @@ class SubclassSetupAndTeardownTest < SetupAndTeardownTest
   teardown :bar
 
   def test_inherited_setup_callbacks
-    assert_equal [:reset_callback_record, :foo, :bar], self.class._setup_callbacks.map(&:raw_filter)
+    assert_equal [:reset_callback_record, :foo, :bar], self.class._setup_callbacks.map(&:filter)
     assert_equal [:foo, :bar], @called_back
-    assert_equal [:foo, :sentinel, :bar], self.class._teardown_callbacks.map(&:raw_filter)
+    assert_equal [:foo, :sentinel, :bar], self.class._teardown_callbacks.map(&:filter)
   end
 
   private