From c507e16dba844c7b066d145bfd5c5d23e7a9c5ad Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 8 Mar 2010 21:40:04 -0800 Subject: Reinstate default_url_options and remove url_options= writer --- actionpack/lib/action_dispatch/routing/url_for.rb | 66 +++++++---------------- actionpack/test/controller/base_test.rb | 42 +++++++-------- actionpack/test/template/url_helper_test.rb | 2 +- 3 files changed, 39 insertions(+), 71 deletions(-) diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 035b0ec4ca..4629e7e002 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -85,43 +85,24 @@ module ActionDispatch extend ActiveSupport::Concern included do - # Including in a class uses an inheritable hash. Modules get a plain hash. - if respond_to?(:class_attribute) - class_attribute :default_url_options - else - mattr_accessor :default_url_options - end - - remove_method :default_url_options - - self.default_url_options = {} - end - - # def default_url_options(options = nil) - # self.class.default_url_options - # end + # TODO: with_routing extends @controller with url_helpers, trickling down to including this module which overrides its default_url_options + unless method_defined?(:default_url_options) + # Including in a class uses an inheritable hash. Modules get a plain hash. + if respond_to?(:class_attribute) + class_attribute :default_url_options + else + mattr_accessor :default_url_options + remove_method :default_url_options + end - def url_options - @url_options ||= begin - # self.class does not respond to default_url_options when the helpers are extended - # onto a singleton - self.class.respond_to?(:default_url_options) ? self.class.default_url_options : {} + self.default_url_options = {} end end - def url_options=(options) - defaults = self.class.respond_to?(:default_url_options) ? self.class.default_url_options : {} - @url_options = defaults.merge(options) + def url_options + default_url_options end - # def merge_options(options) #:nodoc: - # if respond_to?(:default_url_options) && (defaults = default_url_options(options)) - # defaults.merge(options) - # else - # options - # end - # end - # Generate a url based on the options provided, default_url_options and the # routes defined in routes.rb. The following options are supported: # @@ -145,23 +126,14 @@ module ActionDispatch # url_for :controller => 'tasks', :action => 'testing', :host=>'somehost.org', :anchor => 'ok', :only_path => true # => '/tasks/testing#ok' # url_for :controller => 'tasks', :action => 'testing', :trailing_slash=>true # => 'http://somehost.org/tasks/testing/' # url_for :controller => 'tasks', :action => 'testing', :host=>'somehost.org', :number => '33' # => 'http://somehost.org/tasks/testing?number=33' - def url_for(options = {}) - options ||= {} + def url_for(options = nil) case options - when String - options - when Hash - # Handle the deprecated instance level default_url_options - if respond_to?(:default_url_options, true) - ActiveSupport::Deprecation.warn "Overriding the #default_url_options method is deprecated. Instead, set self.url_options = { ... } in a before_filter." - if defaults = default_url_options(options) - options = defaults.merge(options) - end - end - - ActionController::UrlRewriter.rewrite(_router, url_options.merge(options)) - else - polymorphic_url(options) + when String + options + when nil, Hash + ActionController::UrlRewriter.rewrite(_router, url_options.merge(options || {})) + else + polymorphic_url(options) end end end diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb index ade9a7fcba..3e9f8ae048 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -181,7 +181,7 @@ class UrlOptionsTest < ActionController::TestCase rescue_action_in_public! end - def test_default_url_options_are_used_if_set + def test_url_options_override with_routing do |set| set.draw do |map| match 'from_view', :to => 'url_options#from_view', :as => :from_view @@ -206,20 +206,18 @@ class DefaultUrlOptionsTest < ActionController::TestCase rescue_action_in_public! end - def test_default_url_options_are_used_if_set + def test_default_url_options_override with_routing do |set| set.draw do |map| match 'from_view', :to => 'default_url_options#from_view', :as => :from_view match ':controller/:action' end - assert_deprecated do - get :from_view, :route => "from_view_url" + get :from_view, :route => "from_view_url" - assert_equal 'http://www.override.com/from_view?locale=en', @response.body - assert_equal 'http://www.override.com/from_view?locale=en', @controller.send(:from_view_url) - assert_equal 'http://www.override.com/default_url_options/new?locale=en', @controller.url_for(:controller => 'default_url_options') - end + assert_equal 'http://www.override.com/from_view?locale=en', @response.body + assert_equal 'http://www.override.com/from_view?locale=en', @controller.send(:from_view_url) + assert_equal 'http://www.override.com/default_url_options/new?locale=en', @controller.url_for(:controller => 'default_url_options') end end @@ -232,21 +230,19 @@ class DefaultUrlOptionsTest < ActionController::TestCase match ':controller/:action' end - assert_deprecated do - get :from_view, :route => "description_path(1)" - - assert_equal '/en/descriptions/1', @response.body - assert_equal '/en/descriptions', @controller.send(:descriptions_path) - assert_equal '/pl/descriptions', @controller.send(:descriptions_path, "pl") - assert_equal '/pl/descriptions', @controller.send(:descriptions_path, :locale => "pl") - assert_equal '/pl/descriptions.xml', @controller.send(:descriptions_path, "pl", "xml") - assert_equal '/en/descriptions.xml', @controller.send(:descriptions_path, :format => "xml") - assert_equal '/en/descriptions/1', @controller.send(:description_path, 1) - assert_equal '/pl/descriptions/1', @controller.send(:description_path, "pl", 1) - assert_equal '/pl/descriptions/1', @controller.send(:description_path, 1, :locale => "pl") - assert_equal '/pl/descriptions/1.xml', @controller.send(:description_path, "pl", 1, "xml") - assert_equal '/en/descriptions/1.xml', @controller.send(:description_path, 1, :format => "xml") - end + get :from_view, :route => "description_path(1)" + + assert_equal '/en/descriptions/1', @response.body + assert_equal '/en/descriptions', @controller.send(:descriptions_path) + assert_equal '/pl/descriptions', @controller.send(:descriptions_path, "pl") + assert_equal '/pl/descriptions', @controller.send(:descriptions_path, :locale => "pl") + assert_equal '/pl/descriptions.xml', @controller.send(:descriptions_path, "pl", "xml") + assert_equal '/en/descriptions.xml', @controller.send(:descriptions_path, :format => "xml") + assert_equal '/en/descriptions/1', @controller.send(:description_path, 1) + assert_equal '/pl/descriptions/1', @controller.send(:description_path, "pl", 1) + assert_equal '/pl/descriptions/1', @controller.send(:description_path, 1, :locale => "pl") + assert_equal '/pl/descriptions/1.xml', @controller.send(:description_path, "pl", 1, "xml") + assert_equal '/en/descriptions/1.xml', @controller.send(:description_path, 1, :format => "xml") end end diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb index 533a07b6f4..38ab5521fe 100644 --- a/actionpack/test/template/url_helper_test.rb +++ b/actionpack/test/template/url_helper_test.rb @@ -437,7 +437,7 @@ class UrlHelperControllerTest < ActionController::TestCase end with_url_helper_routing do - assert_deprecated { get :show_named_route, :kind => 'url' } + get :show_named_route, :kind => 'url' assert_equal 'http://testtwo.host/url_helper_controller_test/url_helper/show_named_route', @response.body end end -- cgit v1.2.3