Merge pull request #315 from scientist-softserv/extracting-transactions

🎁 Add Transaction for Cleaning Up Split Pages

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

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)

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

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 = {

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

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)

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

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

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

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

spec/transactions/hyrax/transactions/container_decorator_spec.rb

@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe do
+end

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

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