Skip to content

Do not eager load models that are observerd #7064

Closed
wants to merge 1 commit into from
View
79 activemodel/lib/active_model/observing.rb
@@ -13,6 +13,7 @@ module Observing
extend ActiveSupport::Concern
included do
+ Observer.model_loaded self
extend ActiveSupport::DescendantsTracker
end
@@ -106,6 +107,7 @@ def instantiate_observer(observer) #:nodoc:
# Notify observers when the observed class is subclassed.
def inherited(subclass)
super
+ Observer.model_loaded subclass
notify_observers :observed_class_inherited, subclass
end
end
@@ -198,11 +200,21 @@ class Observer
extend ActiveSupport::DescendantsTracker
class << self
- # Attaches the observer to the supplied model classes.
+ # Attaches the observer to the supplied model classes or register them for lazy observation.
def observe(*models)
models.flatten!
- models.collect! { |model| model.respond_to?(:to_sym) ? model.to_s.camelize.constantize : model }
- singleton_class.redefine_method(:observed_classes) { models }
+
+ class_names = models.map do |model|
+ if model.is_a?(String) || model.is_a?(Symbol)
+ model.to_s.camelize
+ else
+ name = model.name
+ ActiveSupport::Deprecation.warn("If you pass '#{name}' to observe you can avoid eager loading #{name}.")
+ name
+ end
+ end.uniq
+
+ singleton_class.redefine_method(:observed_classes) { class_names }
end
# Returns an array of Classes to observe.
@@ -221,14 +233,63 @@ def observed_classes
# The class observed by default is inferred from the observer's class name:
# assert_equal Person, PersonObserver.observed_class
def observed_class
- name[/(.*)Observer/, 1].try :constantize
+ name[/(.*)Observer/, 1]
+ end
+
+ # register model as loaded so it can be connected to it's observers
+ def model_loaded(model)
+ model_name = model.name
+ registered_models[model_name] = model
+ registered_observers.each do |observer_instance|
+ connect!(observer_instance, model) if observer_instance.observed_classes.include?(model_name)
+ end
+ end
+
+ # mark observer as ready to be connected to models
+ def observer_initialized(observer_instance)
+ registered_observers << observer_instance
+ observed_classes = observer_instance.send :observed_classes
+ registered_models.each do |model_name, model|
+ connect!(observer_instance, model) if observed_classes.include?(model_name)
+ end
+ end
+
+ # Load all observed models that would normally be lazy loaded
+ def load_all
+ registered_observers.each do |observer|
+ observer.observed_classes.each(&:constantize)
+ end
+ end
+
+ private
+
+ def registered_observers
+ @@registered_observers ||= []
+ end
+
+ def registered_models
+ @@registered_models ||= {}
+ end
+
+ def connect!(observer, model)
+ ([model] + model.descendants).each do |klass|
+ next if connected?(observer, klass)
+ observer.observed_class_inherited(klass)
+ end
+ end
+
+ def connected?(observer, klass)
+ @@connected ||= {}
+ return true if @@connected[[observer, klass]]
+ @@connected[[observer, klass]] = true
+ false
end
end
# Start observing the declared classes and their subclasses.
# Called automatically by the instance method.
def initialize
- observed_classes.each { |klass| add_observer!(klass) }
+ self.class.observer_initialized(self)
end
def observed_classes #:nodoc:
@@ -245,8 +306,12 @@ def update(observed_method, object, *extra_args, &block) #:nodoc:
# Special method sent by the observed class when it is inherited.
# Passes the new subclass.
def observed_class_inherited(subclass) #:nodoc:
- self.class.observe(observed_classes + [subclass])
- add_observer!(subclass)
+ if name = subclass.name
+ self.class.observe(observed_classes + [name])
+ add_observer!(subclass)
+ else
+ warn("Cannot observe anonymous inherited class #{subclass}.")
+ end
end
protected
View
92 activemodel/test/cases/observing_test.rb
@@ -87,12 +87,23 @@ def setup
test "passes observers to subclasses" do
FooObserver.instance
- bar = Class.new(Foo)
- assert_equal Foo.observers_count, bar.observers_count
+ class ::Foo2 < Foo
+ end
+ assert_equal Foo.observers_count, Foo2.observers_count
+ end
+
+ test "shows helpful warning when inheriting with nameless classes" do
+ FooObserver.instance
+ FooObserver.any_instance.expects(:warn).at_least_once # FooObserver can have multiple instances when other tests ran first
+ Class.new(Foo)
end
end
class ObserverTest < ActiveModel::TestCase
+ class Base
+ include ActiveModel::Observing
+ end
+
def setup
ObservedModel.observers = :foo_observer
FooObserver.singleton_class.instance_eval do
@@ -107,31 +118,41 @@ def teardown
end
end
+ def clear
+ ActiveModel::Observer.send(:registered_observers).clear
+ ActiveModel::Observer.send(:registered_models).clear
+ end
+
test "guesses implicit observable model name" do
- assert_equal Foo, FooObserver.observed_class
+ assert_equal "Foo", FooObserver.observed_class
end
test "tracks implicit observable models" do
instance = FooObserver.new
- assert_equal [Foo], instance.observed_classes
+ assert_equal ["Foo"], instance.observed_classes
end
test "tracks explicit observed model class" do
- FooObserver.observe ObservedModel
+ FooObserver.observe "ObservedModel"
instance = FooObserver.new
- assert_equal [ObservedModel], instance.observed_classes
+ assert_equal ["ObservedModel"], instance.observed_classes
+ end
+
+ test "warns about observing classes" do
+ ActiveSupport::Deprecation.expects(:warn)
+ FooObserver.observe ObservedModel
end
test "tracks explicit observed model as string" do
FooObserver.observe 'observed_model'
instance = FooObserver.new
- assert_equal [ObservedModel], instance.observed_classes
+ assert_equal ["ObservedModel"], instance.observed_classes
end
test "tracks explicit observed model as symbol" do
FooObserver.observe :observed_model
instance = FooObserver.new
- assert_equal [ObservedModel], instance.observed_classes
+ assert_equal ["ObservedModel"], instance.observed_classes
end
test "calls existing observer event" do
@@ -173,9 +194,58 @@ class BarObserver < ActiveModel::Observer
observe :foo
end
- assert_equal [Foo], BarObserver.observed_classes
+ assert_equal ["Foo"], BarObserver.observed_classes
+
+ BarObserver.observe("ObservedModel")
+ assert_equal ["ObservedModel"], BarObserver.observed_classes
+ end
- BarObserver.observe(ObservedModel)
- assert_equal [ObservedModel], BarObserver.observed_classes
+ test "connects model with observers when models are initialized" do
+ class Lazy3Observer < ActiveModel::Observer
+ observe "ObserverTest::Foo3"
+ end
+ Lazy3Observer.instance
+
+ Lazy3Observer.any_instance.expects(:add_observer!).with{|klass| klass == Foo3 }
+ class Foo3 < Base
+ end
+ end
+
+ test "connects model with observers when observers are initialized" do
+ class Foo4 < Base
+ end
+
+ class Lazy4Observer < ActiveModel::Observer
+ observe "ObserverTest::Foo4"
+ end
+
+ Foo4.expects(:add_observer).with{|observer| observer.class == Lazy4Observer }
+ Lazy4Observer.instance
+ end
+
+ test "observes descendants when model is observed after descendants where defined" do
+ class Foo5 < Base
+ end
+
+ class Foo5A < Foo5
+ end
+
+ class Lazy5Observer < ActiveModel::Observer
+ observe "ObserverTest::Foo5"
+ end
+
+ Foo5.expects(:add_observer).with{|observer| observer.class == Lazy5Observer }
+ Foo5A.expects(:add_observer).with{|observer| observer.class == Lazy5Observer }
+ Lazy5Observer.instance
+ end
+
+ test "load_all loads all lazyly observed models" do
+ class EagerObserver < ActiveModel::Observer
+ observe "ObserverTest::DoesNotExit"
+ end
+ EagerObserver.instance # not loaded since it is lazy
+ assert_raise NameError do
+ ActiveModel::Observer.load_all
+ end
end
end
View
12 activerecord/lib/active_record/observer.rb
@@ -83,12 +83,9 @@ module ActiveRecord
# == Loading
#
# Observers register themselves in the model class they observe, since it is the class that
- # notifies them of events when they occur. As a side-effect, when an observer is loaded its
- # corresponding model class is loaded.
+ # notifies them of events when they occur.
#
- # Up to (and including) Rails 2.0.2 observers were instantiated between plugins and
- # application initializers. Now observers are loaded after application initializers,
- # so observed models can make use of extensions.
+ # Observers are loaded after application initializers, so observed models can make use of extensions.
#
# If by any chance you are using observed models in the initialization you can still
# load their observers by calling <tt>ModelObserver.instance</tt> before. Observers are
@@ -98,11 +95,6 @@ class Observer < ActiveModel::Observer
protected
- def observed_classes
- klasses = super
- klasses + klasses.map { |klass| klass.descendants }.flatten
- end
-
def add_observer!(klass)
super
define_callbacks klass
View
8 activerecord/test/cases/lifecycle_test.rb
@@ -137,15 +137,15 @@ def test_before_destroy
def test_auto_observer
topic_observer = TopicaAuditor.instance
assert_nil TopicaAuditor.observed_class
- assert_equal [Topic], TopicaAuditor.observed_classes.to_a
+ assert_equal ([Topic] + Topic.descendants).map(&:name), TopicaAuditor.observed_classes.to_a
topic = Topic.find(1)
assert_equal topic.title, topic_observer.topic.title
end
def test_inferred_auto_observer
topic_observer = TopicObserver.instance
- assert_equal Topic, TopicObserver.observed_class
+ assert_equal "Topic", TopicObserver.observed_class
topic = Topic.find(1)
assert_equal topic.title, topic_observer.topic.title
@@ -176,15 +176,15 @@ def test_observing_subclasses
def test_after_find_can_be_observed_when_its_not_defined_on_the_model
observer = MinimalisticObserver.instance
- assert_equal Minimalistic, MinimalisticObserver.observed_class
+ assert_equal "Minimalistic", MinimalisticObserver.observed_class
minimalistic = Minimalistic.find(1)
assert_equal minimalistic, observer.minimalistic
end
def test_after_find_can_be_observed_when_its_defined_on_the_model
observer = TopicObserver.instance
- assert_equal Topic, TopicObserver.observed_class
+ assert_equal "Topic", TopicObserver.observed_class
topic = Topic.find(1)
assert_equal topic, observer.topic
Something went wrong with that request. Please try again.