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.

apps/dav/lib/Connector/Sabre/FilesPlugin.php

@@ -1,6 +1,7 @@
 <?php
 /**
  * @author Björn Schießle <[email protected]>
+ * @author Joas Schilling <[email protected]>
  * @author Lukas Reschke <[email protected]>
  * @author Morris Jobke <[email protected]>
  * @author Robin Appelman <[email protected]>
@@ -9,7 +10,7 @@
  * @author Thomas Müller <[email protected]>
  * @author Vincent Petry <[email protected]>
  *
- * @copyright Copyright (c) 2016, ownCloud, Inc.
+ * @copyright Copyright (c) 2016, ownCloud GmbH.
  * @license AGPL-3.0
  *
  * This code is free software: you can redistribute it and/or modify
@@ -29,7 +30,6 @@
 namespace OCA\DAV\Connector\Sabre;
 
 use OC\Files\View;
-use OCA\DAV\Upload\FutureFile;
 use OCP\Files\ForbiddenException;
 use Sabre\DAV\Exception\Forbidden;
 use Sabre\DAV\Exception\NotFound;
@@ -157,8 +157,6 @@ public function initialize(\Sabre\DAV\Server $server) {
 		$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']);
@@ -417,52 +415,4 @@ public function sendFileIdHeader($filePath, \Sabre\DAV\INode $node = null) {
 			}
 		}
 	}
-
-	/**
-	 * 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;
-		}
-	}
 }

apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php

@@ -550,85 +550,4 @@ public function testDownloadHeaders($isClumsyAgent, $contentDispositionHeader) {
 
 		$this->plugin->httpGet($request, $response);
 	}
-
-	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);
-	}
 }

apps/files/js/file-upload.js

@@ -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?

apps/files/js/templates/detailsview.handlebars.js

@@ -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});
+})();