Clean up callback implementation

[chrisroberts]

Clean up the callback implementation to make it a bit more agile.

[clyfe]

Possible libs for callback rewrite:

http://api.rubyonrails.org/classes/ActiveSupport/Callbacks.html
http://api.rubyonrails.org/classes/ActiveModel/Callbacks.html
http://api.rubyonrails.org/classes/AbstractController/Callbacks.html

The ActiveSupport is the smallest common denominator that the controller callbacks and model callbacks are based on.
Since an DAV4Rack::Resource has bot "model" functionality and "controller" functionality one of the two callback implementations might be fit for integration.
Bonus: they are faster than method_missing.

I think the most appropriate is the ActiveModel callback.

[clyfe]

Sample implementation:

• nicely decoupled
• same known interface as ActiveRecord
• provides better alternative for the DAV_ prefix hack, namely method_without_callbacks

require 'rubygems'
require 'active_model/callbacks'

class Resource
  def get; p 'get' end
  def put; p 'put' end

  extend ActiveModel::Callbacks
  METHODS = [:get, :put]

  # rewrite "designated" methods to have them wrapped with the "_run_method_callbacks" bock
  METHODS.each do |method|
    define_model_callbacks method
    class_eval <<-OVERRIDE, __FILE__, __LINE__ + 1
      def #{method}_with_callbacks(*args)
        _run_#{method}_callbacks do
          #{method}_without_callbacks(*args)
        end
      end
    OVERRIDE
    alias_method_chain method, :callbacks
  end

  # if we override one of the "designated" methods, make sure to have the overrider wraped with the
  # "_run_method_callbacks" bock, by swapping some of the already (see up) existing bits
  def self.inherited(subclass)
    # ruby magic, this method is called each time a method is added on an inheriting class
    def subclass.method_added(method)
      @@prevent_recursion ||= {}
      if METHODS.include?(method) && @@prevent_recursion[method].nil?
        @@prevent_recursion[method] = true
        alias_method :"#{method}_without_callbacks", method
        alias_method method, :"#{method}_with_callbacks"
      end
    end
  end

end

class R < Resource
  before_get :callback
  def callback; p 'before_get' end

  def get; p 'override get' end
end

r = R.new
r.get
# "before_get"
# "override get"

[clyfe]

Checkout this branch where I implemented these ideas, maybe do a pull ?

https://github.com/clyfe/dav4rack/tree/callbacks_cleanup

[chrisroberts]

After digging into activerecord's callbacks I'm hesitant to merge this as activerecord adds in more than just callbacks (like validations) and I think it may be too heavy for what is needed. If there is a specific feature you need that the activerecord callbacks provide, please let me know. Otherwise, I think we should take what you have here and build it out using activesupport's basic callbacks. It will be a little more work up front but should keep things less bloated.

[clyfe]

The proposed solution only provides callbacks, it does not bring none of the other AciveRecord module (like validations and such).
As far as I can tell, the ActiveModell::Callbacks only expands the ActiveSupport::Callbacks DSL in a particular way too allow you this:

# in lib
extend ActiveModel::Callbacks
define_model_callbacks :create
# usage
before_create :before_create_cb
def before_create_cb; ... end

vs this

# in lib
include ActiveSupport::Callbacks
define_callbacks :create
# usage
set_callback :create, :before, :before_create_cb
def before_create_cb; ... end

I'd say that ActiveModel is giving you a better DSL (+ :only, :except options), while ActiveSupport is more arbitrary.

[chrisroberts]

Ah, my apologies. The multiple times I've been looking at this, I kept seeing 'active_record/callbacks' not 'active_model/callbacks' which was actually there. I am going to pull this into a new branch locally to get prepped for release. Documentation will need to be updated to reflect the new callback API and I would like to get some tests built in for the callbacks before I push out the next release.

[chrisroberts]

This is working fine with Rails 3 as well as non Rails applications, however I ran into some issues with Rails 2 since we have a dependency on a newer active model. Still investigating what is the best way to provide the same functionality when within a Rails 2 environment.

[clyfe]

Side note: let's not break the current (2.4) API before 3.0