diff options
| -rw-r--r-- | actionpack/CHANGELOG.md | 6 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/route_set.rb | 18 | ||||
| -rw-r--r-- | actionpack/test/dispatch/routing/route_set_test.rb | 86 | ||||
| -rw-r--r-- | railties/test/application/routing_test.rb | 71 |
4 files changed, 175 insertions, 6 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index ea82263c1a..a681a2dc79 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -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. diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 8168bd4fcc..a993699e05 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -94,7 +94,12 @@ module ActionDispatch 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 @@ -102,12 +107,14 @@ module ActionDispatch 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 @@ module ActionDispatch def clear! @finalized = false - @url_helpers = nil named_routes.clear set.clear formatter.clear diff --git a/actionpack/test/dispatch/routing/route_set_test.rb b/actionpack/test/dispatch/routing/route_set_test.rb new file mode 100644 index 0000000000..d57b1a5637 --- /dev/null +++ b/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 diff --git a/railties/test/application/routing_test.rb b/railties/test/application/routing_test.rb index a05e39658d..07807c3620 100644 --- a/railties/test/application/routing_test.rb +++ b/railties/test/application/routing_test.rb @@ -229,6 +229,77 @@ module ApplicationTests 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| |