From 88e4731d7d669521c55565db5f9386eb48c7fab9 Mon Sep 17 00:00:00 2001 From: "E. Lynette Rayle" Date: Wed, 22 Jul 2020 14:41:31 -0400 Subject: [PATCH] refactor out default resource processing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- .../hyrax/valkyrie_collection_indexer.rb | 1 + app/services/hyrax/thumbnail_path_service.rb | 2 +- lib/hyrax/specs/shared_specs/indexers.rb | 142 +++++++++++------- spec/hyrax/indexer_spec.rb | 1 + .../indexers/hyrax/permission_indexer_spec.rb | 1 + .../hyrax/valkyrie_collection_indexer_spec.rb | 1 - .../hyrax/valkyrie_work_indexer_spec.rb | 16 +- .../indexers/hyrax/visibility_indexer_spec.rb | 1 + 8 files changed, 101 insertions(+), 64 deletions(-) diff --git a/app/indexers/hyrax/valkyrie_collection_indexer.rb b/app/indexers/hyrax/valkyrie_collection_indexer.rb index b86caaed03..31fce7989a 100644 --- a/app/indexers/hyrax/valkyrie_collection_indexer.rb +++ b/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| diff --git a/app/services/hyrax/thumbnail_path_service.rb b/app/services/hyrax/thumbnail_path_service.rb index fd451052b5..4002f09adf 100644 --- a/app/services/hyrax/thumbnail_path_service.rb +++ b/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) diff --git a/lib/hyrax/specs/shared_specs/indexers.rb b/lib/hyrax/specs/shared_specs/indexers.rb index ca1c733c07..5b808dd76a 100644 --- a/lib/hyrax/specs/shared_specs/indexers.rb +++ b/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 diff --git a/spec/hyrax/indexer_spec.rb b/spec/hyrax/indexer_spec.rb index 9842a45d8e..e5787ab511 100644 --- a/spec/hyrax/indexer_spec.rb +++ b/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 diff --git a/spec/indexers/hyrax/permission_indexer_spec.rb b/spec/indexers/hyrax/permission_indexer_spec.rb index 62381ce318..71afaeebcf 100644 --- a/spec/indexers/hyrax/permission_indexer_spec.rb +++ b/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 diff --git a/spec/indexers/hyrax/valkyrie_collection_indexer_spec.rb b/spec/indexers/hyrax/valkyrie_collection_indexer_spec.rb index 65d908ccc2..775004d0f5 100644 --- a/spec/indexers/hyrax/valkyrie_collection_indexer_spec.rb +++ b/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 diff --git a/spec/indexers/hyrax/valkyrie_work_indexer_spec.rb b/spec/indexers/hyrax/valkyrie_work_indexer_spec.rb index d4465e4e64..32a0ede292 100644 --- a/spec/indexers/hyrax/valkyrie_work_indexer_spec.rb +++ b/spec/indexers/hyrax/valkyrie_work_indexer_spec.rb @@ -3,12 +3,10 @@ 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 @@ -16,6 +14,11 @@ 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 diff --git a/spec/indexers/hyrax/visibility_indexer_spec.rb b/spec/indexers/hyrax/visibility_indexer_spec.rb index 243e0d8d77..ac5e127afa 100644 --- a/spec/indexers/hyrax/visibility_indexer_spec.rb +++ b/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