diff --git a/.rubocop_fixme.yml b/.rubocop_fixme.yml index 1b8769adf4..d1d8c2fc6d 100644 --- a/.rubocop_fixme.yml +++ b/.rubocop_fixme.yml @@ -4,7 +4,6 @@ Security/MarshalLoad: Metrics/ClassLength: Exclude: - - 'app/actors/hyrax/actors/file_set_actor.rb' - 'app/controllers/hyrax/file_sets_controller.rb' - 'app/forms/hyrax/forms/permission_template_form.rb' - 'app/presenters/hyrax/work_show_presenter.rb' diff --git a/app/actors/hyrax/actors/file_set_actor.rb b/app/actors/hyrax/actors/file_set_actor.rb index 824f396b69..347835373a 100644 --- a/app/actors/hyrax/actors/file_set_actor.rb +++ b/app/actors/hyrax/actors/file_set_actor.rb @@ -22,7 +22,7 @@ def create_content(file, relation = :original_file) file_set.label ||= label_for(file) file_set.title = [file_set.label] if file_set.title.blank? return false unless file_set.save # Need to save to get an id - IngestJob.perform_later(wrapper!(file, relation)) + IngestJob.perform_later(wrapper!(file: file, relation: relation)) end # Spawns asynchronous IngestJob with user notification afterward @@ -30,17 +30,19 @@ def create_content(file, relation = :original_file) # @param [Symbol, #to_s] relation # @return [IngestJob] the queued job def update_content(file, relation = :original_file) - IngestJob.perform_later(wrapper!(file, relation), notification: true) + IngestJob.perform_later(wrapper!(file: file, relation: relation), notification: true) end # Spawns async ImportUrlJob to attach remote file to fileset # @param [#to_s] url # @return [IngestUrlJob] the queued job + # @todo Remove as it appears to be untested and not called def import_url(url) file_set.update(import_url: url.to_s) operation = Hyrax::Operation.create!(user: user, operation_type: "Attach File") ImportUrlJob.perform_later(file_set, operation) end + deprecation_deprecate import_url: "appears to be untested and not used, and will be removed unless an issue/PR is submitted verifying otherwise" # @!endgroup @@ -102,9 +104,8 @@ def destroy Hyrax.config.callback.run(:after_destroy, file_set.id, user) end - def file_actor_class - Hyrax::Actors::FileActor - end + class_attribute :file_actor_class + self.file_actor_class = Hyrax::Actors::FileActor private @@ -117,25 +118,8 @@ def build_file_actor(relation) end # uses create! because object must be persisted to serialize for jobs - def wrapper!(file, relation) - JobIoWrapper.create!(wrapper_params(file, relation)) - end - - # helps testing - # @return [Hash] params for JobIoWrapper (new or create) - def wrapper_params(file, relation) - args = { user: user, relation: relation.to_s, file_set_id: file_set.id } - if file.is_a?(Hyrax::UploadedFile) - args[:uploaded_file] = file - args[:path] = file.uploader.path - elsif file.respond_to?(:path) - args[:path] = file.path - args[:original_name] = file.original_filename if file.respond_to?(:original_filename) - args[:original_name] ||= file.original_name if file.respond_to?(:original_name) - else - raise "Require Hyrax::UploadedFile or File-like object, received #{file.class} object: #{file}" - end - args + def wrapper!(file:, relation:) + JobIoWrapper.create_with_varied_file_handling!(user: user, file: file, relation: relation, file_set: file_set) end # For the label, use the original_filename or original_name if it's there. diff --git a/app/models/job_io_wrapper.rb b/app/models/job_io_wrapper.rb index 31cbbe489a..f1152d212d 100644 --- a/app/models/job_io_wrapper.rb +++ b/app/models/job_io_wrapper.rb @@ -24,6 +24,30 @@ class JobIoWrapper < ApplicationRecord after_initialize :static_defaults delegate :read, :size, to: :file + # Responsible for creating a JobIoWrapper from the given parameters, with a + # focus on sniffing out attributes from the given :file. + # + # @param [User] user - The user requesting to create this instance + # @param [#path, Hyrax::UploadedFile] file - The file that is to be uploaded + # @param [String] relation + # @param [FileSet] file_set - The associated file set + # @return [JobIoWrapper] + # @raise ActiveRecord::RecordInvalid - if the instance is not valid + def self.create_with_varied_file_handling!(user:, file:, relation:, file_set:) + args = { user: user, relation: relation.to_s, file_set_id: file_set.id } + if file.is_a?(Hyrax::UploadedFile) + args[:uploaded_file] = file + args[:path] = file.uploader.path + elsif file.respond_to?(:path) + args[:path] = file.path + args[:original_name] = file.original_filename if file.respond_to?(:original_filename) + args[:original_name] ||= file.original_name if file.respond_to?(:original_name) + else + raise "Require Hyrax::UploadedFile or File-like object, received #{file.class} object: #{file}" + end + create!(args) + end + def original_name super || extracted_original_name end diff --git a/spec/actors/hyrax/actors/file_set_actor_spec.rb b/spec/actors/hyrax/actors/file_set_actor_spec.rb index 7e340746a1..9de473a017 100644 --- a/spec/actors/hyrax/actors/file_set_actor_spec.rb +++ b/spec/actors/hyrax/actors/file_set_actor_spec.rb @@ -6,7 +6,6 @@ let(:user) { create(:user) } let(:file_path) { File.join(fixture_path, 'world.png') } let(:file) { fixture_file_upload(file_path, 'image/png') } # we will override for the different types of File objects - let(:wrapper_path) { file.path } # override to match let(:local_file) { File.open(file_path) } let(:file_set) { create(:file_set, content: local_file) } let(:actor) { described_class.new(file_set, user) } @@ -18,37 +17,6 @@ describe 'private' do let(:file_set) { build(:file_set) } # avoid 130+ LDP requests - describe '#wrapper_params' do - let(:baseline_args) { { user: user, file_set_id: file_set.id, relation: relation.to_s } } - let(:wrapper_args) { baseline_args.merge(path: wrapper_path, original_name: actor.send(:label_for, file)) } - - subject { actor.send(:wrapper_params, file, relation) } - - it 'with Rack::Test::UploadedFile returns expected hash' do - expect(subject).to eq(wrapper_args) - expect(actor.send(:wrapper_params, file, :remastered)).to eq(wrapper_args.merge(relation: 'remastered')) - end - - context 'with ::File' do - let(:file) { local_file } - - it 'returns expected hash' do - wrapper_args.delete(:original_name) - expect(subject).to eq(wrapper_args) - end - end - - context 'with Hyrax::UploadedFile' do - let(:file) { Hyrax::UploadedFile.new(user: user, file_set_uri: file_set.uri, file: local_file) } - let(:wrapper_path) { file.uploader.path } - - it 'returns expected hash' do - wrapper_args.delete(:original_name) - expect(subject).to eq(wrapper_args.merge(uploaded_file: file)) - end - end - end - describe '#assign_visibility?' do let(:viz) { Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PUBLIC } @@ -96,9 +64,7 @@ describe '#create_content' do before do - expect(JobIoWrapper).to receive(:create!).with(any_args) do |args| - JobIoWrapper.new(args) # skip persistence - end + expect(JobIoWrapper).to receive(:create_with_varied_file_handling!).with(any_args).and_return(JobIoWrapper.new) expect(IngestJob).to receive(:perform_later).with(JobIoWrapper) end diff --git a/spec/models/job_io_wrapper_spec.rb b/spec/models/job_io_wrapper_spec.rb index 99f21e08bd..50311a182c 100644 --- a/spec/models/job_io_wrapper_spec.rb +++ b/spec/models/job_io_wrapper_spec.rb @@ -14,6 +14,43 @@ expect { subject.save! }.not_to raise_error end + describe '.create_with_wrapped_params!' do + let(:local_file) { File.open(path) } + let(:relation) { :remastered } + + subject { described_class.create_with_varied_file_handling!(user: user, file_set: file_set, file: file, relation: relation) } + + context 'with Rack::Test::UploadedFile' do + let(:file) { Rack::Test::UploadedFile.new(path, 'image/png') } + + it 'creates a JobIoWrapper' do + expected_create_args = { user: user, relation: relation.to_s, file_set_id: file_set.id, path: file.path, original_name: 'world.png' } + expect(JobIoWrapper).to receive(:create!).with(expected_create_args) + subject + end + end + + context 'with ::File' do + let(:file) { local_file } + + it 'creates a JobIoWrapper' do + expected_create_args = { user: user, relation: relation.to_s, file_set_id: file_set.id, path: file.path } + expect(JobIoWrapper).to receive(:create!).with(expected_create_args) + subject + end + end + + context 'with Hyrax::UploadedFile' do + let(:file) { Hyrax::UploadedFile.new(user: user, file_set_uri: file_set.uri, file: local_file) } + + it 'creates a JobIoWrapper' do + expected_create_args = { user: user, relation: relation.to_s, file_set_id: file_set.id, uploaded_file: file, path: file.uploader.path } + expect(JobIoWrapper).to receive(:create!).with(expected_create_args) + subject + end + end + end + describe 'uploaded_file' do let(:other_path) { fixture_path + '/image.jpg' } let(:uploaded_file) { Hyrax::UploadedFile.new(user: user, file_set_uri: file_set.uri, file: File.new(other_path)) }