Skip to content
Browse files

Continued effort to deglobalize the router

  • Loading branch information...
1 parent 9a5be2e commit 36fd9efb5e4bfc9ac3acd4189d4dc457dea8102a Carlhuda committed Feb 25, 2010
View
2 actionpack/lib/action_controller.rb
@@ -32,7 +32,7 @@ module ActionController
autoload :SessionManagement
autoload :Streaming
autoload :Testing
- # ROUTES TODO: Proxy UrlFor to Rails.application.routes.url_helpers
+ autoload :UrlFor
autoload :Verification
end
View
9 actionpack/lib/action_controller/base.rb
@@ -9,7 +9,7 @@ class Base < Metal
include ActionController::Helpers
include ActionController::HideActions
- # include ActionController::UrlFor
+ include ActionController::UrlFor
include ActionController::Redirecting
include ActionController::Rendering
include ActionController::Renderers::All
@@ -82,12 +82,5 @@ def self.filter_parameter_logging(*args, &block)
filter
end
- protected
-
- # Overwrite url rewriter to use request.
- def _url_rewriter
- return ActionController::UrlRewriter unless request
- @_url_rewriter ||= ActionController::UrlRewriter.new(request, params)
- end
end
end
View
21 actionpack/lib/action_controller/metal/url_for.rb
@@ -0,0 +1,21 @@
+module ActionController
+ module UrlFor
+ extend ActiveSupport::Concern
+
+ include ActionDispatch::Routing::UrlFor
+ include ActionController::RackDelegation
+
+ def merge_options(options)
+ super.reverse_merge(
+ :host => request.host_with_port,
+ :protocol => request.protocol,
+ :_path_segments => request.symbolized_path_parameters
+ )
+ end
+
+ def _router
+ raise "In order to use #url_for, you must include the helpers of a particular " \
+ "router. For instance, `include Rails.application.router.named_url_helpers"
+ end
+ end
+end
View
1 actionpack/lib/action_controller/railtie.rb
@@ -1,6 +1,7 @@
require "action_controller"
require "rails"
require "action_view/railtie"
+require "active_support/core_ext/class/subclasses"
module ActionController
class Railtie < Rails::Railtie
View
2 actionpack/lib/action_controller/test_case.rb
@@ -336,7 +336,7 @@ def rescue_action_in_public!
private
def build_request_uri(action, parameters)
unless @request.env['REQUEST_URI']
- options = @controller.__send__(:rewrite_options, parameters)
+ options = @controller.__send__(:merge_options, parameters)
options.update(:only_path => true, :action => action)
url = ActionController::UrlRewriter.new(@request, parameters)
View
21 actionpack/lib/action_controller/url_rewriter.rb
@@ -26,8 +26,14 @@ def to_str
# ROUTES TODO: Class method code smell
def self.rewrite(router, options, path_segments=nil)
+ handle_positional_args(options)
+
rewritten_url = ""
+ # ROUTES TODO: Fix the tests
+ segments = options.delete(:_path_segments)
+ path_segments = path_segments ? path_segments.merge(segments || {}) : segments
+
unless options[:only_path]
rewritten_url << (options[:protocol] || "http")
rewritten_url << "://" unless rewritten_url.match("://")
@@ -52,6 +58,21 @@ def self.rewrite(router, options, path_segments=nil)
protected
+ def self.handle_positional_args(options)
+ return unless args = options.delete(:_positional_args)
+
+ keys = options.delete(:_positional_keys)
+ keys -= options.keys if args.size < keys.size - 1 # take format into account
+
+ args = args.zip(keys).inject({}) do |h, (v, k)|
+ h[k] = v
+ h
+ end
+
+ # Tell url_for to skip default_url_options
+ options.merge!(args)
+ end
+
def self.rewrite_authentication(options)
if options[:user] && options[:password]
"#{Rack::Utils.escape(options.delete(:user))}:#{Rack::Utils.escape(options.delete(:password))}@"
View
23 actionpack/lib/action_dispatch/routing/route_set.rb
@@ -194,24 +194,11 @@ def define_url_helper(route, name, kind, options)
# end
@module.module_eval <<-END_EVAL, __FILE__, __LINE__ + 1
def #{selector}(*args)
- if args.empty? || Hash === args.first
- options = #{hash_access_method}(args.first || {})
- else
- options = #{hash_access_method}(args.extract_options!)
- default = default_url_options(options) if self.respond_to?(:default_url_options, true)
- options = (default ||= {}).merge(options)
-
- keys = #{route.segment_keys.inspect}
- keys -= options.keys if args.size < keys.size - 1 # take format into account
-
- args = args.zip(keys).inject({}) do |h, (v, k)|
- h[k] = v
- h
- end
-
- # Tell url_for to skip default_url_options
- options[:use_defaults] = false
- options.merge!(args)
+ options = #{hash_access_method}(args.extract_options!)
+
+ if args.any?
+ options[:_positional_args] = args
+ options[:_positional_keys] = #{route.segment_keys.inspect}
end
url_for(options)
View
31 actionpack/lib/action_dispatch/routing/url_for.rb
@@ -85,8 +85,6 @@ module UrlFor
extend ActiveSupport::Concern
included do
- # ActionDispatch::Routing::Routes.install_helpers(self)
-
# Including in a class uses an inheritable hash. Modules get a plain hash.
if respond_to?(:class_attribute)
class_attribute :default_url_options
@@ -97,23 +95,16 @@ module UrlFor
self.default_url_options = {}
end
- # Overwrite to implement a number of default options that all url_for-based methods will use. The default options should come in
- # the form of a hash, just like the one you would use for url_for directly. Example:
- #
- # def default_url_options(options)
- # { :project => @project.active? ? @project.url_name : "unknown" }
- # end
- #
- # As you can infer from the example, this is mostly useful for situations where you want to centralize dynamic decisions about the
- # urls as they stem from the business domain. Please note that any individual url_for call can always override the defaults set
- # by this method.
def default_url_options(options = nil)
- # ROUTES TODO: This should probably be an instance method
self.class.default_url_options
end
- def rewrite_options(options) #:nodoc:
- if options.delete(:use_defaults) != false && (defaults = default_url_options(options))
+ def url_options
+ self.class.default_url_options.merge
+ end
+
+ def merge_options(options) #:nodoc:
+ if options.delete(:use_defaults) != false && respond_to?(:default_url_options) && (defaults = default_url_options(options))
defaults.merge(options)
else
options
@@ -149,19 +140,11 @@ def url_for(options = {})
when String
options
when Hash
- _url_rewriter.rewrite(_router, rewrite_options(options))
+ ActionController::UrlRewriter.rewrite(_router, merge_options(options))
else
polymorphic_url(options)
end
end
-
- protected
-
- # ROUTES TODO: Figure out why _url_rewriter is sometimes the class and
- # sometimes an instance.
- def _url_rewriter
- ActionController::UrlRewriter
- end
end
end
end
View
1 actionpack/test/controller/base_test.rb
@@ -220,6 +220,7 @@ def test_ensure_url_for_works_as_expected_when_called_with_no_options_if_default
def test_named_routes_with_path_without_doing_a_request_first
@controller = EmptyController.new
+ @controller.request = @request
with_routing do |set|
set.draw do |map|
View
34 actionpack/test/controller/routing_test.rb
@@ -52,29 +52,11 @@ def test_route_generation_allows_passing_non_string_values_to_generated_helper
end
class MockController
- attr_accessor :routes
-
- def initialize(routes)
- self.routes = routes
- end
-
def url_for(options)
- only_path = options.delete(:only_path)
-
- port = options.delete(:port) || 80
- port_string = port == 80 ? '' : ":#{port}"
-
- protocol = options.delete(:protocol) || "http"
- host = options.delete(:host) || "test.host"
- anchor = "##{options.delete(:anchor)}" if options.key?(:anchor)
+ options[:protocol] ||= "http"
+ options[:host] ||= "test.host"
- path = routes.generate(options)
-
- only_path ? "#{path}#{anchor}" : "#{protocol}://#{host}#{port_string}#{path}#{anchor}"
- end
-
- def request
- @request ||= ActionController::TestRequest.new
+ super(options)
end
end
@@ -268,9 +250,9 @@ def test_optimised_named_route_with_host
end
def setup_for_named_route
- klass = Class.new(MockController)
- rs.install_helpers(klass)
- klass.new(rs)
+ inst = MockController.clone.new
+ inst.class.send(:include, rs.named_url_helpers)
+ inst
end
def test_named_route_without_hash
@@ -759,9 +741,7 @@ def setup_named_route_test
map.users '/admin/users', :controller => 'admin/users', :action => 'index'
end
- klass = Class.new(MockController)
- set.install_helpers(klass)
- klass.new(set)
+ MockController.clone.new.tap { |inst| inst.class.send(:include, set.named_url_helpers)}
end
def test_named_route_hash_access_method

1 comment on commit 36fd9ef

@josh

that meta shit was a total smell.

btw, we should probably rebenchmark name route helpers to make sure we aren't regressing in pref

Please sign in to comment.
Something went wrong with that request. Please try again.