Merge pull request #19808 from byroot/action-parameter

[PoC] Stop shadowing parameters named `action`

actionpack/lib/action_controller/test_case.rb

@@ -18,9 +18,10 @@ def initialize(env = {})
     end
 
     def assign_parameters(routes, controller_path, action, parameters = {})
-      parameters = parameters.symbolize_keys.merge(:controller => controller_path, :action => action)
-      extra_keys = routes.extra_keys(parameters)
+      parameters = parameters.symbolize_keys
+      extra_keys = routes.extra_keys(parameters.merge(:controller => controller_path, :action => action))
       non_path_parameters = get? ? query_parameters : request_parameters
+
       parameters.each do |key, value|
         if value.is_a?(Array) && (value.frozen? || value.any?(&:frozen?))
           value = value.map{ |v| v.duplicable? ? v.dup : v }
@@ -30,7 +31,7 @@ def assign_parameters(routes, controller_path, action, parameters = {})
           value = value.dup
         end
 
-        if extra_keys.include?(key)
+        if extra_keys.include?(key) || key == :action || key == :controller
           non_path_parameters[key] = value
         else
           if value.is_a?(Array)
@@ -43,19 +44,16 @@ def assign_parameters(routes, controller_path, action, parameters = {})
         end
       end
 
+      path_parameters[:controller] = controller_path
+      path_parameters[:action] = action
+
       # Clear the combined params hash in case it was already referenced.
       @env.delete("action_dispatch.request.parameters")
 
       # Clear the filter cache variables so they're not stale
       @filtered_parameters = @filtered_env = @filtered_path = nil
 
-      params = self.request_parameters.dup
-      %w(controller action only_path).each do |k|
-        params.delete(k)
-        params.delete(k.to_sym)
-      end
-      data = params.to_query
-
+      data = request_parameters.to_query
       @env['CONTENT_LENGTH'] = data.length.to_s
       @env['rack.input'] = StringIO.new(data)
     end
@@ -482,12 +480,10 @@ def process(action, *args)
         @controller.request  = @request
         @controller.response = @response
 
-        build_request_uri(action, parameters)
-
-        name = @request.parameters[:action]
+        build_request_uri(controller_class_name, action, parameters)
 
         @controller.recycle!
-        @controller.process(name)
+        @controller.process(action)
 
         if cookies = @request.env['action_dispatch.cookies']
           unless @response.committed?
@@ -603,10 +599,11 @@ def check_required_ivars
         end
       end
 
-      def build_request_uri(action, parameters)
+      def build_request_uri(controller_class_name, action, parameters)
         unless @request.env["PATH_INFO"]
           options = @controller.respond_to?(:url_options) ? @controller.__send__(:url_options).merge(parameters) : parameters
           options.update(
+            :controller => controller_class_name,
             :action => action,
             :relative_url_root => nil,
             :_recall => @request.path_parameters)

actionpack/test/controller/test_case_test.rb

@@ -48,6 +48,14 @@ def test_params
       render text: params.inspect
     end
 
+    def test_query_parameters
+      render text: request.query_parameters.inspect
+    end
+
+    def test_request_parameters
+      render text: request.request_parameters.inspect
+    end
+
     def test_uri
       render text: request.fullpath
     end
@@ -547,6 +555,18 @@ def test_params_passing
     )
   end
 
+  def test_query_param_named_action
+    get :test_query_parameters, params: {action: 'foobar'}
+    parsed_params = eval(@response.body)
+    assert_equal({action: 'foobar'}, parsed_params)
+  end
+
+  def test_request_param_named_action
+    post :test_request_parameters, params: {action: 'foobar'}
+    parsed_params = eval(@response.body)
+    assert_equal({'action' => 'foobar'}, parsed_params)
+  end
+
   def test_kwarg_params_passing_with_session_and_flash
     get :test_params, params: {
       page: {