Upload autorename on client side

Removes the need for POST to collection which would hit against upload
limits.

The client tries to auto rename the file by adding a suffix "(2)".
It tries to use the file list on the client side to guess a
suitable name. In case a file still cannot be uploaded and creates a
conflict, which can happen when the file was concurrently uploaded, the
logic will continue increasing the suffix.
This commit is contained in:
Vincent Petry 2016-09-09 12:19:29 +02:00 committed by Roeland Jago Douma
parent c1feae1684
commit 6a4ea2c15a
No known key found for this signature in database
GPG Key ID: 1E152838F164D13B
5 changed files with 139 additions and 166 deletions

View File

@ -172,8 +172,6 @@ class FilesPlugin extends ServerPlugin {
$this->server = $server;
$this->server->on('propFind', array($this, 'handleGetProperties'));
$this->server->on('propPatch', array($this, 'handleUpdateProperties'));
// RFC5995 to add file to the collection with a suggested name
$this->server->on('method:POST', [$this, 'httpPost']);
$this->server->on('afterBind', array($this, 'sendFileIdHeader'));
$this->server->on('afterWriteContent', array($this, 'sendFileIdHeader'));
$this->server->on('afterMethod:GET', [$this,'httpGet']);
@ -435,52 +433,4 @@ class FilesPlugin extends ServerPlugin {
}
}
}
/**
* POST operation on directories to create a new file
* with suggested name
*
* @param RequestInterface $request request object
* @param ResponseInterface $response response object
* @return null|false
*/
public function httpPost(RequestInterface $request, ResponseInterface $response) {
// TODO: move this to another plugin ?
if (!\OC::$CLI && !\OC::$server->getRequest()->passesCSRFCheck()) {
throw new BadRequest('Invalid CSRF token');
}
list($parentPath, $name) = \Sabre\HTTP\URLUtil::splitPath($request->getPath());
// Making sure the parent node exists and is a directory
$node = $this->tree->getNodeForPath($parentPath);
if ($node instanceof Directory) {
// no Add-Member found
if (empty($name) || $name[0] !== '&') {
// suggested name required
throw new BadRequest('Missing suggested file name');
}
$name = substr($name, 1);
if (empty($name)) {
// suggested name required
throw new BadRequest('Missing suggested file name');
}
// make sure the name is unique
$name = basename(\OC_Helper::buildNotExistingFileNameForView($parentPath, $name, $this->fileView));
$node->createFile($name, $request->getBodyAsStream());
list($parentUrl, ) = \Sabre\HTTP\URLUtil::splitPath($request->getUrl());
$response->setHeader('Content-Location', $parentUrl . '/' . rawurlencode($name));
// created
$response->setStatus(201);
return false;
}
}
}

View File

@ -547,85 +547,4 @@ class FilesPluginTest extends TestCase {
$this->assertEquals("false", $propFind->get(self::HAS_PREVIEW_PROPERTYNAME));
}
public function postCreateFileProvider() {
$baseUrl = 'http://example.com/owncloud/remote.php/webdav/subdir/';
return [
['test.txt', 'some file.txt', 'some file.txt', $baseUrl . 'some%20file.txt'],
['some file.txt', 'some file.txt', 'some file (2).txt', $baseUrl . 'some%20file%20%282%29.txt'],
];
}
/**
* @dataProvider postCreateFileProvider
*/
public function testPostWithAddMember($existingFile, $wantedName, $deduplicatedName, $expectedLocation) {
$request = $this->getMock('Sabre\HTTP\RequestInterface');
$response = $this->getMock('Sabre\HTTP\ResponseInterface');
$request->expects($this->any())
->method('getUrl')
->will($this->returnValue('http://example.com/owncloud/remote.php/webdav/subdir/&' . $wantedName));
$request->expects($this->any())
->method('getPath')
->will($this->returnValue('/subdir/&' . $wantedName));
$request->expects($this->once())
->method('getBodyAsStream')
->will($this->returnValue(fopen('data://text/plain,hello', 'r')));
$this->view->expects($this->any())
->method('file_exists')
->will($this->returnCallback(function($path) use ($existingFile) {
return ($path === '/subdir/' . $existingFile);
}));
$node = $this->createTestNode('\OCA\DAV\Connector\Sabre\Directory', '/subdir');
$node->expects($this->once())
->method('createFile')
->with($deduplicatedName, $this->isType('resource'));
$response->expects($this->once())
->method('setStatus')
->with(201);
$response->expects($this->once())
->method('setHeader')
->with('Content-Location', $expectedLocation);
$this->assertFalse($this->plugin->httpPost($request, $response));
}
public function testPostOnNonDirectory() {
$request = $this->getMock('Sabre\HTTP\RequestInterface');
$response = $this->getMock('Sabre\HTTP\ResponseInterface');
$request->expects($this->any())
->method('getPath')
->will($this->returnValue('/subdir/test.txt/&abc'));
$this->createTestNode('\OCA\DAV\Connector\Sabre\File', '/subdir/test.txt');
$this->assertNull($this->plugin->httpPost($request, $response));
}
/**
* @expectedException \Sabre\DAV\Exception\BadRequest
*/
public function testPostWithoutAddMember() {
$request = $this->getMock('Sabre\HTTP\RequestInterface');
$response = $this->getMock('Sabre\HTTP\ResponseInterface');
$request->expects($this->any())
->method('getPath')
->will($this->returnValue('/subdir/&'));
$node = $this->createTestNode('\OCA\DAV\Connector\Sabre\Directory', '/subdir');
$node->expects($this->never())
->method('createFile');
$this->plugin->httpPost($request, $response);
}
}

View File

@ -100,26 +100,14 @@ OC.FileUpload.prototype = {
/**
* Return the final filename.
* Either this is the original file name or the file name
* after an autorename.
*
* @return {String} file name
*/
getFileName: function() {
// in case of autorename
// autorenamed name
if (this._newName) {
return this._newName;
}
if (this._conflictMode === OC.FileUpload.CONFLICT_MODE_AUTORENAME) {
var locationUrl = this.getResponseHeader('Content-Location');
if (locationUrl) {
this._newName = decodeURIComponent(OC.basename(locationUrl));
return this._newName;
}
}
return this.getFile().name;
},
@ -141,9 +129,20 @@ OC.FileUpload.prototype = {
return OC.joinPaths(this._targetFolder, this.getFile().relativePath || '');
},
/**
* Returns conflict resolution mode.
*
* @return {int} conflict mode
*/
getConflictMode: function() {
return this._conflictMode || OC.FileUpload.CONFLICT_MODE_DETECT;
},
/**
* Set conflict resolution mode.
* See CONFLICT_MODE_* constants.
*
* @param {int} mode conflict mode
*/
setConflictMode: function(mode) {
this._conflictMode = mode;
@ -162,6 +161,30 @@ OC.FileUpload.prototype = {
delete this.data.jqXHR;
},
/**
* Trigger autorename and append "(2)".
* Multiple calls will increment the appended number.
*/
autoRename: function() {
var name = this.getFile().name;
if (!this._renameAttempt) {
this._renameAttempt = 1;
}
var dotPos = name.lastIndexOf('.');
var extPart = '';
if (dotPos > 0) {
this._newName = name.substr(0, dotPos);
extPart = name.substr(dotPos);
} else {
this._newName = name;
}
// generate new name
this._renameAttempt++;
this._newName = this._newName + ' (' + this._renameAttempt + ')' + extPart;
},
/**
* Submit the upload
*/
@ -179,7 +202,7 @@ OC.FileUpload.prototype = {
}
if (this.uploader.fileList) {
this.data.url = this.uploader.fileList.getUploadUrl(file.name, this.getFullPath());
this.data.url = this.uploader.fileList.getUploadUrl(this.getFileName(), this.getFullPath());
}
if (!this.data.headers) {
@ -191,12 +214,9 @@ OC.FileUpload.prototype = {
this.data.type = 'PUT';
delete this.data.headers['If-None-Match'];
if (this._conflictMode === OC.FileUpload.CONFLICT_MODE_DETECT) {
if (this._conflictMode === OC.FileUpload.CONFLICT_MODE_DETECT
|| this._conflictMode === OC.FileUpload.CONFLICT_MODE_AUTORENAME) {
this.data.headers['If-None-Match'] = '*';
} else if (this._conflictMode === OC.FileUpload.CONFLICT_MODE_AUTORENAME) {
// POST to parent folder, with slug
this.data.type = 'POST';
this.data.url = this.uploader.fileList.getUploadUrl('&' + file.name, this.getFullPath());
}
if (file.lastModified) {
@ -212,18 +232,6 @@ OC.FileUpload.prototype = {
'Basic ' + btoa(userName + ':' + (password || ''));
}
if (!this.uploader.isXHRUpload()) {
data.formData = [];
// pass headers as parameters
data.formData.push({name: 'headers', value: JSON.stringify(this.data.headers)});
data.formData.push({name: 'requesttoken', value: OC.requestToken});
if (data.type === 'POST') {
// still add the method to the URL
data.url += '?_method=POST';
}
}
var chunkFolderPromise;
if ($.support.blobSlice
&& this.uploader.fileUploadParam.maxChunkSize
@ -491,6 +499,14 @@ OC.Uploader.prototype = _.extend({
//show "file already exists" dialog
var self = this;
var file = fileUpload.getFile();
// already attempted autorename but the server said the file exists ? (concurrently added)
if (fileUpload.getConflictMode() === OC.FileUpload.CONFLICT_MODE_AUTORENAME) {
// attempt another autorename, defer to let the current callback finish
_.defer(function() {
self.onAutorename(fileUpload);
});
return;
}
// retrieve more info about this file
this.filesClient.getFileInfo(fileUpload.getFullPath()).then(function(status, fileInfo) {
var original = fileInfo;
@ -603,6 +619,13 @@ OC.Uploader.prototype = _.extend({
onAutorename:function(upload) {
this.log('autorename', null, upload);
upload.setConflictMode(OC.FileUpload.CONFLICT_MODE_AUTORENAME);
do {
upload.autoRename();
// if file known to exist on the client side, retry
} while (this.fileList && this.fileList.inList(upload.getFileName()));
// resubmit upload
this.submitUploads([upload]);
},
_trace:false, //TODO implement log handler for JS per class?

View File

@ -0,0 +1,28 @@
(function() {
var template = Handlebars.template, templates = OCA.Files.Templates = OCA.Files.Templates || {};
templates['detailsview'] = template({"1":function(container,depth0,helpers,partials,data) {
var stack1;
return "<ul class=\"tabHeaders\">\n"
+ ((stack1 = helpers.each.call(depth0 != null ? depth0 : {},(depth0 != null ? depth0.tabHeaders : depth0),{"name":"each","hash":{},"fn":container.program(2, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "")
+ "</ul>\n";
},"2":function(container,depth0,helpers,partials,data) {
var helper, alias1=depth0 != null ? depth0 : {}, alias2=helpers.helperMissing, alias3="function", alias4=container.escapeExpression;
return " <li class=\"tabHeader\" data-tabid=\""
+ alias4(((helper = (helper = helpers.tabId || (depth0 != null ? depth0.tabId : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"tabId","hash":{},"data":data}) : helper)))
+ "\" data-tabindex=\""
+ alias4(((helper = (helper = helpers.tabIndex || (depth0 != null ? depth0.tabIndex : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"tabIndex","hash":{},"data":data}) : helper)))
+ "\">\n <a href=\"#\">"
+ alias4(((helper = (helper = helpers.label || (depth0 != null ? depth0.label : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"label","hash":{},"data":data}) : helper)))
+ "</a>\n </li>\n";
},"compiler":[7,">= 4.0.0"],"main":function(container,depth0,helpers,partials,data) {
var stack1, helper, alias1=depth0 != null ? depth0 : {};
return "<div class=\"detailFileInfoContainer\"></div>\n"
+ ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.tabHeaders : depth0),{"name":"if","hash":{},"fn":container.program(1, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "")
+ "<div class=\"tabsContainer\">\n</div>\n<a class=\"close icon-close\" href=\"#\" alt=\""
+ container.escapeExpression(((helper = (helper = helpers.closeLabel || (depth0 != null ? depth0.closeLabel : depth0)) != null ? helper : helpers.helperMissing),(typeof helper === "function" ? helper.call(alias1,{"name":"closeLabel","hash":{},"data":data}) : helper)))
+ "\"></a>\n";
},"useData":true});
})();

View File

@ -144,6 +144,10 @@ describe('OC.Upload tests', function() {
uploader = new OC.Uploader($dummyUploader, {
fileList: fileList
});
var deferred = $.Deferred();
conflictDialogStub.returns(deferred.promise());
deferred.resolve();
});
afterEach(function() {
conflictDialogStub.restore();
@ -158,10 +162,6 @@ describe('OC.Upload tests', function() {
expect(result[1].submit.calledOnce).toEqual(true);
});
it('shows conflict dialog when no client side conflict', function() {
var deferred = $.Deferred();
conflictDialogStub.returns(deferred.promise());
deferred.resolve();
var result = addFiles(uploader, [
{name: 'conflict.txt'},
{name: 'conflict2.txt'},
@ -185,5 +185,58 @@ describe('OC.Upload tests', function() {
expect(result[1].submit.calledOnce).toEqual(false);
expect(result[2].submit.calledOnce).toEqual(true);
});
it('cancels upload when skipping file in conflict mode', function() {
var fileData = {name: 'conflict.txt'};
var uploadData = addFiles(uploader, [
fileData
]);
var upload = new OC.FileUpload(uploader, uploadData[0]);
var deleteStub = sinon.stub(upload, 'deleteUpload');
uploader.onSkip(upload);
expect(deleteStub.calledOnce).toEqual(true);
});
it('overwrites file when choosing replace in conflict mode', function() {
var fileData = {name: 'conflict.txt'};
var uploadData = addFiles(uploader, [
fileData
]);
expect(uploadData[0].submit.notCalled).toEqual(true);
var upload = new OC.FileUpload(uploader, uploadData[0]);
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() {
// needed for _.defer call
var clock = sinon.useFakeTimers();
var fileData = {name: 'conflict.txt'};
var uploadData = addFiles(uploader, [
fileData
]);
expect(uploadData[0].submit.notCalled).toEqual(true);
var upload = new OC.FileUpload(uploader, uploadData[0]);
var getResponseStatusStub = sinon.stub(upload, 'getResponseStatus');
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();
});
});
});