Skip to content

Commit

Permalink
replace ActiveFedora where with Valkyrized where
Browse files Browse the repository at this point in the history
  • Loading branch information
elrayle committed May 4, 2021
1 parent a859788 commit 2b485b0
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 33 deletions.
27 changes: 27 additions & 0 deletions app/services/hyrax/find_objects_via_solr_service.rb
Original file line number Diff line number Diff line change
@@ -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]
# @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 [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?)
query = solr_query_builder.construct_query_for_model(model, field_pairs, join_with, type)
results = solr_service.query(query)
ids = results.map(&:id)
return query_service.find_many_by_ids(ids: ids) if use_valkyrie
ids.map { |id| query_service.find_by_alternate_identifier(alternate_identifier: id.to_str, use_valkyrie: false) }
end
end
end
end
10 changes: 7 additions & 3 deletions app/services/hyrax/multiple_membership_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,15 @@ 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
Hyrax::CollectionType.gids_that_do_not_allow_multiple_membership
Hyrax::CollectionType.gids_that_do_not_allow_multiple_membership.map(&:to_s)
end

def build_error_message(problematic_collections)
Expand All @@ -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
Expand Down
20 changes: 17 additions & 3 deletions app/services/hyrax/solr_query_builder_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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']])
Expand All @@ -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
Expand Down
20 changes: 20 additions & 0 deletions spec/services/hyrax/find_objects_via_solr_service_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# frozen_string_literal: true
require 'spec_helper'

RSpec.describe Hyrax::FindObjectsViaSolrService do
describe ".find_for_model_by_field_pairs", clean_repo: true do
let(:collection1) { valkyrie_create(:hyrax_collection, title: ['Foo']) }
let(:collection2) { valkyrie_create(:hyrax_collection, title: ['Too']) }
let(:collection_ids) { [collection1.id, collection2.id] }
let(:field_pairs) do
{ id: collection_ids.map(&:to_s) }
end

it "returns ActiveFedora objects matching the query" do
expect(described_class.find_for_model_by_field_pairs(model: ::Collection, field_pairs: field_pairs, use_valkyrie: false).map(&:title)).to match_array [['Foo'], ['Too']]
end
it "returns Valkyrie::Resources matching the query" do
expect(described_class.find_for_model_by_field_pairs(model: ::Collection, field_pairs: field_pairs, use_valkyrie: true).map(&:title)).to match_array [['Foo'], ['Too']]
end
end
end
70 changes: 45 additions & 25 deletions spec/services/hyrax/multiple_membership_checker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.map(&:to_s)
}
end
let(:field_pairs_for_col2) do
{
id: [collection2.id],
collection_type_gid_ssim: collection_type_gids.map(&:to_s)
}
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
Expand All @@ -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

Expand All @@ -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 }
Expand All @@ -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

Expand All @@ -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) }
Expand All @@ -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

Expand All @@ -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
Expand Down
11 changes: 9 additions & 2 deletions spec/services/hyrax/solr_query_builder_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 2b485b0

Please sign in to comment.