diff options
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 64 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/polymorphic_routes.rb | 27 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/route_set.rb | 106 | ||||
| -rw-r--r-- | actionpack/test/dispatch/routing/direct_url_helpers_test.rb (renamed from actionpack/test/dispatch/routing/custom_url_helpers_test.rb) | 72 | ||||
| -rw-r--r-- | actionview/lib/action_view/test_case.rb | 8 | ||||
| -rw-r--r-- | railties/test/application/routing_test.rb | 102 |
6 files changed, 314 insertions, 65 deletions
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 3756ef15a2..6123a1f5f5 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -2021,8 +2021,8 @@ module ActionDispatch end module DirectUrls - # Define a custom url helper that will be added to the url helpers - # module. This allows you override and/or replace the default behavior + # Define custom routing behavior that will be added to the application's + # routes. This allows you override and/or replace the default behavior # of routing helpers, e.g: # # direct :homepage do @@ -2037,8 +2037,35 @@ module ActionDispatch # { controller: 'pages', action: 'index', subdomain: 'www' } # end # - # The return value must be a valid set of arguments for `url_for` which - # will actually build the url string. This can be one of the following: + # The above example show how to define a custom url helper but it's also + # possible to alter the behavior of `polymorphic_url` and consequently the + # behavior of `link_to` and `form_for` when passed a model instance, e.g: + # + # direct class: "Basket" do + # [:basket] + # end + # + # NOTE: This custom behavior only applies to simple polymorphic urls where + # a single model instance is passed and not more complicated forms, e.g: + # + # # config/routes.rb + # resource :profile + # namespace :admin do + # resources :users + # end + # + # direct(class: "User") { [:profile] } + # + # # app/views/application/_menu.html.erb + # link_to 'Profile', @current_user + # link_to 'Profile', [:admin, @current_user] + # + # The first `link_to` will generate '/profile' but the second will generate + # the standard polymorphic url of '/admin/users/1'. + # + # The return value from the block passed to `direct` must be a valid set of + # arguments for `url_for` which will actually build the url string. This can + # be one of the following: # # * A string, which is treated as a generated url # * A hash, e.g. { controller: 'pages', action: 'index' } @@ -2046,6 +2073,9 @@ module ActionDispatch # * An Active Model instance # * An Active Model class # + # NOTE: Other url helpers can be called in the block but be careful not to invoke + # your custom url helper again otherwise it will result in a stack overflow error + # # You can also specify default options that will be passed through to # your url helper definition, e.g: # @@ -2053,14 +2083,28 @@ module ActionDispatch # [ :products, options.merge(params.permit(:page, :size)) ] # end # - # NOTE: It is the url helper's responsibility to return the correct - # set of options to be passed to the `url_for` call. - def direct(name, options = nil, &block) - case name + # You can pass options to a polymorphic mapping do - the arity for the block + # needs to be two as the instance is passed as the first argument, e.g: + # + # direct class: 'Basket', anchor: 'items' do |basket, options| + # [:basket, options] + # end + # + # This generates the url '/basket#items' because when the last item in an + # array passed to `polymorphic_url` is a hash then it's treated as options + # to the url helper that gets called. + # + # NOTE: The `direct` method doesn't observe the current scope in routes.rb + # and because of this it's recommended to define them outside of any blocks + # such as `namespace` or `scope`. + def direct(name_or_hash, options = nil, &block) + case name_or_hash + when Hash + @set.add_polymorphic_mapping(name_or_hash, &block) when String, Symbol - @set.add_url_helper(name, options, &block) + @set.add_url_helper(name_or_hash, options, &block) else - raise ArgumentError, "The direct method only accepts a string or symbol" + raise ArgumentError, "The direct method only accepts a hash, string or symbol" end end end diff --git a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb index 432b9bf4c1..512e23c833 100644 --- a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb +++ b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb @@ -103,6 +103,10 @@ module ActionDispatch return polymorphic_url record, options end + if mapping = polymorphic_mapping(record_or_hash_or_array) + return mapping.call(self, [record_or_hash_or_array], options) + end + opts = options.dup action = opts.delete :action type = opts.delete(:routing_type) || :url @@ -123,6 +127,10 @@ module ActionDispatch return polymorphic_path record, options end + if mapping = polymorphic_mapping(record_or_hash_or_array) + return mapping.call(self, [record_or_hash_or_array], options, only_path: true) + end + opts = options.dup action = opts.delete :action type = :path @@ -156,6 +164,11 @@ module ActionDispatch polymorphic_path(record_or_hash, options.merge(action: action)) end + def polymorphic_mapping(record) + return false unless record.respond_to?(:to_model) + _routes.polymorphic_mappings[record.to_model.model_name.name] + end + class HelperMethodBuilder # :nodoc: CACHE = { "path" => {}, "url" => {} } @@ -255,9 +268,13 @@ module ActionDispatch [named_route, args] end - def handle_model_call(target, model) - method, args = handle_model model - target.send(method, *args) + def handle_model_call(target, record) + if mapping = polymorphic_mapping(target, record) + mapping.call(target, [record], {}, only_path: suffix == "path") + else + method, args = handle_model(record) + target.send(method, *args) + end end def handle_list(list) @@ -303,6 +320,10 @@ module ActionDispatch private + def polymorphic_mapping(target, record) + target._routes.polymorphic_mappings[record.to_model.model_name.name] + end + def get_method_for_class(klass) name = @key_strategy.call klass.model_name get_method_for_string name diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 923d063655..8bdf0d1a53 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -160,7 +160,7 @@ module ActionDispatch def add_url_helper(name, defaults, &block) @custom_helpers << name - helper = CustomUrlHelper.new(name, defaults, &block) + helper = DirectUrlHelper.new(name, defaults, &block) @path_helpers_module.module_eval do define_method(:"#{name}_path") do |*args| @@ -177,47 +177,6 @@ module ActionDispatch end end - class CustomUrlHelper - attr_reader :name, :defaults, :block - - def initialize(name, defaults, &block) - @name = name - @defaults = defaults - @block = block - end - - def call(t, args, options, outer_options = {}) - url_options = eval_block(t, args, options) - - case url_options - when String - t.url_for(url_options) - when Hash - t.url_for(url_options.merge(outer_options)) - when ActionController::Parameters - if url_options.permitted? - t.url_for(url_options.to_h.merge(outer_options)) - else - raise ArgumentError, "Generating an URL from non sanitized request parameters is insecure!" - end - when Array - opts = url_options.extract_options! - t.url_for(url_options.push(opts.merge(outer_options))) - else - t.url_for([url_options, outer_options]) - end - end - - private - def eval_block(t, args, options) - t.instance_exec(*args, merge_defaults(options), &block) - end - - def merge_defaults(options) - defaults ? defaults.merge(options) : options - end - end - class UrlHelper def self.create(route, options, route_name, url_strategy) if optimize_helper?(route) @@ -380,7 +339,7 @@ module ActionDispatch attr_accessor :formatter, :set, :named_routes, :default_scope, :router attr_accessor :disable_clear_and_finalize, :resources_path_names attr_accessor :default_url_options - attr_reader :env_key + attr_reader :env_key, :polymorphic_mappings alias :routes :set @@ -422,6 +381,7 @@ module ActionDispatch @set = Journey::Routes.new @router = Journey::Router.new @set @formatter = Journey::Formatter.new self + @polymorphic_mappings = {} end def eager_load! @@ -483,6 +443,7 @@ module ActionDispatch named_routes.clear set.clear formatter.clear + @polymorphic_mappings.clear @prepend.each { |blk| eval_block(blk) } end @@ -554,6 +515,14 @@ module ActionDispatch @_proxy.optimize_routes_generation? end + def polymorphic_url(record_or_hash_or_array, options = {}) + @_proxy.polymorphic_url(record_or_hash_or_array, options) + end + + def polymorphic_path(record_or_hash_or_array, options = {}) + @_proxy.polymorphic_path(record_or_hash_or_array, options) + end + def _routes; @_proxy._routes; end def url_options; {}; end end @@ -629,10 +598,61 @@ module ActionDispatch route end + def add_polymorphic_mapping(options, &block) + defaults = options.dup + klass = defaults.delete(:class) + if klass.nil? + raise ArgumentError, "Missing :class key from polymorphic mapping options" + end + + @polymorphic_mappings[klass] = DirectUrlHelper.new(klass, defaults, &block) + end + def add_url_helper(name, options, &block) named_routes.add_url_helper(name, options, &block) end + class DirectUrlHelper + attr_reader :name, :defaults, :block + + def initialize(name, defaults, &block) + @name = name + @defaults = defaults + @block = block + end + + def call(t, args, options, outer_options = {}) + url_options = eval_block(t, args, options) + + case url_options + when String + t.url_for(url_options) + when Hash + t.url_for(url_options.merge(outer_options)) + when ActionController::Parameters + if url_options.permitted? + t.url_for(url_options.to_h.merge(outer_options)) + else + raise ArgumentError, "Generating an URL from non sanitized request parameters is insecure!" + end + when Array + opts = url_options.extract_options! + t.url_for(url_options.push(opts.merge(outer_options))) + else + t.url_for([url_options, outer_options]) + end + end + + private + def eval_block(t, args, options) + t.instance_exec(*args, merge_defaults(options), &block) + end + + def merge_defaults(options) + defaults ? defaults.merge(options) : options + end + end + class Generator PARAMETERIZE = lambda do |name, value| if name == :controller diff --git a/actionpack/test/dispatch/routing/custom_url_helpers_test.rb b/actionpack/test/dispatch/routing/direct_url_helpers_test.rb index 22e8bbf21e..56bb7f13b3 100644 --- a/actionpack/test/dispatch/routing/custom_url_helpers_test.rb +++ b/actionpack/test/dispatch/routing/direct_url_helpers_test.rb @@ -1,6 +1,6 @@ require "abstract_unit" -class TestCustomUrlHelpers < ActionDispatch::IntegrationTest +class TestDirectUrlHelpers < ActionDispatch::IntegrationTest class Linkable attr_reader :id @@ -17,12 +17,37 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest class Collection < Linkable; end class Product < Linkable; end + class Model + extend ActiveModel::Naming + include ActiveModel::Conversion + + attr_reader :id + + def initialize(id = nil) + @id = id + end + + def model_name + @_model_name ||= ActiveModel::Name.new(self.class, nil, self.class.name.demodulize) + end + + def persisted? + false + end + end + + class Basket < Model; end + class User < Model; end + class Video < Model; end + Routes = ActionDispatch::Routing::RouteSet.new Routes.draw do default_url_options host: "www.example.com" root to: "pages#index" get "/basket", to: "basket#show", as: :basket + get "/profile", to: "users#profile", as: :profile + get "/media/:id", to: "media#show", as: :media resources :categories, :collections, :products @@ -40,6 +65,10 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest direct(:array) { [:admin, :dashboard] } direct(:options) { |options| [:products, options] } direct(:defaults, size: 10) { |options| [:products, options] } + + direct(class: "Basket") { |basket| [:basket] } + direct(class: "User", anchor: "details") { |user, options| [:profile, options] } + direct(class: "Video") { |video| [:media, { id: video.id }] } end APP = build_app Routes @@ -54,6 +83,9 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest @category = Category.new("1") @collection = Collection.new("2") @product = Product.new("3") + @basket = Basket.new + @user = User.new + @video = Video.new("4") @path_params = { "controller" => "pages", "action" => "index" } @unsafe_params = ActionController::Parameters.new(@path_params) @safe_params = ActionController::Parameters.new(@path_params).permit(:controller, :action) @@ -94,6 +126,19 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest assert_equal "/products?size=10", Routes.url_helpers.defaults_path assert_equal "/products?size=20", defaults_path(size: 20) assert_equal "/products?size=20", Routes.url_helpers.defaults_path(size: 20) + + assert_equal "/basket", polymorphic_path(@basket) + assert_equal "/basket", Routes.url_helpers.polymorphic_path(@basket) + + assert_equal "/profile#details", polymorphic_path(@user) + assert_equal "/profile#details", Routes.url_helpers.polymorphic_path(@user) + + assert_equal "/profile#password", polymorphic_path(@user, anchor: "password") + assert_equal "/profile#password", Routes.url_helpers.polymorphic_path(@user, anchor: "password") + + assert_equal "/media/4", polymorphic_path(@video) + assert_equal "/media/4", Routes.url_helpers.polymorphic_path(@video) + assert_equal "/media/4", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_model_call(self, @video) end def test_direct_urls @@ -131,6 +176,21 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest assert_equal "http://www.example.com/products?size=10", Routes.url_helpers.defaults_url assert_equal "http://www.example.com/products?size=20", defaults_url(size: 20) assert_equal "http://www.example.com/products?size=20", Routes.url_helpers.defaults_url(size: 20) + + assert_equal "http://www.example.com/basket", polymorphic_url(@basket) + assert_equal "http://www.example.com/basket", Routes.url_helpers.polymorphic_url(@basket) + assert_equal "http://www.example.com/basket", polymorphic_url(@basket) + assert_equal "http://www.example.com/basket", Routes.url_helpers.polymorphic_url(@basket) + + assert_equal "http://www.example.com/profile#details", polymorphic_url(@user) + assert_equal "http://www.example.com/profile#details", Routes.url_helpers.polymorphic_url(@user) + + assert_equal "http://www.example.com/profile#password", polymorphic_url(@user, anchor: "password") + assert_equal "http://www.example.com/profile#password", Routes.url_helpers.polymorphic_url(@user, anchor: "password") + + assert_equal "http://www.example.com/media/4", polymorphic_url(@video) + assert_equal "http://www.example.com/media/4", Routes.url_helpers.polymorphic_url(@video) + assert_equal "http://www.example.com/media/4", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.url.handle_model_call(self, @video) end def test_raises_argument_error @@ -142,4 +202,14 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest end end end + + def test_missing_class_raises_argument_error + routes = ActionDispatch::Routing::RouteSet.new + + assert_raises ArgumentError do + routes.draw do + direct(fragment: "core") { "http://www.rubyonrails.org" } + end + end + end end diff --git a/actionview/lib/action_view/test_case.rb b/actionview/lib/action_view/test_case.rb index 2b981caa65..ae4fec4337 100644 --- a/actionview/lib/action_view/test_case.rb +++ b/actionview/lib/action_view/test_case.rb @@ -124,6 +124,10 @@ module ActionView @_rendered_views ||= RenderedViewsCollection.new end + def _routes + @controller._routes if @controller.respond_to?(:_routes) + end + # Need to experiment if this priority is the best one: rendered => output_buffer class RenderedViewsCollection def initialize @@ -258,10 +262,6 @@ module ActionView end] end - def _routes - @controller._routes if @controller.respond_to?(:_routes) - end - def method_missing(selector, *args) begin routes = @controller.respond_to?(:_routes) && @controller._routes diff --git a/railties/test/application/routing_test.rb b/railties/test/application/routing_test.rb index 98673f5c1a..6c34c72564 100644 --- a/railties/test/application/routing_test.rb +++ b/railties/test/application/routing_test.rb @@ -264,9 +264,9 @@ module ApplicationTests end { - "development" => ["baz", "http://www.apple.com"], - "production" => ["bar", "http://www.microsoft.com"] - }.each do |mode, (expected_action, expected_url)| + "development" => ["baz", "http://www.apple.com", "/dashboard"], + "production" => ["bar", "http://www.microsoft.com", "/profile"] + }.each do |mode, (expected_action, expected_url, expected_mapping)| test "reloads routes when configuration is changed in #{mode}" do controller :foo, <<-RUBY class FooController < ApplicationController @@ -281,6 +281,25 @@ module ApplicationTests def custom render plain: custom_url end + + def mapping + render plain: url_for(User.new) + end + end + RUBY + + app_file "app/models/user.rb", <<-RUBY + class User + extend ActiveModel::Naming + include ActiveModel::Conversion + + def model_name + @_model_name ||= ActiveModel::Name.new(self.class, nil, "User") + end + + def persisted? + false + end end RUBY @@ -288,8 +307,10 @@ module ApplicationTests Rails.application.routes.draw do get 'foo', to: 'foo#bar' get 'custom', to: 'foo#custom' + get 'mapping', to: 'foo#mapping' direct(:custom) { "http://www.microsoft.com" } + direct(class: "User") { "/profile" } end RUBY @@ -301,12 +322,17 @@ module ApplicationTests get "/custom" assert_equal "http://www.microsoft.com", last_response.body + get "/mapping" + assert_equal "/profile", last_response.body + app_file "config/routes.rb", <<-RUBY Rails.application.routes.draw do get 'foo', to: 'foo#baz' get 'custom', to: 'foo#custom' + get 'mapping', to: 'foo#mapping' direct(:custom) { "http://www.apple.com" } + direct(class: "User") { "/dashboard" } end RUBY @@ -317,6 +343,9 @@ module ApplicationTests get "/custom" assert_equal expected_url, last_response.body + + get "/mapping" + assert_equal expected_mapping, last_response.body end end @@ -379,7 +408,11 @@ module ApplicationTests end def custom - render text: custom_url + render plain: custom_url + end + + def mapping + render plain: url_for(User.new) end end RUBY @@ -392,6 +425,21 @@ module ApplicationTests end RUBY + app_file "app/models/user.rb", <<-RUBY + class User + extend ActiveModel::Naming + include ActiveModel::Conversion + + def model_name + @_model_name ||= ActiveModel::Name.new(self.class, nil, "User") + end + + def persisted? + false + end + end + RUBY + app_file "config/routes.rb", <<-RUBY Rails.application.routes.draw do get 'foo', to: 'foo#index' @@ -412,6 +460,12 @@ module ApplicationTests Rails.application.routes.draw do get 'foo', to: 'foo#index' get 'bar', to: 'bar#index' + + get 'custom', to: 'foo#custom' + direct(:custom) { 'http://www.apple.com' } + + get 'mapping', to: 'foo#mapping' + direct(class: 'User') { '/profile' } end RUBY @@ -425,6 +479,14 @@ module ApplicationTests assert_equal "bar", last_response.body assert_equal "/bar", Rails.application.routes.url_helpers.bar_path + get "/custom" + assert_equal "http://www.apple.com", last_response.body + assert_equal "http://www.apple.com", Rails.application.routes.url_helpers.custom_url + + get "/mapping" + assert_equal "/profile", last_response.body + assert_equal "/profile", Rails.application.routes.url_helpers.polymorphic_path(User.new) + app_file "config/routes.rb", <<-RUBY Rails.application.routes.draw do get 'foo', to: 'foo#index' @@ -442,6 +504,18 @@ module ApplicationTests assert_raises NoMethodError do assert_equal "/bar", Rails.application.routes.url_helpers.bar_path end + + get "/custom" + assert_equal 404, last_response.status + assert_raises NoMethodError do + assert_equal "http://www.apple.com", Rails.application.routes.url_helpers.custom_url + end + + get "/mapping" + assert_equal 404, last_response.status + assert_raises NoMethodError do + assert_equal "/profile", Rails.application.routes.url_helpers.polymorphic_path(User.new) + end end test "named routes are cleared when reloading" do @@ -463,10 +537,27 @@ module ApplicationTests end RUBY + app_file "app/models/user.rb", <<-RUBY + class User + extend ActiveModel::Naming + include ActiveModel::Conversion + + def model_name + @_model_name ||= ActiveModel::Name.new(self.class, nil, "User") + end + + def persisted? + false + end + end + RUBY + app_file "config/routes.rb", <<-RUBY Rails.application.routes.draw do get ':locale/foo', to: 'foo#index', as: 'foo' + get 'users', to: 'foo#users', as: 'users' direct(:microsoft) { 'http://www.microsoft.com' } + direct(class: 'User') { '/profile' } end RUBY @@ -474,10 +565,12 @@ module ApplicationTests assert_equal "foo", last_response.body assert_equal "/en/foo", Rails.application.routes.url_helpers.foo_path(locale: "en") assert_equal "http://www.microsoft.com", Rails.application.routes.url_helpers.microsoft_url + assert_equal "/profile", Rails.application.routes.url_helpers.polymorphic_path(User.new) app_file "config/routes.rb", <<-RUBY Rails.application.routes.draw do get ':locale/bar', to: 'bar#index', as: 'foo' + get 'users', to: 'foo#users', as: 'users' direct(:apple) { 'http://www.apple.com' } end RUBY @@ -491,6 +584,7 @@ module ApplicationTests assert_equal "bar", last_response.body assert_equal "/en/bar", Rails.application.routes.url_helpers.foo_path(locale: "en") assert_equal "http://www.apple.com", Rails.application.routes.url_helpers.apple_url + assert_equal "/users", Rails.application.routes.url_helpers.polymorphic_path(User.new) assert_raises NoMethodError do assert_equal "http://www.microsoft.com", Rails.application.routes.url_helpers.microsoft_url |