Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Clear url helper methods when routes are reloaded

Remove all the old url helper methods when clear! is called on the
route set because it's possible that some routes have been removed.
  • Loading branch information...
commit 01d3a36bfe5d56d85f8a36f2fe10ad96662b4530 1 parent a16da3f
@pixeltrix pixeltrix authored
View
2  actionpack/CHANGELOG.md
@@ -1,5 +1,7 @@
## Rails 4.0.0 (unreleased) ##
+* Clear url helper methods when routes are reloaded. *Andrew White*
+
* Fix a bug in ActionDispatch::Request#raw_post that caused env['rack.input']
to be read but not rewound.
View
6 actionpack/lib/action_dispatch/routing/route_set.rb
@@ -130,6 +130,12 @@ def helper_names
end
def clear!
+ @helpers.each do |helper|
+ @module.module_eval do
+ remove_possible_method helper
+ end
+ end

How about iterating inside the module_eval block instead?

helpers = @helpers
@module.module_eval do
  helpers.each do |helper|
    remove_possible_method helper
  end
end

Might yield some performance difference according to my tests. Btw, can't remove_possible_method be called without module_eval? I think it's public so should work?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
@routes.clear
@helpers.clear
end
View
86 actionpack/test/dispatch/routing/route_set_test.rb
@@ -0,0 +1,86 @@
+require 'abstract_unit'
+
+module ActionDispatch
+ module Routing
+ class RouteSetTest < ActiveSupport::TestCase
+ class SimpleApp
+ def initialize(response)
+ @response = response
+ end
+
+ def call(env)
+ [ 200, { 'Content-Type' => 'text/plain' }, [response] ]
+ end
+ end
+
+ setup do
+ @set = RouteSet.new
+ end
+
+ test "url helpers are added when route is added" do
+ draw do
+ get 'foo', to: SimpleApp.new('foo#index')
+ end
+
+ assert_equal '/foo', url_helpers.foo_path
+ assert_raises NoMethodError do
+ assert_equal '/bar', url_helpers.bar_path
+ end
+
+ draw do
+ get 'foo', to: SimpleApp.new('foo#index')
+ get 'bar', to: SimpleApp.new('bar#index')
+ end
+
+ assert_equal '/foo', url_helpers.foo_path
+ assert_equal '/bar', url_helpers.bar_path
+ end
+
+ test "url helpers are updated when route is updated" do
+ draw do
+ get 'bar', to: SimpleApp.new('bar#index'), as: :bar
+ end
+
+ assert_equal '/bar', url_helpers.bar_path
+
+ draw do
+ get 'baz', to: SimpleApp.new('baz#index'), as: :bar
+ end
+
+ assert_equal '/baz', url_helpers.bar_path
+ end
+
+ test "url helpers are removed when route is removed" do
+ draw do
+ get 'foo', to: SimpleApp.new('foo#index')
+ get 'bar', to: SimpleApp.new('bar#index')
+ end
+
+ assert_equal '/foo', url_helpers.foo_path
+ assert_equal '/bar', url_helpers.bar_path
+
+ draw do
+ get 'foo', to: SimpleApp.new('foo#index')
+ end
+
+ assert_equal '/foo', url_helpers.foo_path
+ assert_raises NoMethodError do
+ assert_equal '/bar', url_helpers.bar_path
+ end
+ end
+
+ private
+ def clear!
+ @set.clear!
+ end
+
+ def draw(&block)
+ @set.draw(&block)
+ end
+
+ def url_helpers
+ @set.url_helpers
+ end
+ end
+ end
+end
View
71 railties/test/application/routing_test.rb
@@ -277,6 +277,77 @@ def baz
end
end
+ test 'routes are added and removed when reloading' do
+ app('development')
+
+ controller :foo, <<-RUBY
+ class FooController < ApplicationController
+ def index
+ render text: "foo"
+ end
+ end
+ RUBY
+
+ controller :bar, <<-RUBY
+ class BarController < ApplicationController
+ def index
+ render text: "bar"
+ end
+ end
+ RUBY
+
+ app_file 'config/routes.rb', <<-RUBY
+ AppTemplate::Application.routes.draw do
+ get 'foo', to: 'foo#index'
+ end
+ RUBY
+
+ get '/foo'
+ assert_equal 'foo', last_response.body
+ assert_equal '/foo', Rails.application.routes.url_helpers.foo_path
+
+ get '/bar'
+ assert_equal 404, last_response.status
+ assert_raises NoMethodError do
+ assert_equal '/bar', Rails.application.routes.url_helpers.bar_path
+ end
+
+ app_file 'config/routes.rb', <<-RUBY
+ AppTemplate::Application.routes.draw do
+ get 'foo', to: 'foo#index'
+ get 'bar', to: 'bar#index'
+ end
+ RUBY
+
+ Rails.application.reload_routes!
+
+ get '/foo'
+ assert_equal 'foo', last_response.body
+ assert_equal '/foo', Rails.application.routes.url_helpers.foo_path
+
+ get '/bar'
+ assert_equal 'bar', last_response.body
+ assert_equal '/bar', Rails.application.routes.url_helpers.bar_path
+
+ app_file 'config/routes.rb', <<-RUBY
+ AppTemplate::Application.routes.draw do
+ get 'foo', to: 'foo#index'
+ end
+ RUBY
+
+ Rails.application.reload_routes!
+
+ get '/foo'
+ assert_equal 'foo', last_response.body
+ assert_equal '/foo', Rails.application.routes.url_helpers.foo_path
+
+ get '/bar'
+ assert_equal 404, last_response.status
+ assert_raises NoMethodError do
+ assert_equal '/bar', Rails.application.routes.url_helpers.bar_path
+ end
+ end
+
test 'resource routing with irregular inflection' do
app_file 'config/initializers/inflection.rb', <<-RUBY
ActiveSupport::Inflector.inflections do |inflect|
@carlosantoniodasilva

How about iterating inside the module_eval block instead?

helpers = @helpers
@module.module_eval do
  helpers.each do |helper|
    remove_possible_method helper
  end
end

Might yield some performance difference according to my tests. Btw, can't remove_possible_method be called without module_eval? I think it's public so should work?

Please sign in to comment.
Something went wrong with that request. Please try again.