Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

4.0's ActiveRecord::DeprecationProxy doesn't handle extend/include #6600

Closed
jdelStrother opened this Issue Jun 2, 2012 · 3 comments

Comments

Projects
None yet
2 participants
Contributor

jdelStrother commented Jun 2, 2012

Hi - I've been starting to experiment with 4.0 and ran into a lot of problems with ActiveRecord plugins. It seems a pretty common pattern to have an on_load hook along the lines of :

ActiveSupport.on_load(:active_record) do
  include MyAwesomeStuff
end

The deprecation proxy (https://github.com/rails/rails/blob/master/activerecord/lib/active_record/model.rb#L82) doesn't handle this properly - it ends up including MyAwesomeStuff into the DeprecationProxy module, which isn't very useful. (We undef all instance_methods from the module, but that doesn't include the 'include' method, since it's private.) Perhaps add this ? -

diff --git a/activerecord/lib/active_record/model.rb b/activerecord/lib/active_record/model.rb
index 105d1e0..265878b 100644
--- a/activerecord/lib/active_record/model.rb
+++ b/activerecord/lib/active_record/model.rb
@@ -82,6 +82,7 @@ module ActiveRecord
     module DeprecationProxy #:nodoc:
       class << self
         instance_methods.each { |m| undef_method m unless m =~ /^__|^object_id$|^instance_eval$/ }
+        undef_method :include

         def method_missing(name, *args, &block)
           if Model.respond_to?(name)

to catch that case?


Then there's plugins that add class methods to ActiveRecord, via 'extend'. Both of these approaches :

ActiveSupport.on_load(:active_record) do
  extend MyAwesomeClassStuff
end
module MyAwesomeStuff
  def self.included(base)
    base.extend(MyAwesomeClassStuff)
  end
end

ActiveSupport.on_load(:active_record) do
  include MyAwesomeStuff
end

are doomed to fail, since they're extending stuff into the Model module, which won't then be available as class methods in ActiveRecord::Base subclasses.


I'm mostly just putting this up as a request for comments - some breakage with 4.0 is probably inevitable, I'm just trying to figure out if we want to try to make these sort of approaches work with 4.0 (with deprecation warnings), or whether we just force plugin authors to update.

Presumably the correct fix is :

ActiveSupport.on_load(:active_record) do
  ActiveRecord::Base.send :include, MyAwesomeStuff
  ActiveRecord::Base.send :extend, MyAwesomeClassStuff
end

? Or are you leaning towards plugins not automagically messing with ActiveRecord at all, and users should be manually include plugins they're interested in -

class User < ActiveRecord::Base
  include ActsAsActingActuary::ActiveRecordMagic
  acts_as_acting_actuary :foo
end

?

Member

jonleighton commented Jun 3, 2012

Thanks, I'll take a look at this.

Contributor

jdelStrother commented Jun 3, 2012

Let me know if I can do anything.

@ghost ghost assigned jonleighton Jun 3, 2012

Member

jonleighton commented Aug 3, 2012

Hey @jdelStrother, I've pushed a fix, could you let me know if you have further issues?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment