From 7b6bfe84f332a3c99656f73cf0251bce0a16ba88 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 1 Mar 2011 17:20:10 -0800 Subject: [PATCH] refactor Reference to a ClassCache object, fix lazy lookup in Middleware so that anonymous classes are supported --- .../lib/action_dispatch/middleware/stack.rb | 13 ++- .../lib/action_dispatch/routing/route_set.rb | 5 +- .../middleware_stack/middleware_test.rb | 50 ++++++++++++ activesupport/CHANGELOG | 6 ++ .../lib/active_support/dependencies.rb | 47 ++++++++--- activesupport/test/class_cache_test.rb | 81 +++++++++++++++++++ activesupport/test/dependencies_test.rb | 6 +- 7 files changed, 187 insertions(+), 21 deletions(-) create mode 100644 actionpack/test/dispatch/middleware_stack/middleware_test.rb create mode 100644 activesupport/test/class_cache_test.rb diff --git a/actionpack/lib/action_dispatch/middleware/stack.rb b/actionpack/lib/action_dispatch/middleware/stack.rb index a5f651297a..fe87abb4d0 100644 --- a/actionpack/lib/action_dispatch/middleware/stack.rb +++ b/actionpack/lib/action_dispatch/middleware/stack.rb @@ -7,12 +7,19 @@ module ActionDispatch attr_reader :args, :block def initialize(klass_or_name, *args, &block) - @ref = ActiveSupport::Dependencies::Reference.new(klass_or_name) + @klass = nil + @name = klass_or_name + + if klass_or_name.respond_to?(:name) + @klass = klass_or_name + @name = @klass.name + end + @args, @block = args, block end def klass - @ref.get + @klass ||= ActiveSupport::Inflector.constantize(@name) end def ==(middleware) @@ -22,7 +29,7 @@ module ActionDispatch when Class klass == middleware else - normalize(@ref.name) == normalize(middleware) + normalize(@name) == normalize(middleware) end end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 4b4e9da173..2f764f88dc 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -50,12 +50,13 @@ module ActionDispatch private def controller_reference(controller_param) + controller_name = "#{controller_param.camelize}Controller" + unless controller = @controllers[controller_param] - controller_name = "#{controller_param.camelize}Controller" controller = @controllers[controller_param] = ActiveSupport::Dependencies.ref(controller_name) end - controller.get + controller.get(controller_name) end def dispatch(controller, action, env) diff --git a/actionpack/test/dispatch/middleware_stack/middleware_test.rb b/actionpack/test/dispatch/middleware_stack/middleware_test.rb new file mode 100644 index 0000000000..aedd5feb38 --- /dev/null +++ b/actionpack/test/dispatch/middleware_stack/middleware_test.rb @@ -0,0 +1,50 @@ +require 'abstract_unit' +require 'action_dispatch/middleware/stack' + +module ActionDispatch + class MiddlewareStack + class MiddlewareTest < ActiveSupport::TestCase + class Omg; end + + { + 'concrete' => Omg, + 'anonymous' => Class.new + }.each do |name, klass| + + define_method("test_#{name}_klass") do + mw = Middleware.new klass + assert_equal klass, mw.klass + end + + define_method("test_#{name}_==") do + mw1 = Middleware.new klass + mw2 = Middleware.new klass + assert_equal mw1, mw2 + end + + end + + def test_string_class + mw = Middleware.new Omg.name + assert_equal Omg, mw.klass + end + + def test_double_equal_works_with_classes + k = Class.new + mw = Middleware.new k + assert_operator mw, :==, k + assert_operator mw, :!=, Class.new + end + + def test_double_equal_works_with_strings + mw = Middleware.new Omg + assert_operator mw, :==, Omg.name + end + + def test_double_equal_normalizes_strings + mw = Middleware.new Omg + assert_operator mw, :==, "::#{Omg.name}" + end + end + end +end diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG index 1b8bcf649c..99cea2586b 100644 --- a/activesupport/CHANGELOG +++ b/activesupport/CHANGELOG @@ -1,5 +1,11 @@ *Rails 3.1.0 (unreleased)* +* ActiveSupport::Dependencies::ClassCache class has been introduced for +holding references to reloadable classes. + +* ActiveSupport::Dependencies::Reference has been refactored to take direct +advantage of the new ClassCache. + * Backports Range#cover? as an alias for Range#include? in Ruby 1.8 [Diego Carrion, fxn] * Added weeks_ago and prev_week to Date/DateTime/Time. [Rob Zolkos, fxn] diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index dab6fdbac6..94a8608aeb 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -524,31 +524,52 @@ module ActiveSupport #:nodoc: explicitly_unloadable_constants.each { |const| remove_constant const } end - class Reference - @@constants = Hash.new { |h, k| h[k] = Inflector.constantize(k) } - - attr_reader :name - - def initialize(name) - @name = name.to_s - @@constants[@name] = name if name.respond_to?(:name) + class ClassCache + def initialize + @store = Hash.new { |h, k| h[k] = Inflector.constantize(k) } end - def get - @@constants[@name] + def empty? + @store.empty? end - def self.clear! - @@constants.clear + def key?(key) + @store.key?(key) + end + + def []=(key, value) + return unless key.respond_to?(:name) + + raise(ArgumentError, 'anonymous classes cannot be cached') unless key.name + + @store[key.name] = value + end + + def [](key) + key = key.name if key.respond_to?(:name) + + @store[key] + end + alias :get :[] + + def new(name) + self[name] = name + self + end + + def clear! + @store.clear end end + Reference = ClassCache.new + def ref(name) references[name] ||= Reference.new(name) end def constantize(name) - ref(name).get + ref(name).get(name) end # Determine if the given constant has been automatically loaded. diff --git a/activesupport/test/class_cache_test.rb b/activesupport/test/class_cache_test.rb new file mode 100644 index 0000000000..3d3ae559e5 --- /dev/null +++ b/activesupport/test/class_cache_test.rb @@ -0,0 +1,81 @@ +require 'abstract_unit' +require 'active_support/dependencies' + +module ActiveSupport + module Dependencies + class ClassCacheTest < ActiveSupport::TestCase + def setup + @cache = ClassCache.new + end + + def test_empty? + assert @cache.empty? + @cache[ClassCacheTest] = ClassCacheTest + assert !@cache.empty? + end + + def test_clear! + assert @cache.empty? + @cache[ClassCacheTest] = ClassCacheTest + assert !@cache.empty? + @cache.clear! + assert @cache.empty? + end + + def test_set_key + @cache[ClassCacheTest] = ClassCacheTest + assert @cache.key?(ClassCacheTest.name) + end + + def test_set_rejects_strings + @cache[ClassCacheTest.name] = ClassCacheTest + assert @cache.empty? + end + + def test_get_with_class + @cache[ClassCacheTest] = ClassCacheTest + assert_equal ClassCacheTest, @cache[ClassCacheTest] + end + + def test_get_with_name + @cache[ClassCacheTest] = ClassCacheTest + assert_equal ClassCacheTest, @cache[ClassCacheTest.name] + end + + def test_get_constantizes + assert @cache.empty? + assert_equal ClassCacheTest, @cache[ClassCacheTest.name] + end + + def test_get_is_an_alias + assert_equal @cache[ClassCacheTest], @cache.get(ClassCacheTest.name) + end + + def test_new + @cache.new ClassCacheTest + assert @cache.key?(ClassCacheTest.name) + end + + def test_new_rejects_strings + @cache.new ClassCacheTest.name + assert !@cache.key?(ClassCacheTest.name) + end + + def test_new_returns_self + v = @cache.new ClassCacheTest.name + assert_equal @cache, v + end + + def test_anonymous_class_fail + assert_raises(ArgumentError) do + @cache.new Class.new + end + + assert_raises(ArgumentError) do + x = Class.new + @cache[x] = x + end + end + end + end +end diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb index bc7f597f1d..b3ada53497 100644 --- a/activesupport/test/dependencies_test.rb +++ b/activesupport/test/dependencies_test.rb @@ -479,13 +479,13 @@ class DependenciesTest < Test::Unit::TestCase with_loading 'dependencies' do c = ActiveSupport::Dependencies.ref("ServiceOne") service_one_first = ServiceOne - assert_equal service_one_first, c.get + assert_equal service_one_first, c.get("ServiceOne") ActiveSupport::Dependencies.clear assert ! defined?(ServiceOne) service_one_second = ServiceOne - assert_not_equal service_one_first, c.get - assert_equal service_one_second, c.get + assert_not_equal service_one_first, c.get("ServiceOne") + assert_equal service_one_second, c.get("ServiceOne") end end