deprecate FindObjectsViaSolrService replaced by SolrQueryService#get_objects #5023
Conversation
| def get_objects(use_valkyrie: Hyrax.config.use_valkyrie?) | ||
| ids = get_ids | ||
| return ids.map { |id| ActiveFedora::Base.find(id) }.to_a unless use_valkyrie | ||
| query_service.find_many_by_ids(ids: ids).to_a |
There was a problem hiding this comment.
is the #to_a desirable, or could this return an Enumerator?
it seems like avoiding loading data that won't be used is attractive.
There was a problem hiding this comment.
That makes sense. I'll remove it and see if everything continues to work. I used to_a because the deprecated method used it. I can't remember the original reason for using to_a.
…objects Use SolrQueryService#get_objects instead of FindObjectsViaSolrService when objects are required. If solr documents are sufficient, then just use SolrQueryService#get. By using SolrQueryService, this allows for the construction of more complex queries even when objects are being returned. Update MultipleMembershipChecker to use SolrQueryService instead of FindObjectViaSolrService.
Co-authored-by: tamsin johnson <tomjohnson@ucsb.edu>
| 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 |
There was a problem hiding this comment.
FindObjectViaSolrService #find_for_model_by_field_pairs needs to continue to return an Array for backward compatibility.
This method is no longer used in any Hyrax code.
| @@ -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) | |||
There was a problem hiding this comment.
The map to strings is required to turn GlobalIDs into string IDs. I'm not sure why this worked before. It definitely does not work with the SolrQueryService.
| 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) |
There was a problem hiding this comment.
Removed .to_a on the returned result to maintain the Enumerable.
| Hyrax::SolrQueryService.new | ||
| .with_model(model: ::Collection) | ||
| .with_field_pairs(field_pairs: field_pairs, join_with: ' OR ') | ||
| .get_objects(use_valkyrie: true).to_a |
There was a problem hiding this comment.
The result is required to be an Array because later it is used in...
collections_to_check |= single_membership_collections(item.member_of_collection_ids)
where the result is collections_to_check.
Use
SolrQueryService#get_objectsinstead ofFindObjectsViaSolrServicewhen objects are required. If solr documents are sufficient, then just useSolrQueryService#get.By using
SolrQueryService, this allows for the construction of more complex queries even when objects are being returned.Also updated
MultipleMembershipCheckerto useSolrQueryServiceinstead ofFindObjectViaSolrService. This was the only use ofFindObjectViaSolrService.Related to: Issue #3820 which is replacing ActiveFedora::Base.where calls.
@samvera/hyrax-code-reviewers