Skip to content

Commit

Permalink
♻️ # ♻️ Favor member_ids_ssim over file_set_ids_ssim
Browse files Browse the repository at this point in the history
For several years Hyrax has index `file_set_ids_ssim` as a verbatim copy
of `member_ids_ssim`.  With Hyrax 5, we're removing the
`file_set_ids_ssim` from indexing; And given that it's been a verbatim
copy since 2017 or so, it's relatively safe to assume that we can favor,
without application impact, the `member_ids_ssim` over the
`file_set_ids_ssim` value.

It would be nice to have `file_set_ids_ssim` but not as a verbatim copy.
Someday, we might have nice things.

In the case of Bulkrax, given that we don't require Hyrax, we need to
have a configurable option as well as one that is a consistent fallback
to what came before.

Related to:
- samvera/hyrax#6513
- samvera/hyrax@7108409
  • Loading branch information
jeremyf committed Feb 23, 2024
1 parent 8ce15d9 commit 062a566
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 17 deletions.
4 changes: 2 additions & 2 deletions app/parsers/bulkrax/parser_export_record_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,14 @@ def each
#
# @see #file_sets
def candidate_file_set_ids
@candidate_file_set_ids ||= works.flat_map { |work| work.fetch("#{Bulkrax.file_model_class.to_s.underscore}_ids_ssim", []) }
@candidate_file_set_ids ||= works.flat_map { |work| work.fetch(Bulkrax.solr_key_for_member_file_ids, []) }
end

# @note Specifically not memoizing this so we can merge values without changing the object.
#
# No sense attempting to query for more than the limit.
def query_kwargs
{ fl: "id,#{Bulkrax.file_model_class.to_s.underscore}_ids_ssim", method: :post, rows: row_limit }
{ fl: "id,member_ids_ssim", method: :post, rows: row_limit }
end

# If we have a limit, we need not query beyond that limit
Expand Down
17 changes: 17 additions & 0 deletions lib/bulkrax.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,21 @@ class Configuration
# @param adapter [Class<Bulkrax::PersistenceLayer::AbstractAdapter>]
attr_writer :persistence_adapter

##
# @param [String]
attr_writer :solr_key_for_member_file_ids

##
# @return [String]
# @see https://github.com/samvera/hyrax/pull/6513
def solr_key_for_member_file_ids
return @solr_key_for_member_file_ids if @solr_key_for_member_file_ids.present?

return "member_ids_ssim" if defined?(Hyrax)

"#{file_model_class.name.to_s.underscore}_ids_ssim"
end

##
# @param coercer [#call]
# @see Bulkrax::FactoryClassFinder
Expand Down Expand Up @@ -157,6 +172,8 @@ def config
:reserved_properties=,
:server_name,
:server_name=,
:solr_key_for_member_file_ids,
:solr_key_for_member_file_ids=,
:use_locking,
:use_locking=,
:use_locking?
Expand Down
30 changes: 15 additions & 15 deletions spec/parsers/bulkrax/parser_export_record_set_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@
RSpec.describe klass do
let(:works) do
[
SolrDocument.new(id: 1, file_set_ids_ssim: ["a", "b", "c"]),
SolrDocument.new(id: 2, file_set_ids_ssim: ["d", "e", "f"])
SolrDocument.new(id: 1, member_ids_ssim: ["a", "b", "c"]),
SolrDocument.new(id: 2, member_ids_ssim: ["d", "e", "f"])
]
end

Expand Down Expand Up @@ -132,7 +132,7 @@
[works[1].id, parser.entry_class],
[collections[0].id, parser.collection_entry_class],
[collections[1].id, parser.collection_entry_class],
[works[0].fetch("file_set_ids_ssim").first, parser.file_set_entry_class]
[works[0].fetch(Bulkrax.solr_key_for_member_file_ids).first, parser.file_set_entry_class]
)
end
end
Expand All @@ -147,12 +147,12 @@
[works[1].id, parser.entry_class],
[collections[0].id, parser.collection_entry_class],
[collections[1].id, parser.collection_entry_class],
[works[0].fetch("file_set_ids_ssim")[0], parser.file_set_entry_class],
[works[0].fetch("file_set_ids_ssim")[1], parser.file_set_entry_class],
[works[0].fetch("file_set_ids_ssim")[2], parser.file_set_entry_class],
[works[1].fetch("file_set_ids_ssim")[0], parser.file_set_entry_class],
[works[1].fetch("file_set_ids_ssim")[1], parser.file_set_entry_class],
[works[1].fetch("file_set_ids_ssim")[2], parser.file_set_entry_class]
[works[0].fetch(Bulkrax.solr_key_for_member_file_ids)[0], parser.file_set_entry_class],
[works[0].fetch(Bulkrax.solr_key_for_member_file_ids)[1], parser.file_set_entry_class],
[works[0].fetch(Bulkrax.solr_key_for_member_file_ids)[2], parser.file_set_entry_class],
[works[1].fetch(Bulkrax.solr_key_for_member_file_ids)[0], parser.file_set_entry_class],
[works[1].fetch(Bulkrax.solr_key_for_member_file_ids)[1], parser.file_set_entry_class],
[works[1].fetch(Bulkrax.solr_key_for_member_file_ids)[2], parser.file_set_entry_class]
)
end
end
Expand All @@ -167,12 +167,12 @@
[works[1].id, parser.entry_class],
[collections[0].id, parser.collection_entry_class],
[collections[1].id, parser.collection_entry_class],
[works[0].fetch("file_set_ids_ssim")[0], parser.file_set_entry_class],
[works[0].fetch("file_set_ids_ssim")[1], parser.file_set_entry_class],
[works[0].fetch("file_set_ids_ssim")[2], parser.file_set_entry_class],
[works[1].fetch("file_set_ids_ssim")[0], parser.file_set_entry_class],
[works[1].fetch("file_set_ids_ssim")[1], parser.file_set_entry_class],
[works[1].fetch("file_set_ids_ssim")[2], parser.file_set_entry_class]
[works[0].fetch(Bulkrax.solr_key_for_member_file_ids)[0], parser.file_set_entry_class],
[works[0].fetch(Bulkrax.solr_key_for_member_file_ids)[1], parser.file_set_entry_class],
[works[0].fetch(Bulkrax.solr_key_for_member_file_ids)[2], parser.file_set_entry_class],
[works[1].fetch(Bulkrax.solr_key_for_member_file_ids)[0], parser.file_set_entry_class],
[works[1].fetch(Bulkrax.solr_key_for_member_file_ids)[1], parser.file_set_entry_class],
[works[1].fetch(Bulkrax.solr_key_for_member_file_ids)[2], parser.file_set_entry_class]
)
end
end
Expand Down

0 comments on commit 062a566

Please sign in to comment.