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

@@ -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}"

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
 

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

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

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