From c1654b2952532a7112a38476884ffbd5f2edbce5 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Tue, 28 Nov 2023 16:35:08 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Work=20through=20fallback=20of?= =?UTF-8?q?=20rodeo=20URLs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit replaces raising exceptions when we can't derive the URL. Instead it opts to return nil and let the caller determine what to do. The primary result is that it's easier to have a rodeo configuration "miss" finding files in the pre-process location due to configuration issues. But alas, that's the nature of a rodeo. Related to: - https://github.com/scientist-softserv/iiif_print/issues/294 --- .../iiif_print/derivative_rodeo_service.rb | 18 ++++++++++++------ .../split_pdfs/derivative_rodeo_splitter.rb | 13 ++++++++++--- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/app/services/iiif_print/derivative_rodeo_service.rb b/app/services/iiif_print/derivative_rodeo_service.rb index 093982ff..f38fed08 100644 --- a/app/services/iiif_print/derivative_rodeo_service.rb +++ b/app/services/iiif_print/derivative_rodeo_service.rb @@ -82,7 +82,8 @@ def named_derivatives_and_generators # @param adapter_name [String] Added as a parameter to make testing just a bit easier. See # {.preprocessed_location_adapter_name} # - # @return [String] + # @return [String] when we have a possible candidate. + # @return [NilClass] when we could not derive a candidate. # rubocop:disable Metrics/MethodLength def self.derivative_rodeo_uri(file_set:, filename: nil, extension: nil, adapter_name: preprocessed_location_adapter_name) # TODO: This is a hack that knows about the inner workings of Hydra::Works, but for @@ -91,6 +92,7 @@ def self.derivative_rodeo_uri(file_set:, filename: nil, extension: nil, adapter_ filename ||= Hydra::Works::DetermineOriginalName.call(file_set.original_file) dirname = derivative_rodeo_preprocessed_directory_for(file_set: file_set, filename: filename) + return nil unless dirname # The aforementioned filename and the following basename and extension are here to allow for # us to take an original file and see if we've pre-processed the derivative file. In the @@ -141,6 +143,7 @@ def self.get_ancestor(filename: nil, file_set:) # @param file_set [FileSet] # @param filename [String] # @return [String] the dirname (without any "/" we hope) + # @return [NilClass] when we cannot infer a URI from the object. def self.derivative_rodeo_preprocessed_directory_for(file_set:, filename:) ancestor, ancestor_type = get_ancestor(filename: filename, file_set: file_set) @@ -152,16 +155,19 @@ def self.derivative_rodeo_preprocessed_directory_for(file_set:, filename:) message = "#{self.class}.#{__method__} #{file_set.class} ID=#{file_set.id} and filename: #{filename.inspect}" \ "has #{ancestor_type} of #{ancestor.class} ID=#{ancestor.id}" Rails.logger.info(message) - ancestor.public_send(parent_work_identifier_property_name) || - raise("Expected #{ancestor.class} ID=#{ancestor.id} (#{ancestor_type} of #{file_set.class} ID=#{file_set.id}) " \ - "to have a present #{parent_work_identifier_property_name.inspect}") + parent_work_identifier = ancestor.public_send(parent_work_identifier_property_name) + return parent_work_identifier if parent_work_identifier.present? + Rails.logger.warn("Expected #{ancestor.class} ID=#{ancestor.id} (#{ancestor_type} of #{file_set.class} ID=#{file_set.id}) " \ + "to have a present #{parent_work_identifier_property_name.inspect}") + nil else # HACK: This makes critical assumptions about how we're creating the title for the file_set; # but we don't have much to fall-back on. Consider making this a configurable function. Or # perhaps this entire method should be more configurable. # TODO: Revisit this implementation. - file_set.title.first.split(".").first || - raise("#{file_set.class} ID=#{file_set.id} has title #{file_set.title.first} from which we cannot infer information.") + candidate = file_set.title.first.split(".").first + return candidate if candidate.present? + nil end # rubocop:enable Style/GuardClause end diff --git a/lib/iiif_print/split_pdfs/derivative_rodeo_splitter.rb b/lib/iiif_print/split_pdfs/derivative_rodeo_splitter.rb index 286ed315..a33954bc 100644 --- a/lib/iiif_print/split_pdfs/derivative_rodeo_splitter.rb +++ b/lib/iiif_print/split_pdfs/derivative_rodeo_splitter.rb @@ -84,7 +84,13 @@ def preprocessed_location_template derivative_rodeo_candidate = IiifPrint::DerivativeRodeoService.derivative_rodeo_uri(file_set: file_set, filename: filename) @preprocessed_location_template = - if rodeo_conformant_uri_exists?(derivative_rodeo_candidate) + if derivative_rodeo_candidate.blank? + message = "#{self.class}##{__method__} could not establish derivative_rodeo_candidate for " \ + "#{file_set.class} ID=#{file_set&.id} #to_param=#{file_set&.to_param} with filename #{filename.inspect}. " \ + "Move along little buddy." + Rails.logger.debug(message) + nil + elsif rodeo_conformant_uri_exists?(derivative_rodeo_candidate) Rails.logger.debug("#{self.class}##{__method__} found existing file at location #{derivative_rodeo_candidate}. High five partner!") derivative_rodeo_candidate elsif file_set.import_url @@ -94,7 +100,8 @@ def preprocessed_location_template handle_original_file_not_in_derivative_rodeo else message = "#{self.class}##{__method__} could not find an existing file at #{derivative_rodeo_candidate} " \ - "nor a remote_url for #{file_set.class} ID=#{file_set.id}. Returning `nil' as we have no possible preprocess. " \ + "nor a remote_url for #{file_set.class} ID=#{file_set.id} #to_param=#{file_set&.to_param}. " \ + "Returning `nil' as we have no possible preprocess. " \ "Maybe the input_uri #{input_uri.inspect} will be adequate." Rails.logger.warn(message) nil @@ -146,7 +153,7 @@ def split_files rescue => e message = "#{self.class}##{__method__} encountered `#{e.class}' “#{e}” for " \ "input_uri: #{input_uri.inspect}, " \ - "output_location_template: #{output_location_template.inspect}, and" \ + "output_location_template: #{output_location_template.inspect}, and " \ "preprocessed_location_template: #{preprocessed_location_template.inspect}." exception = RuntimeError.new(message) exception.set_backtrace(e.backtrace)