replace ActiveFedora where with Valkyrized where

app/services/hyrax/find_objects_via_solr_service.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+module Hyrax
+  # 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_service = Hyrax::SolrService
+    self.query_service = Hyrax.query_service
+
+    class << self
+      # Find objects matching search criteria.
+      # @param model [Class] if not using Valkyrie, this is expected to be an ActiveFedora::Base object that supports #where
+      # @param field_pairs [Hash] a list of pairs of property name and values
+      # @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<ActiveFedora|Valkyrie::Resource>] 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
+      end
+    end
+  end
+end

app/services/hyrax/multiple_membership_checker.rb

@@ -53,7 +53,11 @@ def check(collection_ids:, include_current_members: false)
     def single_membership_collections(collection_ids)
       return [] if collection_ids.blank?
 
-      ::Collection.where(:id => collection_ids, Hyrax.config.collection_type_index_field.to_sym => collection_type_gids_that_disallow_multiple_membership)
+      field_pairs = {
+        :id => collection_ids,
+        Hyrax.config.collection_type_index_field.to_sym => collection_type_gids_that_disallow_multiple_membership
+      }
+      Hyrax::FindObjectsViaSolrService.find_for_model_by_field_pairs(model: ::Collection, field_pairs: field_pairs, use_valkyrie: true)
     end
 
     def collection_type_gids_that_disallow_multiple_membership
@@ -74,7 +78,7 @@ def collection_type_title_from_gid(gid)
     end
 
     def collection_titles_from_list(collection_list)
-      collection_list.each do |collection|
+      collection_list.map do |collection|
         collection.title.first
       end.to_sentence
     end

app/services/hyrax/solr_query_builder_service.rb

@@ -14,9 +14,9 @@ def construct_query_for_ids(id_array)
       end
 
       # Construct a solr query from a list of pairs (e.g. [field name, values])
-      # @param [Array<Array>] field_pairs a list of pairs of property name and values
-      # @param [String] join_with ('AND') the value we're joining the clauses with
-      # @param [String] type ('field') The type of query to run. Either 'raw' or 'field'
+      # @param [Hash] field_pairs a list of pairs of property name and values
+      # @param [String] join_with the value we're joining the clauses with (default: ' AND ')
+      # @param [String] type of query to run. Either 'raw' or 'field' (default: 'field')
       # @return [String] a solr query
       # @example
       #   construct_query([['library_id_ssim', '123'], ['owner_ssim', 'Fred']])
@@ -32,6 +32,20 @@ def default_join_with
         ' AND '
       end
 
+      # Construct a solr query from a list of pairs (e.g. [field name, values]) including the model (e.g. Collection, Monograph)
+      # @param [Class] model class
+      # @param [Hash] field_pairs a list of pairs of property name and values
+      # @param [String] join_with the value we're joining the clauses with (default: ' AND ')
+      # @param [String] type of query to run. Either 'raw' or 'field' (default: 'field')
+      # @return [String] a solr query
+      # @example
+      #   construct_query(Collection, [['library_id_ssim', '123'], ['owner_ssim', 'Fred']])
+      #   # => "_query_:\"{!field f=has_model_ssim}Collection\" AND _query_:\"{!field f=library_id_ssim}123\" AND _query_:\"{!field f=owner_ssim}Fred\""
+      def construct_query_for_model(model, field_pairs, join_with = default_join_with, type = 'field')
+        field_pairs["has_model_ssim"] = model.to_s
+        construct_query(field_pairs, join_with, type)
+      end
+
       private
 
       # @param [Array<Array>] pairs a list of (key, value) pairs. The value itself may

spec/services/hyrax/find_objects_via_solr_service_spec.rb

@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe Hyrax::FindObjectsViaSolrService do
+  describe ".find_for_model_by_field_pairs", clean_repo: true do
+    let(:collection_ids) { [collection1.id, collection2.id] }
+    let(:field_pairs) do
+      { id: collection_ids.map(&:to_s) }
+    end
+    subject(:results) { described_class.find_for_model_by_field_pairs(model: collection1.class, field_pairs: field_pairs, use_valkyrie: use_valkyrie) }
+
+    context "when use_valkyrie is false" do
+      let(:use_valkyrie) { false }
+      let(:collection1) { create(:collection_lw, title: ['Foo']) }
+      let(:collection2) { create(:collection_lw, title: ['Too']) }
+      it "returns ActiveFedora objects matching the query" do
+        expect(results).to be_kind_of Array
+        expect(results.map(&:title)).to match_array [['Foo'], ['Too']]
+      end
+    end
+
+    context "when use_valkyrie is true" do
+      let(:use_valkyrie) { true }
+      context "and objects were created with ActiveFedora" do
+        let(:collection1) { create(:collection_lw, title: ['Foo']) }
+        let(:collection2) { create(:collection_lw, title: ['Too']) }
+
+        it "returns Valkyrie::Resources matching the query" do
+          expect(results).to be_kind_of Array
+          expect(results.map(&:title)).to match_array [['Foo'], ['Too']]
+        end
+      end
+      context "and objects were created with Valkryie" do
+        let(:collection1) { valkyrie_create(:hyrax_collection, title: ['Foo']) }
+        let(:collection2) { valkyrie_create(:hyrax_collection, title: ['Too']) }
+
+        it "returns Valkyrie::Resources matching the query" do
+          expect(results).to be_kind_of Array
+          expect(results.map(&:title)).to match_array [['Foo'], ['Too']]
+        end
+      end
+    end
+  end
+end

spec/services/hyrax/multiple_membership_checker_spec.rb

@@ -15,7 +15,22 @@
     let(:collection_ids) { ['foobar'] }
     let(:included) { false }
     let!(:collection_type) { create(:collection_type, title: 'Greedy', allow_multiple_membership: false) }
-    let(:collection_type_gids) { [collection_type] }
+    let(:collection_types) { [collection_type] }
+    let(:collection_type_gids) { [collection_type.to_global_id] }
+    let(:field_pairs) do
+      {
+        id: collection_ids,
+        collection_type_gid_ssim: collection_type_gids
+      }
+    end
+    let(:field_pairs_for_col2) do
+      {
+        id: [collection2.id],
+        collection_type_gid_ssim: collection_type_gids
+      }
+    end
+    let(:use_valkyrie) { true }
+
     before do
       allow(Hyrax::CollectionType).to receive(:gids_that_do_not_allow_multiple_membership).and_return(collection_type_gids)
     end
@@ -34,46 +49,43 @@
 
       it 'returns nil' do
         expect(checker).to receive(:single_membership_collections).with(collection_ids).once.and_call_original
-        expect(Collection).not_to receive(:where)
+        expect(Hyrax::FindObjectsViaSolrService).not_to receive(:find_for_model_by_field_pairs)
         expect(subject).to be nil
       end
     end
 
     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(Collection).not_to receive(:where)
+        expect(Hyrax::FindObjectsViaSolrService).not_to receive(:find_for_model_by_field_pairs)
         expect(subject).to be nil
       end
     end
 
     context 'when multiple single-membership collection instances are not in the list' do
-      let(:collection) { build(:collection_lw, id: 'collection0', collection_type: collection_type, with_solr_document: true) }
+      let(:collection) { create(:collection_lw, id: 'collection0', collection_type: collection_type, with_solr_document: true) }
       let(:collections) { [collection] }
       let(:collection_ids) { collections.map(&:id) }
 
       it 'returns nil' do
         expect(checker).to receive(:single_membership_collections).with(collection_ids).once.and_call_original
-        expect(Collection).to receive(:where).with(id: collection_ids, collection_type_gid_ssim: collection_type_gids).once.and_return(collections)
+        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(subject).to be nil
       end
     end
 
     context 'when multiple single-membership collection instances are in the list, not including current members' do
-      let(:collection1) { build(:collection_lw, id: 'collection1', title: ['Foo'], collection_type: collection_type, with_solr_document: true) }
-      let(:collection2) { build(:collection_lw, id: 'collection2', title: ['Bar'], collection_type: collection_type, with_solr_document: true) }
+      let(:collection1) { create(:collection_lw, id: 'collection1', title: ['Foo'], collection_type: collection_type, with_solr_document: true) }
+      let(:collection2) { create(:collection_lw, id: 'collection2', title: ['Bar'], collection_type: collection_type, with_solr_document: true) }
       let(:collections) { [collection1, collection2] }
       let(:collection_ids) { collections.map(&:id) }
 
-      before do
-        allow(Collection).to receive(:find).with(collection1.id).and_return(collection1)
-        allow(Collection).to receive(:find).with(collection2.id).and_return(collection2)
-      end
-
       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(Collection).to receive(:where).with(id: collection_ids, collection_type_gid_ssim: collection_type_gids).once.and_return(collections)
+        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(subject).to eq 'Error: You have specified more than one of the same single-membership collection type (type: Greedy, collections: Foo and Bar)'
       end
 
@@ -84,15 +96,16 @@
         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(Collection).to receive(:where).with(id: collection_ids, collection_type_gid_ssim: collection_type_gids).once.and_return(collections)
+          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(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
     end
 
     context 'when multiple single-membership collection instances are in the list, including current members' do
-      let(:collection1) { build(:collection_lw, id: 'collection1', title: ['Foo'], collection_type: collection_type, with_solr_document: true) }
-      let(:collection2) { build(:collection_lw, id: 'collection2', title: ['Bar'], collection_type: collection_type, with_solr_document: true) }
+      let(:collection1) { create(:collection_lw, id: 'collection1', title: ['Foo'], collection_type: collection_type, with_solr_document: true) }
+      let(:collection2) { create(:collection_lw, id: 'collection2', title: ['Bar'], collection_type: collection_type, with_solr_document: true) }
       let(:collections) { [collection1] }
       let(:collection_ids) { collections.map(&:id) }
       let(:included) { true }
@@ -103,8 +116,10 @@
 
       it 'returns an error' do
         expect(item).to receive(:member_of_collection_ids)
-        expect(Collection).to receive(:where).with(id: collection_ids, collection_type_gid_ssim: collection_type_gids).once.and_return(collections)
-        expect(Collection).to receive(:where).with(id: [collection2.id], collection_type_gid_ssim: collection_type_gids).once.and_return([collection2])
+        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(subject).to eq 'Error: You have specified more than one of the same single-membership collection type (type: Greedy, collections: Foo and Bar)'
       end
 
@@ -114,16 +129,18 @@
 
         it 'returns an error' do
           expect(item).to receive(:member_of_collection_ids)
-          expect(Collection).to receive(:where).with(id: collection_ids, collection_type_gid_ssim: collection_type_gids).once.and_return(collections)
-          expect(Collection).to receive(:where).with(id: [collection2.id], collection_type_gid_ssim: collection_type_gids).once.and_return([collection2])
+          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(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
     end
 
     context 'when multiple single-membership collection instances are in the list, but are different collection types' do
-      let(:collection1) { build(:collection_lw, title: ['Foo'], collection_type: collection_type, with_solr_document: true) }
-      let(:collection2) { build(:collection_lw, title: ['Bar'], collection_type: collection_type_2, with_solr_document: true) }
+      let(:collection1) { create(:collection_lw, title: ['Foo'], collection_type: collection_type, with_solr_document: true) }
+      let(:collection2) { create(:collection_lw, title: ['Bar'], collection_type: collection_type_2, with_solr_document: true) }
       let(:collections) { [collection1, collection2] }
       let(:collection_ids) { collections.map(&:id) }
       let(:collection_type_2) { create(:collection_type, title: 'Doc', allow_multiple_membership: false) }
@@ -132,7 +149,8 @@
       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(Collection).to receive(:where).with(id: collection_ids, collection_type_gid_ssim: collection_type_gids).once.and_return(collections)
+        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(subject).to be nil
       end
 
@@ -146,8 +164,10 @@
 
         it 'returns nil' do
           expect(item).to receive(:member_of_collection_ids)
-          expect(Collection).to receive(:where).with(id: collection_ids, collection_type_gid_ssim: collection_type_gids).once.and_return(collections)
-          expect(Collection).to receive(:where).with(id: [collection2.id], collection_type_gid_ssim: collection_type_gids).once.and_return([collection2])
+          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(subject).to be nil
         end
       end

spec/services/hyrax/solr_query_builder_service_spec.rb

@@ -2,17 +2,24 @@
 require 'spec_helper'
 
 RSpec.describe Hyrax::SolrQueryBuilderService do
-  describe '#construct_query_for_ids' do
+  describe '.construct_query_for_ids' do
     it "generates a useable solr query from an array of Fedora ids" do
       expect(described_class.construct_query_for_ids(["my:_ID1_", "my:_ID2_", "my:_ID3_"])).to eq '{!terms f=id}my:_ID1_,my:_ID2_,my:_ID3_'
     end
     it "returns a valid solr query even if given an empty array as input" do
       expect(described_class.construct_query_for_ids([""])).to eq "id:NEVER_USE_THIS_ID"
     end
   end
-  describe "construct_query" do
+
+  describe ".construct_query" do
     it "generates a query clause" do
       expect(described_class.construct_query('id' => "my:_ID1_")).to eq '_query_:"{!field f=id}my:_ID1_"'
     end
   end
+
+  describe ".construct_query_for_model" do
+    it "generates a query clause" do
+      expect(described_class.construct_query_for_model(::Collection, 'id' => "my:_ID1_")).to eq '(_query_:"{!field f=id}my:_ID1_" AND _query_:"{!field f=has_model_ssim}Collection")'
+    end
+  end
 end