Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

always pull repo file if filepath param is missing #5694

Merged
merged 1 commit into from
Jun 29, 2022
Merged
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
3 changes: 2 additions & 1 deletion app/jobs/characterize_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ class CharacterizeJob < Hyrax::ApplicationJob
# @param [String, NilClass] filepath the cached file within the Hyrax.config.working_path
def perform(file_set, file_id, filepath = nil)
raise "#{file_set.class.characterization_proxy} was not found for FileSet #{file_set.id}" unless file_set.characterization_proxy?
filepath = Hyrax::WorkingDirectory.find_or_retrieve(file_id, file_set.id) unless filepath && File.exist?(filepath)
# Ensure a fresh copy of the repo file's latest version is being worked on, if no filepath is directly provided
filepath = Hyrax::WorkingDirectory.copy_repository_resource_to_working_directory(Hydra::PCDM::File.find(file_id), file_set.id) unless filepath && File.exist?(filepath)
characterize(file_set, file_id, filepath)
CreateDerivativesJob.perform_later(file_set, file_id, filepath)
end
Expand Down
6 changes: 4 additions & 2 deletions app/jobs/create_derivatives_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ class CreateDerivativesJob < Hyrax::ApplicationJob
# @param [String, NilClass] filepath the cached file within the Hyrax.config.working_path
def perform(file_set, file_id, filepath = nil)
return if file_set.video? && !Hyrax.config.enable_ffmpeg
filename = Hyrax::WorkingDirectory.find_or_retrieve(file_id, file_set.id, filepath)

file_set.create_derivatives(filename)
# Ensure a fresh copy of the repo file's latest version is being worked on, if no filepath is directly provided
filepath = Hyrax::WorkingDirectory.copy_repository_resource_to_working_directory(Hydra::PCDM::File.find(file_id), file_set.id) unless filepath && File.exist?(filepath)

file_set.create_derivatives(filepath)

# Reload from Fedora and reindex for thumbnail and extracted text
file_set.reload
Expand Down
2 changes: 2 additions & 0 deletions app/services/hyrax/working_directory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ module Hyrax
# @deprecated Use JobIoWrapper instead
class WorkingDirectory
class << self
# @deprecated No longer used anywhere in Hyrax code, naively assumes filenames of versions/revisions are distinct
# Returns the file passed as filepath if that file exists. Otherwise it grabs the file from repository,
# puts it on the disk and returns that path.
# @param [String] repository_file_id identifier for Hydra::PCDM::File
# @param [String] id the identifier of the FileSet
# @param [String, NilClass] filepath path to existing cached copy of the file
# @return [String] path of the working file
def find_or_retrieve(repository_file_id, id, filepath = nil)
no-reply marked this conversation as resolved.
Show resolved Hide resolved
Deprecation.warn("Hyrax::WorkingDirectory.find_or_retrieve() is deprecated and no longer used in Hyrax")
return filepath if filepath && File.exist?(filepath)
repository_file = Hydra::PCDM::File.find(repository_file_id)
working_path = full_filename(id, repository_file.original_name)
Expand Down
14 changes: 12 additions & 2 deletions spec/jobs/characterize_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,18 @@
context 'with valid filepath param' do
let(:filename) { File.join(fixture_path, 'world.png') }

it 'skips Hyrax::WorkingDirectory' do
expect(Hyrax::WorkingDirectory).not_to receive(:find_or_retrieve)
it 'skips Hyrax::WorkingDirectory.copy_repository_resource_to_working_directory' do
expect(Hyrax::WorkingDirectory).not_to receive(:copy_repository_resource_to_working_directory)
expect(Hydra::Works::CharacterizationService).to receive(:run).with(file, filename)
described_class.perform_now(file_set, file.id, filename)
end
end

context 'with no filepath param' do
let(:filename) { nil }

it 'uses Hyrax::WorkingDirectory.copy_repository_resource_to_working_directory to pull the repo file' do
expect(Hyrax::WorkingDirectory).to receive(:copy_repository_resource_to_working_directory)
expect(Hydra::Works::CharacterizationService).to receive(:run).with(file, filename)
described_class.perform_now(file_set, file.id, filename)
end
Expand Down
37 changes: 37 additions & 0 deletions spec/jobs/create_derivatives_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,43 @@
Hyrax.config.enable_ffmpeg = ffmpeg_enabled
end

context "filepath parameter" do
let(:file_set) { create(:file_set) }

let(:file) do
Hydra::PCDM::File.new do |f|
f.content = File.open(File.join(fixture_path, 'world.png'))
f.original_name = 'world.png'
f.mime_type = 'image/png'
end
end

before do
file_set.original_file = file
file_set.save!
end

describe 'with valid filepath param' do
let(:filename) { File.join(fixture_path, 'world.png') }

it 'skips Hyrax::WorkingDirectory.copy_repository_resource_to_working_directory' do
expect(Hyrax::WorkingDirectory).not_to receive(:copy_repository_resource_to_working_directory)
expect(Hydra::Derivatives::ImageDerivatives).to receive(:create)
described_class.perform_now(file_set, file.id, filename)
end
end

describe 'with no filepath param' do
let(:filename) { nil }

it 'Uses Hyrax::WorkingDirectory.copy_repository_resource_to_working_directory to pull the repo file' do
expect(Hyrax::WorkingDirectory).to receive(:copy_repository_resource_to_working_directory)
expect(Hydra::Derivatives::ImageDerivatives).to receive(:create)
described_class.perform_now(file_set, file.id, filename)
end
end
end

context "with an audio file" do
let(:id) { '123' }
let(:file_set) { FileSet.new }
Expand Down