From 49b6b4994eb68a5d9c02478c7ee988101cfb198c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 3 Jan 2012 20:00:40 +0100 Subject: [PATCH] Clean up routes inclusion and add some comments for the next soul that decides to adventure on this code. --- actionpack/lib/action_dispatch/routing.rb | 5 -- .../lib/action_dispatch/routing/route_set.rb | 67 ++++++++++--------- .../lib/action_dispatch/routing/url_for.rb | 23 ++++--- .../controller/action_pack_assertions_test.rb | 14 ---- 4 files changed, 49 insertions(+), 60 deletions(-) diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index fa300b4a16..2f6b9d266d 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -284,10 +284,5 @@ module Routing SEPARATORS = %w( / . ? ) #:nodoc: HTTP_METHODS = [:get, :head, :post, :put, :delete, :options] #:nodoc: - - # A helper module to hold URL related helpers. - module Helpers #:nodoc: - include PolymorphicRoutes - end end end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 10b3e212e6..9d0a3e9993 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -123,11 +123,6 @@ def length routes.length end - def install(destinations = [ActionController::Base, ActionView::Base]) - helper = @module - destinations.each { |d| d.module_eval { include helper } } - end - private def url_helper_name(name, kind = :url) :"#{name}_#{kind}" @@ -276,14 +271,15 @@ def clear! @prepend.each { |blk| eval_block(blk) } end - def install_helpers(destinations) - destinations.each { |d| d.module_eval { include Helpers } } - named_routes.install(destinations) - end - - module MountedHelpers + module MountedHelpers #:nodoc: + extend ActiveSupport::Concern + include UrlFor end + # Contains all the mounted helpers accross different + # engines and the `main_app` helper for the application. + # You can include this in your classes if you want to + # access routes for other engines. def mounted_helpers MountedHelpers end @@ -294,7 +290,7 @@ def define_mounted_helper(name) routes = self MountedHelpers.class_eval do define_method "_#{name}" do - RoutesProxy.new(routes, self._routes_context) + RoutesProxy.new(routes, _routes_context) end end @@ -306,29 +302,40 @@ def #{name} end def url_helpers - routes = self + @url_helpers ||= begin + routes = self + + Module.new do + extend ActiveSupport::Concern + include UrlFor + + # Define url_for in the singleton level so one can do: + # Rails.application.routes.url_helpers.url_for(args) + @_routes = routes + class << self + delegate :url_for, :to => '@_routes' + end - @url_helpers ||= Module.new { - extend ActiveSupport::Concern - include UrlFor + # Make named_routes available in the module singleton + # as well, so one can do: + # Rails.application.routes.url_helpers.posts_path + extend routes.named_routes.module - @_routes = routes - def self.url_for(options) - @_routes.url_for options - end + # Any class that includes this module will get all + # named routes... + include routes.named_routes.module - extend routes.named_routes.module + # plus a singleton class method called _routes ... + included do + singleton_class.send(:redefine_method, :_routes) { routes } + end - # ROUTES TODO: install_helpers isn't great... can we make a module with the stuff that - # we can include? - # Yes plz - JP - included do - routes.install_helpers([self]) - singleton_class.send(:redefine_method, :_routes) { routes } + # And an instance method _routes. Note that + # UrlFor (included in this module) add extra + # conveniences for working with @_routes. + define_method(:_routes) { @_routes || routes } end - - define_method(:_routes) { @_routes || routes } - } + end end def empty? diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 6c2a98ab15..ee6616c5d3 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -8,7 +8,8 @@ module Routing # # Tip: If you need to generate URLs from your models or some other place, # then ActionController::UrlFor is what you're looking for. Read on for - # an introduction. + # an introduction. In general, this module should not be included on its own, + # as it is usually included by url_helpers (as in Rails.application.routes.url_helpers). # # == URL generation from parameters # @@ -84,7 +85,6 @@ module UrlFor include PolymorphicRoutes included do - # 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) @@ -151,16 +151,17 @@ def url_for(options = nil) end protected - def _with_routes(routes) - old_routes, @_routes = @_routes, routes - yield - ensure - @_routes = old_routes - end - def _routes_context - self - end + def _with_routes(routes) + old_routes, @_routes = @_routes, routes + yield + ensure + @_routes = old_routes + end + + def _routes_context + self + end end end end diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index fab70c71d6..bdbf158b36 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -159,20 +159,6 @@ def test_get_post_request_switch assert_equal @response.body, 'request method: GET' end - def test_redirect_to_named_route - with_routing do |set| - set.draw do - match 'route_one', :to => 'action_pack_assertions#nothing', :as => :route_one - match ':controller/:action' - end - set.install_helpers([ActionController::Base, ActionView::Base]) - - process :redirect_to_named_route - assert_redirected_to 'http://test.host/route_one' - assert_redirected_to route_one_url - end - end - def test_string_constraint with_routing do |set| set.draw do -- GitLab