upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-04-22 23:03:00 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 78

Merge pull request #14251 from tomasz-grobelny/upload_reliability_improvements

Browse source 
Limit number of simultaneous MKCOL requests to server to increase upload reliability
This commit is contained in:
Roeland Jago Douma 2019-02-26 11:33:44 +01:00 committed by GitHub
commit 0b7b32e808
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 120 additions and 68 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

41
apps/files/js/file-upload.js
View file

@ -194,6 +194,9 @@ OC.FileUpload.prototype = {
var data = this.data;
var file = this.getFile();
if (self.aborted === true) {
return $.Deferred().resolve().promise();
}
// it was a folder upload, so make sure the parent directory exists already
var folderPromise;
if (file.relativePath) {
@ -243,8 +246,10 @@ OC.FileUpload.prototype = {
}
// wait for creation of the required directory before uploading
$.when(folderPromise, chunkFolderPromise).then(function() {
data.submit();
return Promise.all([folderPromise, chunkFolderPromise]).then(function() {
if (self.aborted !== true) {
data.submit();
}
}, function() {
self.abort();
});
@ -295,6 +300,7 @@ OC.FileUpload.prototype = {
}
this.data.abort();
this.deleteUpload();
this.aborted = true;
},
/**
@ -317,7 +323,7 @@ OC.FileUpload.prototype = {
if (response.errorThrown) {
// attempt parsing Sabre exception is available
var xml = response.jqXHR.responseXML;
if (xml.documentElement.localName === 'error' && xml.documentElement.namespaceURI === 'DAV:') {
if (xml && xml.documentElement.localName === 'error' && xml.documentElement.namespaceURI === 'DAV:') {
var messages = xml.getElementsByTagNameNS('http://sabredav.org/ns', 'message');
var exceptions = xml.getElementsByTagNameNS('http://sabredav.org/ns', 'exception');
if (messages.length) {
@ -554,7 +560,15 @@ OC.Uploader.prototype = _.extend({
var self = this;
_.each(uploads, function(upload) {
self._uploads[upload.data.uploadId] = upload;
upload.submit();
});
self.totalToUpload = _.reduce(uploads, function(memo, upload) { return memo+upload.getFile().size; }, 0);
var semaphore = new OCA.Files.Semaphore(5);
var promises = _.map(uploads, function(upload) {
return semaphore.acquire().then(function(){
return upload.submit().then(function(){
semaphore.release();
});
});
});
},
@ -861,7 +875,6 @@ OC.Uploader.prototype = _.extend({
autoUpload: false,
sequentialUploads: false,
limitConcurrentUploads: 10,
//singleFileUploads is on by default, so the data.files array will always have length 1
/**
* on first add of every selection
* - check all files of originalFiles array with files in dir
@ -886,15 +899,6 @@ OC.Uploader.prototype = _.extend({
// can't link directly due to jQuery not liking cyclic deps on its ajax object
data.uploadId = upload.getId();
// we need to collect all data upload objects before
// starting the upload so we can check their existence
// and set individual conflict actions. Unfortunately,
// there is only one variable that we can use to identify
// the selection a data upload is part of, so we have to
// collect them in data.originalFiles turning
// singleFileUploads off is not an option because we want
// to gracefully handle server errors like 'already exists'
// create a container where we can store the data objects
if ( ! data.originalFiles.selection ) {
// initialize selection and remember number of files to upload
@ -1030,7 +1034,7 @@ OC.Uploader.prototype = _.extend({
self.removeUpload(upload);
if (data.textStatus === 'abort') {
if (data.textStatus === 'abort' || data.errorThrown === 'abort') {
self.showUploadCancelMessage();
} else if (status === 412) {
// file already exists
@ -1134,14 +1138,15 @@ OC.Uploader.prototype = _.extend({
});
fileupload.on('fileuploadprogressall', function(e, data) {
self.log('progress handle fileuploadprogressall', e, data);
var progress = (data.loaded / data.total) * 100;
var total = self.totalToUpload;
var progress = (data.loaded / total) * 100;
var thisUpdate = new Date().getTime();
var diffUpdate = (thisUpdate - lastUpdate)/1000; // eg. 2s
lastUpdate = thisUpdate;
var diffSize = data.loaded - lastSize;
lastSize = data.loaded;
diffSize = diffSize / diffUpdate; // apply timing factor, eg. 1MiB/2s = 0.5MiB/s, unit is byte per second
var remainingSeconds = ((data.total - data.loaded) / diffSize);
var remainingSeconds = ((total - data.loaded) / diffSize);
if(remainingSeconds >= 0) {
bufferTotal = bufferTotal - (buffer[bufferIndex]) + remainingSeconds;
buffer[bufferIndex] = remainingSeconds; //buffer to make it smoother
@ -1164,7 +1169,7 @@ OC.Uploader.prototype = _.extend({
}
self._setProgressBarText(h, h, t('files', '{loadedSize} of {totalSize} ({bitrate})' , {
loadedSize: humanFileSize(data.loaded),
totalSize: humanFileSize(data.total),
totalSize: humanFileSize(total),
bitrate: humanFileSize(data.bitrate / 8) + '/s'
}));
self._setProgressBarValue(progress);

3
apps/files/js/operationprogressbar.js
View file

@ -32,8 +32,9 @@
},
hideCancelButton: function() {
var self = this;
$('#uploadprogresswrapper .stop').fadeOut(function() {
this.$el.trigger(new $.Event('resized'));
self.$el.trigger(new $.Event('resized'));
});
},

144
apps/files/tests/js/fileUploadSpec.js
View file

@ -84,33 +84,41 @@ describe('OC.Upload tests', function() {
}
describe('Adding files for upload', function() {
it('adds file when size is below limits', function() {
it('adds file when size is below limits', function(done) {
var result = addFiles(uploader, [testFile]);
expect(result[0]).not.toEqual(null);
expect(result[0].submit.calledOnce).toEqual(true);
result[0].submit.callsFake(function(){
expect(result[0].submit.calledOnce).toEqual(true);
done();
});
});
it('adds file when free space is unknown', function() {
it('adds file when free space is unknown', function(done) {
var result;
$('#free_space').val(-2);
result = addFiles(uploader, [testFile]);
expect(result[0]).not.toEqual(null);
expect(result[0].submit.calledOnce).toEqual(true);
expect(failStub.notCalled).toEqual(true);
result[0].submit.callsFake(function(){
expect(result[0].submit.calledOnce).toEqual(true);
expect(failStub.notCalled).toEqual(true);
done();
});
});
it('does not add file if it exceeds free space', function() {
it('does not add file if it exceeds free space', function(done) {
var result;
$('#free_space').val(1000);
failStub.callsFake(function(){
expect(failStub.calledOnce).toEqual(true);
expect(failStub.getCall(0).args[1].textStatus).toEqual('notenoughspace');
expect(failStub.getCall(0).args[1].errorThrown).toEqual(
'Not enough free space, you are uploading 5 KB but only 1000 B is left'
);
setTimeout(done, 0);
});
result = addFiles(uploader, [testFile]);
expect(result[0]).toEqual(null);
expect(failStub.calledOnce).toEqual(true);
expect(failStub.getCall(0).args[1].textStatus).toEqual('notenoughspace');
expect(failStub.getCall(0).args[1].errorThrown).toEqual(
'Not enough free space, you are uploading 5 KB but only 1000 B is left'
);
});
});
describe('Upload conflicts', function() {
@ -158,38 +166,60 @@ describe('OC.Upload tests', function() {
fileList.destroy();
});
it('does not show conflict dialog when no client side conflict', function() {
it('does not show conflict dialog when no client side conflict', function(done) {
$('#free_space').val(200000);
var counter = 0;
var fun = function() {
counter++;
if(counter != 2) {
return;
}
expect(result[0].submit.calledOnce).toEqual(true);
expect(result[1].submit.calledOnce).toEqual(true);
setTimeout(done, 0);
};
var result = addFiles(uploader, [{name: 'noconflict.txt'}, {name: 'noconflict2.txt'}]);
result[0].submit.callsFake(fun);
result[1].submit.callsFake(fun);
expect(conflictDialogStub.notCalled).toEqual(true);
expect(result[0].submit.calledOnce).toEqual(true);
expect(result[1].submit.calledOnce).toEqual(true);
});
it('shows conflict dialog when no client side conflict', function() {
it('shows conflict dialog when no client side conflict', function(done) {
var counter = 0;
conflictDialogStub.callsFake(function(){
counter++;
if(counter != 3) {
return $.Deferred().resolve().promise();
}
setTimeout(function() {
expect(conflictDialogStub.callCount).toEqual(3);
expect(conflictDialogStub.getCall(1).args[0].getFileName())
.toEqual('conflict.txt');
expect(conflictDialogStub.getCall(1).args[1])
.toEqual({ name: 'conflict.txt', mimetype: 'text/plain', directory: '/' });
expect(conflictDialogStub.getCall(1).args[2]).toEqual({ name: 'conflict.txt' });
// yes, the dialog must be called several times...
expect(conflictDialogStub.getCall(2).args[0].getFileName()).toEqual('conflict2.txt');
expect(conflictDialogStub.getCall(2).args[1])
.toEqual({ name: 'conflict2.txt', mimetype: 'text/plain', directory: '/' });
expect(conflictDialogStub.getCall(2).args[2]).toEqual({ name: 'conflict2.txt' });
expect(result[0].submit.calledOnce).toEqual(false);
expect(result[1].submit.calledOnce).toEqual(false);
expect(result[2].submit.calledOnce).toEqual(true);
done();
}, 0);
});
var result = addFiles(uploader, [
{name: 'conflict.txt'},
{name: 'conflict2.txt'},
{name: 'noconflict.txt'}
]);
expect(conflictDialogStub.callCount).toEqual(3);
expect(conflictDialogStub.getCall(1).args[0].getFileName())
.toEqual('conflict.txt');
expect(conflictDialogStub.getCall(1).args[1])
.toEqual({ name: 'conflict.txt', mimetype: 'text/plain', directory: '/' });
expect(conflictDialogStub.getCall(1).args[2]).toEqual({ name: 'conflict.txt' });
// yes, the dialog must be called several times...
expect(conflictDialogStub.getCall(2).args[0].getFileName()).toEqual('conflict2.txt');
expect(conflictDialogStub.getCall(2).args[1])
.toEqual({ name: 'conflict2.txt', mimetype: 'text/plain', directory: '/' });
expect(conflictDialogStub.getCall(2).args[2]).toEqual({ name: 'conflict2.txt' });
expect(result[0].submit.calledOnce).toEqual(false);
expect(result[1].submit.calledOnce).toEqual(false);
expect(result[2].submit.calledOnce).toEqual(true);
});
it('cancels upload when skipping file in conflict mode', function() {
it('cancels upload when skipping file in conflict mode', function(done) {
var fileData = {name: 'conflict.txt'};
var uploadData = addFiles(uploader, [
fileData
@ -197,11 +227,14 @@ describe('OC.Upload tests', function() {
var upload = new OC.FileUpload(uploader, uploadData[0]);
var deleteStub = sinon.stub(upload, 'deleteUpload');
deleteStub.callsFake(function(){
expect(deleteStub.calledOnce).toEqual(true);
done();
});
uploader.onSkip(upload);
expect(deleteStub.calledOnce).toEqual(true);
});
it('overwrites file when choosing replace in conflict mode', function() {
it('overwrites file when choosing replace in conflict mode', function(done) {
var fileData = {name: 'conflict.txt'};
var uploadData = addFiles(uploader, [
fileData
@ -210,12 +243,14 @@ describe('OC.Upload tests', function() {
expect(uploadData[0].submit.notCalled).toEqual(true);
var upload = new OC.FileUpload(uploader, uploadData[0]);
uploadData[0].submit.callsFake(function(){
expect(upload.getConflictMode()).toEqual(OC.FileUpload.CONFLICT_MODE_OVERWRITE);
expect(uploadData[0].submit.callCount).toEqual(1);
done();
});
uploader.onReplace(upload);
expect(upload.getConflictMode()).toEqual(OC.FileUpload.CONFLICT_MODE_OVERWRITE);
expect(uploadData[0].submit.calledOnce).toEqual(true);
});
it('autorenames file when choosing replace in conflict mode', function() {
it('autorenames file when choosing replace in conflict mode', function(done) {
// needed for _.defer call
var clock = sinon.useFakeTimers();
var fileData = {name: 'conflict.txt'};
@ -227,20 +262,31 @@ describe('OC.Upload tests', function() {
var upload = new OC.FileUpload(uploader, uploadData[0]);
var getResponseStatusStub = sinon.stub(upload, 'getResponseStatus');
var counter = 0;
uploadData[0].submit.callsFake(function(){
counter++;
if(counter===1)
{
expect(upload.getConflictMode()).toEqual(OC.FileUpload.CONFLICT_MODE_AUTORENAME);
expect(upload.getFileName()).toEqual('conflict (2).txt');
expect(uploadData[0].submit.calledOnce).toEqual(true);
getResponseStatusStub.returns(412);
uploader.fileUploadParam.fail.call($dummyUploader[0], {}, uploadData[0]);
clock.tick(500);
}
if(counter===2)
{
expect(upload.getFileName()).toEqual('conflict (3).txt');
expect(uploadData[0].submit.calledTwice).toEqual(true);
clock.restore();
done();
}
});
uploader.onAutorename(upload);
expect(upload.getConflictMode()).toEqual(OC.FileUpload.CONFLICT_MODE_AUTORENAME);
expect(upload.getFileName()).toEqual('conflict (2).txt');
expect(uploadData[0].submit.calledOnce).toEqual(true);
// in case of server-side conflict, tries to rename again
getResponseStatusStub.returns(412);
uploader.fileUploadParam.fail.call($dummyUploader[0], {}, uploadData[0]);
clock.tick(500);
expect(upload.getFileName()).toEqual('conflict (3).txt');
expect(uploadData[0].submit.calledTwice).toEqual(true);
clock.restore();
});
});
});