From 0777b17dafd54442409c02237be8b5562025ed3f Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue, 1 Jul 2014 15:33:30 -0700
Subject: RouteSet should be in charge of constructing the dispather

Now we can override how requests are dispatched in the routeset object
---
 actionpack/lib/action_dispatch/routing/route_set.rb | 4 ++++
 1 file changed, 4 insertions(+)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 69535faabd..1ee74e6810 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -334,6 +334,10 @@ module ActionDispatch
         @prepend.each { |blk| eval_block(blk) }
       end
 
+      def dispatcher(defaults)
+        Routing::RouteSet::Dispatcher.new(defaults)
+      end
+
       module MountedHelpers #:nodoc:
         extend ActiveSupport::Concern
         include UrlFor
-- 
cgit v1.2.3


From f636652dd52ed36f7438c0436679c987cdfefb82 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 16 Jul 2014 11:54:53 -0700
Subject: extract inner options before delegating to the helper

If we extract the options from the user facing method call ASAP, then we
can simplify internal logic.
---
 actionpack/lib/action_dispatch/routing/route_set.rb | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 1ee74e6810..d24751d28d 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -155,8 +155,8 @@ module ActionDispatch
               @arg_size       = @required_parts.size
             end
 
-            def call(t, args)
-              if args.size == arg_size && !args.last.is_a?(Hash) && optimize_routes_generation?(t)
+            def call(t, args, inner_options)
+              if args.size == arg_size && !inner_options && optimize_routes_generation?(t)
                 options = t.url_options.merge @options
                 options[:path] = optimized_helper(args)
                 ActionDispatch::Http::URL.url_for(options)
@@ -207,15 +207,19 @@ module ActionDispatch
             @route        = route
           end
 
-          def call(t, args)
+          def call(t, args, inner_options)
             controller_options = t.url_options
             options = controller_options.merge @options
-            hash = handle_positional_args(controller_options, args, options, @segment_keys)
+            hash = handle_positional_args(controller_options,
+                                          inner_options || {},
+                                          args,
+                                          options,
+                                          @segment_keys)
+
             t._routes.url_for(hash)
           end
 
-          def handle_positional_args(controller_options, args, result, path_params)
-            inner_options = args.extract_options!
+          def handle_positional_args(controller_options, inner_options, args, result, path_params)
 
             if args.size > 0
               if args.size < path_params.size - 1 # take format into account
@@ -251,7 +255,9 @@ module ActionDispatch
           @module.remove_possible_method name
           @module.module_eval do
             define_method(name) do |*args|
-              helper.call self, args
+              options = nil
+              options = args.pop if args.last.is_a? Hash
+              helper.call self, args, options
             end
           end
 
-- 
cgit v1.2.3


From 1e930e7a488c91beaebfa9682394877e9ad70183 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 16 Jul 2014 16:31:07 -0700
Subject: we do not need to dup the options hash, it is private and a new
 object each call

---
 actionpack/lib/action_dispatch/routing/route_set.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index d24751d28d..4fb32a76eb 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -249,8 +249,8 @@ module ActionDispatch
         #
         #   foo_url(bar, baz, bang, sort_by: 'baz')
         #
-        def define_url_helper(route, name, options)
-          helper = UrlHelper.create(route, options.dup)
+        def define_url_helper(route, name, opts)
+          helper = UrlHelper.create(route, opts)
 
           @module.remove_possible_method name
           @module.module_eval do
-- 
cgit v1.2.3


From 2888f8653e2e0a6394e41cb4e8db2e2d81313eb7 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Thu, 17 Jul 2014 10:47:58 -0700
Subject: use a strategy object for generating urls in named helpers

since we know that the route should be a path or fully qualified, we can
pass a strategy object that handles generation.  This allows us to
eliminate an "if only_path" branch when generating urls.
---
 .../lib/action_dispatch/routing/route_set.rb       | 37 ++++++++++++++--------
 1 file changed, 24 insertions(+), 13 deletions(-)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 4fb32a76eb..3ecf6a76f1 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -134,11 +134,11 @@ module ActionDispatch
         end
 
         class UrlHelper # :nodoc:
-          def self.create(route, options)
+          def self.create(route, options, url_strategy)
             if optimize_helper?(route)
-              OptimizedUrlHelper.new(route, options)
+              OptimizedUrlHelper.new(route, options, url_strategy)
             else
-              new route, options
+              new route, options, url_strategy
             end
           end
 
@@ -146,10 +146,12 @@ module ActionDispatch
             !route.glob? && route.path.requirements.empty?
           end
 
+          attr_reader :url_strategy
+
           class OptimizedUrlHelper < UrlHelper # :nodoc:
             attr_reader :arg_size
 
-            def initialize(route, options)
+            def initialize(route, options, url_strategy)
               super
               @required_parts = @route.required_parts
               @arg_size       = @required_parts.size
@@ -159,7 +161,7 @@ module ActionDispatch
               if args.size == arg_size && !inner_options && optimize_routes_generation?(t)
                 options = t.url_options.merge @options
                 options[:path] = optimized_helper(args)
-                ActionDispatch::Http::URL.url_for(options)
+                url_strategy.call options
               else
                 super
               end
@@ -201,10 +203,11 @@ module ActionDispatch
             end
           end
 
-          def initialize(route, options)
+          def initialize(route, options, url_strategy)
             @options      = options
             @segment_keys = route.segment_keys.uniq
             @route        = route
+            @url_strategy = url_strategy
           end
 
           def call(t, args, inner_options)
@@ -216,7 +219,7 @@ module ActionDispatch
                                           options,
                                           @segment_keys)
 
-            t._routes.url_for(hash)
+            t._routes.url_for(hash, url_strategy)
           end
 
           def handle_positional_args(controller_options, inner_options, args, result, path_params)
@@ -249,8 +252,8 @@ module ActionDispatch
         #
         #   foo_url(bar, baz, bang, sort_by: 'baz')
         #
-        def define_url_helper(route, name, opts)
-          helper = UrlHelper.create(route, opts)
+        def define_url_helper(route, name, opts, url_strategy)
+          helper = UrlHelper.create(route, opts, url_strategy)
 
           @module.remove_possible_method name
           @module.module_eval do
@@ -266,12 +269,20 @@ module ActionDispatch
 
         def define_named_route_methods(name, route)
           define_url_helper route, :"#{name}_path",
-            route.defaults.merge(:use_route => name, :only_path => true)
+            route.defaults.merge(:use_route => name), PATH
+
           define_url_helper route, :"#{name}_url",
-            route.defaults.merge(:use_route => name, :only_path => false)
+            route.defaults.merge(:use_route => name), FULL
         end
       end
 
+      # :stopdoc:
+      # strategy for building urls to send to the client
+      PATH    = ->(options) { ActionDispatch::Http::URL.path_for(options) }
+      FULL    = ->(options) { ActionDispatch::Http::URL.full_url_for(options) }
+      UNKNOWN = ->(options) { ActionDispatch::Http::URL.url_for(options) }
+      # :startdoc:
+
       attr_accessor :formatter, :set, :named_routes, :default_scope, :router
       attr_accessor :disable_clear_and_finalize, :resources_path_names
       attr_accessor :default_url_options, :request_class
@@ -643,7 +654,7 @@ module ActionDispatch
       end
 
       # The +options+ argument must be a hash whose keys are *symbols*.
-      def url_for(options)
+      def url_for(options, url_strategy = UNKNOWN)
         options = default_url_options.merge options
 
         user = password = nil
@@ -677,7 +688,7 @@ module ActionDispatch
         options[:user]        = user
         options[:password]    = password
 
-        ActionDispatch::Http::URL.url_for(options)
+        url_strategy.call options
       end
 
       def call(env)
-- 
cgit v1.2.3


From 212057b912627b9c4056f911a43c83b18ae3ab34 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Thu, 17 Jul 2014 11:21:06 -0700
Subject: pass the route name to define_url_helper

this allows us to avoid 2 hash allocations per named helper definition,
also we can avoid a `merge` and `delete`.
---
 .../lib/action_dispatch/routing/route_set.rb       | 42 +++++++++++-----------
 1 file changed, 21 insertions(+), 21 deletions(-)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 3ecf6a76f1..80c705608d 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -134,11 +134,11 @@ module ActionDispatch
         end
 
         class UrlHelper # :nodoc:
-          def self.create(route, options, url_strategy)
+          def self.create(route, options, route_name, url_strategy)
             if optimize_helper?(route)
-              OptimizedUrlHelper.new(route, options, url_strategy)
+              OptimizedUrlHelper.new(route, options, route_name, url_strategy)
             else
-              new route, options, url_strategy
+              new route, options, route_name, url_strategy
             end
           end
 
@@ -146,12 +146,12 @@ module ActionDispatch
             !route.glob? && route.path.requirements.empty?
           end
 
-          attr_reader :url_strategy
+          attr_reader :url_strategy, :route_name
 
           class OptimizedUrlHelper < UrlHelper # :nodoc:
             attr_reader :arg_size
 
-            def initialize(route, options, url_strategy)
+            def initialize(route, options, route_name, url_strategy)
               super
               @required_parts = @route.required_parts
               @arg_size       = @required_parts.size
@@ -203,11 +203,12 @@ module ActionDispatch
             end
           end
 
-          def initialize(route, options, url_strategy)
+          def initialize(route, options, route_name, url_strategy)
             @options      = options
             @segment_keys = route.segment_keys.uniq
             @route        = route
             @url_strategy = url_strategy
+            @route_name   = route_name
           end
 
           def call(t, args, inner_options)
@@ -219,7 +220,7 @@ module ActionDispatch
                                           options,
                                           @segment_keys)
 
-            t._routes.url_for(hash, url_strategy)
+            t._routes.url_for(hash, route_name, url_strategy)
           end
 
           def handle_positional_args(controller_options, inner_options, args, result, path_params)
@@ -252,8 +253,8 @@ module ActionDispatch
         #
         #   foo_url(bar, baz, bang, sort_by: 'baz')
         #
-        def define_url_helper(route, name, opts, url_strategy)
-          helper = UrlHelper.create(route, opts, url_strategy)
+        def define_url_helper(route, name, opts, route_key, url_strategy)
+          helper = UrlHelper.create(route, opts, route_key, url_strategy)
 
           @module.remove_possible_method name
           @module.module_eval do
@@ -268,11 +269,8 @@ module ActionDispatch
         end
 
         def define_named_route_methods(name, route)
-          define_url_helper route, :"#{name}_path",
-            route.defaults.merge(:use_route => name), PATH
-
-          define_url_helper route, :"#{name}_url",
-            route.defaults.merge(:use_route => name), FULL
+          define_url_helper route, :"#{name}_path", route.defaults, name, PATH
+          define_url_helper route, :"#{name}_url", route.defaults, name, FULL
         end
       end
 
@@ -512,8 +510,8 @@ module ActionDispatch
 
         attr_reader :options, :recall, :set, :named_route
 
-        def initialize(options, recall, set)
-          @named_route = options.delete(:use_route)
+        def initialize(named_route, options, recall, set)
+          @named_route = named_route
           @options     = options.dup
           @recall      = recall.dup
           @set         = set
@@ -629,13 +627,15 @@ module ActionDispatch
       end
 
       def generate_extras(options, recall={})
-        path, params = generate(options, recall)
+        route_key = options.delete :use_route
+        path, params = generate(route_key, options, recall)
         return path, params.keys
       end
 
-      def generate(options, recall = {})
-        Generator.new(options, recall, self).generate
+      def generate(route_key, options, recall = {})
+        Generator.new(route_key, options, recall, self).generate
       end
+      private :generate
 
       RESERVED_OPTIONS = [:host, :protocol, :port, :subdomain, :domain, :tld_length,
                           :trailing_slash, :anchor, :params, :only_path, :script_name,
@@ -654,7 +654,7 @@ module ActionDispatch
       end
 
       # The +options+ argument must be a hash whose keys are *symbols*.
-      def url_for(options, url_strategy = UNKNOWN)
+      def url_for(options, route_name = nil, url_strategy = UNKNOWN)
         options = default_url_options.merge options
 
         user = password = nil
@@ -676,7 +676,7 @@ module ActionDispatch
         path_options = options.dup
         RESERVED_OPTIONS.each { |ro| path_options.delete ro }
 
-        path, params = generate(path_options, recall)
+        path, params = generate(route_name, path_options, recall)
 
         if options.key? :params
           params.merge! options[:params]
-- 
cgit v1.2.3


From 9f63a78d554690efba3819310710634228089d24 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Mon, 28 Jul 2014 14:07:53 -0700
Subject: remove the mounted? method

we know the routes should not be "optimized" when mounting an
application
---
 actionpack/lib/action_dispatch/routing/route_set.rb | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 80c705608d..947391b725 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -641,12 +641,8 @@ module ActionDispatch
                           :trailing_slash, :anchor, :params, :only_path, :script_name,
                           :original_script_name]
 
-      def mounted?
-        false
-      end
-
       def optimize_routes_generation?
-        !mounted? && default_url_options.empty?
+        default_url_options.empty?
       end
 
       def find_script_name(options)
-- 
cgit v1.2.3


From d2d33769030b3a560bdbc9c33e7c189274a0dc3a Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue, 29 Jul 2014 11:07:36 -0700
Subject: eval_block should be private

---
 actionpack/lib/action_dispatch/routing/route_set.rb | 1 +
 1 file changed, 1 insertion(+)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 947391b725..7259b01c99 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -334,6 +334,7 @@ module ActionDispatch
           mapper.instance_exec(&block)
         end
       end
+      private :eval_block
 
       def finalize!
         return if @finalized
-- 
cgit v1.2.3


From a2e926698dfa9bb978958b181b3ed5c80fee8c6e Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue, 29 Jul 2014 11:28:45 -0700
Subject: only ask for the routes module once

we can cache the module on the stack, then reuse it
---
 actionpack/lib/action_dispatch/routing/route_set.rb | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 7259b01c99..f735b9d1c7 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -401,14 +401,16 @@ module ActionDispatch
               def url_options; {}; end
             end
 
+            route_methods = routes.named_routes.module
+
             # 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
+            extend route_methods
 
             # Any class that includes this module will get all
             # named routes...
-            include routes.named_routes.module
+            include route_methods
 
             # plus a singleton class method called _routes ...
             included do
-- 
cgit v1.2.3


From 41931b8af1324f0edf6754fbbfe66433b0b02cf1 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue, 29 Jul 2014 11:32:17 -0700
Subject: pass the module to define_named_route_methods

after this, we can disconnect @module from the instance
---
 actionpack/lib/action_dispatch/routing/route_set.rb | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index f735b9d1c7..aa0803e0e3 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -109,7 +109,7 @@ module ActionDispatch
 
         def add(name, route)
           routes[name.to_sym] = route
-          define_named_route_methods(name, route)
+          define_named_route_methods(@module, name, route)
         end
 
         def get(name)
@@ -253,11 +253,11 @@ module ActionDispatch
         #
         #   foo_url(bar, baz, bang, sort_by: 'baz')
         #
-        def define_url_helper(route, name, opts, route_key, url_strategy)
+        def define_url_helper(mod, route, name, opts, route_key, url_strategy)
           helper = UrlHelper.create(route, opts, route_key, url_strategy)
 
-          @module.remove_possible_method name
-          @module.module_eval do
+          mod.remove_possible_method name
+          mod.module_eval do
             define_method(name) do |*args|
               options = nil
               options = args.pop if args.last.is_a? Hash
@@ -268,9 +268,9 @@ module ActionDispatch
           helpers << name
         end
 
-        def define_named_route_methods(name, route)
-          define_url_helper route, :"#{name}_path", route.defaults, name, PATH
-          define_url_helper route, :"#{name}_url", route.defaults, name, FULL
+        def define_named_route_methods(mod, name, route)
+          define_url_helper mod, route, :"#{name}_path", route.defaults, name, PATH
+          define_url_helper mod, route, :"#{name}_url", route.defaults, name, FULL
         end
       end
 
-- 
cgit v1.2.3


From 0088b08dcaf16176c8f9364d1d786f0c3728d369 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue, 29 Jul 2014 11:48:14 -0700
Subject: helpers should be a Set so it doesn't grow unbounded

since helpers is a set, we can be confident about when to remove methods
from the module.
---
 actionpack/lib/action_dispatch/routing/route_set.rb | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index aa0803e0e3..14c5d663a3 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -90,7 +90,7 @@ module ActionDispatch
 
         def initialize
           @routes  = {}
-          @helpers = []
+          @helpers = Set.new
           @module  = Module.new
         end
 
@@ -100,7 +100,7 @@ module ActionDispatch
 
         def clear!
           @helpers.each do |helper|
-            @module.remove_possible_method helper
+            @module.send :undef_method, helper
           end
 
           @routes.clear
@@ -108,7 +108,11 @@ module ActionDispatch
         end
 
         def add(name, route)
-          routes[name.to_sym] = route
+          key = name.to_sym
+          if routes.key? key
+            undef_named_route_methods @module, name
+          end
+          routes[key] = route
           define_named_route_methods(@module, name, route)
         end
 
@@ -256,7 +260,6 @@ module ActionDispatch
         def define_url_helper(mod, route, name, opts, route_key, url_strategy)
           helper = UrlHelper.create(route, opts, route_key, url_strategy)
 
-          mod.remove_possible_method name
           mod.module_eval do
             define_method(name) do |*args|
               options = nil
@@ -272,6 +275,11 @@ module ActionDispatch
           define_url_helper mod, route, :"#{name}_path", route.defaults, name, PATH
           define_url_helper mod, route, :"#{name}_url", route.defaults, name, FULL
         end
+
+        def undef_named_route_methods(mod, name)
+          mod.send :undef_method, :"#{name}_path"
+          mod.send :undef_method, :"#{name}_url"
+        end
       end
 
       # :stopdoc:
-- 
cgit v1.2.3


From f889831ed65bea14b6b687bdaa4012d73d81b2a6 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue, 29 Jul 2014 12:15:04 -0700
Subject: ask the named routes collection if the route is defined

we should not be accessing internals to figure out if a method is
defined.
---
 actionpack/lib/action_dispatch/routing/route_set.rb | 4 ++++
 1 file changed, 4 insertions(+)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 14c5d663a3..c2583e2be6 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -94,6 +94,10 @@ module ActionDispatch
           @module  = Module.new
         end
 
+        def route_defined?(name)
+          @module.method_defined? name
+        end
+
         def helper_names
           @helpers.map(&:to_s)
         end
-- 
cgit v1.2.3


From d7b726be00379a8e03e443425fba05341c6104cd Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue, 29 Jul 2014 12:15:52 -0700
Subject: oops! :bomb:

use helpers.include? so we don't get any false positives
---
 actionpack/lib/action_dispatch/routing/route_set.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index c2583e2be6..d869b62398 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -95,7 +95,7 @@ module ActionDispatch
         end
 
         def route_defined?(name)
-          @module.method_defined? name
+          @helpers.include? name.to_sym
         end
 
         def helper_names
-- 
cgit v1.2.3


From 2bbcca004cc232cef868cd0e301f274ce5638df0 Mon Sep 17 00:00:00 2001
From: "@schneems and @sgrif" <sean@thoughtbot.com>
Date: Thu, 19 Jun 2014 17:26:29 -0500
Subject: Deprecate `*_path` methods in mailers

Email does not support relative links since there is no implicit host. Therefore all links inside of emails must be fully qualified URLs. All path helpers are now deprecated. When removed, the error will give early indication to developers to use `*_url` methods instead.

Currently if a developer uses a `*_path` helper, their tests and `mail_view` will not catch the mistake. The only way to see the error is by sending emails in production. Preventing sending out emails with non-working path's is the desired end goal of this PR.

Currently path helpers are mixed-in to controllers (the ActionMailer::Base acts as a controller). All `*_url` and `*_path` helpers are made available through the same module. This PR separates this behavior into two modules so we can extend the `*_path` methods to add a Deprecation to them. Once deprecated we can use this same area to raise a NoMethodError and add an informative message directing the developer to use `*_url` instead.

The module with warnings is only mixed in when a controller returns false from the newly added `supports_relative_path?`.

Paired @sgrif & @schneems
---
 .../lib/action_dispatch/routing/route_set.rb       | 120 ++++++++++++---------
 1 file changed, 72 insertions(+), 48 deletions(-)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index d869b62398..0906c67719 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -86,12 +86,13 @@ module ActionDispatch
       # named routes.
       class NamedRouteCollection #:nodoc:
         include Enumerable
-        attr_reader :routes, :helpers, :module
+        attr_reader :routes, :helpers, :url_helpers_module
 
         def initialize
           @routes  = {}
           @helpers = Set.new
-          @module  = Module.new
+          @url_helpers_module  = Module.new
+          @path_helpers_module = Module.new
         end
 
         def route_defined?(name)
@@ -104,7 +105,11 @@ module ActionDispatch
 
         def clear!
           @helpers.each do |helper|
-            @module.send :undef_method, helper
+            if helper =~ /_path$/
+              @path_helpers_module.send :undef_method, helper
+            else
+              @url_helpers_module.send  :undef_method, helper
+            end
           end
 
           @routes.clear
@@ -114,10 +119,12 @@ module ActionDispatch
         def add(name, route)
           key = name.to_sym
           if routes.key? key
-            undef_named_route_methods @module, name
+            @path_helpers_module.send :undef_method, :"#{name}_path"
+            @url_helpers_module.send  :undef_method, :"#{name}_url"
           end
           routes[key] = route
-          define_named_route_methods(@module, name, route)
+          define_url_helper @path_helpers_module, route, :"#{name}_path", route.defaults, name, PATH
+          define_url_helper @url_helpers_module,  route, :"#{name}_url",  route.defaults, name, FULL
         end
 
         def get(name)
@@ -141,6 +148,26 @@ module ActionDispatch
           routes.length
         end
 
+        def path_helpers_module(warn = false)
+          if warn
+            mod = @path_helpers_module
+            Module.new do
+              include mod
+
+              mod.instance_methods(false).each do |meth|
+                define_method("#{meth}_with_warning") do |*args, &block|
+                  ActiveSupport::Deprecation.warn("The method `#{meth}` cannot be used here as a full URL is required. Use `#{meth.to_s.sub(/_path$/, '_url')}` instead")
+                  send("#{meth}_without_warning", *args, &block)
+                end
+
+                alias_method_chain meth, :warning
+              end
+            end
+          else
+            @path_helpers_module
+          end
+        end
+
         class UrlHelper # :nodoc:
           def self.create(route, options, route_name, url_strategy)
             if optimize_helper?(route)
@@ -263,7 +290,7 @@ module ActionDispatch
         #
         def define_url_helper(mod, route, name, opts, route_key, url_strategy)
           helper = UrlHelper.create(route, opts, route_key, url_strategy)
-
+          mod.remove_possible_method name
           mod.module_eval do
             define_method(name) do |*args|
               options = nil
@@ -274,16 +301,6 @@ module ActionDispatch
 
           helpers << name
         end
-
-        def define_named_route_methods(mod, name, route)
-          define_url_helper mod, route, :"#{name}_path", route.defaults, name, PATH
-          define_url_helper mod, route, :"#{name}_url", route.defaults, name, FULL
-        end
-
-        def undef_named_route_methods(mod, name)
-          mod.send :undef_method, :"#{name}_path"
-          mod.send :undef_method, :"#{name}_url"
-        end
       end
 
       # :stopdoc:
@@ -396,44 +413,51 @@ module ActionDispatch
         RUBY
       end
 
-      def url_helpers
-        @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, :optimize_routes_generation?, :to => '@_routes'
-              attr_reader :_routes
-              def url_options; {}; end
-            end
+      def url_helpers(include_path_helpers = true)
+        routes = self
 
-            route_methods = routes.named_routes.module
+        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, :optimize_routes_generation?, to: '@_routes'
+            attr_reader :_routes
+            def url_options; {}; end
+          end
 
-            # Make named_routes available in the module singleton
-            # as well, so one can do:
-            # Rails.application.routes.url_helpers.posts_path
-            extend route_methods
+          route_methods = routes.named_routes.url_helpers_module
 
-            # Any class that includes this module will get all
-            # named routes...
-            include route_methods
+          # Make named_routes available in the module singleton
+          # as well, so one can do:
+          # Rails.application.routes.url_helpers.posts_path
+          extend route_methods
 
-            # plus a singleton class method called _routes ...
-            included do
-              singleton_class.send(:redefine_method, :_routes) { routes }
-            end
+          # Any class that includes this module will get all
+          # named routes...
+          include route_methods
 
-            # And an instance method _routes. Note that
-            # UrlFor (included in this module) add extra
-            # conveniences for working with @_routes.
-            define_method(:_routes) { @_routes || routes }
+          if include_path_helpers
+            path_helpers = routes.named_routes.path_helpers_module
+          else
+            path_helpers = routes.named_routes.path_helpers_module(true)
           end
+
+          include path_helpers
+          extend path_helpers
+
+          # plus a singleton class method called _routes ...
+          included do
+            singleton_class.send(:redefine_method, :_routes) { routes }
+          end
+
+          # 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
       end
 
-- 
cgit v1.2.3


From cf6658c2842d92c7d23928ec6f72074dce345f15 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 30 Jul 2014 14:38:50 -0700
Subject: `add` will remove the method if it exists already

---
 actionpack/lib/action_dispatch/routing/route_set.rb | 1 -
 1 file changed, 1 deletion(-)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 0906c67719..459097f327 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -290,7 +290,6 @@ module ActionDispatch
         #
         def define_url_helper(mod, route, name, opts, route_key, url_strategy)
           helper = UrlHelper.create(route, opts, route_key, url_strategy)
-          mod.remove_possible_method name
           mod.module_eval do
             define_method(name) do |*args|
               options = nil
-- 
cgit v1.2.3


From 210b338db20b1cdd0684f40bd78b52ed16148b99 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 30 Jul 2014 14:46:07 -0700
Subject: split path_helpers and url_helpers

this lets us avoid hard coding a regexp for separating path and url
helpers in the clear! method.
---
 .../lib/action_dispatch/routing/route_set.rb       | 43 +++++++++++++---------
 1 file changed, 25 insertions(+), 18 deletions(-)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 459097f327..42d6ee40db 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -86,45 +86,54 @@ module ActionDispatch
       # named routes.
       class NamedRouteCollection #:nodoc:
         include Enumerable
-        attr_reader :routes, :helpers, :url_helpers_module
+        attr_reader :routes, :url_helpers_module
 
         def initialize
           @routes  = {}
-          @helpers = Set.new
+          @path_helpers = Set.new
+          @url_helpers = Set.new
           @url_helpers_module  = Module.new
           @path_helpers_module = Module.new
         end
 
         def route_defined?(name)
-          @helpers.include? name.to_sym
+          key = name.to_sym
+          @path_helpers.include?(key) || @url_helpers.include?(key)
         end
 
         def helper_names
-          @helpers.map(&:to_s)
+          @path_helpers.map(&:to_s) + @url_helpers.map(&:to_s)
         end
 
         def clear!
-          @helpers.each do |helper|
-            if helper =~ /_path$/
-              @path_helpers_module.send :undef_method, helper
-            else
-              @url_helpers_module.send  :undef_method, helper
-            end
+          @path_helpers.each do |helper|
+            @path_helpers_module.send :undef_method, helper
+          end
+
+          @url_helpers.each do |helper|
+            @url_helpers_module.send  :undef_method, helper
           end
 
           @routes.clear
-          @helpers.clear
+          @path_helpers.clear
+          @url_helpers.clear
         end
 
         def add(name, route)
-          key = name.to_sym
+          key       = name.to_sym
+          path_name = :"#{name}_path"
+          url_name  = :"#{name}_url"
+
           if routes.key? key
-            @path_helpers_module.send :undef_method, :"#{name}_path"
-            @url_helpers_module.send  :undef_method, :"#{name}_url"
+            @path_helpers_module.send :undef_method, path_name
+            @url_helpers_module.send  :undef_method, url_name
           end
           routes[key] = route
-          define_url_helper @path_helpers_module, route, :"#{name}_path", route.defaults, name, PATH
-          define_url_helper @url_helpers_module,  route, :"#{name}_url",  route.defaults, name, FULL
+          define_url_helper @path_helpers_module, route, path_name, route.defaults, name, PATH
+          define_url_helper @url_helpers_module,  route, url_name,  route.defaults, name, FULL
+
+          @path_helpers << path_name
+          @url_helpers << url_name
         end
 
         def get(name)
@@ -297,8 +306,6 @@ module ActionDispatch
               helper.call self, args, options
             end
           end
-
-          helpers << name
         end
       end
 
-- 
cgit v1.2.3


From d9108abcad66412f9e8604479cdcc7795a3262b1 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 30 Jul 2014 14:46:45 -0700
Subject: fix variable name

---
 actionpack/lib/action_dispatch/routing/route_set.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 42d6ee40db..444c36a72b 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -435,16 +435,16 @@ module ActionDispatch
             def url_options; {}; end
           end
 
-          route_methods = routes.named_routes.url_helpers_module
+          url_helpers = routes.named_routes.url_helpers_module
 
           # Make named_routes available in the module singleton
           # as well, so one can do:
           # Rails.application.routes.url_helpers.posts_path
-          extend route_methods
+          extend url_helpers
 
           # Any class that includes this module will get all
           # named routes...
-          include route_methods
+          include url_helpers
 
           if include_path_helpers
             path_helpers = routes.named_routes.path_helpers_module
-- 
cgit v1.2.3


From 09603275e9f57ca133bd92557c1213cdfa02cff8 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 30 Jul 2014 14:51:28 -0700
Subject: avoid instrospection on the module

we already know what helpers are path helpers, so just iterate through
that list and define the helpers with warnings
---
 actionpack/lib/action_dispatch/routing/route_set.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 444c36a72b..6e8016d360 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -160,10 +160,11 @@ module ActionDispatch
         def path_helpers_module(warn = false)
           if warn
             mod = @path_helpers_module
+            helpers = @path_helpers
             Module.new do
               include mod
 
-              mod.instance_methods(false).each do |meth|
+              helpers.each do |meth|
                 define_method("#{meth}_with_warning") do |*args, &block|
                   ActiveSupport::Deprecation.warn("The method `#{meth}` cannot be used here as a full URL is required. Use `#{meth.to_s.sub(/_path$/, '_url')}` instead")
                   send("#{meth}_without_warning", *args, &block)
-- 
cgit v1.2.3


From 68aea29cf5e5b856fdfa0b3087bf68cbd6310bd6 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 30 Jul 2014 14:53:28 -0700
Subject: remove alias_method_chain

we can `super` in to the previous implementation.
---
 actionpack/lib/action_dispatch/routing/route_set.rb | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 6e8016d360..4155efa03c 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -165,12 +165,10 @@ module ActionDispatch
               include mod
 
               helpers.each do |meth|
-                define_method("#{meth}_with_warning") do |*args, &block|
+                define_method(meth) do |*args, &block|
                   ActiveSupport::Deprecation.warn("The method `#{meth}` cannot be used here as a full URL is required. Use `#{meth.to_s.sub(/_path$/, '_url')}` instead")
-                  send("#{meth}_without_warning", *args, &block)
+                  super(*args, &block)
                 end
-
-                alias_method_chain meth, :warning
               end
             end
           else
-- 
cgit v1.2.3


From 3429b0ccba036d09d1e6357e559be05efe3da969 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 30 Jul 2014 18:11:24 -0700
Subject: remove useless deup

every call to default_resources_path_names allocates a new hash, no need
to dup
---
 actionpack/lib/action_dispatch/routing/route_set.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 4155efa03c..96e23c2464 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -327,7 +327,7 @@ module ActionDispatch
 
       def initialize(request_class = ActionDispatch::Request)
         self.named_routes = NamedRouteCollection.new
-        self.resources_path_names = self.class.default_resources_path_names.dup
+        self.resources_path_names = self.class.default_resources_path_names
         self.default_url_options = {}
         self.request_class = request_class
 
-- 
cgit v1.2.3


From 3e9158bb95de1770c5a529a903fe15b19a473439 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Thu, 31 Jul 2014 11:21:53 -0700
Subject: do a hash lookup for collision detection

hash lookup should be faster than searching an array.
---
 actionpack/lib/action_dispatch/routing/route_set.rb | 4 ++++
 1 file changed, 4 insertions(+)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 96e23c2464..518d4ca09c 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -140,6 +140,10 @@ module ActionDispatch
           routes[name.to_sym]
         end
 
+        def key?(name)
+          routes.key? name.to_sym
+        end
+
         alias []=   add
         alias []    get
         alias clear clear!
-- 
cgit v1.2.3


From 8cbcd19d7079db1b5df6ddb3b813fea57cd5cc38 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Fri, 1 Aug 2014 11:45:52 -0700
Subject: always return a string from find_script_name

this allows us to avoid nil checks on the return value
---
 actionpack/lib/action_dispatch/routing/route_set.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 518d4ca09c..ce1fe2e451 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -694,7 +694,7 @@ module ActionDispatch
       end
 
       def find_script_name(options)
-        options.delete :script_name
+        options.delete(:script_name) { '' }
       end
 
       # The +options+ argument must be a hash whose keys are *symbols*.
@@ -713,7 +713,7 @@ module ActionDispatch
         original_script_name = options.delete(:original_script_name)
         script_name = find_script_name options
 
-        if script_name && original_script_name
+        if original_script_name
           script_name = original_script_name + script_name
         end
 
-- 
cgit v1.2.3


From 3300fdedc748993b378288c6cbc3113885c955ed Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Mon, 4 Aug 2014 18:20:07 -0700
Subject: avoid testing only_path

we know that this call only wants the path returned, so lets call a
method that returns the path.
---
 actionpack/lib/action_dispatch/routing/route_set.rb | 4 ++++
 1 file changed, 4 insertions(+)

(limited to 'actionpack/lib/action_dispatch/routing/route_set.rb')

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index ce1fe2e451..5b3651aaee 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -697,6 +697,10 @@ module ActionDispatch
         options.delete(:script_name) { '' }
       end
 
+      def path_for(options, route_name = nil) # :nodoc:
+        url_for(options, route_name, PATH)
+      end
+
       # The +options+ argument must be a hash whose keys are *symbols*.
       def url_for(options, route_name = nil, url_strategy = UNKNOWN)
         options = default_url_options.merge options
-- 
cgit v1.2.3