Merge pull request #33697 from nextcloud/fix/improve-bulk-upload

Improve bulk upload and add a config switch for it

5 files changed, 54 insertions, 48 deletions

diff --git a/apps/dav/lib/BulkUpload/BulkUploadPlugin.php b/apps/dav/lib/BulkUpload/BulkUploadPlugin.php
index 6ab4b21a74b..0d5cce88d0d 100644
--- a/apps/dav/lib/BulkUpload/BulkUploadPlugin.php
+++ b/apps/dav/lib/BulkUpload/BulkUploadPlugin.php
@@ -3,6 +3,7 @@
  * @copyright Copyright (c) 2021, Louis Chemineau <louis@chmn.me>
  *
  * @author Louis Chemineau <louis@chmn.me>
+ * @author Côme Chilliet <come.chilliet@nextcloud.com>
  *
  * @license AGPL-3.0
  *
@@ -27,19 +28,19 @@ use Sabre\DAV\Server;
 use Sabre\DAV\ServerPlugin;
 use Sabre\HTTP\RequestInterface;
 use Sabre\HTTP\ResponseInterface;
+use OCP\Files\DavUtil;
 use OCP\Files\Folder;
 use OCP\AppFramework\Http;
 use OCA\DAV\Connector\Sabre\MtimeSanitizer;
 
 class BulkUploadPlugin extends ServerPlugin {
+	private Folder $userFolder;
+	private LoggerInterface $logger;
 
-	/** @var Folder */
-	private $userFolder;
-
-	/** @var LoggerInterface */
-	private $logger;
-
-	public function __construct(Folder $userFolder, LoggerInterface $logger) {
+	public function __construct(
+		Folder $userFolder,
+		LoggerInterface $logger
+	) {
 		$this->userFolder = $userFolder;
 		$this->logger = $logger;
 	}
@@ -95,6 +96,8 @@ class BulkUploadPlugin extends ServerPlugin {
 				$writtenFiles[$headers['x-file-path']] = [
 					"error" => false,
 					"etag" => $node->getETag(),
+					"fileid" => DavUtil::getDavFileId($node->getId()),
+					"permissions" => DavUtil::getDavPermissions($node),
 				];
 			} catch (\Exception $e) {
 				$this->logger->error($e->getMessage(), ['path' => $headers['x-file-path']]);
diff --git a/apps/dav/lib/Capabilities.php b/apps/dav/lib/Capabilities.php
index 41d1b983587..b8096d3395a 100644
--- a/apps/dav/lib/Capabilities.php
+++ b/apps/dav/lib/Capabilities.php
@@ -4,6 +4,7 @@
  *
  * @author Thomas Müller <thomas.mueller@tmit.eu>
  * @author Louis Chemineau <louis@chmn.me>
+ * @author Côme Chilliet <come.chilliet@nextcloud.com>
  *
  * @license AGPL-3.0
  *
@@ -23,15 +24,24 @@
 namespace OCA\DAV;
 
 use OCP\Capabilities\ICapability;
+use OCP\IConfig;
 
 class Capabilities implements ICapability {
+	private IConfig $config;
+
+	public function __construct(IConfig $config) {
+		$this->config = $config;
+	}
+
 	public function getCapabilities() {
-		return [
+		$capabilities = [
 			'dav' => [
 				'chunking' => '1.0',
-				// disabled because of https://github.com/nextcloud/desktop/issues/4243
-				// 'bulkupload' => '1.0',
 			]
 		];
+		if ($this->config->getSystemValueBool('bulkupload.enabled', true)) {
+			$capabilities['dav']['bulkupload'] = '1.0';
+		}
+		return $capabilities;
 	}
 }
diff --git a/apps/dav/lib/Connector/Sabre/Node.php b/apps/dav/lib/Connector/Sabre/Node.php
index 87f2fea394f..5fb811ad1ab 100644
--- a/apps/dav/lib/Connector/Sabre/Node.php
+++ b/apps/dav/lib/Connector/Sabre/Node.php
@@ -38,9 +38,9 @@ namespace OCA\DAV\Connector\Sabre;
 use OC\Files\Mount\MoveableMount;
 use OC\Files\Node\File;
 use OC\Files\Node\Folder;
-use OC\Files\Storage\Wrapper\Wrapper;
 use OC\Files\View;
 use OCA\DAV\Connector\Sabre\Exception\InvalidPath;
+use OCP\Files\DavUtil;
 use OCP\Files\FileInfo;
 use OCP\Files\IRootFolder;
 use OCP\Files\StorageNotAvailableException;
@@ -252,10 +252,8 @@ abstract class Node implements \Sabre\DAV\INode {
 	 * @return string|null
 	 */
 	public function getFileId() {
-		if ($this->info->getId()) {
-			$instanceId = \OC_Util::getInstanceId();
-			$id = sprintf('%08d', $this->info->getId());
-			return $id . $instanceId;
+		if ($id = $this->info->getId()) {
+			return DavUtil::getDavFileId($id);
 		}
 
 		return null;
@@ -381,35 +379,7 @@ abstract class Node implements \Sabre\DAV\INode {
 	 * @return string
 	 */
 	public function getDavPermissions() {
-		$p = '';
-		if ($this->info->isShared()) {
-			$p .= 'S';
-		}
-		if ($this->info->isShareable()) {
-			$p .= 'R';
-		}
-		if ($this->info->isMounted()) {
-			$p .= 'M';
-		}
-		if ($this->info->isReadable()) {
-			$p .= 'G';
-		}
-		if ($this->info->isDeletable()) {
-			$p .= 'D';
-		}
-		if ($this->info->isUpdateable()) {
-			$p .= 'NV'; // Renameable, Moveable
-		}
-		if ($this->info->getType() === \OCP\Files\FileInfo::TYPE_FILE) {
-			if ($this->info->isUpdateable()) {
-				$p .= 'W';
-			}
-		} else {
-			if ($this->info->isCreatable()) {
-				$p .= 'CK';
-			}
-		}
-		return $p;
+		return DavUtil::getDavPermissions($this->info);
 	}
 
 	public function getOwner() {
diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php
index 2cfcb3f5393..f98dba22925 100644
--- a/apps/dav/lib/Server.php
+++ b/apps/dav/lib/Server.php
@@ -313,7 +313,10 @@ class Server {
 						$view
 					));
 					$this->server->addPlugin(
-						new BulkUploadPlugin($userFolder, $logger)
+						new BulkUploadPlugin(
+							$userFolder,
+							$logger
+						)
 					);
 				}
 				$this->server->addPlugin(new \OCA\DAV\CalDAV\BirthdayCalendar\EnablePlugin(
diff --git a/apps/dav/tests/unit/CapabilitiesTest.php b/apps/dav/tests/unit/CapabilitiesTest.php
index 7abfd08854b..6a9a0c1180a 100644
--- a/apps/dav/tests/unit/CapabilitiesTest.php
+++ b/apps/dav/tests/unit/CapabilitiesTest.php
@@ -24,6 +24,7 @@
 namespace OCA\DAV\Tests\unit;
 
 use OCA\DAV\Capabilities;
+use OCP\IConfig;
 use Test\TestCase;
 
 /**
@@ -31,12 +32,31 @@ use Test\TestCase;
  */
 class CapabilitiesTest extends TestCase {
 	public function testGetCapabilities() {
-		$capabilities = new Capabilities();
+		$config = $this->createMock(IConfig::class);
+		$config->expects($this->once())
+			->method('getSystemValueBool')
+			->with('bulkupload.enabled', $this->isType('bool'))
+			->willReturn(false);
+		$capabilities = new Capabilities($config);
 		$expected = [
 			'dav' => [
 				'chunking' => '1.0',
-				// disabled because of https://github.com/nextcloud/desktop/issues/4243
-				// 'bulkupload' => '1.0',
+			],
+		];
+		$this->assertSame($expected, $capabilities->getCapabilities());
+	}
+
+	public function testGetCapabilitiesWithBulkUpload() {
+		$config = $this->createMock(IConfig::class);
+		$config->expects($this->once())
+			->method('getSystemValueBool')
+			->with('bulkupload.enabled', $this->isType('bool'))
+			->willReturn(true);
+		$capabilities = new Capabilities($config);
+		$expected = [
+			'dav' => [
+				'chunking' => '1.0',
+				'bulkupload' => '1.0',
 			],
 		];
 		$this->assertSame($expected, $capabilities->getCapabilities());