refactor out default resource processing

* adds documentation of usage
* raise exceptions if required indexer_class and resource are not set
* add shared spec for ‘a Work indexer’ that calls shared specs related to works
* add core metadata test to ‘a Collection indexer’
* allow default visibility to be passed in using ‘restricted’ if not passed in

app/indexers/hyrax/valkyrie_collection_indexer.rb

@@ -7,6 +7,7 @@ class ValkyrieCollectionIndexer < Hyrax::ValkyrieIndexer
     include Hyrax::ResourceIndexer
     include Hyrax::PermissionIndexer
     include Hyrax::VisibilityIndexer
+    include Hyrax::Indexer(:core_metadata)
 
     def to_solr
       super.tap do |index_document|

app/services/hyrax/thumbnail_path_service.rb

@@ -5,7 +5,7 @@ class << self
       # @param [#id] object - to get the thumbnail for
       # @return [String] a path to the thumbnail
       def call(object)
-        return default_image unless object.try(:thumbnail_id)&.to_s&.present?
+        return default_image if object.try(:thumbnail_id).blank?
 
         thumb = fetch_thumbnail(object)
 

lib/hyrax/specs/shared_specs/indexers.rb

@@ -1,12 +1,21 @@
 # frozen_string_literal: true
 
+# These shared specs test various aspects of valkyrie based indexers by calling the #to_solr method.
+# All tests require two variables to be set in the caller using let statements:
+#   * indexer_class - class of the indexer being tested
+#   * resource - a Hyrax::Resource that defines attributes and values consistent with the indexer
+#
+# NOTE: It is important that the resource has required values that the indexer #to_solr customizations expects to be available.
 RSpec.shared_examples 'a Hyrax::Resource indexer' do
-  subject(:indexer)  { indexer_class.new(resource: the_resource) }
-  let(:the_resource) { defined?(resource) ? resource : Hyrax::Resource.new }
+  before do
+    raise 'indexer_class must be set with `let(:indexer_class)`' unless defined? indexer_class
+    raise 'resource must be set with `let(:resource)` and is expected to be a kind of Hyrax::Resource' unless defined?(resource) && resource.kind_of?(Hyrax::Resource)
+    resource.alternate_ids = ids
+  end
+  subject(:indexer)  { indexer_class.new(resource: resource) }
   let(:ids)          { ['id1', 'id2'] }
 
   describe '#to_solr' do
-    before { the_resource.alternate_ids = ids }
     it 'indexes alternate_ids' do
       expect(indexer.to_solr)
         .to include(alternate_ids_sm: a_collection_containing_exactly(*ids))
@@ -15,23 +24,18 @@
 end
 
 RSpec.shared_examples 'a permission indexer' do
-  subject(:indexer) { indexer_class.new(resource: the_resource) }
-  let(:the_resource) do
-    if defined?(resource)
-      Hyrax::VisibilityWriter.new(resource: resource)
-          .assign_access_for(visibility: Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PUBLIC)
-      resource.permission_manager.edit_groups = edit_groups
-      resource.permission_manager.edit_users  = edit_users
-      resource.permission_manager.read_users  = read_users
-      resource.permission_manager.acl.save
-      resource
-    else
-      FactoryBot.valkyrie_create(:hyrax_work, :public,
-                                 read_users: read_users,
-                                 edit_groups: edit_groups,
-                                 edit_users: edit_users)
-    end
+  before do
+    raise 'indexer_class must be set with `let(:indexer_class)`' unless defined? indexer_class
+    # NOTE: resource must be persisted for these tests to pass
+    raise 'resource must be set with `let(:resource)` and is expected to be a kind of Hyrax::Resource' unless defined?(resource) && resource.kind_of?(Hyrax::Resource)
+    Hyrax::VisibilityWriter.new(resource: resource)
+        .assign_access_for(visibility: Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PUBLIC)
+    resource.permission_manager.edit_groups = edit_groups
+    resource.permission_manager.edit_users  = edit_users
+    resource.permission_manager.read_users  = read_users
+    resource.permission_manager.acl.save
   end
+  subject(:indexer) { indexer_class.new(resource: resource) }
   let(:edit_groups) { [:managers] }
   let(:edit_users)  { [FactoryBot.create(:user)] }
   let(:read_users)  { [FactoryBot.create(:user)] }
@@ -52,17 +56,23 @@
 end
 
 RSpec.shared_examples 'a visibility indexer' do
-  subject(:indexer)  { indexer_class.new(resource: the_resource) }
-  let(:the_resource) { defined?(resource) ? resource : FactoryBot.build(:hyrax_work) }
+  before do
+    raise 'indexer_class must be set with `let(:indexer_class)`' unless defined? indexer_class
+    raise 'resource must be set with `let(:resource)` and is expected to be a kind of Hyrax::Resource' unless defined?(resource) && resource.kind_of?(Hyrax::Resource)
+    # optionally can pass in default_visibility by setting it with a let statement if your application changes the default; Hyrax defines this as 'restricted'
+    # See samvera/hyrda-head hydra-access-controls/app/models/concerns/hydra/access_controls/access_rights.rb for possible VISIBILITY_TEXT_VALUE_...'
+  end
+  subject(:indexer)  { indexer_class.new(resource: resource) }
 
   describe '#to_solr' do
-    it 'indexes visibility' do
-      expect(indexer.to_solr).to include(visibility_ssi: 'restricted')
+    it 'indexes default visibility as restricted or passed in default' do
+      expected_value = defined?(default_visibility) ? default_visibility : 'restricted'
+      expect(indexer.to_solr).to include(visibility_ssi: expected_value)
     end
 
     context 'when resource is public' do
       before do
-        Hyrax::VisibilityWriter.new(resource: the_resource)
+        Hyrax::VisibilityWriter.new(resource: resource)
           .assign_access_for(visibility: Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PUBLIC)
       end
 
@@ -73,9 +83,37 @@
   end
 end
 
+RSpec.shared_examples 'a Core metadata indexer' do
+  before do
+    raise 'indexer_class must be set with `let(:indexer_class)`' unless defined? indexer_class
+    # NOTE: The resource's class is expected to have or inherit `include Hyrax::Schema(:core_metadata)`
+    raise 'resource must be set with `let(:resource)` and is expected to be a kind of Hyrax::Resource' unless defined?(resource) && resource.kind_of?(Hyrax::Resource)
+    resource.title = titles
+  end
+  subject(:indexer) { indexer_class.new(resource: resource) }
+  let(:titles)      { ['Comet in Moominland', 'Finn Family Moomintroll'] }
+
+  describe '#to_solr' do
+    it 'indexes title as text' do
+      expect(indexer.to_solr)
+        .to include(title_tesim: a_collection_containing_exactly(*titles))
+    end
+
+    it 'indexes title as string' do
+      expect(indexer.to_solr)
+        .to include(title_sim: a_collection_containing_exactly(*titles))
+    end
+  end
+end
+
 RSpec.shared_examples 'a Basic metadata indexer' do
-  subject(:indexer) { indexer_class.new(resource: the_resource) }
-  let(:the_resource) { defined?(resource) ? resource : resource_class.new }
+  before do
+    raise 'indexer_class must be set with `let(:indexer_class)`' unless defined? indexer_class
+    # NOTE: The resource's class is expected to to have or inherit `include Hyrax::Schema(:basic_metadata)`
+    raise 'resource must be set with `let(:resource)` and is expected to be a kind of Hyrax::Resource' unless defined?(resource) && resource.kind_of?(Hyrax::Resource)
+    attributes.each { |k, v| resource.set_value(k, v) }
+  end
+  subject(:indexer) { indexer_class.new(resource: resource) }
 
   let(:attributes) do
     {
@@ -84,14 +122,7 @@
     }
   end
 
-  let(:resource_class) do
-    Class.new(Hyrax::Work) do
-      include Hyrax::Schema(:basic_metadata)
-    end
-  end
-
   describe '#to_solr' do
-    before { attributes.each { |k, v| the_resource.set_value(k, v) } }
     it 'indexes basic metadata' do
       expect(indexer.to_solr)
         .to include(keyword_sim:   a_collection_containing_exactly(*attributes[:keyword]),
@@ -101,10 +132,32 @@
   end
 end
 
+RSpec.shared_examples 'a Work indexer' do
+  before do
+    raise 'indexer_class must be set with `let(:indexer_class)`' unless defined? indexer_class
+    # NOTE: resource must be persisted for permission tests to pass
+    raise 'resource must be set with `let(:resource)` and is expected to be a kind of Hyrax::Work' unless defined?(resource) && resource.kind_of?(Hyrax::Work)
+    # optionally can pass in default_visibility by setting it with a let statement if your application changes the default; Hyrax defines this as 'restricted'
+    # See samvera/hyrda-head hydra-access-controls/app/models/concerns/hydra/access_controls/access_rights.rb for possible VISIBILITY_TEXT_VALUE_...'
+  end
+  subject(:indexer) { indexer_class.new(resource: resource) }
+
+  it_behaves_like 'a Hyrax::Resource indexer'
+  it_behaves_like 'a Core metadata indexer'
+  it_behaves_like 'a permission indexer'
+  it_behaves_like 'a visibility indexer'
+end
+
 RSpec.shared_examples 'a Collection indexer' do
-  subject(:indexer) { indexer_class.new(resource: the_resource) }
-  let(:the_resource) { defined?(resource) ? resource : FactoryBot.valkyrie_create(:hyrax_collection) }
+  before do
+    raise 'indexer_class must be set with `let(:indexer_class)`' unless defined? indexer_class
+    # NOTE: resource must be persisted for permission tests to pass
+    raise 'resource must be set with `let(:resource)` and is expected to be a kind of Hyrax::PcdmCollection' unless defined?(resource) && resource.kind_of?(Hyrax::PcdmCollection)
+  end
+  subject(:indexer) { indexer_class.new(resource: resource) }
 
+  it_behaves_like 'a Hyrax::Resource indexer'
+  it_behaves_like 'a Core metadata indexer'
   it_behaves_like 'a permission indexer'
   it_behaves_like 'a visibility indexer'
 
@@ -120,22 +173,3 @@
     end
   end
 end
-
-RSpec.shared_examples 'a Core metadata indexer' do
-  subject(:indexer) { indexer_class.new(resource: the_resource) }
-  let(:titles)      { ['Comet in Moominland', 'Finn Family Moomintroll'] }
-  let(:the_resource) { defined?(resource) ? resource : Hyrax::Work.new }
-
-  describe '#to_solr' do
-    before { the_resource.title = titles }
-    it 'indexes title as text' do
-      expect(indexer.to_solr)
-        .to include(title_tesim: a_collection_containing_exactly(*titles))
-    end
-
-    it 'indexes title as string' do
-      expect(indexer.to_solr)
-        .to include(title_sim: a_collection_containing_exactly(*titles))
-    end
-  end
-end

spec/hyrax/indexer_spec.rb

@@ -13,6 +13,7 @@
   end
 
   context 'with core metadata schema' do
+    let(:resource) { work }
     it_behaves_like 'a Core metadata indexer'
   end
 

spec/indexers/hyrax/permission_indexer_spec.rb

@@ -8,6 +8,7 @@
       include Hyrax::PermissionIndexer
     end
   end
+  let(:resource) { FactoryBot.valkyrie_create(:hyrax_resource) }
 
   it_behaves_like 'a permission indexer'
 end

spec/indexers/hyrax/valkyrie_collection_indexer_spec.rb

@@ -6,6 +6,5 @@
   let(:resource) { FactoryBot.valkyrie_create(:hyrax_collection) }
   let(:indexer_class) { described_class }
 
-  it_behaves_like 'a Hyrax::Resource indexer'
   it_behaves_like 'a Collection indexer'
 end

spec/indexers/hyrax/valkyrie_work_indexer_spec.rb

@@ -3,19 +3,22 @@
 require 'hyrax/specs/shared_specs'
 
 RSpec.describe Hyrax::ValkyrieWorkIndexer do
+  let(:resource) { FactoryBot.valkyrie_create(:hyrax_work) }
   let(:indexer_class) { described_class }
 
-  it_behaves_like 'a Hyrax::Resource indexer'
-  it_behaves_like 'a Core metadata indexer'
-  it_behaves_like 'a permission indexer'
-  it_behaves_like 'a visibility indexer'
+  it_behaves_like 'a Work indexer'
 
   context 'when extending with basic metadata' do
     let(:indexer_class) do
       Class.new(described_class) do
         include Hyrax::Indexer(:basic_metadata)
       end
     end
+    let(:resource) do
+      Class.new(Hyrax::Work) do
+        include Hyrax::Schema(:basic_metadata)
+      end.new
+    end
 
     it_behaves_like 'a Basic metadata indexer'
   end
@@ -42,9 +45,6 @@ def to_solr
     let(:resource) { Hyrax.persister.save(resource: Hyrax::Test::Custom::Work.new(broader: ['term1', 'term2'])) }
     let(:indexer_class) { Hyrax::Test::Custom::WorkIndexer }
 
-    it_behaves_like 'a Hyrax::Resource indexer'
-    it_behaves_like 'a Core metadata indexer'
-    it_behaves_like 'a permission indexer'
-    it_behaves_like 'a visibility indexer'
+    it_behaves_like 'a Work indexer'
   end
 end

spec/indexers/hyrax/visibility_indexer_spec.rb

@@ -8,6 +8,7 @@
       include Hyrax::VisibilityIndexer
     end
   end
+  let(:resource) { Hyrax::Resource.new }
 
   it_behaves_like 'a visibility indexer'
 end