From edcccf4990c28a1078172f89db0b16de40c59b90 Mon Sep 17 00:00:00 2001
From: Vitalii Khustochka <xyctka@gmail.com>
Date: Mon, 9 Mar 2020 18:24:44 -0500
Subject: [PATCH] Only compute :only and :except callback conditions once

Ref: https://github.com/rails/rails/issues/38679

`_normalize_callback_options` mutates the options hash, but
doesn't remove the `:only` and `:except` conditions.

So if you call `before_action` & al with the same option hash
you end up with multiple instance of identical `:if` / `:unless` procs
---
 .../lib/abstract_controller/callbacks.rb      |  2 +-
 actionpack/test/abstract/callbacks_test.rb    | 42 +++++++++++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/actionpack/lib/abstract_controller/callbacks.rb b/actionpack/lib/abstract_controller/callbacks.rb
index 983d65f944..3084886ec5 100644
--- a/actionpack/lib/abstract_controller/callbacks.rb
+++ b/actionpack/lib/abstract_controller/callbacks.rb
@@ -69,7 +69,7 @@ def _normalize_callback_options(options)
       end
 
       def _normalize_callback_option(options, from, to) # :nodoc:
-        if from = options[from]
+        if from = options.delete(from)
           _from = Array(from).map(&:to_s).to_set
           from = proc { |c| _from.include? c.action_name }
           options[to] = Array(options[to]).unshift(from)
diff --git a/actionpack/test/abstract/callbacks_test.rb b/actionpack/test/abstract/callbacks_test.rb
index 4512ea27b3..e64f580fb2 100644
--- a/actionpack/test/abstract/callbacks_test.rb
+++ b/actionpack/test/abstract/callbacks_test.rb
@@ -158,6 +158,48 @@ def setup
       end
     end
 
+    class CallbacksWithReusedConditions < ControllerWithCallbacks
+      options = { only: :index }
+      before_action :list, options
+      before_action :authenticate, options
+
+      def index
+        self.response_body = @list.join(", ")
+      end
+
+      def public_data
+        @authenticated = "false"
+        self.response_body = @authenticated
+      end
+
+      private
+        def list
+          @list = ["Hello", "World"]
+        end
+
+        def authenticate
+          @list ||= []
+          @authenticated = "true"
+        end
+    end
+
+    class TestCallbacksWithReusedConditions < ActiveSupport::TestCase
+      def setup
+        @controller = CallbacksWithReusedConditions.new
+      end
+
+      test "when :only is specified, both actions triggered on that action" do
+        @controller.process(:index)
+        assert_equal "Hello, World", @controller.response_body
+        assert_equal "true", @controller.instance_variable_get("@authenticated")
+      end
+
+      test "when :only is specified, both actions are not triggered on other actions" do
+        @controller.process(:public_data)
+        assert_equal "false", @controller.response_body
+      end
+    end
+
     class CallbacksWithArrayConditions < ControllerWithCallbacks
       before_action :list, only: [:index, :listy]
       before_action :authenticate, except: [:index, :listy]
-- 
GitLab