Skip to content

Commit

Permalink
🐛 Work through fallback of rodeo URLs
Browse files Browse the repository at this point in the history
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:

- #294
  • Loading branch information
jeremyf committed Nov 28, 2023
1 parent 29baf08 commit c1654b2
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 9 deletions.
18 changes: 12 additions & 6 deletions app/services/iiif_print/derivative_rodeo_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)

Expand All @@ -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
Expand Down
13 changes: 10 additions & 3 deletions lib/iiif_print/split_pdfs/derivative_rodeo_splitter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit c1654b2

Please sign in to comment.