From 250bc33233a253d58581b9ab58f4fc0b8dca5b15 Mon Sep 17 00:00:00 2001
From: Wilson Bilkovich <wilsonb@gmail.com>
Date: Fri, 30 Jun 2017 15:53:32 -0700
Subject: [PATCH] Properly register "custom" URL helpers as named helpers.

CustomUrlHelpers were introduced in ce7d5fb2e6, closing issue #22512.
They currently register themselves in an ivar that is never accessed.
This change removes the @custom_helpers special-case, and registers them
the way named routes are normally handled.

Without this, you can get route_defined?(:example_url) == false, while
still being able to call url_helpers.example_url and example_path.

Various popular gems such as 'rspec-rails' make use of route_defined?()
when determining how to proxy method calls or whether to define a route.
---
 .../lib/action_dispatch/routing/route_set.rb  | 28 +++++++------------
 .../routing/custom_url_helpers_test.rb        |  6 ++++
 2 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 68bd6d806b..73df6b693c 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -73,7 +73,6 @@ 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
@@ -96,23 +95,9 @@ module ActionDispatch
             @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)
@@ -158,21 +143,28 @@ module ActionDispatch
           routes.length
         end
 
+        # Given a +name+, defines name_path and name_url helpers.
+        # Used by 'direct', 'resolve', and 'polymorphic' route helpers.
         def add_url_helper(name, defaults, &block)
-          @custom_helpers << name
           helper = CustomUrlHelper.new(name, defaults, &block)
+          _path_helper = :"#{name}_path"
+          _url_helper = :"#{name}_url"
 
           @path_helpers_module.module_eval do
-            define_method(:"#{name}_path") do |*args|
+            define_method(:"#{_path_helper}") do |*args|
               helper.call(self, args, true)
             end
           end
+          @path_helpers << _path_helper
 
           @url_helpers_module.module_eval do
-            define_method(:"#{name}_url") do |*args|
+            define_method(:"#{_url_helper}") do |*args|
               helper.call(self, args, false)
             end
           end
+          @url_helpers << _url_helper
+
+          self
         end
 
         class UrlHelper
diff --git a/actionpack/test/dispatch/routing/custom_url_helpers_test.rb b/actionpack/test/dispatch/routing/custom_url_helpers_test.rb
index 338992dda5..cbbed66056 100644
--- a/actionpack/test/dispatch/routing/custom_url_helpers_test.rb
+++ b/actionpack/test/dispatch/routing/custom_url_helpers_test.rb
@@ -322,4 +322,10 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest
       end
     end
   end
+
+  def test_defining_direct_url_registers_helper_method
+    assert_equal "http://www.example.com/basket", Routes.url_helpers.symbol_url
+    assert_equal true, Routes.named_routes.route_defined?(:symbol_url), "'symbol_url' named helper not found"
+    assert_equal true, Routes.named_routes.route_defined?(:symbol_path), "'symbol_path' named helper not found"
+  end
 end