Skip to content
63 changes: 51 additions & 12 deletions app/controllers/public/resources_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,30 +25,53 @@ def iiif
page_number = params[:page] || 1

redirect_resource do |resource|
resource.image? ? resource.content_converted_iiif_url(page_number) : resource.content_iiif_url(page_number)
if resource.converted_pages?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do the other endpoints in this file need the resource.converted_pages? check and fork to use the new URLs you defined in resource.rb?

e.g. image_api can use content_converted_pages_image_api_url, info can use content_converted_pages_info_url, same sort of thing for preview and thumbnail

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point; I've added all the ones that seem relevant but I admit to not having a thorough sense of all the contexts in which these endpoints are used so I haven't tested fully. Thumbnails and previews seem to work as expected at least.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • info: redirects to info.json, the IIIF Image Information doc (dimensions, tile sizes, formats). Viewers fetch this once before requesting any tiles
  • image_api: the full IIIF Image API endpoint (with params like size, rotation, etc). This is the most-fetched endpoint by IIIF viewers with deep zoom capabilities
  • iiif: convenience redirect to an image or page as a JPEG (at full max res /full/max/0/default.jpg)
  • preview and thumbnail are pretty self explanatory, they are fixed size JPEG derivatives

resource.content_converted_pages_iiif_url(page_number.to_i)
elsif resource.image?
resource.content_converted_iiif_url(page_number)
else
resource.content_iiif_url(page_number)
end
end
end

def image_api
page_number = params[:page] || 1

redirect_resource do |resource|
resource.content_image_api_url(
page_number,
params[:region],
params[:size],
params[:rotation],
params[:quality],
params[:format]
)
if resource.converted_pages?
resource.content_converted_pages_image_api_url(
page_number,
params[:region],
params[:size],
params[:rotation],
params[:quality],
params[:format]
)
else
resource.content_image_api_url(
page_number,
params[:region],
params[:size],
params[:rotation],
params[:quality],
params[:format]
)
end
end
end

def info
page_number = params[:page] || 1

redirect_resource do |resource|
resource.image? ? resource.content_converted_info_url(page_number) : resource.content_info_url(page_number)
if resource.converted_pages?
resource.content_converted_pages_info_url(page_number)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we do page_number.to_i everywhere the new helpers are called, since it does a numeric comparison after passing the arg to content_converted_pages_base_url?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch; seems safer to me to just do the conversion inside content_converted_pages_base_url rather than having to double check the type of page_number everywhere any of these methods are called, so I added that in, but if you think it would be better to convert each argument before passing it we can do that instead (or in addition).

elsif resource.image?
resource.content_converted_info_url(page_number)
else
resource.content_info_url(page_number)
end
end
end

Expand All @@ -64,14 +87,30 @@ def manifest
end

def preview
page_number = params[:page] || 1

redirect_resource do |resource|
resource.image? ? resource.content_converted_preview_url : resource.content_preview_url
if resource.converted_pages?
resource.content_converted_pages_preview_url(page_number)
elsif resource.image?
resource.content_converted_preview_url
else
resource.content_preview_url
end
end
end

def thumbnail
page_number = params[:page] || 1

redirect_resource do |resource|
resource.image? ? resource.content_converted_thumbnail_url : resource.content_thumbnail_url
if resource.converted_pages?
resource.content_converted_pages_thumbnail_url(page_number)
elsif resource.image?
resource.content_converted_thumbnail_url
else
resource.content_thumbnail_url
end
end
end

Expand Down
100 changes: 92 additions & 8 deletions app/jobs/convert_image_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,35 +5,119 @@ class ConvertImageJob < ApplicationJob

# In the event the job is started before the file is fully uploaded, we'll retry
retry_on Exceptions::FileNotUploadedError, wait: 10.seconds
retry_on Exceptions::PDFExtractionError, wait: 10.seconds
retry_on Exceptions::PDFPageConversionError, wait: 10.seconds

def perform(resource_id)
# Only convert if the passed resource is an image
resource = Resource.find(resource_id)
return unless resource.image?

# Only convert if there is content attached

# Return early if no content is attached
content = resource.content
return unless content.attached?

raise Exceptions::FileNotUploadedError unless resource.content_uploaded?

# Route to appropriate converter based on content type
if resource.image?
convert_image(resource)
elsif resource.pdf?
convert_pdf(resource)
end
end

private

# Convert a single image file to TIFF
# Maintains backwards compatibility with existing image conversion workflow
# @param resource [Resource] The resource containing the image to convert
def convert_image(resource)
content = resource.content

content.open do |file|
begin
# Convert the image
# Convert the image to TIFF using existing service
filepath = Images::Convert.to_tiff(file)
filename = Images::Convert.filename content.filename.to_s, FILE_EXTENSION_TIFF

# Upload the converted content
# Upload the converted content to content_converted (single file attachment)
resource.content_converted.attach(
io: File.open(filepath),
content_type: CONTENT_TYPE_TIFF,
filename:,
)
rescue MiniMagick::Error => e
# Content cannot be converted
Rails.logger.error e.message
# Content cannot be converted - log and continue (non-fatal)
Rails.logger.error "Error converting image for resource #{resource.id}: #{e.message}"
Rails.logger.error e.backtrace.join("\n")
ensure
# Clean up temporary file
File.delete(filepath) if filepath && File.exist?(filepath)
end
end
end

# Convert a multi-page PDF to an ordered set of TIFF files
# @param resource [Resource] The resource containing the PDF to convert
def convert_pdf(resource)
content = resource.content
temp_files = []

begin
content.open do |file|
# Extract page count from PDF
page_count = Images::ConvertPdf.page_count(file)

# Process each page
page_count.times do |page_number|
begin
# Extract page as intermediate image
temp_image_path = Images::ConvertPdf.extract_page(file, page_number)
temp_files << temp_image_path

# Convert extracted page to TIFF
tiff_path = Images::ConvertPdf.page_to_tiff(File.new(temp_image_path))
temp_files << tiff_path

# Generate filename for this page (e.g., "document_page_001.tif")
base_filename = File.basename(content.filename.to_s, '.*')
page_filename = "#{base_filename}_page_#{page_number + 1}.#{FILE_EXTENSION_TIFF}"

# Attach TIFF to content_converted_pages in order
resource.content_converted_pages.attach(
io: File.open(tiff_path),
content_type: CONTENT_TYPE_TIFF,
filename: page_filename,
)

# Clean up intermediate files as we go
Images::ConvertPdf.cleanup_temp_files([temp_image_path, tiff_path])
temp_files -= [temp_image_path, tiff_path]

rescue Exceptions::PDFExtractionError => e
Rails.logger.error "Failed to extract page #{page_number} from PDF for resource #{resource.id}: #{e.message}"
# Continue with next page rather than failing entire job
rescue Exceptions::PDFPageConversionError => e
Rails.logger.error "Failed to convert page #{page_number} to TIFF for resource #{resource.id}: #{e.message}"
# Continue with next page rather than failing entire job
end
end

# Store page count on resource for tracking
resource.update(pages_count: page_count)

Comment thread
blms marked this conversation as resolved.
# Regenerate manifest with converted pages
CreateManifestJob.perform_later(resource.id)

Rails.logger.info "Successfully converted PDF resource #{resource.id} with #{page_count} pages"

end
rescue Exceptions::EmptyPDFError => e
Rails.logger.error "PDF resource #{resource.id} is empty: #{e.message}"
rescue Exceptions::PDFExtractionError => e
Rails.logger.error "Failed to extract pages from PDF resource #{resource.id}: #{e.message}"
ensure
# Ensure all temporary files are cleaned up
Images::ConvertPdf.cleanup_temp_files(temp_files)
end
end
end
55 changes: 39 additions & 16 deletions app/models/concerns/attachable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@ def delete_attachments
attachment = self.send(name)
return unless attachment.attached?

attachment.purge
# Handle both has_one_attached and has_many_attached
if attachment.is_a?(ActiveStorage::Attached::Many)
attachment.purge_all
else
attachment.purge
end
end
end

Expand All @@ -31,14 +36,21 @@ def set_attachment_keys

self.class.list_attachments&.each do |name|
attachment = self.send(name)

# Only set the storage key for a new record
next unless attachment.new_record?

# If the "storage_key" attribute is set on the metadata, this blob was created as a direct upload
next if attachment.metadata && attachment.metadata[:storage_key].present?

attachment.key = "#{self.storage_key}/#{ActiveStorage::Blob.generate_unique_secure_token}"

# Handle both has_one_attached and has_many_attached
if attachment.is_a?(ActiveStorage::Attached::Many)
# For has_many_attached, iterate through each attachment
attachment.each do |attach|
next unless attach.new_record?
next if attach.metadata && attach.metadata[:storage_key].present?
attach.key = "#{self.storage_key}/#{ActiveStorage::Blob.generate_unique_secure_token}"
end
else
# For has_one_attached
next unless attachment.new_record?
next if attachment.metadata && attachment.metadata[:storage_key].present?
attachment.key = "#{self.storage_key}/#{ActiveStorage::Blob.generate_unique_secure_token}"
end
end
end
end
Expand All @@ -47,18 +59,20 @@ def set_attachment_keys

# This method overrides model#has_one_attached in order to facilitate generating a <name>_url method for easily
# accessing the attachment URL in serializers.
def has_one_attached(name, dependent: :purge_later)
super
generate_url_method name
# Pass generate_urls: false when the model defines its own <name>_*_url methods (e.g. page-indexed URLs).
def has_one_attached(name, dependent: :purge_later, generate_urls: true)
super(name, dependent: dependent)
generate_url_method name if generate_urls
generate_remove_method name
@attachments << name
end

# This method overrides model#has_many_attached in order to facilitate generating a <name>_url method for easily
# accessing the attachment URL in serializers.
def has_many_attached(name, dependent: :purge_later)
super
generate_url_method name
# Pass generate_urls: false when the model defines its own <name>_*_url methods (e.g. page-indexed URLs).
def has_many_attached(name, dependent: :purge_later, generate_urls: true)
super(name, dependent: dependent)
generate_url_method name if generate_urls
generate_remove_method name
@attachments << name
end
Expand Down Expand Up @@ -163,7 +177,16 @@ def generate_url_method(name)
end

def attachment_preloads
@attachments.map{ |a| { "#{a}_attachment".to_sym => :blob } }
@attachments.map do |a|
# For has_many_attached, the association is plural: {name}_attachments
# For has_one_attached, the association is singular: {name}_attachment
# Check if the reflection exists to determine which pattern to use
if reflect_on_association("#{a}_attachments")
{ "#{a}_attachments".to_sym => :blob }
else
{ "#{a}_attachment".to_sym => :blob }
end
end
end

def list_attachments
Expand Down
Loading