Added association delete callbacks

lib/active_fedora/associations.rb

@@ -38,6 +38,7 @@ module ClassMethods
       def has_many(association_id, options={})
         raise "You must specify a property name for #{name}" if !options[:property]
         reflection = create_has_many_reflection(association_id, options)
+        add_association_callbacks(reflection.name, reflection.options)
         collection_accessor_methods(reflection, HasManyAssociation)
       end
 
@@ -152,7 +153,7 @@ def has_and_belongs_to_many(association_id, options = {}, &extension)
         reflection = create_has_and_belongs_to_many_reflection(association_id, options, &extension)
         collection_accessor_methods(reflection, HasAndBelongsToManyAssociation)
         #configure_after_destroy_method_for_has_and_belongs_to_many(reflection)
-        #add_association_callbacks(reflection.name, options)
+        add_association_callbacks(reflection.name, options)
       end
 
 
@@ -171,6 +172,20 @@ def create_has_and_belongs_to_many_reflection(association_id, options)
           create_reflection(:has_and_belongs_to_many, association_id, options, self)
         end
 
+        def add_association_callbacks(association_name, options)
+          callbacks = %w(before_add after_add before_remove after_remove)
+          callbacks.each do |callback_name|
+            full_callback_name = "#{callback_name}_for_#{association_name}"
+            defined_callbacks = options[callback_name.to_sym]
+            class_attribute full_callback_name.to_sym
+            if options.has_key?(callback_name.to_sym)
+              self.send((full_callback_name +'=').to_sym, [defined_callbacks].flatten)
+            else
+              self.send((full_callback_name +'=').to_sym, [])
+            end
+          end
+        end
+
         def association_accessor_methods(reflection, association_proxy_class)
           redefine_method(reflection.name) do |*params|
             force_reload = params.first unless params.empty?

lib/active_fedora/associations/association_collection.rb

@@ -215,10 +215,28 @@ def remove_records(*records)
           records = flatten_deeper(records)
           records.each { |record| raise_on_type_mismatch(record) }
 
-          #records.each { |record| callback(:before_remove, record) }
+          records.each { |record| callback(:before_remove, record) }
           old_records = records.reject { |r| r.new_record? }
           yield(records, old_records)
-          #records.each { |record| callback(:after_remove, record) }
+          records.each { |record| callback(:after_remove, record) }
+        end
+
+        def callback(method, record)
+          callbacks_for(method).each do |callback|
+            case callback
+            when Symbol
+              @owner.send(callback, record)
+            when Proc
+              callback.call(@owner, record)
+            else
+              callback.send(method, @owner, record)
+            end
+          end
+        end
+
+        def callbacks_for(callback_name)
+          full_callback_name = "#{callback_name}_for_#{@reflection.name}"
+          @owner.class.send(full_callback_name.to_sym) || []
         end
 
         def ensure_owner_is_not_new

lib/active_fedora/associations/has_and_belongs_to_many_association.rb

@@ -48,10 +48,16 @@ def delete_records(records)
 
             if (@reflection.options[:inverse_of])
               r.remove_relationship(@reflection.options[:inverse_of], @owner)
+              # It looks like inverse_of points at a predicate, not at a relationship name,
+              # which is what we should have done. Now we need a way to look up the
+              # reflection by predicate
+              name = r.class.reflection_name_for_predicate(@reflection.options[:inverse_of])
+              r.send(name).reset
               r.save
             end
           end
         end
+
     end
   end
 end

lib/active_fedora/base.rb

@@ -252,7 +252,6 @@ def create_date
       elsif @inner_object.respond_to? :createdDate
         Array(@inner_object.createdDate).first
       else
-        puts @inner_object.inspect
         @inner_object.profile['objCreateDate']
       end
     end

lib/active_fedora/reflection.rb

@@ -8,8 +8,13 @@ module Reflection # :nodoc:
     end
 
 
-
     module ClassMethods
+      def reflection_name_for_predicate(predicate)
+        reflections.each do |k, v|
+          return k if v.options[:property] == predicate
+        end
+      end
+
       def create_reflection(macro, name, options, active_fedora)
         case macro
           when :has_many, :belongs_to, :has_and_belongs_to_many

spec/integration/associations_spec.rb

@@ -160,16 +160,20 @@ class Topic < ActiveFedora::Base
 
     describe "of has_many_and_belongs_to" do
       before do
-        @topic1 = Topic.new
-        @topic1.save
-        @topic2 = Topic.new
-        @topic2.save
+        @topic1 = Topic.create
+        @topic2 = Topic.create
+        @book = Book.create
       end
-      it "habtm should set relationships bidirectionally" do
-        @book = Book.new
+      it "habtm should set and remove relationships bidirectionally" do
         @book.topics << @topic1
-        @book.topics.map(&:pid).should == [@topic1.pid]
-        Topic.find(@topic1.pid).books.should == [] #Can't have saved it because @book isn't saved yet.
+        @book.topics.should == [@topic1]
+        @topic1.books.should == [@book]
+        @topic1.reload.books.should == [@book]
+
+        @book.topics.delete(@topic1)
+        #@topic1.books.delete(@book)
+        @book.topics.should == []
+        @topic1.books.should == []
       end
       after do
         @topic1.delete
@@ -389,8 +393,16 @@ class Page < ActiveFedora::Base
         @reloaded_book.pages.should == [@p2]
       end
 
+      it "should allow replacing the children" do
+        @p3 = Page.create()
+        @p4 = Page.create()
+        @book.pages = [@p3, @p4]
+        @book.save
+
+        @book.reload.pages.should == [@p3, @p4]
+      end
+
       it "should allow a child to be deleted from the has_and_belongs_to_many association" do
-        pending "This isn't working and we ought to fix it"
         @reloaded_book = LibraryBook.find(@book.pid)
         @reloaded_book.pages.delete(@p1)
         @reloaded_book.save
@@ -402,6 +414,34 @@ class Page < ActiveFedora::Base
     end
   end
 
+  describe "association hooks" do
+    before :all do
+      class LibraryBook < ActiveFedora::Base
+        has_and_belongs_to_many :pages, :property=>:is_part_of, after_remove: :say_hi
+
+      end
+      class Page < ActiveFedora::Base
+        has_many :library_books, :property=>:is_part_of
+      end
+
+    end
+    after :all do
+      Object.send(:remove_const, :LibraryBook)
+      Object.send(:remove_const, :Page)
+    end
+
+    describe "removing association" do
+      subject {LibraryBook.new}
+      before do
+        @p1 = subject.pages.build
+        @p2 = subject.pages.build
+      end
+      it "should run the hooks" do
+        subject.should_receive(:say_hi).with(@p2)
+        subject.pages.delete(@p2)
+      end
+    end
+  end
 
 
   describe "when a object is deleted" do

spec/unit/has_and_belongs_to_many_collection_spec.rb

@@ -29,31 +29,4 @@
 
   end
 
-  it "should call remove_relationship" do
-    subject = stub("subject", :new_record? => false, :pid => 'subject:a', :internal_uri => 'info:fedora/subject:a')
-    predicate = stub(:klass => mock.class, :options=>{:property=>'predicate'}, :class_name=> nil)
-    ActiveFedora::SolrService.stub(:query).and_return([])
-    ac = ActiveFedora::Associations::HasAndBelongsToManyAssociation.new(subject, predicate)
-    object = stub("object", :new_record? => false, :pid => 'object:b', :save => nil)
-
-    subject.should_receive(:remove_relationship).with('predicate', object)
-
-    ac.delete(object)
-
-  end
-
-  it "should call remove_relationship on subject and object when inverse_of given" do
-    subject = stub("subject", :new_record? => false, :pid => 'subject:a', :internal_uri => 'info:fedora/subject:a')
-    predicate = stub(:klass => mock.class, :options=>{:property=>'predicate', :inverse_of => 'inverse_predicate'}, :class_name=> nil)
-    ActiveFedora::SolrService.stub(:query).and_return([])
-    ac = ActiveFedora::Associations::HasAndBelongsToManyAssociation.new(subject, predicate)
-    object = stub("object", :new_record? => false, :pid => 'object:b', :save => nil)
-
-    subject.should_receive(:remove_relationship).with('predicate', object)
-    object.should_receive(:remove_relationship).with('inverse_predicate', subject)
-
-    ac.delete(object)
-
-  end
-
 end

spec/unit/has_many_collection_spec.rb

@@ -14,32 +14,4 @@
 
   end
 
-  it "should call remove_relationship" do
-    subject = stub("subject", :new_record? => false, :pid => 'subject:a', :internal_uri => 'info:fedora/subject:a')
-    predicate = stub(:klass => mock.class, :options=>{:property=>'predicate'}, :class_name=> nil)
-    ActiveFedora::SolrService.stub(:query).and_return([])
-    ac = ActiveFedora::Associations::HasManyAssociation.new(subject, predicate)
-    object = stub("object", :new_record? => false, :pid => 'object:b', :save => nil)
-
-    object.should_receive(:remove_relationship).with('predicate', subject)
-
-    ac.delete(object)
-
-  end
-
-  it "should be able to replace the collection" do
-    @owner = stub(:new_record? => false, :to_ary => nil, :internal_uri => 'info:fedora/changeme:99')
-    @reflection = stub(:klass => mock.class, :options=>{:property=>'predicate'}, :class_name=> nil)
-    ac = ActiveFedora::Associations::HasManyAssociation.new(@owner, @reflection)
-    @target = [stub(:to_ary => nil, :new_record? => false, :remove_relationship=>true), stub(:to_ary => nil, :new_record? => false, :remove_relationship=>true), stub(:to_ary => nil, :new_record? => false, :remove_relationship=>true)]
-    ac.target = @target 
-
-    @expected1 = stub(:new_record? => false, :add_relationship=>true, :save=>true, :to_ary => nil)
-    @expected2 = stub(:new_record? => false, :add_relationship=>true, :save=>true, :to_ary => nil)
-    ac.replace([@expected1, @expected2])
-    ac.target.should == [@expected1, @expected2]
-
-  end
-
-
 end