diff --git a/app/services/hyrax/find_objects_via_solr_service.rb b/app/services/hyrax/find_objects_via_solr_service.rb index 21045fc0cf..628b0b800e 100644 --- a/app/services/hyrax/find_objects_via_solr_service.rb +++ b/app/services/hyrax/find_objects_via_solr_service.rb @@ -1,9 +1,11 @@ # frozen_string_literal: true module Hyrax + # @deprecated This class is being replaced by Hyrax::SolrQueryService #get_objects method. + # # Methods in this class search solr to get the ids and then use the query service to find the objects. class FindObjectsViaSolrService class_attribute :solr_query_builder, :solr_service, :query_service - self.solr_query_builder = Hyrax::SolrQueryBuilderService + self.solr_query_builder = Hyrax::SolrQueryService self.solr_service = Hyrax::SolrService self.query_service = Hyrax.query_service @@ -14,13 +16,15 @@ class << self # @param join_with [String] the value we're joining the clauses with (default: ' OR ' for backward compatibility with ActiveFedora where) # @param type [String] The type of query to run. Either 'raw' or 'field' (default: 'field') # @param use_valkyrie [Boolean] if true, return Valkyrie resource(s); otherwise, return ActiveFedora object(s) - # @return [Array] objects matching the query + # @return [Array] objects matching the query def find_for_model_by_field_pairs(model:, field_pairs:, join_with: ' OR ', type: 'field', use_valkyrie: Hyrax.config.use_valkyrie?) - return model.where(field_pairs).to_a unless use_valkyrie - query = solr_query_builder.construct_query_for_model(model, field_pairs, join_with, type) - results = solr_service.query(query) - ids = results.map(&:id) - query_service.find_many_by_ids(ids: ids).to_a + Deprecation.warn("'##{__method__}' will be removed in Hyrax 4.0. " \ + "Instead, use 'Hyrax::SolrQueryService.new.with_model(...).with_field_pairs(...).get_objects'.") + + solr_query_builder.new + .with_model(model: model) + .with_field_pairs(field_pairs: field_pairs, join_with: join_with, type: type) + .get_objects(use_valkyrie: use_valkyrie).to_a end end end diff --git a/app/services/hyrax/multiple_membership_checker.rb b/app/services/hyrax/multiple_membership_checker.rb index a2c461caec..35f0f6ecc6 100644 --- a/app/services/hyrax/multiple_membership_checker.rb +++ b/app/services/hyrax/multiple_membership_checker.rb @@ -55,9 +55,12 @@ def single_membership_collections(collection_ids) field_pairs = { :id => collection_ids, - Hyrax.config.collection_type_index_field.to_sym => collection_type_gids_that_disallow_multiple_membership + Hyrax.config.collection_type_index_field.to_sym => collection_type_gids_that_disallow_multiple_membership&.map(&:to_s) } - Hyrax::FindObjectsViaSolrService.find_for_model_by_field_pairs(model: ::Collection, field_pairs: field_pairs, use_valkyrie: true) + Hyrax::SolrQueryService.new + .with_model(model: ::Collection) + .with_field_pairs(field_pairs: field_pairs, join_with: ' OR ') + .get_objects(use_valkyrie: true).to_a end def collection_type_gids_that_disallow_multiple_membership diff --git a/app/services/hyrax/solr_query_service.rb b/app/services/hyrax/solr_query_service.rb index ee4e7801af..550d0ea917 100644 --- a/app/services/hyrax/solr_query_service.rb +++ b/app/services/hyrax/solr_query_service.rb @@ -4,6 +4,9 @@ module Hyrax # Methods in this class are providing functionality previously supported by ActiveFedora::SolrQueryBuilder. # It includes methods to build and execute a query. class SolrQueryService + class_attribute :query_service + self.query_service = Hyrax.query_service + attr_reader :query, :solr_service def initialize(query: [], solr_service: Hyrax::SolrService) @@ -17,6 +20,21 @@ def get solr_service.get(build) end + ## + # @return [Array] ids of documents matching the current query + def get_ids # rubocop:disable Naming/AccessorMethodName + results = get + results['response']['docs'].map { |doc| doc['id'] } + end + + ## + # @return [Array] objects matching the current query + def get_objects(use_valkyrie: Hyrax.config.use_valkyrie?) + ids = get_ids + return ids.map { |id| ActiveFedora::Base.find(id) } unless use_valkyrie + query_service.find_many_by_ids(ids: ids) + end + ## # @return [Integer] the number of results that match the query in solr def count diff --git a/spec/services/hyrax/multiple_membership_checker_spec.rb b/spec/services/hyrax/multiple_membership_checker_spec.rb index acf9b5f1d5..b6570eaac9 100644 --- a/spec/services/hyrax/multiple_membership_checker_spec.rb +++ b/spec/services/hyrax/multiple_membership_checker_spec.rb @@ -20,13 +20,13 @@ let(:field_pairs) do { id: collection_ids, - collection_type_gid_ssim: collection_type_gids + collection_type_gid_ssim: collection_type_gids.map(&:to_s) } end let(:field_pairs_for_col2) do { id: [collection2.id], - collection_type_gid_ssim: collection_type_gids + collection_type_gid_ssim: collection_type_gids.map(&:to_s) } end let(:use_valkyrie) { true } @@ -49,7 +49,7 @@ it 'returns nil' do expect(checker).to receive(:single_membership_collections).with(collection_ids).once.and_call_original - expect(Hyrax::FindObjectsViaSolrService).not_to receive(:find_for_model_by_field_pairs) + expect(Hyrax::SolrQueryService).not_to receive(:new) expect(subject).to be nil end end @@ -57,7 +57,7 @@ context 'when there are no single-membership collection instances' do it 'returns nil' do expect(checker).to receive(:single_membership_collections).with(collection_ids).once.and_return([]) - expect(Hyrax::FindObjectsViaSolrService).not_to receive(:find_for_model_by_field_pairs) + expect(Hyrax::SolrQueryService).not_to receive(:new) expect(subject).to be nil end end @@ -69,8 +69,10 @@ it 'returns nil' do expect(checker).to receive(:single_membership_collections).with(collection_ids).once.and_call_original - expect(Hyrax::FindObjectsViaSolrService).to receive(:find_for_model_by_field_pairs) - .with(model: ::Collection, field_pairs: field_pairs, use_valkyrie: true).once.and_return(collections) + expect(Hyrax::SolrQueryService).to receive(:new).and_return(inst = double) + expect(inst).to receive(:with_model).with(model: ::Collection).once.and_return(inst_with_model = double) + expect(inst_with_model).to receive(:with_field_pairs).with(field_pairs: field_pairs, join_with: ' OR ').once.and_return(inst_with_full_query = double) + expect(inst_with_full_query).to receive(:get_objects).with(use_valkyrie: true).once.and_return(collections) expect(subject).to be nil end end @@ -84,8 +86,10 @@ it 'returns an error' do expect(item).not_to receive(:member_of_collection_ids) expect(checker).to receive(:single_membership_collections).with(collection_ids).once.and_call_original - expect(Hyrax::FindObjectsViaSolrService).to receive(:find_for_model_by_field_pairs) - .with(model: ::Collection, field_pairs: field_pairs, use_valkyrie: true).once.and_return(collections) + expect(Hyrax::SolrQueryService).to receive(:new).and_return(inst = double) + expect(inst).to receive(:with_model).with(model: ::Collection).once.and_return(inst_with_model = double) + expect(inst_with_model).to receive(:with_field_pairs).with(field_pairs: field_pairs, join_with: ' OR ').once.and_return(inst_with_full_query = double) + expect(inst_with_full_query).to receive(:get_objects).with(use_valkyrie: true).once.and_return(collections) expect(subject).to eq 'Error: You have specified more than one of the same single-membership collection type (type: Greedy, collections: Foo and Bar)' end @@ -96,8 +100,10 @@ it 'returns an error' do expect(item).not_to receive(:member_of_collection_ids) expect(checker).to receive(:single_membership_collections).with(collection_ids).once.and_call_original - expect(Hyrax::FindObjectsViaSolrService).to receive(:find_for_model_by_field_pairs) - .with(model: ::Collection, field_pairs: field_pairs, use_valkyrie: true).once.and_return(collections) + expect(Hyrax::SolrQueryService).to receive(:new).and_return(inst = double) + expect(inst).to receive(:with_model).with(model: ::Collection).once.and_return(inst_with_model = double) + expect(inst_with_model).to receive(:with_field_pairs).with(field_pairs: field_pairs, join_with: ' OR ').once.and_return(inst_with_full_query = double) + expect(inst_with_full_query).to receive(:get_objects).with(use_valkyrie: true).once.and_return(collections) expect(subject).to eq 'Error: You have specified more than one of the same single-membership collection type (type: Greedy, collections: Foo and Bar)' end end @@ -116,10 +122,13 @@ it 'returns an error' do expect(item).to receive(:member_of_collection_ids) - expect(Hyrax::FindObjectsViaSolrService).to receive(:find_for_model_by_field_pairs) - .with(model: ::Collection, field_pairs: field_pairs, use_valkyrie: true).once.and_return(collections) - expect(Hyrax::FindObjectsViaSolrService).to receive(:find_for_model_by_field_pairs) - .with(model: ::Collection, field_pairs: field_pairs_for_col2, use_valkyrie: true).once.and_return([collection2]) + expect(Hyrax::SolrQueryService).to receive(:new).and_return(inst1 = double, inst2 = double) + expect(inst1).to receive(:with_model).with(model: ::Collection).and_return(inst1_with_model = double) + expect(inst1_with_model).to receive(:with_field_pairs).with(field_pairs: field_pairs, join_with: ' OR ').and_return(inst1_with_full_query = double) + expect(inst1_with_full_query).to receive(:get_objects).with(use_valkyrie: true).and_return(collections) + expect(inst2).to receive(:with_model).with(model: ::Collection).and_return(inst2_with_model = double) + expect(inst2_with_model).to receive(:with_field_pairs).with(field_pairs: field_pairs_for_col2, join_with: ' OR ').and_return(inst2_with_full_query = double) + expect(inst2_with_full_query).to receive(:get_objects).with(use_valkyrie: true).and_return([collection2]) expect(subject).to eq 'Error: You have specified more than one of the same single-membership collection type (type: Greedy, collections: Foo and Bar)' end @@ -129,10 +138,13 @@ it 'returns an error' do expect(item).to receive(:member_of_collection_ids) - expect(Hyrax::FindObjectsViaSolrService).to receive(:find_for_model_by_field_pairs) - .with(model: ::Collection, field_pairs: field_pairs, use_valkyrie: true).once.and_return(collections) - expect(Hyrax::FindObjectsViaSolrService).to receive(:find_for_model_by_field_pairs) - .with(model: ::Collection, field_pairs: field_pairs_for_col2, use_valkyrie: true).once.and_return([collection2]) + expect(Hyrax::SolrQueryService).to receive(:new).and_return(inst1 = double, inst2 = double) + expect(inst1).to receive(:with_model).with(model: ::Collection).and_return(inst1_with_model = double) + expect(inst1_with_model).to receive(:with_field_pairs).with(field_pairs: field_pairs, join_with: ' OR ').and_return(inst1_with_full_query = double) + expect(inst1_with_full_query).to receive(:get_objects).with(use_valkyrie: true).and_return(collections) + expect(inst2).to receive(:with_model).with(model: ::Collection).and_return(inst2_with_model = double) + expect(inst2_with_model).to receive(:with_field_pairs).with(field_pairs: field_pairs_for_col2, join_with: ' OR ').and_return(inst2_with_full_query = double) + expect(inst2_with_full_query).to receive(:get_objects).with(use_valkyrie: true).and_return([collection2]) expect(subject).to eq 'Error: You have specified more than one of the same single-membership collection type (type: Greedy, collections: Foo and Bar)' end end @@ -149,8 +161,10 @@ it 'returns nil' do expect(item).not_to receive(:member_of_collection_ids) expect(checker).to receive(:single_membership_collections).with(collection_ids).once.and_call_original - expect(Hyrax::FindObjectsViaSolrService).to receive(:find_for_model_by_field_pairs) - .with(model: ::Collection, field_pairs: field_pairs, use_valkyrie: true).once.and_return(collections) + expect(Hyrax::SolrQueryService).to receive(:new).and_return(inst = double) + expect(inst).to receive(:with_model).with(model: ::Collection).once.and_return(inst_with_model = double) + expect(inst_with_model).to receive(:with_field_pairs).with(field_pairs: field_pairs, join_with: ' OR ').once.and_return(inst_with_full_query = double) + expect(inst_with_full_query).to receive(:get_objects).with(use_valkyrie: true).once.and_return(collections) expect(subject).to be nil end @@ -164,10 +178,13 @@ it 'returns nil' do expect(item).to receive(:member_of_collection_ids) - expect(Hyrax::FindObjectsViaSolrService).to receive(:find_for_model_by_field_pairs) - .with(model: ::Collection, field_pairs: field_pairs, use_valkyrie: true).once.and_return(collections) - expect(Hyrax::FindObjectsViaSolrService).to receive(:find_for_model_by_field_pairs) - .with(model: ::Collection, field_pairs: field_pairs_for_col2, use_valkyrie: true).once.and_return([collection2]) + expect(Hyrax::SolrQueryService).to receive(:new).and_return(inst1 = double, inst2 = double) + expect(inst1).to receive(:with_model).with(model: ::Collection).and_return(inst1_with_model = double) + expect(inst1_with_model).to receive(:with_field_pairs).with(field_pairs: field_pairs, join_with: ' OR ').and_return(inst1_with_full_query = double) + expect(inst1_with_full_query).to receive(:get_objects).with(use_valkyrie: true).and_return(collections) + expect(inst2).to receive(:with_model).with(model: ::Collection).and_return(inst2_with_model = double) + expect(inst2_with_model).to receive(:with_field_pairs).with(field_pairs: field_pairs_for_col2, join_with: ' OR ').and_return(inst2_with_full_query = double) + expect(inst2_with_full_query).to receive(:get_objects).with(use_valkyrie: true).and_return([collection2]) expect(subject).to be nil end end diff --git a/spec/services/hyrax/solr_query_service_spec.rb b/spec/services/hyrax/solr_query_service_spec.rb index 6f439ea4cc..fac24d49b1 100644 --- a/spec/services/hyrax/solr_query_service_spec.rb +++ b/spec/services/hyrax/solr_query_service_spec.rb @@ -25,6 +25,48 @@ end end + describe '#get_ids' do + let!(:work1) { create(:work, id: 'wk1', creator: ['Mark']) } + let!(:work2) { create(:work, id: 'wk2', creator: ['Fred']) } + let!(:work3) { create(:work, id: 'wk3', creator: ['Mark']) } + subject(:solr_query_service) { described_class.new } + + before { solr_query_service.with_field_pairs(field_pairs: { creator_tesim: 'Mark' }) } + + it 'get ids for solr document matching the query' do + ids = solr_query_service.get_ids + expect(ids.count).to eq 2 + expect(ids).to match_array ['wk1', 'wk3'] + end + end + + describe '#get_objects' do + let!(:work1) { create(:work, id: 'wk1', creator: ['Mark']) } + let!(:work2) { create(:work, id: 'wk2', creator: ['Fred']) } + let!(:work3) { create(:work, id: 'wk3', creator: ['Mark']) } + subject(:solr_query_service) { described_class.new } + + before { solr_query_service.with_field_pairs(field_pairs: { creator_tesim: 'Mark' }) } + + context "when use_valkyrie is false" do + it 'get ActiveFedora::Base objects matching the query' do + objects = solr_query_service.get_objects(use_valkyrie: false) + expect(objects.count).to eq 2 + expect(objects.first).to be_kind_of ActiveFedora::Base + expect(objects.map(&:id)).to match_array ['wk1', 'wk3'] + end + end + + context "when use_valkyrie is true" do + it 'get Valkyrie::Resource objects matching the query' do + objects = solr_query_service.get_objects(use_valkyrie: true) + expect(objects.count).to eq 2 + expect(objects.first).to be_kind_of Valkyrie::Resource + expect(objects.map(&:id)).to match_array ['wk1', 'wk3'] + end + end + end + describe '#count' do let!(:work1) { create(:work, id: 'wk1', creator: ['Mark']) } let!(:work2) { create(:work, id: 'wk2', creator: ['Fred']) }