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

Download hosted files to tmp and move to dropbox when uploading #6159

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
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
31 changes: 1 addition & 30 deletions app/jobs/master_file_management_jobs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}"

Expand All @@ -57,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
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}"
Expand Down
11 changes: 6 additions & 5 deletions app/models/master_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -694,14 +694,15 @@ 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)
FileMover.move(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
Expand Down
20 changes: 16 additions & 4 deletions app/services/file_locator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -48,6 +48,7 @@ def download_url

def initialize(source, opts = {})
@source = source
@filename = opts[:filename]
@auth_header = opts[:auth_header]
end

Expand Down Expand Up @@ -89,17 +90,28 @@ 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
ensure
@local_file.close
end

def exist?
case uri.scheme
when 's3'
Expand Down
65 changes: 65 additions & 0 deletions app/services/file_mover.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# 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
masaball marked this conversation as resolved.
Show resolved Hide resolved
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_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
43 changes: 39 additions & 4 deletions spec/models/master_file_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
81 changes: 81 additions & 0 deletions spec/services/file_mover_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
# 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
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, 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(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 source to s3 dest', s3: true 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)
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 'moves file from source to dest' do
expect(@source_object).to receive(:download_file).with(fs_dest.uri.path)
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 '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
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

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