From 0b9515ea390f3d765a6cc8605a7a4d588f3aedd8 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Wed, 17 Jan 2024 15:14:30 -0500 Subject: [PATCH 1/2] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Extract=20direct=20Act?= =?UTF-8?q?iveFedora=20calls=20to=20adapter?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- README.md | 4 ++ .../concerns/iiif_print/solr/document.rb | 2 +- .../manifest_builder_service_behavior.rb | 2 +- lib/iiif_print.rb | 33 +--------- lib/iiif_print/catalog_search_builder.rb | 4 +- lib/iiif_print/configuration.rb | 16 +++++ lib/iiif_print/engine.rb | 6 ++ lib/iiif_print/homepage_search_builder.rb | 4 +- lib/iiif_print/persistence_layer.rb | 40 ++++++++++++ .../active_fedora_adapter.rb | 65 +++++++++++++++++++ .../persistence_layer/valkyrie_adapter.rb | 45 +++++++++++++ spec/iiif_print/configuration_spec.rb | 6 ++ spec/spec_helper.rb | 2 +- 13 files changed, 191 insertions(+), 38 deletions(-) create mode 100644 lib/iiif_print/persistence_layer.rb create mode 100644 lib/iiif_print/persistence_layer/active_fedora_adapter.rb create mode 100644 lib/iiif_print/persistence_layer/valkyrie_adapter.rb diff --git a/README.md b/README.md index ba249873..16405cb6 100644 --- a/README.md +++ b/README.md @@ -115,6 +115,10 @@ uv = createUV('#uv', { ## Configuration to enable IiifPrint features **NOTE: WorkTypes and models are used synonymously here.** +### Persistence Layer Adapter + +We created IiifPrint with an assumption of ActiveFedora. However, as Hyrax now supports Valkyrie, we need an alternate approach. We introduced `IiifPrint::Configuration#persistence_layer` as a configuration option. By default it will use `ActiveFedora` methods; but you can switch adapters to use Valkyrie instead. (See `IiifPrint::PersistentLayer` for more details). + ### IIIF URL configuration If you set EXTERNAL_IIIF_URL in your environment, then IiifPrint will use that URL as the root for your IIIF URLs. It will also switch from using the file set ID to using the SHA1 of the file as the identifier. This enables using serverless_iiif or Cantaloupe (refered to as the service) by pointing the service to the same S3 bucket that FCREPO writes the uploaded files to. By setting it up that way you do not need the service to connect to FCREPO or Hyrax at all, both natively support connecting to an S3 bucket to get their data. diff --git a/app/models/concerns/iiif_print/solr/document.rb b/app/models/concerns/iiif_print/solr/document.rb index 84e7e996..1e0806a6 100644 --- a/app/models/concerns/iiif_print/solr/document.rb +++ b/app/models/concerns/iiif_print/solr/document.rb @@ -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) diff --git a/app/services/iiif_print/manifest_builder_service_behavior.rb b/app/services/iiif_print/manifest_builder_service_behavior.rb index d198946a..9c49ff3d 100644 --- a/app/services/iiif_print/manifest_builder_service_behavior.rb +++ b/app/services/iiif_print/manifest_builder_service_behavior.rb @@ -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 } ) diff --git a/lib/iiif_print.rb b/lib/iiif_print.rb index d89e426d..c5499f86 100644 --- a/lib/iiif_print.rb +++ b/lib/iiif_print.rb @@ -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_adapter, to: :config + delegate :parent_for, :grandparent_for, :solr_construct_query, :solr_query, :solr_name, :clean_for_tests!, to: :persistence_adapter end DEFAULT_MODEL_CONFIGURATION = { diff --git a/lib/iiif_print/catalog_search_builder.rb b/lib/iiif_print/catalog_search_builder.rb index e230881a..1afd661b 100644 --- a/lib/iiif_print/catalog_search_builder.rb +++ b/lib/iiif_print/catalog_search_builder.rb @@ -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 diff --git a/lib/iiif_print/configuration.rb b/lib/iiif_print/configuration.rb index 03bf5b55..90ed3125 100644 --- a/lib/iiif_print/configuration.rb +++ b/lib/iiif_print/configuration.rb @@ -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) diff --git a/lib/iiif_print/engine.rb b/lib/iiif_print/engine.rb index b314e35f..82131d65 100644 --- a/lib/iiif_print/engine.rb +++ b/lib/iiif_print/engine.rb @@ -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" diff --git a/lib/iiif_print/homepage_search_builder.rb b/lib/iiif_print/homepage_search_builder.rb index b55506ef..255adcc1 100644 --- a/lib/iiif_print/homepage_search_builder.rb +++ b/lib/iiif_print/homepage_search_builder.rb @@ -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 diff --git a/lib/iiif_print/persistence_layer.rb b/lib/iiif_print/persistence_layer.rb new file mode 100644 index 00000000..15b8a00f --- /dev/null +++ b/lib/iiif_print/persistence_layer.rb @@ -0,0 +1,40 @@ +module IiifPrint + ## + # The PersistenceLayer module provides the namespace for other adapters: + # + # - {IiifPrint::PersistenceLayer::ActiveFedoraAdapter} + # - {IiifPrint::PersistenceLayer::ValkyrieAdapter} + # + # And the defining interface in the {IiifPrint::PersistenceLayer::AbstractAdapter} + module PersistenceLayer + # @abstract + class AbstractAdapter + ## + # @abstract + def self.parent_for(*); end + + ## + # @abstract + def self.grandparent_for(*); end + + ## + # @abstract + def self.solr_field_query(*); end + + ## + # @abstract + def self.clean_for_tests! + return false unless Rails.env.test? + yield + end + + ## + # @abstract + def self.solr_query(*args); end + + ## + # @abstract + def self.solr_name(*args); end + end + end +end diff --git a/lib/iiif_print/persistence_layer/active_fedora_adapter.rb b/lib/iiif_print/persistence_layer/active_fedora_adapter.rb new file mode 100644 index 00000000..e6299fb8 --- /dev/null +++ b/lib/iiif_print/persistence_layer/active_fedora_adapter.rb @@ -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 diff --git a/lib/iiif_print/persistence_layer/valkyrie_adapter.rb b/lib/iiif_print/persistence_layer/valkyrie_adapter.rb new file mode 100644 index 00000000..8617c404 --- /dev/null +++ b/lib/iiif_print/persistence_layer/valkyrie_adapter.rb @@ -0,0 +1,45 @@ +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) + Hyrax.index_adapter.find_parents(resource: file_set).first + 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 = Hyrax.index_adapter.find_parents(resource: file_set).first + return nil unless parent + Hyrax.index_adapter.find_parents(resource: parent).first + 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 diff --git a/spec/iiif_print/configuration_spec.rb b/spec/iiif_print/configuration_spec.rb index 0ae66f03..4d8f3b54 100644 --- a/spec/iiif_print/configuration_spec.rb +++ b/spec/iiif_print/configuration_spec.rb @@ -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 eq(IiifPrint::PersistenceLayer::ActiveFedoraAdapter) } + 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 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 94023e1b..ad534478 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -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 From 447f18e08085721fdcfa44648231d09a34cd0265 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Thu, 18 Jan 2024 10:50:40 -0500 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=8E=81=20Add=20Transaction=20for=20Cl?= =?UTF-8?q?eaning=20Up=20Split=20Pages?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Valkyrie leverages transactions instead of the actor stack; as such we need to mirror the actor stack behavior as a transaction (or listener). In this case, we should use a transaction. Related to: - https://github.com/scientist-softserv/iiif_print/issues/312 --- .../actors/file_set_actor_decorator.rb | 2 +- .../iiif_print_container_decorator.rb | 32 +++++++++++++++++ ...nditionally_destroy_children_from_split.rb | 31 +++++++++++++++++ lib/iiif_print.rb | 18 ++++++++-- lib/iiif_print/data/work_derivatives.rb | 2 +- lib/iiif_print/engine.rb | 3 ++ lib/iiif_print/persistence_layer.rb | 28 ++++++++++++--- .../active_fedora_adapter.rb | 18 ++++++++++ .../persistence_layer/valkyrie_adapter.rb | 6 ++++ .../destroy_pdf_child_works_service.rb | 17 ++-------- .../transactions/container_decorator_spec.rb | 6 ++++ .../iiif_print_container_decorator_spec.rb | 21 ++++++++++++ ...onally_destroy_children_from_split_spec.rb | 34 +++++++++++++++++++ 13 files changed, 195 insertions(+), 23 deletions(-) create mode 100644 app/transactions/hyrax/transactions/iiif_print_container_decorator.rb create mode 100644 app/transactions/hyrax/transactions/steps/conditionally_destroy_children_from_split.rb create mode 100644 spec/transactions/hyrax/transactions/container_decorator_spec.rb create mode 100644 spec/transactions/hyrax/transactions/iiif_print_container_decorator_spec.rb create mode 100644 spec/transactions/hyrax/transactions/steps/conditionally_destroy_children_from_split_spec.rb diff --git a/app/actors/iiif_print/actors/file_set_actor_decorator.rb b/app/actors/iiif_print/actors/file_set_actor_decorator.rb index 98d48965..1b7b3122 100644 --- a/app/actors/iiif_print/actors/file_set_actor_decorator.rb +++ b/app/actors/iiif_print/actors/file_set_actor_decorator.rb @@ -48,7 +48,7 @@ def destroy # we destroy the children before the file_set, because we need the parent relationship IiifPrint::SplitPdfs::DestroyPdfChildWorksService.conditionally_destroy_spawned_children_of( file_set: file_set, - work: file_set.parent + work: IiifPrint.parent_for(file_set) ) # and now back to your regularly scheduled programming super diff --git a/app/transactions/hyrax/transactions/iiif_print_container_decorator.rb b/app/transactions/hyrax/transactions/iiif_print_container_decorator.rb new file mode 100644 index 00000000..1b5dd0ae --- /dev/null +++ b/app/transactions/hyrax/transactions/iiif_print_container_decorator.rb @@ -0,0 +1,32 @@ +module Hyrax + module Transactions + ## + # This decorator does the following: + # + # - Prepend the {ConditionallyDestroyChildrenFromSplit} transaction to the "file_set.destroy" + # step. The prependment corresponds to the behavior for + # {IiifPrint::Actors::FileSetActorDecorator#destroy} + # + # For more information about adjusting transactions, see + # [Transitioning workshop solution for adding transaction](https://github.com/samvera-labs/transitioning-to-valkyrie-workshop/commit/bcab2bb8f65078e88395c68f72be00e7ffad57ec) + # + # @see https://github.com/samvera/hyrax/blob/f875d61dc87229cf1f05eb2bb6d414b5ef314616/lib/hyrax/transactions/container.rb + class IiifPrintContainerDecorator + extend Dry::Container::Mixin + + namespace 'file_set' do |ops| + ops.register 'iiif_print_conditionally_destroy_spawned_children' do + Steps::ConditionallyDestroyChildrenFromSplit.new + end + ops.register 'destroy' do + Hyrax::Transactions::FileSetDestroy.new( + steps: (['file_set.iiif_print_conditionally_destroy_spawned_children'] + + Hyrax::Transactions::FileSetDestroy::DEFAULT_STEPS) + ) + end + end + end + end +end + +Hyrax::Transactions::Container.merge(Hyrax::Transactions::IiifPrintContainerDecorator) diff --git a/app/transactions/hyrax/transactions/steps/conditionally_destroy_children_from_split.rb b/app/transactions/hyrax/transactions/steps/conditionally_destroy_children_from_split.rb new file mode 100644 index 00000000..20ddf9a5 --- /dev/null +++ b/app/transactions/hyrax/transactions/steps/conditionally_destroy_children_from_split.rb @@ -0,0 +1,31 @@ +module Hyrax + module Transactions + module Steps + ## + # For a FileSet that is a PDF, we need to delete any works and file_sets that are the result of + # splitting that PDF into constituent images of each page of the PDF. This is responsible for + # that work. + class ConditionallyDestroyChildrenFromSplit + include Dry::Monads[:result] + + ## + # @param resource [Hyrax::FileSet] + def call(resource) + return Failure(:resource_not_persisted) unless resource.persisted? + + parent = IiifPrint.persistence_adapter.parent_for(resource) + return Success(true) unless parent + + # We do not care about the results of this call; as it is conditionally looking for things + # to destroy. + IiifPrint::SplitPdfs::DestroyPdfChildWorksService.conditionally_destroy_spawned_children_of( + file_set: resource, + work: parent + ) + + Success(true) + end + end + end + end +end diff --git a/lib/iiif_print.rb b/lib/iiif_print.rb index c5499f86..7b3dbe7e 100644 --- a/lib/iiif_print.rb +++ b/lib/iiif_print.rb @@ -44,8 +44,22 @@ def self.config(&block) end class << self - delegate :skip_splitting_pdf_files_that_end_with_these_texts, :persistence_adapter, to: :config - delegate :parent_for, :grandparent_for, :solr_construct_query, :solr_query, :solr_name, :clean_for_tests!, to: :persistence_adapter + delegate( + :persistence_adapter, + :skip_splitting_pdf_files_that_end_with_these_texts, + to: :config + ) + + delegate( + :clean_for_tests!, + :destroy_children_split_from, + :grandparent_for, + :solr_construct_query, + :solr_name, + :solr_query, + :parent_for, + to: :persistence_adapter + ) end DEFAULT_MODEL_CONFIGURATION = { diff --git a/lib/iiif_print/data/work_derivatives.rb b/lib/iiif_print/data/work_derivatives.rb index 3ab70bee..86ad596d 100644 --- a/lib/iiif_print/data/work_derivatives.rb +++ b/lib/iiif_print/data/work_derivatives.rb @@ -239,7 +239,7 @@ def primary_file_path # of the first assigned file path for single-file work. work_file = parent return if work_file.nil? - work_files = work_file.parent + work_files = IiifPrint.parent_for(work_file) return if work_files.nil? work_files.assigned[0] else diff --git a/lib/iiif_print/engine.rb b/lib/iiif_print/engine.rb index 82131d65..04a5596f 100644 --- a/lib/iiif_print/engine.rb +++ b/lib/iiif_print/engine.rb @@ -11,7 +11,10 @@ module IiifPrint class Engine < ::Rails::Engine isolate_namespace IiifPrint + config.eager_load_paths += %W[#{config.root}/app/transactions] + initializer 'requires' do + require 'hyrax/transactions/iiif_print_container_decorator' 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) diff --git a/lib/iiif_print/persistence_layer.rb b/lib/iiif_print/persistence_layer.rb index 15b8a00f..c46b9ee1 100644 --- a/lib/iiif_print/persistence_layer.rb +++ b/lib/iiif_print/persistence_layer.rb @@ -9,17 +9,31 @@ module IiifPrint module PersistenceLayer # @abstract class AbstractAdapter + ## + # @param file_set [Object] + # @param work [Object] + # @param model [Class] The class name for which we'll split children. + def self.destroy_children_split_from(file_set:, work:, model:) + raise NotImplementedError, "#{self}.{__method__}" + end + ## # @abstract - def self.parent_for(*); end + def self.parent_for(*) + raise NotImplementedError, "#{self}.{__method__}" + end ## # @abstract - def self.grandparent_for(*); end + def self.grandparent_for(*) + raise NotImplementedError, "#{self}.{__method__}" + end ## # @abstract - def self.solr_field_query(*); end + def self.solr_field_query(*) + raise NotImplementedError, "#{self}.{__method__}" + end ## # @abstract @@ -30,11 +44,15 @@ def self.clean_for_tests! ## # @abstract - def self.solr_query(*args); end + def self.solr_query(*args) + raise NotImplementedError, "#{self}.{__method__}" + end ## # @abstract - def self.solr_name(*args); end + def self.solr_name(*args) + raise NotImplementedError, "#{self}.{__method__}" + end end end end diff --git a/lib/iiif_print/persistence_layer/active_fedora_adapter.rb b/lib/iiif_print/persistence_layer/active_fedora_adapter.rb index e6299fb8..e7ca2468 100644 --- a/lib/iiif_print/persistence_layer/active_fedora_adapter.rb +++ b/lib/iiif_print/persistence_layer/active_fedora_adapter.rb @@ -60,6 +60,24 @@ def self.solr_name(field_name) ::ActiveFedora.index_field_mapper.solr_name(field_name.to_s) end end + + ## + # @param file_set [Object] + # @param work [Object] + # @param model [Class] The class name for which we'll split children. + def self.destroy_children_split_from(file_set:, work:, model:) + # look first for children by the file set id they were split from + children = model.where(split_from_pdf_id: file_set.id) + if children.blank? + # find works where file name and work `to_param` are both in the title + children = model.where(title: file_set.label).where(title: work.to_param) + end + return if children.blank? + children.each do |rcd| + rcd.destroy(eradicate: true) + end + true + end end end end diff --git a/lib/iiif_print/persistence_layer/valkyrie_adapter.rb b/lib/iiif_print/persistence_layer/valkyrie_adapter.rb index 8617c404..105aa256 100644 --- a/lib/iiif_print/persistence_layer/valkyrie_adapter.rb +++ b/lib/iiif_print/persistence_layer/valkyrie_adapter.rb @@ -40,6 +40,12 @@ def self.solr_query(*args) def self.solr_name(field_name) Hyrax.config.index_field_mapper.solr_name(field_name.to_s) end + + ## + # @todo implement this logic + def self.destroy_children_split_from(file_set:, work:, model:) + super + end end end end diff --git a/lib/iiif_print/split_pdfs/destroy_pdf_child_works_service.rb b/lib/iiif_print/split_pdfs/destroy_pdf_child_works_service.rb index d50daf35..61edde61 100644 --- a/lib/iiif_print/split_pdfs/destroy_pdf_child_works_service.rb +++ b/lib/iiif_print/split_pdfs/destroy_pdf_child_works_service.rb @@ -12,21 +12,10 @@ def self.conditionally_destroy_spawned_children_of(file_set:, work:) return unless child_model return unless file_set.class.pdf_mime_types.include?(file_set.mime_type) + # NOTE: The IiifPrint::PendingRelationship is an ActiveRecord object; hence we don't need to + # leverage an adapter. IiifPrint::PendingRelationship.where(parent_id: work.id, file_id: file_set.id).find_each(&:destroy) - destroy_spawned_children(model: child_model, file_set: file_set, work: work) - end - - private_class_method def self.destroy_spawned_children(model:, file_set:, work:) - # look first for children by the file set id they were split from - children = model.where(split_from_pdf_id: file_set.id) - if children.blank? - # find works where file name and work `to_param` are both in the title - children = model.where(title: file_set.label).where(title: work.to_param) - end - return if children.blank? - children.each do |rcd| - rcd.destroy(eradicate: true) - end + IiifPrint.destroy_children_split_from(file_set: file_set, work: work, model: child_model) end end end diff --git a/spec/transactions/hyrax/transactions/container_decorator_spec.rb b/spec/transactions/hyrax/transactions/container_decorator_spec.rb new file mode 100644 index 00000000..0024a18a --- /dev/null +++ b/spec/transactions/hyrax/transactions/container_decorator_spec.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe do +end diff --git a/spec/transactions/hyrax/transactions/iiif_print_container_decorator_spec.rb b/spec/transactions/hyrax/transactions/iiif_print_container_decorator_spec.rb new file mode 100644 index 00000000..aa6032b2 --- /dev/null +++ b/spec/transactions/hyrax/transactions/iiif_print_container_decorator_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Hyrax::Transactions::Container do + describe 'file_set.destroy' do + subject(:transaction_step) { described_class['file_set.destroy'] } + describe '#steps' do + subject { transaction_step.steps } + it { + is_expected.to match_array(["file_set.iiif_print_conditionally_destroy_spawned_children", + "file_set.remove_from_work", + "file_set.delete"]) + } + end + end + describe 'file_set.iiif_print_conditionally_destroy_spawned_children' do + subject(:transaction_step) { described_class['file_set.iiif_print_conditionally_destroy_spawned_children'] } + it { is_expected.to be_a Hyrax::Transactions::Steps::ConditionallyDestroyChildrenFromSplit } + end +end diff --git a/spec/transactions/hyrax/transactions/steps/conditionally_destroy_children_from_split_spec.rb b/spec/transactions/hyrax/transactions/steps/conditionally_destroy_children_from_split_spec.rb new file mode 100644 index 00000000..8696a3da --- /dev/null +++ b/spec/transactions/hyrax/transactions/steps/conditionally_destroy_children_from_split_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Hyrax::Transactions::Steps::ConditionallyDestroyChildrenFromSplit do + describe '#call' do + let(:file_set) { double(Hyrax::FileSet, persisted?: persisted) } + subject { described_class.new.call(file_set) } + + describe 'with an unsaved resource' do + let(:persisted) { false } + it { is_expected.to be_failure } + end + + describe 'with a saved resource' do + let(:persisted) { true } + before { expect(IiifPrint.persistence_adapter).to receive(:parent_for).and_return(parent) } + + context 'without a parent' do + let(:parent) { nil } + it { is_expected.to be_success } + end + + context 'with a parent' do + let(:parent) { double(Valkyrie::Resource) } + it do + expect(IiifPrint::SplitPdfs::DestroyPdfChildWorksService).to receive(:conditionally_destroy_spawned_children_of) + .with(file_set: file_set, work: parent) + is_expected.to be_success + end + end + end + end +end