Merge branch 'master' into bug/filtered_parameters_class

6 files changed, 301 insertions, 36 deletions

diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb
index b91ffb8419..9aa4b92df2 100644
--- a/actionpack/lib/action_dispatch/routing/inspector.rb
+++ b/actionpack/lib/action_dispatch/routing/inspector.rb
@@ -196,7 +196,7 @@ module ActionDispatch
         @buffer << @view.render(partial: "routes/route", collection: routes)
       end
 
-      # the header is part of the HTML page, so we don't construct it here.
+      # The header is part of the HTML page, so we don't construct it here.
       def header(routes)
       end
 
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 8d9f70e3c6..8ad17504ae 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -17,9 +17,9 @@ module ActionDispatch
         CALL  = ->(app, req) { app.call req.env }
 
         def initialize(app, constraints, strategy)
-          # Unwrap Constraints objects.  I don't actually think it's possible
+          # Unwrap Constraints objects. I don't actually think it's possible
           # to pass a Constraints object to this constructor, but there were
-          # multiple places that kept testing children of this object.  I
+          # multiple places that kept testing children of this object. I
           # *think* they were just being defensive, but I have no idea.
           if app.is_a?(self.class)
             constraints += app.constraints
@@ -218,7 +218,7 @@ module ActionDispatch
         private
           def add_wildcard_options(options, formatted, path_ast)
             # Add a constraint for wildcard route to make it non-greedy and match the
-            # optional format part of the route by default
+            # optional format part of the route by default.
             if formatted != false
               path_ast.grep(Journey::Nodes::Star).each_with_object({}) { |node, hash|
                 hash[node.name.to_sym] ||= /.+?/
@@ -396,7 +396,7 @@ module ActionDispatch
       end
 
       module Base
-        # Matches a url pattern to one or more routes.
+        # Matches a URL pattern to one or more routes.
         #
         # You should not use the +match+ method in your router
         # without specifying an HTTP method.
@@ -406,7 +406,7 @@ module ActionDispatch
         #   # sets :controller, :action and :id in params
         #   match ':controller/:action/:id', via: [:get, :post]
         #
-        # Note that +:controller+, +:action+ and +:id+ are interpreted as url
+        # Note that +:controller+, +:action+ and +:id+ are interpreted as URL
         # query parameters and thus available through +params+ in an action.
         #
         # If you want to expose your action to GET, use +get+ in the router:
@@ -455,7 +455,7 @@ module ActionDispatch
         #
         # === Options
         #
-        # Any options not seen here are passed on as params with the url.
+        # Any options not seen here are passed on as params with the URL.
         #
         # [:controller]
         #   The route's controller.
@@ -660,7 +660,7 @@ module ActionDispatch
                 else
                   prefix_options = options.slice(*_route.segment_keys)
                   prefix_options[:relative_url_root] = "".freeze
-                  # we must actually delete prefix segment keys to avoid passing them to next url_for
+                  # We must actually delete prefix segment keys to avoid passing them to next url_for.
                   _route.segment_keys.each { |k| options.delete(k) }
                   _routes.url_helpers.send("#{name}_path", prefix_options)
                 end
@@ -1238,7 +1238,7 @@ module ActionDispatch
         #
         #   resource :profile
         #
-        # creates six different routes in your application, all mapping to
+        # This creates six different routes in your application, all mapping to
         # the +Profiles+ controller (note that the controller is named after
         # the plural):
         #
@@ -1323,14 +1323,14 @@ module ActionDispatch
         #
         #     resources :posts, path_names: { new: "brand_new" }
         #
-        #   The above example will now change /posts/new to /posts/brand_new
+        #   The above example will now change /posts/new to /posts/brand_new.
         #
         # [:path]
         #   Allows you to change the path prefix for the resource.
         #
         #     resources :posts, path: 'postings'
         #
-        #   The resource and all segments will now route to /postings instead of /posts
+        #   The resource and all segments will now route to /postings instead of /posts.
         #
         # [:only]
         #   Only generate routes for the given actions.
@@ -1525,7 +1525,7 @@ module ActionDispatch
           end
         end
 
-        # See ActionDispatch::Routing::Mapper::Scoping#namespace
+        # See ActionDispatch::Routing::Mapper::Scoping#namespace.
         def namespace(path, options = {})
           if resource_scope?
             nested { super }
@@ -1545,7 +1545,7 @@ module ActionDispatch
           !parent_resource.singleton? && @scope[:shallow]
         end
 
-        # Matches a url pattern to one or more routes.
+        # Matches a URL pattern to one or more routes.
         # For more information, see match[rdoc-ref:Base#match].
         #
         #   match 'path' => 'controller#action', via: patch
@@ -1904,7 +1904,7 @@ module ActionDispatch
             ast = Journey::Parser.parse path
 
             mapping = Mapping.build(@scope, @set, ast, controller, default_action, to, via, formatted, options_constraints, anchor, options)
-            @set.add_route(mapping, ast, as, anchor)
+            @set.add_route(mapping, as)
           end
 
           def match_root_route(options)
@@ -2003,7 +2003,7 @@ module ActionDispatch
         #     concerns :commentable
         #   end
         #
-        # concerns also work in any routes helper that you want to use:
+        # Concerns also work in any routes helper that you want to use:
         #
         #   namespace :posts do
         #     concerns :commentable
@@ -2020,6 +2020,120 @@ module ActionDispatch
         end
       end
 
+      module CustomUrls
+        # Define custom url helpers that will be added to the application's
+        # routes. This allows you to override and/or replace the default behavior
+        # of routing helpers, e.g:
+        #
+        #   direct :homepage do
+        #     "http://www.rubyonrails.org"
+        #   end
+        #
+        #   direct :commentable do |model|
+        #     [ model, anchor: model.dom_id ]
+        #   end
+        #
+        #   direct :main do
+        #     { controller: "pages", action: "index", subdomain: "www" }
+        #   end
+        #
+        # 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" }
+        #   * An array, which is passed to `polymorphic_url`
+        #   * 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:
+        #
+        #   direct :browse, page: 1, size: 10 do |options|
+        #     [ :products, options.merge(params.permit(:page, :size).to_h.symbolize_keys) ]
+        #   end
+        #
+        # In this instance the `params` object comes from the context in which the the
+        # block is executed, e.g. generating a URL inside a controller action or a view.
+        # If the block is executed where there isn't a params object such as this:
+        #
+        #   Rails.application.routes.url_helpers.browse_path
+        #
+        # then it will raise a `NameError`. Because of this you need to be aware of the
+        # context in which you will use your custom URL helper when defining it.
+        #
+        # NOTE: The `direct` method can't be used inside of a scope block such as
+        # `namespace` or `scope` and will raise an error if it detects that it is.
+        def direct(name, options = {}, &block)
+          unless @scope.root?
+            raise RuntimeError, "The direct method can't be used inside a routes scope block"
+          end
+
+          @set.add_url_helper(name, options, &block)
+        end
+
+        # Define custom polymorphic mappings of models to URLs. This alters the
+        # behavior of `polymorphic_url` and consequently the behavior of
+        # `link_to` and `form_for` when passed a model instance, e.g:
+        #
+        #   resource :basket
+        #
+        #   resolve "Basket" do
+        #     [:basket]
+        #   end
+        #
+        # This will now generate "/basket" when a `Basket` instance is passed to
+        # `link_to` or `form_for` instead of the standard "/baskets/:id".
+        #
+        # 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
+        #
+        #   resolve("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".
+        #
+        # You can pass options to a polymorphic mapping - the arity for the block
+        # needs to be two as the instance is passed as the first argument, e.g:
+        #
+        #   resolve "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 `resolve` method can't be used inside of a scope block such as
+        # `namespace` or `scope` and will raise an error if it detects that it is.
+        def resolve(*args, &block)
+          unless @scope.root?
+            raise RuntimeError, "The resolve method can't be used inside a routes scope block"
+          end
+
+          options = args.extract_options!
+          args = args.flatten(1)
+
+          args.each do |klass|
+            @set.add_polymorphic_mapping(klass, options, &block)
+          end
+        end
+      end
+
       class Scope # :nodoc:
         OPTIONS = [:path, :shallow_path, :as, :shallow_prefix, :module,
                    :controller, :action, :path_names, :constraints,
@@ -2040,6 +2154,14 @@ module ActionDispatch
           scope_level == :nested
         end
 
+        def null?
+          @hash.nil? && @parent.nil?
+        end
+
+        def root?
+          @parent.null?
+        end
+
         def resources?
           scope_level == :resources
         end
@@ -2113,6 +2235,7 @@ module ActionDispatch
       include Scoping
       include Concerns
       include Resources
+      include CustomUrls
     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..e89ea8b21d 100644
--- a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb
+++ b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb
@@ -40,7 +40,7 @@ module ActionDispatch
     #
     # Example usage:
     #
-    #   edit_polymorphic_path(@post)              # => "/posts/1/edit"
+    #   edit_polymorphic_path(@post)           # => "/posts/1/edit"
     #   polymorphic_path(@post, format: :pdf)  # => "/posts/1.pdf"
     #
     # == Usage with mounted engines
@@ -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], false)
+        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], true)
+        end
+
         opts   = options.dup
         action = opts.delete :action
         type   = :path
@@ -156,6 +164,14 @@ module ActionDispatch
           polymorphic_path(record_or_hash, options.merge(action: action))
         end
 
+        def polymorphic_mapping(record)
+          if record.respond_to?(:to_model)
+            _routes.polymorphic_mappings[record.to_model.model_name.name]
+          else
+            _routes.polymorphic_mappings[record.class.name]
+          end
+        end
+
         class HelperMethodBuilder # :nodoc:
           CACHE = { "path" => {}, "url" => {} }
 
@@ -255,9 +271,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], suffix == "path")
+            else
+              method, args = handle_model(record)
+              target.send(method, *args)
+            end
           end
 
           def handle_list(list)
@@ -303,6 +323,14 @@ module ActionDispatch
 
           private
 
+            def polymorphic_mapping(target, record)
+              if record.respond_to?(:to_model)
+                target._routes.polymorphic_mappings[record.to_model.model_name.name]
+              else
+                target._routes.polymorphic_mappings[record.class.name]
+              end
+            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/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb
index dabc045007..3bcb341758 100644
--- a/actionpack/lib/action_dispatch/routing/redirection.rb
+++ b/actionpack/lib/action_dispatch/routing/redirection.rb
@@ -36,6 +36,8 @@ module ActionDispatch
         uri.host   ||= req.host
         uri.port   ||= req.port unless req.standard_port?
 
+        req.commit_flash
+
         body = %(<html><body>You are being <a href="#{ERB::Util.unwrapped_html_escape(uri.to_s)}">redirected</a>.</body></html>)
 
         headers = {
@@ -144,7 +146,7 @@ module ActionDispatch
       #
       #   get 'docs/:article', to: redirect('/wiki/%{article}')
       #
-      # Note that if you return a path without a leading slash then the url is prefixed with the
+      # Note that if you return a path without a leading slash then the URL is prefixed with the
       # current SCRIPT_NAME environment variable. This is typically '/' but may be different in
       # a mounted engine or where the application is deployed to a subdirectory of a website.
       #
@@ -163,7 +165,7 @@ module ActionDispatch
       # Note that the +do end+ syntax for the redirect block wouldn't work, as Ruby would pass
       # the block to +get+ instead of +redirect+. Use <tt>{ ... }</tt> instead.
       #
-      # The options version of redirect allows you to supply only the parts of the url which need
+      # The options version of redirect allows you to supply only the parts of the URL which need
       # to change, it also supports interpolation of the path similar to the first example.
       #
       #   get 'stores/:name',       to: redirect(subdomain: 'stores', path: '/%{name}')
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 5b873aeab7..129e90037e 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -73,6 +73,7 @@ module ActionDispatch
           @routes = {}
           @path_helpers = Set.new
           @url_helpers = Set.new
+          @custom_helpers = Set.new
           @url_helpers_module  = Module.new
           @path_helpers_module = Module.new
         end
@@ -88,16 +89,30 @@ module ActionDispatch
 
         def clear!
           @path_helpers.each do |helper|
-            @path_helpers_module.send :undef_method, helper
+            @path_helpers_module.send :remove_method, helper
           end
 
           @url_helpers.each do |helper|
-            @url_helpers_module.send  :undef_method, helper
+            @url_helpers_module.send  :remove_method, helper
+          end
+
+          @custom_helpers.each do |helper|
+            path_name = :"#{helper}_path"
+            url_name = :"#{helper}_url"
+
+            if @path_helpers_module.method_defined?(path_name)
+              @path_helpers_module.send :remove_method, path_name
+            end
+
+            if @url_helpers_module.method_defined?(url_name)
+              @url_helpers_module.send :remove_method, url_name
+            end
           end
 
           @routes.clear
           @path_helpers.clear
           @url_helpers.clear
+          @custom_helpers.clear
         end
 
         def add(name, route)
@@ -143,6 +158,23 @@ module ActionDispatch
           routes.length
         end
 
+        def add_url_helper(name, defaults, &block)
+          @custom_helpers << name
+          helper = CustomUrlHelper.new(name, defaults, &block)
+
+          @path_helpers_module.module_eval do
+            define_method(:"#{name}_path") do |*args|
+              helper.call(self, args, true)
+            end
+          end
+
+          @url_helpers_module.module_eval do
+            define_method(:"#{name}_url") do |*args|
+              helper.call(self, args, false)
+            end
+          end
+        end
+
         class UrlHelper
           def self.create(route, options, route_name, url_strategy)
             if optimize_helper?(route)
@@ -263,7 +295,7 @@ module ActionDispatch
         end
 
         private
-          # Create a url helper allowing ordered parameters to be associated
+          # Create a URL helper allowing ordered parameters to be associated
           # with corresponding dynamic segments, so you can do:
           #
           #   foo_url(bar, baz, bang)
@@ -305,7 +337,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
 
@@ -347,6 +379,7 @@ module ActionDispatch
         @set    = Journey::Routes.new
         @router = Journey::Router.new @set
         @formatter = Journey::Formatter.new self
+        @polymorphic_mappings = {}
       end
 
       def eager_load!
@@ -408,6 +441,7 @@ module ActionDispatch
         named_routes.clear
         set.clear
         formatter.clear
+        @polymorphic_mappings.clear
         @prepend.each { |blk| eval_block(blk) }
       end
 
@@ -452,17 +486,50 @@ module ActionDispatch
 
           # Define url_for in the singleton level so one can do:
           # Rails.application.routes.url_helpers.url_for(args)
-          @_routes = routes
+          proxy_class = Class.new do
+            include UrlFor
+            include routes.named_routes.path_helpers_module
+            include routes.named_routes.url_helpers_module
+
+            attr_reader :_routes
+
+            def initialize(routes)
+              @_routes = routes
+            end
+
+            def optimize_routes_generation?
+              @_routes.optimize_routes_generation?
+            end
+          end
+
+          @_proxy = proxy_class.new(routes)
+
           class << self
             def url_for(options)
-              @_routes.url_for(options)
+              @_proxy.url_for(options)
+            end
+
+            def full_url_for(options)
+              @_proxy.full_url_for(options)
+            end
+
+            def route_for(name, *args)
+              @_proxy.route_for(name, *args)
             end
 
             def optimize_routes_generation?
-              @_routes.optimize_routes_generation?
+              @_proxy.optimize_routes_generation?
             end
 
-            attr_reader :_routes
+            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
 
@@ -506,7 +573,7 @@ module ActionDispatch
         routes.empty?
       end
 
-      def add_route(mapping, path_ast, name, anchor)
+      def add_route(mapping, name)
         raise ArgumentError, "Invalid route name: '#{name}'" unless name.blank? || name.to_s.match(/^[_a-z]\w*$/i)
 
         if name && named_routes[name]
@@ -523,20 +590,58 @@ module ActionDispatch
         if route.segment_keys.include?(:controller)
           ActiveSupport::Deprecation.warn(<<-MSG.squish)
             Using a dynamic :controller segment in a route is deprecated and
-            will be removed in Rails 5.1.
+            will be removed in Rails 5.2.
           MSG
         end
 
         if route.segment_keys.include?(:action)
           ActiveSupport::Deprecation.warn(<<-MSG.squish)
             Using a dynamic :action segment in a route is deprecated and
-            will be removed in Rails 5.1.
+            will be removed in Rails 5.2.
           MSG
         end
 
         route
       end
 
+      def add_polymorphic_mapping(klass, options, &block)
+        @polymorphic_mappings[klass] = CustomUrlHelper.new(klass, options, &block)
+      end
+
+      def add_url_helper(name, options, &block)
+        named_routes.add_url_helper(name, options, &block)
+      end
+
+      class CustomUrlHelper
+        attr_reader :name, :defaults, :block
+
+        def initialize(name, defaults, &block)
+          @name = name
+          @defaults = defaults
+          @block = block
+        end
+
+        def call(t, args, only_path = false)
+          options = args.extract_options!
+          url = t.full_url_for(eval_block(t, args, options))
+
+          if only_path
+            "/" + url.partition(%r{(?<!/)/(?!/)}).last
+          else
+            url
+          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
@@ -751,8 +856,7 @@ module ActionDispatch
               params[key] = URI.parser.unescape(value)
             end
           end
-          old_params = req.path_parameters
-          req.path_parameters = old_params.merge params
+          req.path_parameters = params
           app = route.app
           if app.matches?(req) && app.dispatcher?
             begin
diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb
index 3e564f13d8..008216cc80 100644
--- a/actionpack/lib/action_dispatch/routing/url_for.rb
+++ b/actionpack/lib/action_dispatch/routing/url_for.rb
@@ -113,10 +113,10 @@ module ActionDispatch
         default_url_options
       end
 
-      # Generate a url based on the options provided, default_url_options and the
+      # Generate a URL based on the options provided, default_url_options and the
       # routes defined in routes.rb. The following options are supported:
       #
-      # * <tt>:only_path</tt> - If true, the relative url is returned. Defaults to +false+.
+      # * <tt>:only_path</tt> - If true, the relative URL is returned. Defaults to +false+.
       # * <tt>:protocol</tt> - The protocol to connect to. Defaults to 'http'.
       # * <tt>:host</tt> - Specifies the host the link should be targeted at.
       #   If <tt>:only_path</tt> is false, this option must be
@@ -164,6 +164,10 @@ module ActionDispatch
       # implicitly used by +url_for+ can always be overwritten like shown on the
       # last +url_for+ calls.
       def url_for(options = nil)
+        full_url_for(options)
+      end
+
+      def full_url_for(options = nil) # :nodoc:
         case options
         when nil
           _routes.url_for(url_options.symbolize_keys)
@@ -192,6 +196,10 @@ module ActionDispatch
         end
       end
 
+      def route_for(name, *args) # :nodoc:
+        public_send(:"#{name}_url", *args)
+      end
+
       protected
 
         def optimize_routes_generation?