From 26f53257563fa905d2d4b65d5e337f7574a190ed Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 11 May 2020 14:15:26 -0700 Subject: [PATCH] Clean up the generate method signature Journey is a private API inside Rails now, so lets start cleaning up some of its signatures. In this case, we always wan to parameterize things the same way, so lets just do that in the code (rather than passing some strategy object in). --- .../lib/action_dispatch/journey/formatter.rb | 14 ++++++++------ .../lib/action_dispatch/routing/route_set.rb | 10 +--------- actionpack/test/journey/router_test.rb | 16 ---------------- 3 files changed, 9 insertions(+), 31 deletions(-) diff --git a/actionpack/lib/action_dispatch/journey/formatter.rb b/actionpack/lib/action_dispatch/journey/formatter.rb index 7d1f5ab7c9..ca47b94c7e 100644 --- a/actionpack/lib/action_dispatch/journey/formatter.rb +++ b/actionpack/lib/action_dispatch/journey/formatter.rb @@ -15,12 +15,12 @@ module ActionDispatch @cache = nil end - def generate(name, options, path_parameters, parameterize = nil) + def generate(name, options, path_parameters) constraints = path_parameters.merge(options) missing_keys = nil match_route(name, constraints) do |route| - parameterized_parts = extract_parameterized_parts(route, options, path_parameters, parameterize) + parameterized_parts = extract_parameterized_parts(route, options, path_parameters) # Skip this route unless a name has been provided or it is a # standard Rails route since we can't determine whether an options @@ -62,7 +62,7 @@ module ActionDispatch end private - def extract_parameterized_parts(route, options, recall, parameterize = nil) + def extract_parameterized_parts(route, options, recall) parameterized_parts = recall.merge(options) keys_to_keep = route.parts.reverse_each.drop_while { |part| @@ -73,9 +73,11 @@ module ActionDispatch !keys_to_keep.include?(bad_key) end - if parameterize - parameterized_parts.each do |k, v| - parameterized_parts[k] = parameterize.call(k, v) + parameterized_parts.each do |k, v| + if k == :controller + parameterized_parts[k] = v + else + parameterized_parts[k] = v.to_param end end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 9b1162998f..e7773d80d3 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -650,14 +650,6 @@ module ActionDispatch end class Generator - PARAMETERIZE = lambda do |name, value| - if name == :controller - value - else - value.to_param - end - end - attr_reader :options, :recall, :set, :named_route def initialize(named_route, options, recall, set) @@ -744,7 +736,7 @@ module ActionDispatch # Generates a path from routes, returns [path, params]. # If no route is generated the formatter will raise ActionController::UrlGenerationError def generate - @set.formatter.generate(named_route, options, recall, PARAMETERIZE) + @set.formatter.generate(named_route, options, recall) end def different_controller? diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index fe0e3a975b..4e7374a7d4 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -224,22 +224,6 @@ module ActionDispatch assert_equal "/", path end - def test_generate_calls_param_proc - get "/:controller(/:action)", to: "foo#bar" - - parameterized = [] - params = [ [:controller, "tasks"], - [:action, "show"] ] - - @formatter.generate( - nil, - Hash[params], - {}, - lambda { |k, v| parameterized << [k, v]; v }) - - assert_equal params.map(&:to_s).sort, parameterized.map(&:to_s).sort - end - def test_generate_id get "/:controller(/:action)", to: "foo#bar"