From 7b53016b985dea3ac943ae4ea61a59b8689c1477 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Fri, 6 Sep 2019 14:23:38 +0200 Subject: [PATCH] Refactor `define_url_helper` to share the same instance between _path and _url Before this patch each named routes generates two UrlHelper instances. Both are almost identical, their only difference is the `url_strategy` property. This patch get rid of that property and instead pass it as an argument to call, so effectively it's stored in the closure. --- .../lib/action_dispatch/routing/route_set.rb | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 44f8cb1791..98cb58c47a 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -110,8 +110,10 @@ module ActionDispatch @url_helpers_module.undef_method url_name end routes[key] = route - define_url_helper @path_helpers_module, route, path_name, route.defaults, name, PATH - define_url_helper @url_helpers_module, route, url_name, route.defaults, name, UNKNOWN + + helper = UrlHelper.create(route, route.defaults, name) + define_url_helper @path_helpers_module, path_name, helper, PATH + define_url_helper @url_helpers_module, url_name, helper, UNKNOWN @path_helpers << path_name @url_helpers << url_name @@ -169,11 +171,11 @@ module ActionDispatch end class UrlHelper - def self.create(route, options, route_name, url_strategy) + def self.create(route, options, route_name) if optimize_helper?(route) - OptimizedUrlHelper.new(route, options, route_name, url_strategy) + OptimizedUrlHelper.new(route, options, route_name) else - new route, options, route_name, url_strategy + new(route, options, route_name) end end @@ -181,18 +183,18 @@ module ActionDispatch route.path.requirements.empty? && !route.glob? end - attr_reader :url_strategy, :route_name + attr_reader :route_name class OptimizedUrlHelper < UrlHelper attr_reader :arg_size - def initialize(route, options, route_name, url_strategy) + def initialize(route, options, route_name) super @required_parts = @route.required_parts @arg_size = @required_parts.size end - def call(t, args, inner_options) + def call(t, args, inner_options, url_strategy) if args.size == arg_size && !inner_options && optimize_routes_generation?(t) options = t.url_options.merge @options options[:path] = optimized_helper(args) @@ -249,15 +251,14 @@ module ActionDispatch end end - def initialize(route, options, route_name, url_strategy) + def initialize(route, options, route_name) @options = options @segment_keys = route.segment_keys.uniq @route = route - @url_strategy = url_strategy @route_name = route_name end - def call(t, args, inner_options) + def call(t, args, inner_options, url_strategy) controller_options = t.url_options options = controller_options.merge @options hash = handle_positional_args(controller_options, @@ -312,8 +313,7 @@ module ActionDispatch # # foo_url(bar, baz, bang, sort_by: 'baz') # - def define_url_helper(mod, route, name, opts, route_key, url_strategy) - helper = UrlHelper.create(route, opts, route_key, url_strategy) + def define_url_helper(mod, name, helper, url_strategy) mod.define_method(name) do |*args| last = args.last options = \ @@ -323,7 +323,7 @@ module ActionDispatch when ActionController::Parameters args.pop.to_h end - helper.call self, args, options + helper.call(self, args, options, url_strategy) end end end