Skip to content

Commit

Permalink
♻️ Extract direct ActiveFedora calls to adapter
Browse files Browse the repository at this point in the history
With Valkyrie looming large, this change refactors the code towards an
adapter pattern; one that will allow for creating a Valkyrie adapter and
then configuring IIIFPrint accordingly.

We'll also need to consider how we duplicate the actor work within a
transaction and/or publisher/listener setup.

There are likely other places where we're making assumptions about
presenters and/or the data models we're processing.
  • Loading branch information
jeremyf committed Jan 17, 2024
1 parent 2dfd14b commit 0565150
Show file tree
Hide file tree
Showing 12 changed files with 165 additions and 38 deletions.
2 changes: 1 addition & 1 deletion app/models/concerns/iiif_print/solr/document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def digest_sha1

def method_missing(method_name, *args, &block)
super unless iiif_print_solr_field_names.include? method_name.to_s
self[::ActiveFedora.index_field_mapper.solr_name(method_name.to_s)]
self[IiifPrint.solr_name(method_name.to_s)]
end

def respond_to_missing?(method_name, include_private = false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def get_solr_hits(ids)
results = []
ids.each_slice(SOLR_QUERY_PAGE_SIZE) do |paged_ids|
query = "id:(#{paged_ids.join(' OR ')})"
results += ActiveFedora::SolrService.query(
results += IiifPrint.solr_query(
query,
{ fq: "-has_model_ssim:FileSet", rows: paged_ids.size, method: :post }
)
Expand Down
33 changes: 2 additions & 31 deletions lib/iiif_print.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,37 +44,8 @@ def self.config(&block)
end

class << self
delegate :skip_splitting_pdf_files_that_end_with_these_texts, to: :config
end

##
# Return the immediate parent of the given :file_set.
#
# @param file_set [FileSet]
# @return [#work?, Hydra::PCDM::Work]
# @return [NilClass] when no parent is found.
def self.parent_for(file_set)
# fallback to Fedora-stored relationships if work's aggregation of
# file set is not indexed in Solr
file_set.parent || file_set.member_of.find(&:work?)
end

##
# Return the parent's parent of the given :file_set.
#
# @param file_set [FileSet]
# @return [#work?, Hydra::PCDM::Work]
# @return [NilClass] when no grand parent is found.
def self.grandparent_for(file_set)
parent_of_file_set = parent_for(file_set)
# HACK: This is an assumption about the file_set structure, namely that an image page split from
# a PDF is part of a file set that is a child of a work that is a child of a single work. That
# is, it only has one grand parent. Which is a reasonable assumption for IIIF Print but is not
# valid when extended beyond IIIF Print. That is GenericWork does not have a parent method but
# does have a parents method.
parent_of_file_set.try(:parent_works).try(:first) ||
parent_of_file_set.try(:parents).try(:first) ||
parent_of_file_set&.member_of&.find(&:work?)
delegate :skip_splitting_pdf_files_that_end_with_these_texts, :persistence_layer, to: :config
delegate :parent_for, :grandparent_for, :solr_construct_query, :solr_query, :solr_name, :clean_for_tests!, to: :persistence_layer
end

DEFAULT_MODEL_CONFIGURATION = {
Expand Down
4 changes: 2 additions & 2 deletions lib/iiif_print/catalog_search_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ class CatalogSearchBuilder < Hyrax::CatalogSearchBuilder
# rubocop:enable Naming/PredicateName
def show_parents_only(solr_parameters)
query = if blacklight_params["include_child_works"] == 'true'
ActiveFedora::SolrQueryBuilder.construct_query(is_child_bsi: 'true')
IiifPrint.solr_construct_query(is_child_bsi: 'true')
else
ActiveFedora::SolrQueryBuilder.construct_query(is_child_bsi: nil)
IiifPrint.solr_construct_query(is_child_bsi: nil)
end
solr_parameters[:fq] += [query]
end
Expand Down
16 changes: 16 additions & 0 deletions lib/iiif_print/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,22 @@ module IiifPrint
class Configuration
attr_writer :after_create_fileset_handler

attr_writer :persistence_adapter
def persistence_adapter
@persistent_adapter || default_persistence_adapter
end

def default_persistence_adapter
# There's probably some configuration of Hyrax we could use to better refine this; but it's
# likely a reasonable guess. The main goal is to not break existing implementations and
# maintain an upgrade path.
if Gem::Version.new(Hyrax::VERSION) >= Gem::Version.new('6.0.0')
IiifPrint::PersistenceLayer::ValkyrieAdapter
else
IiifPrint::PersistenceLayer::ActiveFedoraAdapter
end
end

# @param file_set [FileSet]
# @param user [User]
def handle_after_create_fileset(file_set, user)
Expand Down
6 changes: 6 additions & 0 deletions lib/iiif_print/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ module IiifPrint
class Engine < ::Rails::Engine
isolate_namespace IiifPrint

initializer 'requires' do
require 'iiif_print/persistence_layer'
require 'iiif_print/persistence_layer/active_fedora_adapter' if defined?(ActiveFedora)
require 'iiif_print/persistence_layer/valkyrie_adapter' if defined?(Valkyrie)
end

# rubocop:disable Metrics/BlockLength
config.to_prepare do
require "iiif_print/jobs/create_relationships_job"
Expand Down
4 changes: 2 additions & 2 deletions lib/iiif_print/homepage_search_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ class HomepageSearchBuilder < Hyrax::HomepageSearchBuilder

def show_parents_only(solr_parameters)
query = if blacklight_params["include_child_works"] == 'true'
ActiveFedora::SolrQueryBuilder.construct_query(is_child_bsi: 'true')
IiifPrint.solr_construct_query(is_child_bsi: 'true')
else
ActiveFedora::SolrQueryBuilder.construct_query(is_child_bsi: nil)
IiifPrint.solr_construct_query(is_child_bsi: nil)
end
solr_parameters[:fq] += [query]
end
Expand Down
20 changes: 20 additions & 0 deletions lib/iiif_print/persistence_layer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
module IiifPrint
module PersistenceLayer
class AbstractAdapter
def self.parent_for(*); end

def self.grandparent_for(*); end

def self.solr_field_query(*); end

def self.clean_for_tests!
return false unless Rails.env.test?
yield
end

def self.solr_query(*args); end

def self.solr_name(*args); end
end
end
end
65 changes: 65 additions & 0 deletions lib/iiif_print/persistence_layer/active_fedora_adapter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
module IiifPrint
module PersistenceLayer
class ActiveFedoraAdapter < AbstractAdapter
##
# Return the immediate parent of the given :file_set.
#
# @param file_set [FileSet]
# @return [#work?, Hydra::PCDM::Work]
# @return [NilClass] when no parent is found.
def self.parent_for(file_set)
# fallback to Fedora-stored relationships if work's aggregation of
# file set is not indexed in Solr
file_set.parent || file_set.member_of.find(&:work?)
end

##
# Return the parent's parent of the given :file_set.
#
# @param file_set [FileSet]
# @return [#work?, Hydra::PCDM::Work]
# @return [NilClass] when no grand parent is found.
def self.grandparent_for(file_set)
parent_of_file_set = parent_for(file_set)
# HACK: This is an assumption about the file_set structure, namely that an image page split from
# a PDF is part of a file set that is a child of a work that is a child of a single work. That
# is, it only has one grand parent. Which is a reasonable assumption for IIIF Print but is not
# valid when extended beyond IIIF Print. That is GenericWork does not have a parent method but
# does have a parents method.
parent_of_file_set.try(:parent_works).try(:first) ||
parent_of_file_set.try(:parents).try(:first) ||
parent_of_file_set&.member_of&.find(&:work?)
end

def self.solr_construct_query(*args)
if defined?(Hyrax::SolrQueryBuilderService)
Hyrax::SolrQueryBuilderService.construct_query(*args)
else
ActiveFedora::SolrQueryBuilderService.construct_query(*args)
end
end

def self.clean_for_tests!
super do
ActiveFedora::Cleaner.clean!
end
end

def self.solr_query(*args)
if defined?(Hyrax::SolrService)
Hyrax::SolrService.query(*args)
else
ActiveFedora::SolrService.query(*args)
end
end

def self.solr_name(field_name)
if defined?(Hyrax) && Hyrax.config.respond_to?(:index_field_mapper)
Hyrax.config.index_field_mapper.solr_name(field_name.to_s)
else
::ActiveFedora.index_field_mapper.solr_name(field_name.to_s)
end
end
end
end
end
43 changes: 43 additions & 0 deletions lib/iiif_print/persistence_layer/valkyrie_adapter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
module IiifPrint
module PersistenceLayer
class ValkyrieAdapter < AbstractAdapter
##
# Return the immediate parent of the given :file_set.
#
# @param file_set [FileSet]
# @return [#work?, Hydra::PCDM::Work]
# @return [NilClass] when no parent is found.
def self.parent_for(file_set)
raise NotImplementedError
end

##
# Return the parent's parent of the given :file_set.
#
# @param file_set [FileSet]
# @return [#work?, Hydra::PCDM::Work]
# @return [NilClass] when no grand parent is found.
def self.grandparent_for(file_set)
raise NotImplementedError
end

def self.solr_construct_query(*args)
Hyrax::SolrQueryBuilderService.construct_query(*args)
end

def self.clean_for_tests!
# For Fedora backed repositories, we'll want to consider some cleaning mechanism. For
# database backed repositories, we can rely on the database_cleaner gem.
raise NotImplementedError
end

def self.solr_query(*args)
Hyrax::SolrService.query(*args)
end

def self.solr_name(field_name)
Hyrax.config.index_field_mapper.solr_name(field_name.to_s)
end
end
end
end
6 changes: 6 additions & 0 deletions spec/iiif_print/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
RSpec.describe IiifPrint::Configuration do
let(:config) { described_class.new }

describe '#persistence_adapter' do
subject { config.persistence_adapter }

it { is_expected.to be_a PersistenceLayer::AbstractAdapter }
end

describe '#ancestory_identifier_function' do
subject(:function) { config.ancestory_identifier_function }
it "is expected to be a lambda with an arity of one" do
Expand Down
2 changes: 1 addition & 1 deletion spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
::Noid::Rails.config.minter_class = minter_class
Hyrax.config.noid_minter_class = minter_class

ActiveFedora::Cleaner.clean!
IiifPrint.clean_for_tests!
DatabaseCleaner.clean_with(:truncation)

begin
Expand Down

0 comments on commit 0565150

Please sign in to comment.