Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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

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.

Suggested change
if resource.converted_pages
if resource.converted_pages?

typo?

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?

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