From 5ab76a5efbe0efefd746573520f15f23a35e12dd Mon Sep 17 00:00:00 2001 From: Mason Ballengee Date: Mon, 6 Jan 2025 16:34:49 -0500 Subject: [PATCH 1/4] Download hosted files to tmp and move to dropbox when uploading Interntet host files such as google drive or sharepoint were being downloaded multiple times during processing. By creating a tempfile and moving it into the dropbox, this should remove or at least minimize excess downloads. --- app/jobs/master_file_management_jobs.rb | 30 +---------------- app/models/master_file.rb | 12 ++++--- app/services/file_locator.rb | 21 +++++++++--- app/services/file_mover.rb | 45 +++++++++++++++++++++++++ spec/models/master_file_spec.rb | 43 ++++++++++++++++++++--- 5 files changed, 109 insertions(+), 42 deletions(-) create mode 100644 app/services/file_mover.rb diff --git a/app/jobs/master_file_management_jobs.rb b/app/jobs/master_file_management_jobs.rb index f35de08acd..44ab6e70d9 100644 --- a/app/jobs/master_file_management_jobs.rb +++ b/app/jobs/master_file_management_jobs.rb @@ -18,34 +18,6 @@ module MasterFileManagementJobs class Move < ActiveJob::Base queue_as :master_file_management_move - def s3_to_s3(source, dest) - source_object = FileLocator::S3File.new(source.source).object - dest_object = FileLocator::S3File.new(dest.source).object - if dest_object.copy_from(source_object, multipart_copy: source_object.size > 15.megabytes) - source_object.delete if FileLocator.new(dest.source).exists? - end - end - - def s3_to_file(source, dest) - source_object = FileLocator::S3File.new(source.source).object - FileUtils.mkdir_p File.dirname(dest.uri.path) unless File.exist? File.dirname(dest.uri.path) - if source_object.download_file(dest.uri.path) - source_object.delete - end - end - - def file_to_s3(source, dest) - dest_object = FileLocator::S3File.new(dest.source).object - if dest_object.upload_file(source.uri.path) - FileUtils.rm(source.uri.path) - end - end - - def file_to_file(source, dest) - FileUtils.mkdir_p File.dirname(dest.location) unless File.exist? File.dirname(dest.location) - FileUtils.mv source.location, dest.location - end - def perform(id, newpath) Rails.logger.debug "Moving masterfile to #{newpath}" @@ -58,7 +30,7 @@ def perform(id, newpath) elsif old_locator.exists? new_locator = FileLocator.new(newpath) copy_method = "#{old_locator.uri.scheme}_to_#{new_locator.uri.scheme}".to_sym - send(copy_method, old_locator, new_locator) + FileMover.send(copy_method, old_locator, new_locator) masterfile.file_location = newpath masterfile.save Rails.logger.info "#{oldpath} has been moved to #{newpath}" diff --git a/app/models/master_file.rb b/app/models/master_file.rb index df55db16d9..9c54fd811f 100644 --- a/app/models/master_file.rb +++ b/app/models/master_file.rb @@ -208,9 +208,8 @@ def setContent(file, file_name: nil, file_size: nil, auth_header: nil, dropbox_d self.file_location = file.to_s self.file_size = FileLocator::S3File.new(file).object.size else - self.file_location = file.to_s - self.file_size = file_size - self.title = file_name + local_file = FileLocator.new(file, filename: file_name, auth_header: auth_header).local_location + saveOriginal(File.open(local_file), file_name, dropbox_dir) end else #Batch saveOriginal(file, File.basename(file.path), dropbox_dir) @@ -682,6 +681,7 @@ def ffmpeg_frame_options(file_source, output_path, offset, new_width, new_height def saveOriginal(file, original_name = nil, dropbox_dir = media_object.collection.dropbox_absolute_path) realpath = File.realpath(file.path) + self.file_size = file.size if original_name.present? # If we have a temp name from an upload, rename to the original name supplied by the user unless File.basename(realpath) == original_name @@ -694,14 +694,16 @@ def saveOriginal(file, original_name = nil, dropbox_dir = media_object.collectio path = File.join(parent_dir, duplicate_file_name(original_name, num)) num += 1 end - FileUtils.move(realpath, path) + old_locator = FileLocator.new(realpath) + new_locator = FileLocator.new(path) + copy_method = "#{old_locator.uri.scheme}_to_#{new_locator.uri.scheme}".to_sym + FileMover.send(copy_method, old_locator, new_locator) realpath = path end create_working_file!(realpath) end self.file_location = realpath - self.file_size = file.size.to_s ensure file.close end diff --git a/app/services/file_locator.rb b/app/services/file_locator.rb index 0ef83705e2..081e4d2af1 100644 --- a/app/services/file_locator.rb +++ b/app/services/file_locator.rb @@ -16,7 +16,7 @@ require 'aws-sdk-s3' class FileLocator - attr_reader :source, :auth_header + attr_reader :source, :filename, :auth_header class S3File attr_reader :bucket, :key @@ -48,6 +48,7 @@ def download_url def initialize(source, opts = {}) @source = source + @filename = opts[:filename] @auth_header = opts[:auth_header] end @@ -89,17 +90,29 @@ def location end end - # If S3, download object to /tmp + # If S3 or http(s), download object to /tmp def local_location @local_location ||= begin - if uri.scheme == 's3' + case uri.scheme + when 's3' S3File.new(uri).local_file.path - else + when 'file' location + else + local_file.path end end end + def local_file + @local_file ||= Tempfile.new(filename) + File.binwrite(@local_file, reader.read) + @local_file.rewind + @local_file + ensure + @local_file.close + end + def exist? case uri.scheme when 's3' diff --git a/app/services/file_mover.rb b/app/services/file_mover.rb new file mode 100644 index 0000000000..ea7efe8984 --- /dev/null +++ b/app/services/file_mover.rb @@ -0,0 +1,45 @@ +# Copyright 2011-2024, The Trustees of Indiana University and Northwestern +# University. Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software distributed +# under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR +# CONDITIONS OF ANY KIND, either express or implied. See the License for the +# specific language governing permissions and limitations under the License. +# --- END LICENSE_HEADER BLOCK --- + +class FileMover + class << self + def s3_to_s3(source, dest) + source_object = FileLocator::S3File.new(source.source).object + dest_object = FileLocator::S3File.new(dest.source).object + if dest_object.copy_from(source_object, multipart_copy: source_object.size > 15.megabytes) + source_object.delete if FileLocator.new(dest.source).exists? + end + end + + def s3_to_file(source, dest) + source_object = FileLocator::S3File.new(source.source).object + FileUtils.mkdir_p File.dirname(dest.uri.path) unless File.exist? File.dirname(dest.uri.path) + if source_object.download_file(dest.uri.path) + source_object.delete + end + end + + def file_to_s3(source, dest) + dest_object = FileLocator::S3File.new(dest.source).object + if dest_object.upload_file(source.uri.path) + FileUtils.rm(source.uri.path) + end + end + + def file_to_file(source, dest) + FileUtils.mkdir_p File.dirname(dest.location) unless File.exist? File.dirname(dest.location) + FileUtils.mv source.location, dest.location + end + end +end \ No newline at end of file diff --git a/spec/models/master_file_spec.rb b/spec/models/master_file_spec.rb index 9c00267597..f3fafea222 100644 --- a/spec/models/master_file_spec.rb +++ b/spec/models/master_file_spec.rb @@ -423,15 +423,50 @@ let(:file_name) { "sample.mp4" } let(:file_size) { 12345 } let(:auth_header) { {"Authorization"=>"Bearer ya29.a0AfH6SMC6vSj4D6po1aDxAr6JmY92azh3lxevSuPKxf9QPPSKmMzqbZvI7B3oIACqqMVono1P0XD2F1Jl_rkayoI6JGz-P2cpg44-55oJFcWychAvUliWeRKf1cifMo9JF10YmXxhIfrG5mu7Ahy9FZpudN92p2JhvTI"} } + let(:fixture) { File.expand_path('../../fixtures/videoshort.mp4',__FILE__) } + let(:tempfile) { Tempfile.new('foo') } + let(:media_path) { File.expand_path("../../master_files-#{SecureRandom.uuid}",__FILE__)} + let(:dropbox_path) { File.expand_path("../../collection-#{SecureRandom.uuid}",__FILE__)} + let(:upload) { ActionDispatch::Http::UploadedFile.new :tempfile => tempfile, :filename => original, :type => 'video/mp4' } + let!(:media_object) { FactoryBot.create(:media_object, sections: [subject]) } + let(:collection) { Admin::Collection.new } subject { FactoryBot.create(:master_file) } - it "should set the right properties" do + before(:each) do allow(subject).to receive(:reloadTechnicalMetadata!).and_return(nil) - subject.setContent(file, file_name: file_name, file_size: file_size, auth_header: auth_header) - expect(subject.file_location).to eq(file.to_s) + @old_media_path = Settings.encoding.working_file_path + FileUtils.mkdir_p media_path + FileUtils.cp fixture, tempfile + allow_any_instance_of(FileLocator).to receive(:local_location).and_return(tempfile.path) + allow_any_instance_of(File).to receive(:size).and_return(file_size) + allow(media_object).to receive(:collection).and_return(collection) + FileUtils.mkdir_p dropbox_path + allow(collection).to receive(:dropbox_absolute_path).and_return(File.absolute_path(dropbox_path)) + end + + after(:each) do + Settings.encoding.working_file_path = @old_media_path + File.unlink subject.file_location + FileUtils.rm_rf media_path + FileUtils.rm_rf dropbox_path + end + + it "should move an uploaded file into the root of the collection's dropbox" do + Settings.encoding.working_file_path = nil + subject.setContent(file, file_name: file_name, file_size: file_size, auth_header: auth_header, dropbox_dir: collection.dropbox_absolute_path) + expect(subject.file_location).to eq(File.realpath(File.join(collection.dropbox_absolute_path, file_name))) + end + + it "should copy an uploaded file to the media path" do + Settings.encoding.working_file_path = media_path + subject.setContent(file, file_name: file_name, file_size: file_size, auth_header: auth_header, dropbox_dir: collection.dropbox_absolute_path) + expect(File.fnmatch("#{media_path}/*/#{file_name}", subject.working_file_path.first)).to be true + end + + it "should set the right properties" do + subject.setContent(file, file_name: file_name, file_size: file_size, auth_header: auth_header, dropbox_dir: collection.dropbox_absolute_path) expect(subject.file_size).to eq(file_size) - expect(subject.title).to eq(file_name) expect(subject.instance_variable_get(:@auth_header)).to eq(auth_header) end end From 2d49a12984398d45f148273c8a47485a05689419 Mon Sep 17 00:00:00 2001 From: Mason Ballengee Date: Tue, 7 Jan 2025 15:33:56 -0500 Subject: [PATCH 2/4] Add tests for new FileMover service --- spec/services/file_mover_spec.rb | 93 ++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 spec/services/file_mover_spec.rb diff --git a/spec/services/file_mover_spec.rb b/spec/services/file_mover_spec.rb new file mode 100644 index 0000000000..c3be7f5e22 --- /dev/null +++ b/spec/services/file_mover_spec.rb @@ -0,0 +1,93 @@ +# Copyright 2011-2024, The Trustees of Indiana University and Northwestern +# University. Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software distributed +# under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR +# CONDITIONS OF ANY KIND, either express or implied. See the License for the +# specific language governing permissions and limitations under the License. +# --- END LICENSE_HEADER BLOCK --- + +require 'rails_helper' +require 'fakefs/safe' + +describe FileMover, type: :service do + let(:fs_location) { '/path/to/source/test.mp4' } + let(:fs_file) { FileLocator.new("file://#{fs_location}") } + let(:fs_dest) { FileLocator.new('file:///path/to/dest/test.mp4') } + + context 's3 methods' do + let(:s3_file) { FileLocator.new('s3://source_bucket/test.mp4') } + let(:s3_dest) { FileLocator.new('s3://dest_bucket/test.mp4') } + + before :each do + @source_object = double(key: 'test.mp4', bucket_name: 'source_bucket') + allow(Aws::S3::Object).to receive(:new).and_call_original + allow(Aws::S3::Object).to receive(:new).with(key: 'test.mp4', bucket_name: 'source_bucket').and_return(@source_object) + allow(@source_object).to receive(:size).and_return(1) + allow(@source_object).to receive(:delete).and_return(true) + allow(@source_object).to receive(:download_file).and_return(true) + end + + describe '.s3_to_s3' do + it 'copies file from source to dest' do + expect_any_instance_of(Aws::S3::Object).to receive(:copy_from).with(Aws::S3::Object.new(key: 'test.mp4', bucket_name: 'source_bucket'), multipart_copy: false) + described_class.s3_to_s3(s3_file, s3_dest) + end + + it 'deletes file after copying' do + allow_any_instance_of(Aws::S3::Object).to receive(:copy_from).and_return(true) + expect(@source_object).to receive(:delete) + described_class.s3_to_s3(s3_file, s3_dest) + end + end + + describe '.s3_to_file' do + it 'copies file from source to dest' do + expect(@source_object).to receive(:download_file).with(fs_dest.uri.path) + described_class.s3_to_file(s3_file, fs_dest) + end + + it 'deletes file after copying' do + expect(@source_object).to receive(:delete) + described_class.s3_to_file(s3_file, fs_dest) + end + end + + describe '.file_to_s3' do + it 'copies file from source to dest' do + expect_any_instance_of(Aws::S3::Object).to receive(:upload_file).with(fs_file.uri.path) + described_class.file_to_s3(fs_file, s3_dest) + end + + it 'deletes file after copying' do + allow_any_instance_of(Aws::S3::Object).to receive(:upload_file).and_return(true) + expect(FileUtils).to receive(:rm).with(fs_file.uri.path) + described_class.file_to_s3(fs_file, s3_dest) + end + end + end + + describe '.file_to_file' do + before :each do + FakeFS.activate! + FileUtils.mkdir_p File.dirname(fs_location) + File.open(fs_location, 'w'){} + end + + after :each do + FakeFS.deactivate! + end + + it 'moves file from source to dest' do + expect(File.exist? fs_location).to be true + described_class.file_to_file(fs_file, fs_dest) + expect(File.exist? fs_location).to be false + expect(File.exist? '/path/to/dest/test.mp4').to be true + end + end +end \ No newline at end of file From da3320140bc8005ad9f5c2b03023cc6ca8fab4b6 Mon Sep 17 00:00:00 2001 From: Mason Ballengee Date: Wed, 8 Jan 2025 14:29:10 -0500 Subject: [PATCH 3/4] Create 'FileMover.move' convenience method This commit also includes some tightening up of the test suite and removing an unneeded `File#rewind` call from `FileLocator#local_file`. --- app/jobs/master_file_management_jobs.rb | 3 +- app/models/master_file.rb | 3 +- app/services/file_locator.rb | 1 - app/services/file_mover.rb | 60 ++++++++++++++------- spec/services/file_mover_spec.rb | 72 ++++++++++++------------- 5 files changed, 78 insertions(+), 61 deletions(-) diff --git a/app/jobs/master_file_management_jobs.rb b/app/jobs/master_file_management_jobs.rb index 44ab6e70d9..f01ddb76ab 100644 --- a/app/jobs/master_file_management_jobs.rb +++ b/app/jobs/master_file_management_jobs.rb @@ -29,8 +29,7 @@ def perform(id, newpath) Rails.logger.info "Masterfile #{newpath} already moved" elsif old_locator.exists? new_locator = FileLocator.new(newpath) - copy_method = "#{old_locator.uri.scheme}_to_#{new_locator.uri.scheme}".to_sym - FileMover.send(copy_method, old_locator, new_locator) + FileMover.move(old_locator, new_locator) masterfile.file_location = newpath masterfile.save Rails.logger.info "#{oldpath} has been moved to #{newpath}" diff --git a/app/models/master_file.rb b/app/models/master_file.rb index 9c54fd811f..3951ca10ee 100644 --- a/app/models/master_file.rb +++ b/app/models/master_file.rb @@ -696,8 +696,7 @@ def saveOriginal(file, original_name = nil, dropbox_dir = media_object.collectio end old_locator = FileLocator.new(realpath) new_locator = FileLocator.new(path) - copy_method = "#{old_locator.uri.scheme}_to_#{new_locator.uri.scheme}".to_sym - FileMover.send(copy_method, old_locator, new_locator) + FileMover.move(old_locator, new_locator) realpath = path end diff --git a/app/services/file_locator.rb b/app/services/file_locator.rb index 081e4d2af1..24e6045b12 100644 --- a/app/services/file_locator.rb +++ b/app/services/file_locator.rb @@ -107,7 +107,6 @@ def local_location def local_file @local_file ||= Tempfile.new(filename) File.binwrite(@local_file, reader.read) - @local_file.rewind @local_file ensure @local_file.close diff --git a/app/services/file_mover.rb b/app/services/file_mover.rb index ea7efe8984..b5320cccd4 100644 --- a/app/services/file_mover.rb +++ b/app/services/file_mover.rb @@ -14,32 +14,52 @@ class FileMover class << self - def s3_to_s3(source, dest) - source_object = FileLocator::S3File.new(source.source).object - dest_object = FileLocator::S3File.new(dest.source).object - if dest_object.copy_from(source_object, multipart_copy: source_object.size > 15.megabytes) - source_object.delete if FileLocator.new(dest.source).exists? - end + def move(source, dest, method: nil) + new(source, dest, method: method).move end + end + + def initialize(source, dest, method: nil) + @source = source + @dest = dest + @method = method + end + + def move + send(method, @source, @dest) + end + + def method + @method || "#{@source.uri.scheme}_to_#{@dest.uri.scheme}".to_sym + end + + private - def s3_to_file(source, dest) - source_object = FileLocator::S3File.new(source.source).object - FileUtils.mkdir_p File.dirname(dest.uri.path) unless File.exist? File.dirname(dest.uri.path) - if source_object.download_file(dest.uri.path) - source_object.delete - end + def s3_to_s3(source, dest) + source_object = FileLocator::S3File.new(source.source).object + dest_object = FileLocator::S3File.new(dest.source).object + if dest_object.copy_from(source_object, multipart_copy: source_object.size > 15.megabytes) + source_object.delete if FileLocator.new(dest.source).exists? end + end - def file_to_s3(source, dest) - dest_object = FileLocator::S3File.new(dest.source).object - if dest_object.upload_file(source.uri.path) - FileUtils.rm(source.uri.path) - end + def s3_to_file(source, dest) + source_object = FileLocator::S3File.new(source.source).object + FileUtils.mkdir_p File.dirname(dest.uri.path) unless File.exist? File.dirname(dest.uri.path) + if source_object.download_file(dest.uri.path) + source_object.delete end + end - def file_to_file(source, dest) - FileUtils.mkdir_p File.dirname(dest.location) unless File.exist? File.dirname(dest.location) - FileUtils.mv source.location, dest.location + def file_to_s3(source, dest) + dest_object = FileLocator::S3File.new(dest.source).object + if dest_object.upload_file(source.uri.path) + FileUtils.rm(source.uri.path) end end + + def file_to_file(source, dest) + FileUtils.mkdir_p File.dirname(dest.location) unless File.exist? File.dirname(dest.location) + FileUtils.mv source.location, dest.location + end end \ No newline at end of file diff --git a/spec/services/file_mover_spec.rb b/spec/services/file_mover_spec.rb index c3be7f5e22..6b453e01f5 100644 --- a/spec/services/file_mover_spec.rb +++ b/spec/services/file_mover_spec.rb @@ -16,78 +16,78 @@ require 'fakefs/safe' describe FileMover, type: :service do - let(:fs_location) { '/path/to/source/test.mp4' } - let(:fs_file) { FileLocator.new("file://#{fs_location}") } - let(:fs_dest) { FileLocator.new('file:///path/to/dest/test.mp4') } - - context 's3 methods' do + describe '.move' do + let(:fs_location) { '/path/to/source/test.mp4' } + let(:fs_file) { FileLocator.new("file://#{fs_location}") } + let(:fs_dest) { FileLocator.new('file:///path/to/dest/test.mp4') } let(:s3_file) { FileLocator.new('s3://source_bucket/test.mp4') } let(:s3_dest) { FileLocator.new('s3://dest_bucket/test.mp4') } - before :each do - @source_object = double(key: 'test.mp4', bucket_name: 'source_bucket') + before(:each, s3: true) do + @source_object = double(key: 'test.mp4', bucket_name: 'source_bucket', size: 1) + @dest_object = double(key: 'test.mp4', bucket_name: 'dest_bucket', exists?: true) allow(Aws::S3::Object).to receive(:new).and_call_original allow(Aws::S3::Object).to receive(:new).with(key: 'test.mp4', bucket_name: 'source_bucket').and_return(@source_object) - allow(@source_object).to receive(:size).and_return(1) + allow(Aws::S3::Object).to receive(:new).with(key: 'test.mp4', bucket_name: 'dest_bucket').and_return(@dest_object) allow(@source_object).to receive(:delete).and_return(true) allow(@source_object).to receive(:download_file).and_return(true) + allow(@dest_object).to receive(:copy_from).and_return(true) end - describe '.s3_to_s3' do + describe 's3 source to s3 dest', s3: true do it 'copies file from source to dest' do - expect_any_instance_of(Aws::S3::Object).to receive(:copy_from).with(Aws::S3::Object.new(key: 'test.mp4', bucket_name: 'source_bucket'), multipart_copy: false) - described_class.s3_to_s3(s3_file, s3_dest) + expect(@dest_object).to receive(:copy_from).with(Aws::S3::Object.new(key: 'test.mp4', bucket_name: 'source_bucket'), multipart_copy: false) + described_class.move(s3_file, s3_dest) end it 'deletes file after copying' do - allow_any_instance_of(Aws::S3::Object).to receive(:copy_from).and_return(true) expect(@source_object).to receive(:delete) - described_class.s3_to_s3(s3_file, s3_dest) + described_class.move(s3_file, s3_dest) end end - describe '.s3_to_file' do + describe 's3 source to filesystem dest', s3: true do it 'copies file from source to dest' do expect(@source_object).to receive(:download_file).with(fs_dest.uri.path) - described_class.s3_to_file(s3_file, fs_dest) + described_class.move(s3_file, fs_dest) end it 'deletes file after copying' do expect(@source_object).to receive(:delete) - described_class.s3_to_file(s3_file, fs_dest) + described_class.move(s3_file, fs_dest) end end - describe '.file_to_s3' do + describe 'filesystem source to s3 dest', s3: true do it 'copies file from source to dest' do - expect_any_instance_of(Aws::S3::Object).to receive(:upload_file).with(fs_file.uri.path) - described_class.file_to_s3(fs_file, s3_dest) + expect(@dest_object).to receive(:upload_file).with(fs_file.uri.path) + described_class.move(fs_file, s3_dest) end it 'deletes file after copying' do - allow_any_instance_of(Aws::S3::Object).to receive(:upload_file).and_return(true) + allow(@dest_object).to receive(:upload_file).and_return(true) expect(FileUtils).to receive(:rm).with(fs_file.uri.path) - described_class.file_to_s3(fs_file, s3_dest) + described_class.move(fs_file, s3_dest) end end - end - describe '.file_to_file' do - before :each do - FakeFS.activate! - FileUtils.mkdir_p File.dirname(fs_location) - File.open(fs_location, 'w'){} - end + describe 'file system source to filesystem dest' do + before :each do + FakeFS.activate! + FileUtils.mkdir_p File.dirname(fs_location) + File.open(fs_location, 'w'){} + end - after :each do - FakeFS.deactivate! - end + after :each do + FakeFS.deactivate! + end - it 'moves file from source to dest' do - expect(File.exist? fs_location).to be true - described_class.file_to_file(fs_file, fs_dest) - expect(File.exist? fs_location).to be false - expect(File.exist? '/path/to/dest/test.mp4').to be true + it 'moves file from source to dest' do + expect(File.exist? fs_location).to be true + described_class.move(fs_file, fs_dest) + expect(File.exist? fs_location).to be false + expect(File.exist? '/path/to/dest/test.mp4').to be true + end end end end \ No newline at end of file From 25f83b595a528a24dca3fc9949efc7f6606d6499 Mon Sep 17 00:00:00 2001 From: Mason Ballengee Date: Wed, 8 Jan 2025 16:55:00 -0500 Subject: [PATCH 4/4] Reduce amount of test lifecycles in FileMover spec --- spec/services/file_mover_spec.rb | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/spec/services/file_mover_spec.rb b/spec/services/file_mover_spec.rb index 6b453e01f5..8b8d9459bd 100644 --- a/spec/services/file_mover_spec.rb +++ b/spec/services/file_mover_spec.rb @@ -35,37 +35,25 @@ end describe 's3 source to s3 dest', s3: true do - it 'copies file from source to dest' do + it 'moves file from source to dest' do expect(@dest_object).to receive(:copy_from).with(Aws::S3::Object.new(key: 'test.mp4', bucket_name: 'source_bucket'), multipart_copy: false) - described_class.move(s3_file, s3_dest) - end - - it 'deletes file after copying' do expect(@source_object).to receive(:delete) described_class.move(s3_file, s3_dest) end end describe 's3 source to filesystem dest', s3: true do - it 'copies file from source to dest' do + it 'moves file from source to dest' do expect(@source_object).to receive(:download_file).with(fs_dest.uri.path) - described_class.move(s3_file, fs_dest) - end - - it 'deletes file after copying' do expect(@source_object).to receive(:delete) described_class.move(s3_file, fs_dest) end end describe 'filesystem source to s3 dest', s3: true do - it 'copies file from source to dest' do - expect(@dest_object).to receive(:upload_file).with(fs_file.uri.path) - described_class.move(fs_file, s3_dest) - end - - it 'deletes file after copying' do + it 'moves file from source to dest' do allow(@dest_object).to receive(:upload_file).and_return(true) + expect(@dest_object).to receive(:upload_file).with(fs_file.uri.path) expect(FileUtils).to receive(:rm).with(fs_file.uri.path) described_class.move(fs_file, s3_dest) end