Permalink
Browse files

Clear url helper methods when routes are reloaded

Clear url helper methods when routes are reloaded by removing the
methods explicitly rather than just clearing the module because it
didn't work properly and could be the source of a memory leak.

Closes #8488.
  • Loading branch information...
1 parent 3bf4ddf commit 6ab1a9540b73555f2cabb54860b9ca17e8f226cb @pixeltrix pixeltrix committed Dec 14, 2012
View
@@ -1,5 +1,11 @@
## Rails 3.2.10 (unreleased) ##
+* Clear url helper methods when routes are reloaded by removing the methods
+ explicitly rather than just clearing the module because it didn't work
+ properly and could be the source of a memory leak.
+
+ *Andrew White*
+
* Fix a bug in ActionDispatch::Request#raw_post that caused env['rack.input']
to be read but not rewound.
@@ -94,20 +94,27 @@ class NamedRouteCollection #:nodoc:
attr_reader :routes, :helpers, :module
def initialize
- clear!
+ @routes = {}
+ @helpers = []
+
+ @module = Module.new do
+ instance_methods.each { |selector| remove_method(selector) }
+ end
end
def helper_names
self.module.instance_methods.map(&:to_s)
end
def clear!
+ @helpers.each do |helper|
+ @module.module_eval do
+ remove_possible_method helper
+ end
+ end
+
@routes = {}
@helpers = []
-
- @module ||= Module.new do
- instance_methods.each { |selector| remove_method(selector) }
- end
end
def add(name, route)
@@ -291,7 +298,6 @@ def finalize!
def clear!
@finalized = false
- @url_helpers = nil
named_routes.clear
set.clear
formatter.clear
@@ -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
@@ -229,6 +229,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|

0 comments on commit 6ab1a95

Please sign in to comment.