add tests for better coverage of pcdm objects

README.md

@@ -1,5 +1,6 @@
 # Hydra::PCDM
 [![Build Status](https://travis-ci.org/projecthydra-labs/hydra-pcdm.svg?branch=master)](https://travis-ci.org/projecthydra-labs/hydra-pcdm)
+[![Coverage Status](https://coveralls.io/repos/projecthydra-labs/hydra-pcdm/badge.svg)](https://coveralls.io/r/projecthydra-labs/hydra-pcdm)
 
 Hydra implementation of Portland Common Data Models (PCDM)
 

lib/hydra/pcdm/models/concerns/collection_behavior.rb

@@ -27,9 +27,6 @@ module CollectionBehavior
     #   6) Hydra::PCDM::Collection can have descriptive metadata
     #   7) Hydra::PCDM::Collection can have access metadata
 
-    # TODO: Make members private adding to an aggregations has to go through the following methods.
-
-
 
     def << arg
 
@@ -46,7 +43,7 @@ def << arg
     end
 
     def collections= collections
-      raise ArgumentError, "each collection must be a Hydra::PCDM::Collection" unless collections.all? { |c| Hydra::PCDM.collection? c }
+      raise ArgumentError, "each collection must be a pcdm collection" unless collections.all? { |c| Hydra::PCDM.collection? c }
       raise ArgumentError, "a collection can't be an ancestor of itself" if collection_ancestor?(collections)
       self.members = self.objects + collections
     end
@@ -56,7 +53,7 @@ def collections
     end
 
     def objects= objects
-      raise ArgumentError, "each object must be a Hydra::PCDM::Object" unless objects.all? { |o| Hydra::PCDM.object? o }
+      raise ArgumentError, "each object must be a pcdm object" unless objects.all? { |o| Hydra::PCDM.object? o }
       self.members = self.collections + objects
     end
 
@@ -92,8 +89,6 @@ def ancestor? collection
     #   * Are there any default properties to set for Collection's access metadata?
     #   * Is there a way to override default properties defined in this class?
 
-    # TODO: Add ORE.aggregates related Hydra::PCDM::Objects.
-
   end
 end
 

lib/hydra/pcdm/models/concerns/object_behavior.rb

@@ -20,21 +20,17 @@ module ObjectBehavior
 
 
     # behavior:
-
     #   1) Hydra::PCDM::Object can aggregate (pcdm:hasMember) Hydra::PCDM::Object
+    #   2) Hydra::PCDM::Object can aggregate (ore:aggregates) Hydra::PCDM::Object  (Object related to the Object)
 
-    #   2) Hydra::PCDM::Object can contain (pcdm:hasFile) Hydra::PCDM::File
-    #   3) Hydra::PCDM::Object can contain (pcdm:hasRelatedFile) Hydra::PCDM::File
-
-    #   4) Hydra::PCDM::Object can NOT aggregate Hydra::PCDM::Collection
-    #   5) Hydra::PCDM::Object can NOT aggregate non-PCDM object
+    #   3) Hydra::PCDM::Object can contain (pcdm:hasFile) Hydra::PCDM::File
+    #   4) Hydra::PCDM::Object can contain (pcdm:hasRelatedFile) Hydra::PCDM::File
 
-    #   6) Hydra::PCDM::Object can have descriptive metadata
-    #   7) Hydra::PCDM::Object can have access metadata
-    # TODO: add code to enforce behavior rules
-
-    # TODO: Make members private so adding to an aggregations has to go through the following methods.
+    #   5) Hydra::PCDM::Object can NOT aggregate Hydra::PCDM::Collection
+    #   6) Hydra::PCDM::Object can NOT aggregate non-PCDM object
 
+    #   7) Hydra::PCDM::Object can have descriptive metadata
+    #   8) Hydra::PCDM::Object can have access metadata
 
 
     def << arg
@@ -45,29 +41,52 @@ def << arg
       #       Want to override << on obj1.objects to check that new_object is_a? Hydra::PCDM::Object
 
       # check that arg is an instance of Hydra::PCDM::Object or Hydra::PCDM::File
-      raise ArgumentError, "argument must be either a Hydra::PCDM::Object or Hydra::PCDM::File" unless
+      raise ArgumentError, "argument must be either a pcdm object or a pcdm file" unless
           ( Hydra::PCDM.object? arg ) || ( Hydra::PCDM.file? arg )
       members << arg  if Hydra::PCDM.object? arg
       files   << arg  if Hydra::PCDM.file? arg
     end
 
     def objects= objects
-      raise ArgumentError, "each object must be a Hydra::PCDM::Object" unless objects.all? { |o| Hydra::PCDM.object? o }
+      raise ArgumentError, "each object must be a pcdm object" unless objects.all? { |o| Hydra::PCDM.object? o }
+      raise ArgumentError, "an object can't be an ancestor of itself" if object_ancestor?(objects)
       self.members = objects
     end
 
     def objects
       members.to_a.select { |m| Hydra::PCDM.object? m }
     end
 
+    def object_ancestor? objects
+      objects.each do |check|
+        return true if check.id == self.id
+        return true if ancestor?(check)
+      end
+      false
+    end
+
+    def ancestor? object
+      return false if object.objects.empty?
+      current_objects = object.objects
+      next_batch = []
+      while !current_objects.empty? do
+        current_objects.each do |c|
+          return true if c.id == self.id
+          next_batch += c.objects
+        end
+        current_objects = next_batch
+      end
+      false
+    end
+
     def contains= files
       # check that file is an instance of Hydra::PCDM::File
-      raise ArgumentError, "each file must be a Hydra::PCDM::File" unless
+      raise ArgumentError, "each file must be a pcdm file" unless
           files.all? { |f| Hydra::PCDM.file? f }
       super(files)
     end
 
-    # TODO: Add hasRelatedFiles
+    # TODO: implement hasRelatedFiles
 
     # TODO: RDF metadata can be added using property definitions.
     #   * How to distinguish between descriptive and access metadata?

spec/hydra/pcdm/models/collection_spec.rb

@@ -54,16 +54,18 @@
     end
 
     context 'with unacceptable collections' do
+      let(:error_message) { "each collection must be a pcdm collection" }
+
       it 'should NOT aggregate Hydra::PCDM::Objects in collections aggregation' do
-        expect{ collection1.collections = [object1] }.to raise_error(ArgumentError,"each collection must be a Hydra::PCDM::Collection")
+        expect{ collection1.collections = [object1] }.to raise_error(ArgumentError,error_message)
       end
 
       it 'should NOT aggregate non-PCDM objects in collections aggregation' do
-        expect{ collection1.collections = [non_PCDM_object] }.to raise_error(ArgumentError,"each collection must be a Hydra::PCDM::Collection")
+        expect{ collection1.collections = [non_PCDM_object] }.to raise_error(ArgumentError,error_message)
       end
 
       it 'should NOT aggregate AF::Base objects in collections aggregation' do
-        expect{ collection1.collections = [af_base_object] }.to raise_error(ArgumentError,"each collection must be a Hydra::PCDM::Collection")
+        expect{ collection1.collections = [af_base_object] }.to raise_error(ArgumentError,error_message)
       end
     end
 
@@ -179,12 +181,13 @@ class Cullection < Hydra::PCDM::Collection
     end
 
     context "with unacceptable objects" do
+      let(:error_message) { "each object must be a pcdm object" }
       it 'should NOT aggregate Hydra::PCDM::Collection in objects aggregation' do
-        expect{ collection1.objects = [collection2] }.to raise_error(ArgumentError,"each object must be a Hydra::PCDM::Object")
+        expect{ collection1.objects = [collection2] }.to raise_error(ArgumentError,error_message)
       end
 
       it 'should NOT aggregate non-PCDM objects in collections aggregation' do
-        expect{ collection1.objects = [non_PCDM_object] }.to raise_error(ArgumentError,"each object must be a Hydra::PCDM::Object")
+        expect{ collection1.objects = [non_PCDM_object] }.to raise_error(ArgumentError,error_message)
       end
     end
 

spec/hydra/pcdm/models/object_spec.rb

@@ -2,70 +2,150 @@
 
 describe Hydra::PCDM::Object do
 
+  let(:object1) { Hydra::PCDM::Object.create }
+  let(:object2) { Hydra::PCDM::Object.create }
+  let(:object3) { Hydra::PCDM::Object.create }
+  let(:object4) { Hydra::PCDM::Object.create }
+  let(:object5) { Hydra::PCDM::Object.create }
+
+  let(:file1) { Hydra::PCDM::File.new }
+  let(:file2) { Hydra::PCDM::File.new }
+
+  let(:collection1) { Hydra::PCDM::Collection.create }
+  let(:non_PCDM_object)  { "I'm not a PCDM object" }
+  let(:af_base_object)   { ActiveFedora::Base.create }
+
   # TEST the following behaviors...
   #   1) Hydra::PCDM::Object can aggregate (pcdm:hasMember) Hydra::PCDM::Object
+  #   2) Hydra::PCDM::Collection can aggregate (ore:aggregates) Hydra::PCDM::Object  (Object related to the Object)
 
-  #   2) Hydra::PCDM::Object can contain (pcdm:hasFile) Hydra::PCDM::File
-  #   3) Hydra::PCDM::Object can contain (pcdm:hasRelatedFile) Hydra::PCDM::File
+  #   3) Hydra::PCDM::Object can contain (pcdm:hasFile) Hydra::PCDM::File
+  #   4) Hydra::PCDM::Object can contain (pcdm:hasRelatedFile) Hydra::PCDM::File
 
-  #   4) Hydra::PCDM::Object can NOT aggregate Hydra::PCDM::Collection
-  #   5) Hydra::PCDM::Object can NOT aggregate non-PCDM object
+  #   5) Hydra::PCDM::Object can NOT aggregate Hydra::PCDM::Collection
+  #   6) Hydra::PCDM::Object can NOT aggregate non-PCDM object
 
-  #   6) Hydra::PCDM::Object can have descriptive metadata
-  #   7) Hydra::PCDM::Object can have access metadata
+  #   7) Hydra::PCDM::Object can have descriptive metadata
+  #   8) Hydra::PCDM::Object can have access metadata
 
-  # subject { Hydra::PCDM::Object.new }
-
-  # NOTE: This method is named 'members' because of the definition 'aggregates: members' in Hydra::PCDM::Object
   describe '#objects=' do
-    #   1) Hydra::PCDM::Object can aggregate (pcdm:hasMember) Hydra::PCDM::Object
-
     it 'should aggregate objects' do
-
-      # TODO: This test needs refinement with before and after managing objects in fedora.
-
-      object1 = Hydra::PCDM::Object.create
-      object2 = Hydra::PCDM::Object.create
-      object3 = Hydra::PCDM::Object.create
-
       object1.objects = [object2, object3]
       object1.save
       expect(object1.objects).to eq [object2, object3]
     end
 
-    xit 'should add an object to the objects aggregation' do
-
-      object1 = Hydra::PCDM::Object.create
-      object2 = Hydra::PCDM::Object.create
-      object3 = Hydra::PCDM::Object.create
-      object4 = Hydra::PCDM::Object.create
-
+    xit 'should add a object to the objects aggregation' do
       object1.objects = [object2,object3]
       object1.save
       object1.objects << object4
       expect(object1.objects).to eq [object2,object3,object4]
     end
 
-    it 'should NOT aggregate Hydra::PCDM::Collection in objects aggregation' do
-      # 4) Hydra::PCDM::Object can NOT aggregate Hydra::PCDM::Collection
+    it 'should aggregate objects in a sub-object of a object' do
+      object1.objects = [object2]
+      object1.save
+      object2.objects = [object3]
+      object2.save
+      expect(object1.objects).to eq [object2]
+      expect(object2.objects).to eq [object3]
+    end
+
+    context 'with unacceptable objects' do
+      let(:error_message) { "each object must be a pcdm object" }
+
+      it 'should NOT aggregate Hydra::PCDM::Collections in objects aggregation' do
+        expect{ object1.objects = [collection1] }.to raise_error(ArgumentError,error_message)
+      end
+
+      it 'should NOT aggregate non-PCDM objects in objects aggregation' do
+        expect{ object1.objects = [non_PCDM_object] }.to raise_error(ArgumentError,error_message)
+      end
 
-      object1 = Hydra::PCDM::Object.create
-      collection1 = Hydra::PCDM::Collection.create
-      expect{ object1.objects = [collection1] }.to raise_error(ArgumentError,"each object must be a Hydra::PCDM::Object")
+      it 'should NOT aggregate AF::Base objects in objects aggregation' do
+        expect{ object1.objects = [af_base_object] }.to raise_error(ArgumentError,error_message)
+      end
     end
 
-    it 'should NOT aggregate non-PCDM objects in objects aggregation' do
-      #   5) Hydra::PCDM::Collection can NOT aggregate non-PCDM objects
+    context 'with acceptable objects' do
+      describe 'aggregates objects that implement Hydra::PCDM' do
+        before do
+          class DummyIncObject < ActiveFedora::Base
+            include Hydra::PCDM::ObjectBehavior
+          end
+        end
+        after { Object.send(:remove_const, :DummyIncObject) }
+        let(:iobject1) { DummyIncObject.create }
+        before do
+          object1.objects = [iobject1]
+          object1.save
+        end
+        subject { object1.objects }
+        it { is_expected.to eq [iobject1]}
+      end
+
+      describe 'aggregates collections that extend Hydra::PCDM' do
+        before do
+          class DummyExtObject < Hydra::PCDM::Object
+          end
+        end
+        after { Object.send(:remove_const, :DummyExtObject) }
+        let(:eobject1) { DummyExtObject.create }
+        before do
+          object1.objects = [eobject1]
+          object1.save
+        end
+        subject { object1.objects }
+        it { is_expected.to eq [eobject1]}
+      end
+    end
 
-      object1 = Hydra::PCDM::Object.create
-      string1 = "non-PCDM object"
-      expect{ object1.objects = [string1] }.to raise_error(ArgumentError,"each object must be a Hydra::PCDM::Object")
+    describe "adding objects that are ancestors" do
+      let(:error_message) { "an object can't be an ancestor of itself" }
+
+      context "when the source object is the same" do
+        it "raises an error" do
+          expect{ object1.objects = [object1]}.to raise_error(ArgumentError, error_message)
+        end
+      end
+
+      before do
+        object1.objects = [object2]
+        object1.save
+      end
+
+      it "raises and error" do
+        expect{ object2.objects = [object1]}.to raise_error(ArgumentError, error_message)
+      end
+
+      context "with more ancestors" do
+        before do
+          object2.objects = [object3]
+          object2.save
+        end
+
+        it "raises an error" do
+          expect{ object3.objects = [object1]}.to raise_error(ArgumentError, error_message)
+        end
+
+        context "with a more complicated example" do
+          before do
+            object3.objects = [object4, object5]
+            object3.save
+          end
+
+          it "raises errors" do
+            expect{ object4.objects = [object1]}.to raise_error(ArgumentError, error_message)
+            expect{ object4.objects = [object2]}.to raise_error(ArgumentError, error_message)
+          end
+        end
+      end
     end
   end
 
   describe '#objects' do
     it 'should return empty array when no members' do
-      object1 = Hydra::PCDM::Object.new
+      object1.save
       expect(object1.objects).to eq []
     end
   end
@@ -100,12 +180,10 @@
     it { is_expected.to eq [file1, file2] }
   end
 
-  describe '#related_files' do
-    xit 'should contain related files' do
-      #   3) Hydra::PCDM::Object can contain (pcdm:hasRelatedFile) Hydra::PCDM::File
-
+  describe 'Related files' do
+    xit 'should allow related files' do
       # TODO Write test
-
     end
   end
+
 end