Skip to content

Commit

Permalink
Lots of progress refactoring observing.
Browse files Browse the repository at this point in the history
- Added class called `ObserveeSet` to hold all the models observed by a
given observer.
- Created a DeferredModelLoader which is a sort of proxy a model
classes (it whines unless you instantiate it with the stringy name of a
model) that also checks what `Object.const_defined?` has to say about
it, but it never actually tries to load the model unless explicitly
asked.
- Still need to set up inheritance and enablement and generally fix
ObserverArray which hasn't been ported for the updated code yet.
  • Loading branch information
mvastola committed Jun 14, 2017
1 parent 433742b commit aeca112
Show file tree
Hide file tree
Showing 13 changed files with 224 additions and 70 deletions.
23 changes: 18 additions & 5 deletions NOTES-MV.md
Original file line number Diff line number Diff line change
@@ -1,22 +1,35 @@
## Strategy
## TODO
- [ ] Implement `#instantiate_observers`
- [x] New structure to replace ObserverArray
- [ ] Each ORM (i.e. `ActiveRecord::Base`) will have a class method `#pending_observees` returning a array of all fully-qualified model names (as strings) to begin observing upon inheritance.
- [ ] Each ORM observer class (i.e. `ActiveRecord::Observer`) will have a class method (`#descendant_instances`) returning an array of all observers (objects).
- [ ] Each observer will have an `#current_observees` class method which will return an array (of strings) the models the observer observes.
- [ ] Each model will have an `#observers` class method which will obtain an array of observer objects observing the model.
- [x] Only strings can be 'observed'. (Anything that responds to #to_s is converted. Warnings are raised for Classes.)
- [ ] Upon setting up observers, instantiate them, but don't rush to load the classes they observe. instead, hook into those by monitoring the inherit callbacks of `Active*::Base`
- [ ] This means observers specify their classes with symbols or strings. *NOT* constants.
- [x] This means observers specify their classes with symbols or strings. *NOT* constants.
- [ ] ObserverArray should *only* store Observer instances. No need to store observer classes at all. (Can create a delegator to helpful methods in the class.)

## TODO
## Checks
- [ ] Probably a better idea to change functionality since this is a major version change rather than have deprecators strewn all over the place.
- [ ] I have a strong bias for `prepend` nowadays. Check to see if when I use `prepend` it's really preferable over `include`/`extend`.
- [ ] Document all changes (especially non-breaking ones).
- [ ] Document all changes (especially backward-incompatible ones).
- [ ] Make sure documentation is pretty and where it belongs.
- [ ] Optimize autoloading, etc. (Perhaps last?)
- [ ] Writeup upgrade instructions (noting backwards-incompatible changes).
- [ ] Perhaps write an upgrade generator(?)
- [ ] Cause deprecator to raise errors in testing mode.
- [ ] Cause deprecator to raise errors in testing mode. (Fix/complete it in general)
- [ ] Confirm bootstrapper works for non-rails.
- [ ] Make sure enablement is working (I'm pretty sure I broke it)
- [ ] Make sure I didn't break anything that was threadsafe
- [ ] Add tests for any new functionality you write
- [ ] Check for all TODOs, FIXMEs, etc

## Summary of Changes
- Added Bundler confifiguration setting to make `:github` source in `Gemfile` use `HTTPS`. (Was triggering warning with latest Bundler.)
- Restructured directories and code locations so that it would make intuitive sense (and also so that code would be compatible with autoloaders without manual `require`s or additional configuration).
- Removed reliance on `rails` and `railties` gems.
- You can no longer redefine the method #observed_classes in ActiveModel::Observer subclasses. You can either use `observe :class` or `observed_classes = :class`

## Stack Trace
- `activesupport-5.0.1/lib/active_support/dependencies.rb:509` in `load_missing_constant`: Circular dependency detected while autoloading constant `User` (`RuntimeError`)
Expand Down
41 changes: 41 additions & 0 deletions lib/active_model/observee_set.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
require 'active_model'
module ActiveModel
class ObserveeSet # aka Classes Observed by an Observer

attr_reader :observer
attr_reader :observees
attr_predicate :observees
def initialize(observer) #:nodoc:
@observer = observer
@observees ||= Set.new
end

def add(*new_models)
new_models = new_models.flatten.map do |model|
Observing::DeferredModelLoader.new(model)
end.compact
observees.merge(new_models)
end
alias_method :<<, :add
alias_method :merge, :add

def replace(*models)
observees.clear
add(*models)
end
alias_method :observees=, :replace

def all
observees.to_a.freeze
end

def loaded
all.select(&:model_defined?).freeze
end

def pending
all.reject(&:model_defined?).freeze
end

end
end
118 changes: 67 additions & 51 deletions lib/active_model/observer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ module ActiveModel
# <tt>ProductManagerObserver</tt> to <tt>ProductManager</tt>, and so on. If
# you want to name your observer differently than the class you're interested
# in observing, you can use the <tt>Observer.observe</tt> class method which
# takes either the concrete class (<tt>Product</tt>) or a symbol for that
# class (<tt>:product</tt>):
# takes either a string or a symbol containing the name of that class
# (<tt>:product</tt>):
#
# class AuditObserver < ActiveModel::Observer
# observe :account
Expand Down Expand Up @@ -65,63 +65,83 @@ module ActiveModel
# ActiveRecord::Observer.
class Observer
include Singleton
prepend Rails::Observers::Deprecation
include Rails::Observers::Deprecation
extend ActiveSupport::DescendantsTracker

class << self
# Attaches the observer to the supplied model classes.
# Attaches the observer to the supplied model class names.
#
# class AuditObserver < ActiveModel::Observer
# observe :account, :balance
# observe 'Account', 'Balance'
# end
#
# AuditObserver.observed_classes # => [Account, Balance]
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 }
instance.observees = models
end
end

# Returns an array of Classes to observe.
#
# AccountObserver.observed_classes # => [Account]
#
# You can override this instead of using the +observe+ helper.
#
# class AuditObserver < ActiveModel::Observer
# def self.observed_classes
# [Account, Balance]
# end
# end
def observed_classes
unless defined?(@observed_classes)
@observed_classes = Set.new
@observed_classes << default_observed_class if default_observed_class
@observed_classes.freeze
end
@observed_classes
end
# Start observing the declared classes and their subclasses.
# Called automatically by the instance method.
def initialize
@observees = ObserveeSet.new(self)
@default_observee = self.class.name.sub!(/Observer\z/, '').freeze
observees << default_observee if default_observee?
end

# Returns the class observed by default. It's inferred from the observer's
# class name.
#
# PersonObserver.default_observed_class # => Person
# AccountObserver.default_observed_class # => Account
def default_observed_class
return @default_observed_class if defined?(@default_observed_class)
@default_observed_class = self.name.sub!(/Observer\z/, '').try(:safe_constantize)
end
deprecated_alias_method :observed_class, :default_observed_class
# Returns an array of names of classes to observe.
#
# AccountObserver.observees # => [:account]
#
# By default, the observed class name is obtained by removing 'Observer' from
# from the end of the observer's name. If the observer's class name doesn't end
# in 'Observer', no class is observed by default.


# Attaches the observer to the supplied model class names.
#
# AuditObserver.observees = [ 'Account', 'Balance' ]
attr_reader :observees
def observees=(models)
observees.replace(models)
end

delegate :observed_classes, :to => :class
# Attaches the observer to a new model class *without* removing the previous ones.
def observe!(klass)
observees << klass.is_a?(String) ? klass : klass.name
end

# Start observing the declared classes and their subclasses.
# Called automatically by the instance method.
def initialize #:nodoc:
observed_classes.each { |klass| add_observer!(klass) }
# Returns an array of observed models (as strings), both those that are being observed,
# and those that will be once they are loaded
#
# AuditObserver.observed_classes # => [ 'Account', 'Balance' ]
def observed_classes
observees.all.map(&:model_name).freeze
end

# Returns an array of observed models (as strings) that are loaded.
#
# AuditObserver.loaded_observed_classes # => [ 'Account', 'Balance' ]
def loaded_observed_classes
observees.loaded.map(&:model_name).freeze
end

# Returns an array of as-of-yet unloaded models (as strings) that will be observed.
#
# AuditObserver.pending_observed_classes # => [ 'Account', 'Balance' ]
def pending_observed_classes
observees.pending.map(&:model_name).freeze
end


# Returns the class (if any) observed by default. It's inferred from the observer's
# class name and obtained by simply removing the word 'Observer' from the end (if present).
#
# PersonObserver.default_observee # => Person
# AccountObserver.default_observee # => Account
attr_reader :default_observee
attr_predicate :default_observee

# TODO: Add a method_added callback to watch that no one redefines observed_class(es) and
# raise fire a deprecation error if so.

# Send observed_method(object) if the method exists and
# the observer is enabled for the given object's class.
Expand All @@ -133,17 +153,13 @@ 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)
observed_classes << subclass
observer!(subclass)
end

protected
def add_observer!(klass) #:nodoc:
klass.add_observer(self)
end

# Returns true if notifications are disabled for this object.
def disabled_for?(object) #:nodoc:
def disabled_for?(object)
klass = object.class
return false unless klass.respond_to?(:observers)
klass.observers.disabled_for?(self)
Expand Down
16 changes: 9 additions & 7 deletions lib/active_model/observing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,27 @@ module Observing
extend ActiveSupport::Autoload
autoload :ClassMethods
autoload :InstanceMethods
autoload :DeferredModelLoader

class << self

def prepend_features(klass)
klass.mattr_reader(:observer_orm) { self }
class << klass
include ::ActiveSupport::DescendantsTracker
prepend ::ActiveModel::Observing::ClassMethods
end
klass.mattr_reader :observer_orm { klass }

klass.prepend ::ActiveModel::Observing::InstanceMethods
add_instantiation_hooks!(klass)
end
# Previously, people called include ActiveModel::Observing
# so let's be backwards compatible
alias_method :append_features, :prepend_features

private

def add_instantiation_hooks!(klass)
@@reloader_class = begin
::ActiveSupport::Reloader
rescue NameError
::ActiveSupport::Reloader
rescue NameError
::ActionDispatch::Reloader
end
ActiveSupport.on_load :after_initialize, :yield => true do
Expand All @@ -34,6 +35,7 @@ def add_instantiation_hooks!(klass)
end
end
end

end
end
end
7 changes: 4 additions & 3 deletions lib/active_model/observing/class_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ module ClassMethods
prepend Rails::Observers::Deprecation
def inherited(subclass)
super
subclass.observer_orm.instantiate_observers
notify_observers :observed_class_inherited, subclass
#TODO: uncomment
#subclass.observer_orm.instantiate_observers
#notify_observers :observed_class_inherited, subclass
end

# Activates the observers assigned.
#
# class ORM
Expand Down
56 changes: 56 additions & 0 deletions lib/active_model/observing/deferred_model_loader.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
module ActiveModel
module Observing
class DeferredModelLoader
include Comparable
extend ActiveSupport::DescendantsTracker
class << self
def load_all
descendants.reject(&:loaded?).each(&:load!)
end

def load_if(&block)
descendants.reject(&:loaded?).select(&block).each(&:load!)
end
end

attr_reader :model_name
def initialize(model_name)
if [ Class, Module ].any? { |mod| model_name.is_a?(mod) }
warn ArgumentError, "Specifying models to observe as classes " +
"(as opposed to the stringy names of classes) is highly deprecated and " +
"very likely to result in circular dependencies. (From observation of #{model} " +
"by #{observer}.)"
model_name = model_name.name
elsif model_name.is_a?(self.class)
model_name = model_name.name
end

if [ String, Symbol ].none? { |mod| model_name.is_a?(mod) }
__raise__ ArgumentError, "Argument #{model_name} must be a String or Symbol."
elsif !model_name.respond_to?(:to_s)
__raise__ ArgumentError, "Argument #{model_name} must respond to #to_s."
end
@model_name = model_name.to_s.camelize.freeze
end

def <=>(other)
self.model_name <=> other.model_name
end

def model_defined?
Object.const_defined?(model_name)
end

def model_assigned?
defined?(@model)
end

def model(load_model = false)
return @model if model_assigned?
return nil unless load_model || model_defined?
@model = Object.const_get(model_name)
end

end
end
end
2 changes: 1 addition & 1 deletion lib/active_record/observing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
module ActiveRecord
module Observing
def self.prepended(klass)
klass.include ActiveModel::Observing
klass.prepend ActiveModel::Observing
end
end
end
4 changes: 2 additions & 2 deletions lib/active_resource/observing.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
require 'active_resource'
module ActiveResource
module Observing
def self.prepended(context)
context.include ActiveModel::Observing
def self.prepended(klass)
klass.prepend ActiveModel::Observing
end

def create(*)
Expand Down
5 changes: 5 additions & 0 deletions lib/core_ext.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
__FILE__.sub(/\.rb\z/, '').tap do |path|
[ 'string', 'module' ].each do |file|
require File.join(path, file)
end
end
5 changes: 5 additions & 0 deletions lib/core_ext/module.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
__FILE__.sub(/\.rb\z/, '').tap do |path|
[ 'attr_predicate' ].each do |file|
require File.join(path, file)
end
end
Loading

0 comments on commit aeca112

Please sign in to comment.