Skip to content

Commit

Permalink
Access copy regeneration and image rotation features, using Derivativ…
Browse files Browse the repository at this point in the history
…o 3; Fix for speeding through ProcessDigitalObjectImportJob for deleted DigitalObjects
  • Loading branch information
elohanlon committed Dec 13, 2023
1 parent 47403da commit d18177d
Show file tree
Hide file tree
Showing 16 changed files with 375 additions and 301 deletions.
2 changes: 2 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ Metrics/CyclomaticComplexity:
- 'app/models/concerns/digital_object/persistence.rb'
- 'app/controllers/digital_objects_controller.rb'
- 'app/models/digital_object/base.rb'
- 'app/models/concerns/digital_object/publishing.rb'

Metrics/PerceivedComplexity:
Exclude:
Expand All @@ -76,6 +77,7 @@ Metrics/PerceivedComplexity:
- 'app/models/concerns/digital_object/persistence.rb'
- 'app/controllers/digital_objects_controller.rb'
- 'app/models/digital_object/base.rb'
- 'app/models/concerns/digital_object/publishing.rb'

Metrics/MethodLength:
Enabled: false
Expand Down

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,11 @@
</label>
<div class="col-sm-9">
<%= digitalObject.getPerformDerivativeProcessing() %>
<% if(!digitalObject.getPerformDerivativeProcessing()) { %>
<button type="button" class="regenerate-access-copy-button btn btn-link btn-xs">
<span class="glyphicon glyphicon-refresh"></span> Regenerate access copy
</button>
<% } %>
</div>
</div>
</div>
Expand Down
31 changes: 24 additions & 7 deletions app/controllers/digital_objects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class DigitalObjectsController < ApplicationController

before_action :set_digital_object, only: [:show, :edit, :update, :destroy, :undestroy, :data_for_ordered_child_editor,
:download, :download_access_copy, :download_poster, :download_service_copy,
:add_parent, :remove_parents, :mods, :xacml, :media_view, :rotate_image, :swap_order_of_first_two_child_assets,
:add_parent, :remove_parents, :mods, :xacml, :media_view, :rotate_image, :regenerate_access_copy, :swap_order_of_first_two_child_assets,
:download_transcript, :update_transcript,
:download_index_document, :update_index_document,
:download_captions, :update_captions,
Expand Down Expand Up @@ -261,16 +261,33 @@ def rotate_image
end

rotate_by = params[:rotate_by].to_i
@digital_object.fedora_object.orientation -= rotate_by
@digital_object.featured_region = @digital_object.rotated_region(rotate_by) if @digital_object.featured_region.present?

if @digital_object.save && @digital_object.destroy_and_regenerate_derivatives!
@digital_object.fedora_object.orientation += rotate_by
# TODO: Maybe later, preserve and rotate previous region if image is rotated.
# For now, don't worry about it and just CLEAR the previous region. Users almost never select a
# region BEFORE rotating an image.
@digital_object.featured_region = nil
# @digital_object.featured_region = @digital_object.rotated_region(rotate_by) if @digital_object.featured_region.present?

if @digital_object.save && @digital_object.regenerate_derivatives!(access_copy: true, image_server_cache: true)
render json: { success: true }
else
render json: { errors: ['An error occurred during image regeneration.'] }
end
end

def regenerate_access_copy
unless @digital_object.is_a?(DigitalObject::Asset)
render json: { errors: ["Only Assets have access copies. This is a #{@digital_object.digital_object_type.display_label}."] }
return
end

if @digital_object.regenerate_derivatives!(access_copy: true, image_server_cache: true)
render json: { success: true }
else
render json: { errors: ['An error occurred during access copy regeneration.'] }
end
end

def update_featured_region
unless @digital_object.is_a?(DigitalObject::Asset)
render json: { errors: ["Only Assets can have featured regions. This is a #{@digital_object.digital_object_type.display_label} of type #{@digital_object.dc_type}"] }
Expand All @@ -283,7 +300,7 @@ def update_featured_region

@digital_object.featured_region = params[:region]
@digital_object.region_selection_event = { 'updatedBy' => current_user.email }
if @digital_object.save && @digital_object.destroy_and_regenerate_derivatives!
if @digital_object.save && @digital_object.regenerate_derivatives!(access_copy: false, image_server_cache: true)
render json: { success: true }.merge(@digital_object.region_selection_event)
else
errors = @digital_object.errors[:featured_region]
Expand Down Expand Up @@ -413,7 +430,7 @@ def require_appropriate_project_permissions!
associated_project = Project.find_by(project_find_criteria)
publish_requirements << :create
require_project_permission!(associated_project, publish_requirements)
when 'update', 'reorder_child_digital_objects', 'add_parent', 'remove_parents', 'rotate_image', 'swap_order_of_first_two_child_assets', 'update_transcript', 'update_index_document', 'update_captions', 'update_synchronized_transcript', 'clear_synchronized_transcript_and_reimport_transcript',
when 'update', 'reorder_child_digital_objects', 'add_parent', 'remove_parents', 'rotate_image', 'regenerate_access_copy', 'swap_order_of_first_two_child_assets', 'update_transcript', 'update_index_document', 'update_captions', 'update_synchronized_transcript', 'clear_synchronized_transcript_and_reimport_transcript',
require_project_permission!(@digital_object.project, :update)
# Also require publish permission if params[:publish] is set to true (note: applies to the 'update' action)
publish_requirements << :update
Expand Down
3 changes: 3 additions & 0 deletions app/jobs/process_digital_object_import_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ class ProcessDigitalObjectImportJob
FIND_DIGITAL_OBJECT_IMPORT_RETRY_DELAY = 10

def self.perform(digital_object_import_id)
# If the import job was previously deleted (and does not exist in the database), return immediately
return unless DigitalObjectImport.exists?(digital_object_import_id)

# Retrieve DigitalObjectImport instance from table
# If we encounter an error (e.g. Mysql2::Error), wait and try again.
digital_object_import = find_digital_object_import_with_retry(digital_object_import_id)
Expand Down
1 change: 1 addition & 0 deletions app/jobs/request_derivatives_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def payload_for_derivative_request(asset, requested_derivatives)
derivative_request: {
identifier: asset.pid,
delivery_target: 'hyacinth2',
adjust_orientation: asset.fedora_object.orientation,
main_uri: Addressable::URI.encode("file://#{asset.filesystem_location}"),
requested_derivatives: requested_derivatives,
# access_uri can be nil, if no access copy currently exists
Expand Down
62 changes: 31 additions & 31 deletions app/models/concerns/digital_object/assets/featured_region.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,36 +42,36 @@ def asset_image_height
width_val.to_s.to_i
end

def rotated_region(rotate_by)
original_region = self.featured_region
return original_region if original_region.blank?

original_image_width = self.asset_image_width
original_image_height = self.asset_image_height
original_region_left, original_region_top, original_region_width, original_region_height = original_region.split(',').map(&:to_i)
original_region_right = original_region_left + original_region_width
original_region_bottom = original_region_top + original_region_height
rotate_by = rotate_by % 360
left = original_region_left
width = original_region_width
top = original_region_top
height = original_region_height
case rotate_by
when 90
left = original_image_height - original_region_bottom
width = original_region_height
top = original_region_left
height = original_region_width
when 180
left = original_image_width - original_region_right
top = original_image_height - original_region_bottom
when 270
left = original_region_top
width = original_region_height
top = original_image_width - original_region_right
height = original_region_width
end
"#{left},#{top},#{width},#{height}"
end
# NOTE: Commenting out this method for now because it isn't being used, but it might be again in the near future.
# def rotated_region(rotate_by)
# original_region = self.featured_region
# return original_region if original_region.blank?

# original_image_width = self.asset_image_width
# original_image_height = self.asset_image_height
# original_region_left, original_region_top, original_region_width, original_region_height = original_region.split(',').map(&:to_i)
# original_region_right = original_region_left + original_region_width
# original_region_bottom = original_region_top + original_region_height
# rotate_by = rotate_by % 360
# left = original_region_left
# width = original_region_width
# top = original_region_top
# height = original_region_height
# case rotate_by
# when 90
# left = original_image_height - original_region_bottom
# width = original_region_height
# top = original_region_left
# height = original_region_width
# when 180
# left = original_image_width - original_region_right
# top = original_image_height - original_region_bottom
# when 270
# left = original_region_top
# width = original_region_height
# top = original_image_width - original_region_right
# height = original_region_width
# end
# "#{left},#{top},#{width},#{height}"
# end
end
4 changes: 2 additions & 2 deletions app/models/concerns/digital_object/assets/file_import.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ def do_file_import
# Add original filename property to content datastream using <info:fedora/fedora-system:def/model#downloadFilename> relationship
@fedora_object.rels_int.add_relationship(content_ds, 'info:fedora/fedora-system:def/model#downloadFilename', original_filename, true) # last param *true* means that this is a literal value rather than a relationship

# Assume top-left orientation at upload time. This can be corrected later in the app.
@fedora_object.rels_int.add_relationship(content_ds, :orientation, 'top-left', true) # last param *true* means that this is a literal value rather than a relationship
# Assume 0-degree orientation at upload time. An app user can update this later.
@fedora_object.orientation = 0

self.original_file_path = (@import_file_original_file_path || @import_file_import_path) # This also updates the 'content' datastream label
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/concerns/digital_object/publishing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def execute_publish_action_for_target(publish_action, publish_target, do_ezid_up
rescue Errno::ECONNREFUSED
@errors.add(:publish_target, "Connection to server refused for Publish Target URL: #{publish_target.publish_target_field('publish_url')}")
rescue RestClient::ExceptionWithResponse => e
@errors.add(:publish_target, "#{e.response.code} response received for Publish Target URL: #{publish_target.publish_target_field('publish_url')}")
@errors.add(:publish_target, "#{response&.code || 'Undefined'} response received for Publish Target URL: #{publish_target.publish_target_field('publish_url')}")
end
success
end
Expand Down
28 changes: 25 additions & 3 deletions app/models/digital_object/asset.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ def run_after_create_logic
end

def run_after_save_logic
run_derivative_updates_if_necessary
end

def run_derivative_updates_if_necessary
# Conditionally run derivative processing on save
RequestDerivativesJob.perform_later(self.pid) if self.perform_derivative_processing
# Always update the image service on save, regardless of whether derivatives are ready
Expand Down Expand Up @@ -271,9 +275,27 @@ def handle_service_copy_upload(import_file_data)
end
end

def destroy_and_regenerate_derivatives!
raise 'TODO: Reimplement'
#generate_derivatives(true)
def regenerate_derivatives!(access_copy:, image_server_cache:)
destroy_access_copy! if access_copy
destroy_image_server_cache! if image_server_cache
self.perform_derivative_processing = true
self.save
end

def destroy_access_copy!
if self.access_copy_location && File.exist?(self.access_copy_location)
File.delete(self.access_copy_location)
end
if @fedora_object.datastreams['access']
@fedora_object.datastreams['access'].delete
end
end

def destroy_image_server_cache!
RestClient.delete(
"#{IMAGE_SERVER_CONFIG['url']}/api/v1/resources/#{self.pid}",
Authorization: "Bearer #{IMAGE_SERVER_CONFIG['token']}"
)
end

# def generate_derivatives(delete_existing_derivatives = false)
Expand Down
2 changes: 1 addition & 1 deletion config/environments/development.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Hyacinth::Application.configure do
# Settings specified here will take precedence over those in config/application.rb.

config.log_level = :debug
config.log_level = :error

# In the development environment your application's code is reloaded on
# every request. This slows down response time but is perfect for development
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
put 'add_parent', action: 'add_parent'
put 'remove_parents', action: 'remove_parents'
post 'rotate_image', action: 'rotate_image'
post 'regenerate_access_copy', action: 'regenerate_access_copy'
get 'featured_region', action: 'query_featured_region'
post 'featured_region', action: 'update_featured_region'
post 'swap_order_of_first_two_child_assets', action: 'swap_order_of_first_two_child_assets'
Expand Down
4 changes: 2 additions & 2 deletions config/templates/hyacinth.template.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ development:
default_asset_home: <%= Rails.root.join('tmp', 'development', 'asset_home') %>
default_service_copy_home: <%= Rails.root.join('tmp', 'development', 'service_copy_home') %>
access_copy_directory: <%= Rails.root.join('tmp', 'development', 'access') %>
# access_copy_file_group: digital-projects
# access_copy_file_group: 'digital-projects'
# access_copy_file_permissions: '0640'
upload_directory: <%= Rails.root.join('tmp', 'development', 'upload') %>
csv_export_directory: <%= Rails.root.join('tmp', 'development', 'csv_exports') %>
Expand All @@ -22,7 +22,7 @@ test:
default_asset_home: <%= Rails.root.join('tmp', 'test', 'asset_home') %>
default_service_copy_home: <%= Rails.root.join('tmp', 'test', 'service_copy_home') %>
access_copy_directory: <%= Rails.root.join('tmp', 'test', 'access') %>
# access_copy_file_group: digital-projects
# access_copy_file_group: 'digital-projects'
# access_copy_file_permissions: '0640'
upload_directory: <%= Rails.root.join('tmp', 'test', 'upload') %>
csv_export_directory: <%= Rails.root.join('tmp', 'test', 'csv_exports') %>
Expand Down
6 changes: 4 additions & 2 deletions spec/jobs/process_digital_object_import_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,10 @@

describe ".perform" do
let(:digital_object_import_id) {
allow(DigitalObjectImport).to receive(:find).with(12345).and_return(digital_object_import)
12345
import_id = 12345
allow(DigitalObjectImport).to receive(:find).with(import_id).and_return(digital_object_import)
allow(DigitalObjectImport).to receive(:exists?).with(import_id).and_return(true)
import_id
}
let(:import_job) {
import_job = ImportJob.new
Expand Down
2 changes: 1 addition & 1 deletion spec/models/digital_object/asset_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@
asset.validate_featured_region
expect(asset.errors).not_to be_empty
end
it "rotates a region appropriately when image is rotated" do
pending "rotates a region appropriately when image is rotated" do
asset.featured_region = good_region
region_params = good_region.split(',')
# get a rotated value for 90 more degrees
Expand Down

0 comments on commit d18177d

Please sign in to comment.