From 2e6658ae510e17e9e6e98ebd784066752ea6027c Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Fri, 26 May 2017 13:12:21 +0930 Subject: Clarify intentions around method redefinitions Don't use remove_method or remove_possible_method just before a new definition: at best the purpose is unclear, and at worst it creates a race condition. Instead, prefer redefine_method when practical, and silence_redefinition_of_method otherwise. --- actionpack/lib/action_controller/metal/renderers.rb | 2 +- actionpack/lib/action_controller/test_case.rb | 3 ++- actionpack/lib/action_dispatch/routing/route_set.rb | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/renderers.rb b/actionpack/lib/action_controller/metal/renderers.rb index 26752571f8..b81d3ef539 100644 --- a/actionpack/lib/action_controller/metal/renderers.rb +++ b/actionpack/lib/action_controller/metal/renderers.rb @@ -85,7 +85,7 @@ module ActionController def self.remove(key) RENDERERS.delete(key.to_sym) method_name = _render_with_renderer_method_name(key) - remove_method(method_name) if method_defined?(method_name) + remove_possible_method(method_name) end def self._render_with_renderer_method_name(key) diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 50a96bce98..74d557fc18 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -4,6 +4,7 @@ require "rack/session/abstract/id" require "active_support/core_ext/hash/conversions" require "active_support/core_ext/object/to_query" require "active_support/core_ext/module/anonymous" +require "active_support/core_ext/module/redefine_method" require "active_support/core_ext/hash/keys" require "active_support/testing/constant_lookup" require_relative "template_assertions" @@ -19,7 +20,7 @@ module ActionController # the database on the main thread, so they could open a txn, then the # controller thread will open a new connection and try to access data # that's only visible to the main thread's txn. This is the problem in #23483. - remove_method :new_controller_thread + silence_redefinition_of_method :new_controller_thread def new_controller_thread # :nodoc: yield end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 357eaec572..445e86b13c 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -3,6 +3,7 @@ require_relative "../journey" require "active_support/core_ext/object/to_query" require "active_support/core_ext/hash/slice" +require "active_support/core_ext/module/redefine_method" require "active_support/core_ext/module/remove_method" require "active_support/core_ext/array/extract_options" require "action_controller/metal/exceptions" @@ -546,7 +547,7 @@ module ActionDispatch # plus a singleton class method called _routes ... included do - singleton_class.send(:redefine_method, :_routes) { routes } + redefine_singleton_method(:_routes) { routes } end # And an instance method _routes. Note that -- cgit v1.2.3