Merge pull request #451 from seanpdoyle/singleton-callbacks

Singletons: Fix bug that prevented callbacks

Author: rafaelfranca

lib/active_resource/base.rb

@@ -1482,6 +1482,11 @@ def save!
       save || raise(ResourceInvalid.new(self))
     end
 
+    ##
+    # :method: destroy
+    # :call-seq:
+    #     destroy
+    #
     # Deletes the resource from the remote service.
     #
     # There's a series of callbacks associated with <tt>destroy</tt>. If any
@@ -1498,9 +1503,9 @@ def save!
     #   new_id = new_person.id # => 7
     #   new_person.destroy
     #   Person.find(new_id) # 404 (Resource Not Found)
-    def destroy
+    def destroy(path = element_path)
       run_callbacks :destroy do
-        connection.delete(element_path, self.class.headers)
+        connection.delete(path, self.class.headers)
       end
     end
 
@@ -1699,9 +1704,9 @@ def connection(refresh = false)
       end
 
       # Update the resource on the remote service.
-      def _update
+      def _update(path = element_path(prefix_options))
         run_callbacks :update do
-          connection.put(element_path(prefix_options), encode, self.class.headers).tap do |response|
+          connection.put(path, encode, self.class.headers).tap do |response|
             load_attributes_from_response(response)
           end
         end
@@ -1712,9 +1717,9 @@ def _update
       # There's a series of callbacks associated with <tt>create</tt>. If any
       # of the <tt>before_create</tt> callbacks throw +:abort+ the action is
       # cancelled.
-      def create
+      def create(path = collection_path)
         run_callbacks :create do
-          connection.post(collection_path, encode, self.class.headers).tap do |response|
+          connection.post(path, encode, self.class.headers).tap do |response|
             self.id = id_from_response(response)
             load_attributes_from_response(response)
           end

lib/active_resource/singleton.rb

@@ -109,31 +109,32 @@ def find_singleton(options)
           instantiate_record(resp, prefix_options)
         end
     end
+
+    ##
+    # :method: destroy
+    # :call-seq:
+    #     destroy
+    #
     # Deletes the resource from the remote service.
     #
     # ==== Examples
     #   weather = Weather.find
     #   weather.destroy
     #   Weather.find # 404 (Resource Not Found)
-    def destroy
-      connection.delete(singleton_path, self.class.headers)
+    def destroy(path = singleton_path)
+      super
     end
 
 
     protected
       # Update the resource on the remote service
-      def _update
-        connection.put(singleton_path(prefix_options), encode, self.class.headers).tap do |response|
-          load_attributes_from_response(response)
-        end
+      def _update(path = singleton_path)
+        super
       end
 
       # Create (i.e. \save to the remote service) the \new resource.
-      def create
-        connection.post(singleton_path, encode, self.class.headers).tap do |response|
-          self.id = id_from_response(response)
-          load_attributes_from_response(response)
-        end
+      def create(path = singleton_path)
+        super
       end
 
     private

test/cases/callbacks_test.rb

@@ -2,11 +2,13 @@
 
 require "abstract_unit"
 require "active_support/core_ext/hash/conversions"
+require "active_support/concern"
+require "fixtures/weather"
 
-class Developer < ActiveResource::Base
-  self.site = "http://37s.sunrise.i:3000"
+module CallbackHistory
+  extend ActiveSupport::Concern
 
-  class << self
+  class_methods do
     def callback_string(callback_method)
       "history << [#{callback_method.to_sym.inspect}, :string]"
     end
@@ -31,28 +33,44 @@ def callback_object(callback_method)
     end
   end
 
-  ActiveResource::Callbacks::CALLBACKS.each do |callback_method|
-    next if callback_method.to_s =~ /^around_/
-    define_callback_method(callback_method)
-    send(callback_method, callback_proc(callback_method))
-    send(callback_method, callback_object(callback_method))
-    send(callback_method) { |model| model.history << [ callback_method, :block ] }
+  included do
+    ActiveResource::Callbacks::CALLBACKS.each do |callback_method|
+      next if callback_method.to_s =~ /^around_/
+      define_callback_method(callback_method)
+      send(callback_method, callback_proc(callback_method))
+      send(callback_method, callback_object(callback_method))
+      send(callback_method) { |model| model.history << [ callback_method, :block ] }
+    end
   end
 
   def history
     @history ||= []
   end
 end
 
+class Developer < ActiveResource::Base
+  include CallbackHistory
+
+  self.site = "http://37s.sunrise.i:3000"
+end
+
+Weather.include CallbackHistory
+
 class CallbacksTest < ActiveSupport::TestCase
   def setup
     @developer_attrs = { id: 1, name: "Guillermo", salary: 100_000 }
     @developer = { "developer" => @developer_attrs }.to_json
+    @weather_attrs = { status: "Sunny", temperature: 67 }
+    @weather = { weather: @weather_attrs }.to_json
     ActiveResource::HttpMock.respond_to do |mock|
       mock.post   "/developers.json",   {}, @developer, 201, "Location" => "/developers/1.json"
       mock.get    "/developers/1.json", {}, @developer
       mock.put    "/developers/1.json", {}, nil, 204
       mock.delete "/developers/1.json", {}, nil, 200
+      mock.get    "/weather.json", {}, @weather
+      mock.post   "/weather.json", {}, @weather, 201, "Location" => "/weather.json"
+      mock.delete "/weather.json", {}, nil
+      mock.put    "/weather.json", {}, nil, 204
     end
   end
 
@@ -101,6 +119,36 @@ def test_create
     ], developer.history
   end
 
+  def test_create_singleton
+    weather = Weather.create(@weather_attrs)
+    assert_equal [
+      [ :before_validation,           :method ],
+      [ :before_validation,           :proc   ],
+      [ :before_validation,           :object ],
+      [ :before_validation,           :block  ],
+      [ :after_validation,            :method ],
+      [ :after_validation,            :proc   ],
+      [ :after_validation,            :object ],
+      [ :after_validation,            :block  ],
+      [ :before_save,                 :method ],
+      [ :before_save,                 :proc   ],
+      [ :before_save,                 :object ],
+      [ :before_save,                 :block  ],
+      [ :before_create,               :method ],
+      [ :before_create,               :proc   ],
+      [ :before_create,               :object ],
+      [ :before_create,               :block  ],
+      [ :after_create,                :method ],
+      [ :after_create,                :proc   ],
+      [ :after_create,                :object ],
+      [ :after_create,                :block  ],
+      [ :after_save,                  :method ],
+      [ :after_save,                  :proc   ],
+      [ :after_save,                  :object ],
+      [ :after_save,                  :block  ]
+    ], weather.history
+  end
+
   def test_reload
     developer = Developer.find(1)
     developer.reload
@@ -147,6 +195,37 @@ def test_update
     ], developer.history
   end
 
+  def test_update_singleton
+    weather = Weather.find
+    weather.save
+    assert_equal [
+      [ :before_validation,           :method ],
+      [ :before_validation,           :proc   ],
+      [ :before_validation,           :object ],
+      [ :before_validation,           :block  ],
+      [ :after_validation,            :method ],
+      [ :after_validation,            :proc   ],
+      [ :after_validation,            :object ],
+      [ :after_validation,            :block  ],
+      [ :before_save,                 :method ],
+      [ :before_save,                 :proc   ],
+      [ :before_save,                 :object ],
+      [ :before_save,                 :block  ],
+      [ :before_update,               :method ],
+      [ :before_update,               :proc   ],
+      [ :before_update,               :object ],
+      [ :before_update,               :block  ],
+      [ :after_update,                :method ],
+      [ :after_update,                :proc   ],
+      [ :after_update,                :object ],
+      [ :after_update,                :block  ],
+      [ :after_save,                  :method ],
+      [ :after_save,                  :proc   ],
+      [ :after_save,                  :object ],
+      [ :after_save,                  :block  ]
+    ], weather.history
+  end
+
   def test_destroy
     developer = Developer.find(1)
     developer.destroy
@@ -162,6 +241,21 @@ def test_destroy
     ], developer.history
   end
 
+  def test_destroy_singleton
+    weather = Weather.find
+    weather.destroy
+    assert_equal [
+      [ :before_destroy,              :method ],
+      [ :before_destroy,              :proc   ],
+      [ :before_destroy,              :object ],
+      [ :before_destroy,              :block  ],
+      [ :after_destroy,               :method ],
+      [ :after_destroy,               :proc   ],
+      [ :after_destroy,               :object ],
+      [ :after_destroy,               :block  ]
+    ], weather.history
+  end
+
   def test_delete
     developer = Developer.find(1)
     Developer.delete(developer.id)