Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

callbacks don't work from extended modules

#575

If an AR instance extends a module that adds callbacks to the instance's metaclass, then those callbacks do not get called when the callback chain is invoked.

I have included a patch that fixes the problem as well as adds a test case. All tests pass in activesupport after the patch.

Please see the test case included in the patch for a very clear description of the problem.

This patch is meant to be applied from SRC_ROOT/activesupport with -p0.

This patch is for Rails 2.1.0.

Reported by Christopher J. Bottaro · July 8th, 2008 @ 06:32 PM

State: wontfix
Milestone: 2.x
Assigned to: josh josh
Importance: none

Activity

  1. Christopher J. Bottaro
    Christopher J. Bottaro

    I forgot to mention, this patch is for Rails 2.1.0.

    July 8th, 2008 @ 06:29 PM

  2. Pratik
    Pratik
    • Assigned user set to josh

    July 14th, 2008 @ 02:49 PM

  3. Repository
    Repository
    • State changed from new to resolved

    (from [be078ee162fcae883a5621a30929879cd783a238]) Run callbacks from object's metaclass [#575 state:resolved]

    http://github.com/rails/rails/co...

    July 16th, 2008 @ 03:58 AM

  4. josh
    josh
    • State changed from resolved to open

    We had to revert this. Apparently it broke a ton of shit.

    Maybe we don't want callbacks on the metaclass, but a set of callbacks for objects as well as classes.

    Lets try a new API

    module ModuleWithCallbacks
      def self.extended(obj)
        obj.before_save :module_callback
      end
    
      def module_callback
        # ...
      end
    end
    
    p = Person.new
    p.extend(ModuleWithCallbacks)
    
    # or also do
    p.before_save :module_callback
    def p.module_callback
      ...
    end
    

    We could have a instance variable on the object that tracks its callbacks

    July 17th, 2008 @ 03:26 AM

  5. Christopher J. Bottaro
    Christopher J. Bottaro

    Just out of curiosity, what did it break?

    It broke our webapp because we try to serialize AR instances and it complained about not being able to serialize the object's metaclass. We just decided that we shouldn't have been serializing AR instances in the first place, heh.

    July 17th, 2008 @ 03:55 AM

  6. Christopher J. Bottaro
    Christopher J. Bottaro

    Btw, your solution of having callback chains on classes as well as instances of classes (objects) sounds great.

    July 17th, 2008 @ 03:59 AM

  7. josh
    josh

    Check out the latest Memoizable stuff for reference.

    http://github.com/rails/rails/tr...

    I've been playing around with a pattern that works for Classes and Objects.

    I don't have time to work on this right now, but I do plan to get back to in in the next coming weeks.

    (BTW, I'm getting obsessed with "object-oriented"/prototype driven development)

    July 19th, 2008 @ 01:30 AM

  8. josh
    josh
    • State changed from open to wontfix

    It looks like this is going take alot more work. If you ever decide its worth pursuing, open and new ticket for refactoring Callbacks and assign it to me.

    July 22nd, 2008 @ 04:30 PM