refactor to use consistent method names for getting parents and children

lib/hydra/pcdm/collection_indexer.rb

@@ -3,7 +3,7 @@ class CollectionIndexer < ObjectIndexer
 
     def generate_solr_document
       super.tap do |solr_doc|
-        solr_doc["members_ssim"]        = object.member_ids
+        solr_doc["members_ssim"]           = object.member_ids
         solr_doc["child_collections_ssim"] = object.child_collections.map { |o| o.id }
       end
     end

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

@@ -1,6 +1,18 @@
 require 'active_fedora/aggregation'
 
 module Hydra::PCDM
+
+  # behavior:
+  #   1) Hydra::PCDM::Collection can aggregate (pcdm:hasMember)  Hydra::PCDM::Collection (no infinite loop, e.g., A -> B -> C -> A)
+  #   2) Hydra::PCDM::Collection can aggregate (pcdm:hasMember)  Hydra::PCDM::Object
+  #   3) Hydra::PCDM::Collection can aggregate (ore:aggregates) Hydra::PCDM::Object  (Object related to the Collection)
+
+  #   4) Hydra::PCDM::Collection can NOT aggregate non-PCDM object
+  #   5) Hydra::PCDM::Collection can NOT contain (pcdm:hasFile)  Hydra::PCDM::File
+
+  #   6) Hydra::PCDM::Collection can have descriptive metadata
+  #   7) Hydra::PCDM::Collection can have access metadata
+  #
   module CollectionBehavior
     extend ActiveSupport::Concern
 
@@ -22,17 +34,6 @@ def indexer
       end
     end
 
-    # behavior:
-    #   1) Hydra::PCDM::Collection can aggregate (pcdm:hasMember)  Hydra::PCDM::Collection (no infinite loop, e.g., A -> B -> C -> A)
-    #   2) Hydra::PCDM::Collection can aggregate (pcdm:hasMember)  Hydra::PCDM::Object
-    #   3) Hydra::PCDM::Collection can aggregate (ore:aggregates) Hydra::PCDM::Object  (Object related to the Collection)
-
-    #   4) Hydra::PCDM::Collection can NOT aggregate non-PCDM object
-    #   5) Hydra::PCDM::Collection can NOT contain (pcdm:hasFile)  Hydra::PCDM::File
-
-    #   6) Hydra::PCDM::Collection can have descriptive metadata
-    #   7) Hydra::PCDM::Collection can have access metadata
-    #
     def pcdm_object?
       false
     end
@@ -41,25 +42,43 @@ def pcdm_collection?
       true
     end
 
+    def parents
+      aggregated_by
+    end
+
+    def parent_collections
+      aggregated_by.select { |parent| parent.class.included_modules.include?(Hydra::PCDM::CollectionBehavior) }
+    end
+
     def child_collections= collections
       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 = objects + collections
+      self.members = child_objects + collections
     end
 
     def child_collections
       members.to_a.select { |m| Hydra::PCDM.collection? m }
     end
 
-    def objects= objects
+    def child_objects= objects
       raise ArgumentError, "each object must be a pcdm object" unless objects.all? { |o| Hydra::PCDM.object? o }
       self.members = child_collections + objects
     end
 
-    def objects
+    def objects= objects
+      warn "[DEPRECATION] `objects=` is deprecated.  Please use `child_objects=` instead.  This has a target date for removal of 07-31-2015"
+      self.child_objects= objects
+    end
+
+    def child_objects
       members.to_a.select { |m| Hydra::PCDM.object? m }
     end
 
+    def objects
+      warn "[DEPRECATION] `objects` is deprecated.  Please use `child_objects` instead.  This has a target date for removal of 07-31-2015"
+      self.child_objects
+    end
+
     def collection_ancestor? collections
       collections.each do |check|
         return true if ancestor?(check)

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

@@ -45,12 +45,6 @@ def create_reflection(macro, name, options, active_fedora)
       end
     end
 
-    def objects= objects
-      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
-
     # @return [Boolean] whether this instance is a PCDM Object.
     def pcdm_object?
       true
@@ -61,10 +55,6 @@ def pcdm_collection?
       false
     end
 
-    def objects
-      members.to_a.select { |m| Hydra::PCDM.object? m }
-    end
-
     def parents
       aggregated_by
     end
@@ -77,6 +67,26 @@ def parent_collections
       aggregated_by.select { |parent| parent.class.included_modules.include?(Hydra::PCDM::CollectionBehavior) }
     end
 
+    def child_objects= objects
+      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= objects
+      warn "[DEPRECATION] `objects=` is deprecated.  Please use `child_objects=` instead.  This has a target date for removal of 07-31-2015"
+      self.child_objects= objects
+    end
+
+    def child_objects
+      members.to_a.select { |m| Hydra::PCDM.object? m }
+    end
+
+    def objects
+      warn "[DEPRECATION] `objects` is deprecated.  Please use `child_objects` instead.  This has a target date for removal of 07-31-2015"
+      self.child_objects
+    end
+
     def object_ancestor? objects
       objects.each do |check|
         return true if ancestor?(check)
@@ -86,13 +96,13 @@ def object_ancestor? objects
 
     def ancestor? object
       return true if object == self
-      return false if object.objects.empty?
-      current_objects = object.objects
+      return false if object.child_objects.empty?
+      current_objects = object.child_objects
       next_batch = []
       while !current_objects.empty? do
         current_objects.each do |c|
           return true if c == self
-          next_batch += c.objects
+          next_batch += c.child_objects
         end
         current_objects = next_batch
       end

lib/hydra/pcdm/object_indexer.rb

@@ -2,7 +2,7 @@ module Hydra::PCDM
   class ObjectIndexer < ActiveFedora::IndexingService
     def generate_solr_document
       super.tap do |solr_doc|
-        solr_doc["objects_ssim"]     = object.objects.map { |o| o.id }
+        solr_doc["child_objects_ssim"]     = object.child_objects.map { |o| o.id }
       end
     end
 

lib/hydra/pcdm/services/collection/get_objects.rb

@@ -11,7 +11,7 @@ class GetObjectsFromCollection
     def self.call( parent_collection )
       raise ArgumentError, "parent_collection must be a pcdm collection" unless Hydra::PCDM.collection? parent_collection
 
-      parent_collection.objects
+      parent_collection.child_objects
     end
 
   end

lib/hydra/pcdm/services/object/get_objects.rb

@@ -11,7 +11,7 @@ class GetObjectsFromObject
     def self.call( parent_object )
       raise ArgumentError, "parent_object must be a pcdm object" unless Hydra::PCDM.object? parent_object
 
-      parent_object.objects
+      parent_object.child_objects
     end
 
   end

spec/hydra/pcdm/collection_indexer_spec.rb

@@ -1,15 +1,15 @@
 require 'spec_helper'
 
 describe Hydra::PCDM::CollectionIndexer do
-  let(:collection) { Hydra::PCDM::Collection.new }
+  let(:collection)         { Hydra::PCDM::Collection.new }
   let(:child_collections1) { Hydra::PCDM::Collection.new(id: '123') }
   let(:child_collections2) { Hydra::PCDM::Collection.new(id: '456') }
-  let(:object1) { Hydra::PCDM::Object.new(id: '789') }
-  let(:indexer) { described_class.new(collection) }
+  let(:child_object1)      { Hydra::PCDM::Object.new(id: '789') }
+  let(:indexer)            { described_class.new(collection) }
 
   before do
     allow(collection).to receive(:child_collections).and_return([child_collections1, child_collections2])
-    allow(collection).to receive(:objects).and_return([object1])
+    allow(collection).to receive(:child_objects).and_return([child_object1])
     allow(collection).to receive(:member_ids).and_return(['123', '456', '789'])
   end
 
@@ -18,7 +18,7 @@
 
     it "has fields" do
       expect(subject['child_collections_ssim']).to eq ['123', '456']
-      expect(subject['objects_ssim']).to eq ['789']
+      expect(subject['child_objects_ssim']).to eq ['789']
       expect(subject['members_ssim']).to eq ['123', '456', '789']
     end
   end

spec/hydra/pcdm/models/collection_spec.rb

@@ -17,11 +17,49 @@
     end
   end
 
-  describe '#objects=' do
+  describe '#child_objects=' do
     it 'should aggregate objects' do
-      collection1.objects = [object1,object2]
+      collection1.child_objects = [object1,object2]
       collection1.save
-      expect(collection1.objects).to eq [object1,object2]
+      expect(collection1.child_objects).to eq [object1,object2]
+    end
+  end
+
+  context 'when aggregated by other objects' do
+
+    before do
+      # Using before(:all) and instance variable because regular :let syntax had a significant impact on performance
+      # All of the tests in this context are describing idempotent behavior, so isolation between examples isn't necessary.
+      @collection1 = Hydra::PCDM::Collection.new
+      @collection2 = Hydra::PCDM::Collection.new
+      @collection =  Hydra::PCDM::Collection.new
+      @collection1.members << @collection
+      @collection2.members << @collection
+      allow(@collection).to receive(:id).and_return("banana")
+      proxies = [
+          build_proxy(container: @collection1),
+          build_proxy(container: @collection2),
+      ]
+      allow(ActiveFedora::Aggregation::Proxy).to receive(:where).with(proxyFor_ssim: @collection.id).and_return(proxies)
+    end
+
+    describe 'parents' do
+      subject { @collection.parents }
+      it "finds all nodes that aggregate the object with hasMember" do
+        expect(subject).to include(@collection1, @collection2)
+        expect(subject.count).to eq 2
+      end
+    end
+
+    describe 'parent_collections' do
+      subject { @collection.parent_collections }
+      it "finds collections that aggregate the object with hasMember" do
+        expect(subject).to include(@collection1, @collection2)
+        expect(subject.count).to eq 2
+      end
+    end
+    def build_proxy(container:)
+      instance_double(ActiveFedora::Aggregation::Proxy, container: container)
     end
   end