Skip to content
Browse files

Get rid of the ActiveRecord::Model::DeprecationProxy thing.

I think it's going to be too much pain to try to transition the
:active_record load hook from executing against Base to executing
against Model.

For example, after Model is included in Base, and modules included in
Model will no longer get added to the ancestors of Base.

So plugins which wish to be compatible with both Model and Base should
use the :active_record_model load hook which executes *before* Base gets
loaded.

In general, ActiveRecord::Model is an advanced feature at the moment and
probably most people will continue to inherit from ActiveRecord::Base
for the time being.
  • Loading branch information...
1 parent 18e979e commit 83846838252397b3781eed165ca301e05db39293 @jonleighton jonleighton committed Oct 19, 2012
View
9 activerecord/CHANGELOG.md
@@ -945,7 +945,14 @@
* Plugins & libraries etc that add methods to `ActiveRecord::Base`
will not be compatible with `ActiveRecord::Model`. Those libraries
should add to `ActiveRecord::Model` instead (which is included in
- `Base`), or better still, avoid monkey-patching AR and instead
+ `Base`). This should be done using the `:active_record_model`
+ load hook, which executes before `ActiveRecord::Base` loads:
+
+ ActiveSupport.on_load(:active_record_model) do
+ include MyPlugin
+ end
+
+ Or better still, avoid monkey-patching AR and instead
provide a module that users can include where they need it.
* To minimise the risk of conflicts with other code, it is
View
2 activerecord/lib/active_record/attribute_methods/dirty.rb
@@ -2,7 +2,7 @@
require 'active_support/deprecation'
module ActiveRecord
- ActiveSupport.on_load(:active_record_config) do
+ ActiveSupport.on_load(:active_record_model) do
mattr_accessor :partial_writes, instance_accessor: false
self.partial_writes = true
end
View
2 activerecord/lib/active_record/attribute_methods/read.rb
@@ -1,5 +1,5 @@
module ActiveRecord
- ActiveSupport.on_load(:active_record_config) do
+ ActiveSupport.on_load(:active_record_model) do
mattr_accessor :attribute_types_cached_by_default, instance_accessor: false
end
View
2 activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb
@@ -1,6 +1,6 @@
module ActiveRecord
- ActiveSupport.on_load(:active_record_config) do
+ ActiveSupport.on_load(:active_record_model) do
mattr_accessor :time_zone_aware_attributes, instance_accessor: false
self.time_zone_aware_attributes = false
View
4 activerecord/lib/active_record/base.rb
@@ -323,6 +323,6 @@ module ActiveRecord #:nodoc:
class Base
include ActiveRecord::Model
end
-end
-ActiveSupport.run_load_hooks(:active_record, ActiveRecord::Model::DeprecationProxy.new)
+ ActiveSupport.run_load_hooks(:active_record, Base)
+end
View
2 activerecord/lib/active_record/core.rb
@@ -3,7 +3,7 @@
require 'thread'
module ActiveRecord
- ActiveSupport.on_load(:active_record_config) do
+ ActiveSupport.on_load(:active_record_model) do
##
# :singleton-method:
#
View
2 activerecord/lib/active_record/explain.rb
@@ -1,7 +1,7 @@
require 'active_support/lazy_load_hooks'
module ActiveRecord
- ActiveSupport.on_load(:active_record_config) do
+ ActiveSupport.on_load(:active_record_model) do
mattr_accessor :auto_explain_threshold_in_seconds, instance_accessor: false
end
View
2 activerecord/lib/active_record/inheritance.rb
@@ -1,6 +1,6 @@
module ActiveRecord
- ActiveSupport.on_load(:active_record_config) do
+ ActiveSupport.on_load(:active_record_model) do
# Determine whether to store the full constant name including namespace when using STI
mattr_accessor :store_full_sti_class, instance_accessor: false
self.store_full_sti_class = true
View
2 activerecord/lib/active_record/locking/optimistic.rb
@@ -1,5 +1,5 @@
module ActiveRecord
- ActiveSupport.on_load(:active_record_config) do
+ ActiveSupport.on_load(:active_record_model) do
mattr_accessor :lock_optimistically, instance_accessor: false
self.lock_optimistically = true
end
View
45 activerecord/lib/active_record/model.rb
@@ -115,52 +115,21 @@ def inheritance_column
'type'
end
end
-
- class DeprecationProxy < BasicObject #:nodoc:
- def initialize(model = Model, base = Base)
- @model = model
- @base = base
- end
-
- def method_missing(name, *args, &block)
- if @model.respond_to?(name, true)
- @model.send(name, *args, &block)
- else
- ::ActiveSupport::Deprecation.warn(
- "The object passed to the active_record load hook was previously ActiveRecord::Base " \
- "(a Class). Now it is ActiveRecord::Model (a Module). You have called `#{name}' which " \
- "is only defined on ActiveRecord::Base. Please change your code so that it works with " \
- "a module rather than a class. (Model is included in Base, so anything added to Model " \
- "will be available on Base as well.)"
- )
- @base.send(name, *args, &block)
- end
- end
-
- alias send method_missing
-
- def extend(*mods)
- ::ActiveSupport::Deprecation.warn(
- "The object passed to the active_record load hook was previously ActiveRecord::Base " \
- "(a Class). Now it is ActiveRecord::Model (a Module). You have called `extend' which " \
- "would add singleton methods to Model. This is presumably not what you want, since the " \
- "methods would not be inherited down to Base. Rather than using extend, please use " \
- "ActiveSupport::Concern + include, which will ensure that your class methods are " \
- "inherited."
- )
- @base.extend(*mods)
- end
- end
end
- # This hook is where config accessors on Model get defined.
+ # This hook is where config accessors on Model should be defined.
#
# We don't want to just open the Model module and add stuff to it in other files, because
# that would cause Model to load, which causes all sorts of loading order issues.
#
# We need this hook rather than just using the :active_record one, because users of the
# :active_record hook may need to use config options.
- ActiveSupport.run_load_hooks(:active_record_config, Model)
+ #
+ # Users who wish to include a module in Model that they want to also
+ # get inherited by Base should do so using this load hook. After Base
+ # has included Model, any modules subsequently included in Model won't
+ # be inherited by Base.
+ ActiveSupport.run_load_hooks(:active_record_model, Model)
# Load Base at this point, because the active_record load hook is run in that file.
Base
View
2 activerecord/lib/active_record/model_schema.rb
@@ -1,6 +1,6 @@
module ActiveRecord
- ActiveSupport.on_load(:active_record_config) do
+ ActiveSupport.on_load(:active_record_model) do
mattr_accessor :primary_key_prefix_type, instance_accessor: false
mattr_accessor :table_name_prefix, instance_accessor: false
View
2 activerecord/lib/active_record/nested_attributes.rb
@@ -3,7 +3,7 @@
require 'active_support/core_ext/hash/indifferent_access'
module ActiveRecord
- ActiveSupport.on_load(:active_record_config) do
+ ActiveSupport.on_load(:active_record_model) do
mattr_accessor :nested_attributes_options, instance_accessor: false
self.nested_attributes_options = {}
end
View
2 activerecord/lib/active_record/serialization.rb
@@ -1,5 +1,5 @@
module ActiveRecord #:nodoc:
- ActiveSupport.on_load(:active_record_config) do
+ ActiveSupport.on_load(:active_record_model) do
mattr_accessor :include_root_in_json, instance_accessor: false
self.include_root_in_json = true
end
View
2 activerecord/lib/active_record/timestamp.rb
@@ -1,6 +1,6 @@
module ActiveRecord
- ActiveSupport.on_load(:active_record_config) do
+ ActiveSupport.on_load(:active_record_model) do
mattr_accessor :record_timestamps, instance_accessor: false
self.record_timestamps = true
end
View
36 activerecord/test/cases/inclusion_test.rb
@@ -82,42 +82,6 @@ def test_mirrored_configuration
def test_included_twice
@klass.send :include, ActiveRecord::Model
end
-
- def test_deprecation_proxy
- proxy = ActiveRecord::Model::DeprecationProxy.new
-
- assert_equal ActiveRecord::Model.name, proxy.name
- assert_equal ActiveRecord::Base.superclass, assert_deprecated { proxy.superclass }
-
- sup, sup2 = nil, nil
- ActiveSupport.on_load(:__test_active_record_model_deprecation) do
- sup = superclass
- sup2 = send(:superclass)
- end
- assert_deprecated do
- ActiveSupport.run_load_hooks(:__test_active_record_model_deprecation, proxy)
- end
- assert_equal ActiveRecord::Base.superclass, sup
- assert_equal ActiveRecord::Base.superclass, sup2
- end
-
- test "including in deprecation proxy" do
- model, base = ActiveRecord::Model.dup, ActiveRecord::Base.dup
- proxy = ActiveRecord::Model::DeprecationProxy.new(model, base)
-
- mod = Module.new
- proxy.include mod
- assert model < mod
- end
-
- test "extending in deprecation proxy" do
- model, base = ActiveRecord::Model.dup, ActiveRecord::Base.dup
- proxy = ActiveRecord::Model::DeprecationProxy.new(model, base)
-
- mod = Module.new
- assert_deprecated { proxy.extend mod }
- assert base.singleton_class < mod
- end
end
class InclusionFixturesTest < ActiveRecord::TestCase

3 comments on commit 8384683

@rafaelfranca
Ruby on Rails member

This broke the railties tests. I did this fix but I'm not sure if this is the best way to deal with it.

diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb
index d7e35fb..b3dd9ba 100644
--- a/activerecord/lib/active_record/railtie.rb
+++ b/activerecord/lib/active_record/railtie.rb
@@ -80,7 +80,7 @@ module ActiveRecord
             if File.file?(filename)
               cache = Marshal.load File.binread filename
               if cache.version == ActiveRecord::Migrator.current_version
-                ActiveRecord::Model.connection.schema_cache = cache
+                ActiveRecord::Base.connection.schema_cache = cache
               else
                 warn "Ignoring db/schema_cache.dump because it has expired. The current schema version is #{ActiveRecord::Migrator.current_version}, but the one in the cache is #{cache.version}."
               end
@@ -122,8 +122,8 @@ module ActiveRecord

       ActiveSupport.on_load(:active_record) do
         ActionDispatch::Reloader.send(hook) do
-          ActiveRecord::Model.clear_reloadable_connections!
-          ActiveRecord::Model.clear_cache!
+          ActiveRecord::Base.clear_reloadable_connections!
+          ActiveRecord::Base.clear_cache!
         end
       end
     end
@@ -135,10 +135,10 @@ module ActiveRecord

     config.after_initialize do |app|
       ActiveSupport.on_load(:active_record) do
-        ActiveRecord::Model.instantiate_observers
+        ActiveRecord::Base.instantiate_observers

         ActionDispatch::Reloader.to_prepare do
-          ActiveRecord::Model.instantiate_observers
+          ActiveRecord::Base.instantiate_observers
         end
       end
@jonleighton
Ruby on Rails member

Sorry for breaking the tests. Looks like @jeremy reverted it so I'll pick this back up in the next week or so.

@jeremy
Ruby on Rails member
jeremy commented on 8384683 Oct 21, 2012

Thanks @jonleighton - sorry for the unceremonious revert! ❤️

Please sign in to comment.
Something went wrong with that request. Please try again.