Skip to content

Commit

Permalink
Handle remote uploads as file transfers
Browse files Browse the repository at this point in the history
Apache expects a response to the request within 60 seconds
(ProxyTimeout). Avoid that by responding with a transfer for the upload,
which the browser will query the status of instead.
  • Loading branch information
robinkar committed Aug 20, 2024
1 parent 3c7cdbd commit 508c2ce
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 8 deletions.
11 changes: 9 additions & 2 deletions apps/dashboard/app/controllers/files_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,16 @@ def upload
parse_path(upload_path)
validate_path!

@path.handle_upload(params[:file].tempfile)
# Need to remove the tempfile from list of Rack tempfiles to prevent it
# being cleaned up once request completes since Rclone uses the files.
request.env[Rack::RACK_TEMPFILES].reject! { |f| f.path == params[:file].tempfile.path } unless posix_file?

render json: {}
@transfer = @path.handle_upload(params[:file].tempfile)
if @transfer
render "transfers/show"
else
render json: {}
end
rescue AllowlistPolicy::Forbidden => e
render json: { error_message: e.message }, status: :forbidden
rescue Errno::EACCES => e
Expand Down
1 change: 1 addition & 0 deletions apps/dashboard/app/javascript/files/file_ops.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const EVENTNAME = {


let fileOps = null;
export {fileOps};

jQuery(function() {
fileOps = new FileOps();
Expand Down
14 changes: 14 additions & 0 deletions apps/dashboard/app/javascript/files/uppy_ops.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import XHRUpload from '@uppy/xhr-upload'
import _ from 'lodash';
import {CONTENTID, EVENTNAME as DATATABLE_EVENTNAME} from './data_table.js';
import { maxFileSize, csrfToken, uppyLocale } from '../config.js';
import { fileOps } from './file_ops.js';

let uppy = null;

Expand Down Expand Up @@ -113,6 +114,7 @@ jQuery(function() {

uppy.on('complete', (result) => {
if(result.successful.length > 0){
result.successful.forEach(handleUploadSuccess);
reloadTable();
}
});
Expand Down Expand Up @@ -192,3 +194,15 @@ function reloadTable() {
$(CONTENTID).trigger(DATATABLE_EVENTNAME.reloadTable,{});
}

// Uploads may return the status of a transfer for remote uploads.
function handleUploadSuccess(result) {
// These extra checks might not be needed.
const body = result?.response?.body;
if (!body || !(typeof body === "object" && !Array.isArray(body) && body !== null)) {
return;
}
if (Object.keys(body).length > 0 && !body.completed) {
fileOps.reportTransfer(body);
}
}

11 changes: 9 additions & 2 deletions apps/dashboard/app/models/remote_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,15 @@ def write(content)
end

def handle_upload(tempfile)
# FIXME: upload to the remote asynchronously
RcloneUtil.moveto(remote, path, tempfile.path)
# Start a transfer that moves the Rack tempfile to the remote.
transfer = RemoteTransfer.build(
action: "mv",
files: { tempfile.path => path.to_s },
src_remote: RcloneUtil::LOCAL_FS_NAME,
dest_remote: remote,
tempfile: tempfile
)
transfer.tap(&:perform_later)
end

def mime_type
Expand Down
7 changes: 3 additions & 4 deletions apps/dashboard/app/models/remote_transfer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,24 +43,23 @@ class RemoteTransfer < Transfer
end
end

attr_accessor :src_remote, :dest_remote, :filesizes, :transferred
attr_accessor :src_remote, :dest_remote, :tempfile, :filesizes, :transferred

class << self
def transfers
# all transfers stored in the Transfer class
Transfer.transfers
end

def build(action:, files:, src_remote:, dest_remote:)
def build(action:, files:, src_remote:, dest_remote:, tempfile: nil)
if files.is_a?(Array)
# rm action will want to provide an array of files
# so if it is an Array we convert it to a hash:
#
# convert [a1, a2, a3] to {a1 => nil, a2 => nil, a3 => nil}
files = Hash[files.map { |f| [f, nil] }].with_indifferent_access
end

self.new(action: action, files: files, src_remote: src_remote, dest_remote: dest_remote)
self.new(action: action, files: files, src_remote: src_remote, dest_remote: dest_remote, tempfile: tempfile)
end
end

Expand Down

0 comments on commit 508c2ce

Please sign in to comment.