Browse files

hides the per thread registry instance, and caches singleton methods

Existing code was delegating to the instance with delegate
macro calls, or invoking the instance method to reach
the object and call its instance methods.

But the point is to have a clean class-level interface where
the thread local instance is hidden in the implementation.

References #11c6973.
References #10198.
  • Loading branch information...
1 parent 67bb49b commit e5ef3abdd2336c34cd853a1f845f79b8b19fbb1b @fxn fxn committed Apr 13, 2013
View
6 activerecord/lib/active_record/scoping.rb
@@ -36,7 +36,7 @@ def populate_with_current_scope_attributes
# to get the current_scope for the +Board+ model, then you would use the
# following code:
#
- # registry = ActiveRecord::Scoping::ScopeRegistry.instance
+ # registry = ActiveRecord::Scoping::ScopeRegistry
# registry.set_value_for(:current_scope, "Board", some_new_scope)
#
# Now when you run:
@@ -52,10 +52,6 @@ def populate_with_current_scope_attributes
class ScopeRegistry # :nodoc:
extend ActiveSupport::PerThreadRegistry
- class << self
- delegate :value_for, :set_value_for, to: :instance
- end
-
VALID_SCOPE_TYPES = [:current_scope, :ignore_default_scope]
def initialize
View
4 activerecord/test/cases/base_test.rb
@@ -1377,9 +1377,9 @@ def test_current_scope_is_reset
UnloadablePost.send(:current_scope=, UnloadablePost.all)
UnloadablePost.unloadable
- assert_not_nil ActiveRecord::Scoping::ScopeRegistry.instance.value_for(:current_scope, "UnloadablePost")
+ assert_not_nil ActiveRecord::Scoping::ScopeRegistry.value_for(:current_scope, "UnloadablePost")
ActiveSupport::Dependencies.remove_unloadable_constants!
- assert_nil ActiveRecord::Scoping::ScopeRegistry.instance.value_for(:current_scope, "UnloadablePost")
+ assert_nil ActiveRecord::Scoping::ScopeRegistry.value_for(:current_scope, "UnloadablePost")
ensure
Object.class_eval{ remove_const :UnloadablePost } if defined?(UnloadablePost)
end
View
4 activesupport/lib/active_support/notifications.rb
@@ -193,10 +193,6 @@ def instrumenter
class InstrumentationRegistry # :nodoc:
extend ActiveSupport::PerThreadRegistry
- class << self
- delegate :instrumenter_for, to: :instance
- end
-
def initialize
@registry = {}
end
View
48 activesupport/lib/active_support/per_thread_registry.rb
@@ -1,7 +1,11 @@
module ActiveSupport
# This module is used to encapsulate access to thread local variables.
#
- # Given
+ # Instead of polluting the thread locals namespace:
+ #
+ # Thread.current[:connection_handler]
+ #
+ # you define a class that extends this module:
#
# module ActiveRecord
# class RuntimeRegistry
@@ -11,34 +15,40 @@ module ActiveSupport
# end
# end
#
- # <tt>ActiveRecord::RuntimeRegistry</tt> gets an +instance+ class method
- # that returns an instance of the class unique to the current thread. Thus,
- # instead of polluting +Thread.current+
- #
- # Thread.current[:connection_handler]
- #
- # you write
+ # and invoke the declared instance accessors as class methods. So
#
- # ActiveRecord::RuntimeRegistry.instance.connection_handler
+ # ActiveRecord::RuntimeRegistry.connection_handler = connection_handler
#
- # A +method_missing+ handler that proxies to the thread local instance is
- # installed in the extended class so the call above can be shortened to
+ # sets a connection handler local to the current thread, and
#
# ActiveRecord::RuntimeRegistry.connection_handler
#
- # The instance is stored as a thread local keyed by the name of the class,
- # that is the string "ActiveRecord::RuntimeRegistry" in the example above.
+ # returns a connection handler local to the current thread.
+ #
+ # This feature is accomplished by instantiating the class and storing the
+ # instance as a thread local keyed by the class name. In the example above
+ # a key "ActiveRecord::RuntimeRegistry" is stored in <tt>Thread.current</tt>.
+ # The class methods proxy to said thread local instance.
#
# If the class has an initializer, it must accept no arguments.
module PerThreadRegistry
- def instance
- Thread.current[self.name] ||= new
- end
-
protected
- def method_missing(*args, &block)
- instance.public_send(*args, &block)
+ def method_missing(name, *args, &block)
@frodsan
frodsan added a line comment Apr 13, 2013

This method should be marked as :nodoc:, right?

@fxn
Ruby on Rails member
fxn added a line comment Apr 13, 2013

Yessir!

@frodsan
frodsan added a line comment Apr 13, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ # Caches the method definition as a singleton method of the receiver.
+ singleton_class.class_eval do
@frodsan
frodsan added a line comment Apr 13, 2013

I'm not sure but this can be changed to +define_singleton_method+ :?

@fxn
Ruby on Rails member
fxn added a line comment Apr 13, 2013

Oh yes, didn't remember that one, would you change it?

@frodsan
frodsan added a line comment Apr 13, 2013

#10207 😄

@fxn
Ruby on Rails member
fxn added a line comment Apr 13, 2013

Much better!

@tenderlove
Ruby on Rails member
tenderlove added a line comment Oct 8, 2013

This isn't thread safe. It will cause method redefinition warnings in concurrent systems (since there is no lock).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ define_method(name) do |*a, &b|
+ per_thread_registry_instance.public_send(name, *a, &b)
+ end
+ end
+
+ send(name, *args, &block)
+ end
+
+ private
+
+ def per_thread_registry_instance
+ Thread.current[name] ||= new
end
end
end

0 comments on commit e5ef3ab

Please sign in to comment.