From 697991a20114ba409e8bc991bfba0a08a25ff60f Mon Sep 17 00:00:00 2001 From: "ara.t.howard" Date: Wed, 4 Apr 2012 16:28:19 -0600 Subject: carefully and quickly merge url_for options --- actionpack/lib/action_dispatch/routing/url_for.rb | 18 +++++++++++++++++- actionpack/test/controller/url_for_test.rb | 9 +++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 94db36ce1f..f2773615a3 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -147,7 +147,7 @@ module ActionDispatch when String options when nil, Hash - _routes.url_for((options || {}).symbolize_keys.reverse_merge!(url_options)) + _routes.url_for(_merge_url_for_options(options, url_options)) else polymorphic_url(options) end @@ -155,6 +155,22 @@ module ActionDispatch protected + def _merge_url_for_options(h1, h2) + opts = {} + + h1.keys.each do |k| + s = k.to_sym + opts[s] = h1[k] unless opts.has_key?(s) + end if h1 + + h2.keys.each do |k| + s = k.to_sym + opts[s] = h2[k] unless opts.has_key?(s) + end if h2 + + opts + end + def optimize_routes_generation? return @_optimized_routes if defined?(@_optimized_routes) @_optimized_routes = _routes.optimize_routes_generation? && default_url_options.empty? diff --git a/actionpack/test/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb index aa233d6135..5b06d94d18 100644 --- a/actionpack/test/controller/url_for_test.rb +++ b/actionpack/test/controller/url_for_test.rb @@ -356,6 +356,15 @@ module AbstractController assert_equal("/c/a", W.new.url_for(HashWithIndifferentAccess.new('controller' => 'c', 'action' => 'a', 'only_path' => true))) end + def test_with_stringified_default_url_options + W.default_url_options['controller'] = 'd' + W.default_url_options['only_path'] = false + assert_equal("/c", W.new.url_for(:controller => 'c', :only_path => true)) + + W.default_url_options['action'] = 'b' + assert_equal("/c/a", W.new.url_for(:controller => 'c', :action => 'a', :only_path => true)) + end + def test_url_params_with_nil_to_param_are_not_in_url assert_equal("/c/a", W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :id => Struct.new(:to_param).new(nil))) end -- cgit v1.2.3 From 99c08c702eea705471e07bc83a9bdea7edadfb7b Mon Sep 17 00:00:00 2001 From: "ara.t.howard" Date: Thu, 5 Apr 2012 07:30:54 -0600 Subject: url_for: avoid a nil check. unroll a loop. handle most common case 1st. --- actionpack/lib/action_dispatch/routing/url_for.rb | 31 ++++++++++------------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index f2773615a3..bab501f27c 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -144,10 +144,21 @@ module ActionDispatch # # => 'http://somehost.org/tasks/testing?number=33' def url_for(options = nil) case options + when nil + _routes.url_for(url_options) + when Hash + symbolized = {} + options.keys.each do |k| + sym = k.to_sym + symbolized[sym] = options[k] unless symbolized.has_key?(sym) + end + url_options.keys.each do |k| + sym = k.to_sym + symbolized[sym] = url_options[k] unless symbolized.has_key?(sym) + end + _routes.url_for(symbolized) when String options - when nil, Hash - _routes.url_for(_merge_url_for_options(options, url_options)) else polymorphic_url(options) end @@ -155,22 +166,6 @@ module ActionDispatch protected - def _merge_url_for_options(h1, h2) - opts = {} - - h1.keys.each do |k| - s = k.to_sym - opts[s] = h1[k] unless opts.has_key?(s) - end if h1 - - h2.keys.each do |k| - s = k.to_sym - opts[s] = h2[k] unless opts.has_key?(s) - end if h2 - - opts - end - def optimize_routes_generation? return @_optimized_routes if defined?(@_optimized_routes) @_optimized_routes = _routes.optimize_routes_generation? && default_url_options.empty? -- cgit v1.2.3