Skip to content

Commit 3850799

Browse files
committed
S3 direct multipart upload optimisation.
The change was needed to avoid doubling of upload time as the original solution does a complete re-read of the S3 object on final MOVE of NextCloud chunked upload. See nextcloud#27034 for details. Signed-off-by: Bernd.Rederlechner@t-systems.com <bernd.rederlechner@t-systems.com>
1 parent 5d04a3f commit 3850799

23 files changed

+679
-19
lines changed

apps/dav/composer/composer/autoload_classmap.php

+2
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@
243243
'OCA\\DAV\\Search\\EventsSearchProvider' => $baseDir . '/../lib/Search/EventsSearchProvider.php',
244244
'OCA\\DAV\\Search\\TasksSearchProvider' => $baseDir . '/../lib/Search/TasksSearchProvider.php',
245245
'OCA\\DAV\\Server' => $baseDir . '/../lib/Server.php',
246+
'OCA\\DAV\\Service\\CustomPropertiesService' => $baseDir . '/../lib/Service/CustomPropertiesService.php',
246247
'OCA\\DAV\\Settings\\CalDAVSettings' => $baseDir . '/../lib/Settings/CalDAVSettings.php',
247248
'OCA\\DAV\\Storage\\PublicOwnerWrapper' => $baseDir . '/../lib/Storage/PublicOwnerWrapper.php',
248249
'OCA\\DAV\\SystemTag\\SystemTagMappingNode' => $baseDir . '/../lib/SystemTag/SystemTagMappingNode.php',
@@ -255,6 +256,7 @@
255256
'OCA\\DAV\\Traits\\PrincipalProxyTrait' => $baseDir . '/../lib/Traits/PrincipalProxyTrait.php',
256257
'OCA\\DAV\\Upload\\AssemblyStream' => $baseDir . '/../lib/Upload/AssemblyStream.php',
257258
'OCA\\DAV\\Upload\\ChunkingPlugin' => $baseDir . '/../lib/Upload/ChunkingPlugin.php',
259+
'OCA\\DAV\\Upload\\ChunkingV2Plugin' => $baseDir . '/../lib/Upload/ChunkingV2Plugin.php',
258260
'OCA\\DAV\\Upload\\CleanupService' => $baseDir . '/../lib/Upload/CleanupService.php',
259261
'OCA\\DAV\\Upload\\FutureFile' => $baseDir . '/../lib/Upload/FutureFile.php',
260262
'OCA\\DAV\\Upload\\RootCollection' => $baseDir . '/../lib/Upload/RootCollection.php',

apps/dav/composer/composer/autoload_static.php

+2
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ class ComposerStaticInitDAV
258258
'OCA\\DAV\\Search\\EventsSearchProvider' => __DIR__ . '/..' . '/../lib/Search/EventsSearchProvider.php',
259259
'OCA\\DAV\\Search\\TasksSearchProvider' => __DIR__ . '/..' . '/../lib/Search/TasksSearchProvider.php',
260260
'OCA\\DAV\\Server' => __DIR__ . '/..' . '/../lib/Server.php',
261+
'OCA\\DAV\\Service\\CustomPropertiesService' => __DIR__ . '/..' . '/../lib/Service/CustomPropertiesService.php',
261262
'OCA\\DAV\\Settings\\CalDAVSettings' => __DIR__ . '/..' . '/../lib/Settings/CalDAVSettings.php',
262263
'OCA\\DAV\\Storage\\PublicOwnerWrapper' => __DIR__ . '/..' . '/../lib/Storage/PublicOwnerWrapper.php',
263264
'OCA\\DAV\\SystemTag\\SystemTagMappingNode' => __DIR__ . '/..' . '/../lib/SystemTag/SystemTagMappingNode.php',
@@ -270,6 +271,7 @@ class ComposerStaticInitDAV
270271
'OCA\\DAV\\Traits\\PrincipalProxyTrait' => __DIR__ . '/..' . '/../lib/Traits/PrincipalProxyTrait.php',
271272
'OCA\\DAV\\Upload\\AssemblyStream' => __DIR__ . '/..' . '/../lib/Upload/AssemblyStream.php',
272273
'OCA\\DAV\\Upload\\ChunkingPlugin' => __DIR__ . '/..' . '/../lib/Upload/ChunkingPlugin.php',
274+
'OCA\\DAV\\Upload\\ChunkingV2Plugin' => __DIR__ . '/..' . '/../lib/Upload/ChunkingV2Plugin.php',
273275
'OCA\\DAV\\Upload\\CleanupService' => __DIR__ . '/..' . '/../lib/Upload/CleanupService.php',
274276
'OCA\\DAV\\Upload\\FutureFile' => __DIR__ . '/..' . '/../lib/Upload/FutureFile.php',
275277
'OCA\\DAV\\Upload\\RootCollection' => __DIR__ . '/..' . '/../lib/Upload/RootCollection.php',

apps/dav/lib/BackgroundJob/UploadCleanup.php

+9-1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
namespace OCA\DAV\BackgroundJob;
3030

3131
use OC\User\NoUserException;
32+
use OCA\DAV\Service\CustomPropertiesService;
3233
use OCP\AppFramework\Utility\ITimeFactory;
3334
use OCP\BackgroundJob\IJobList;
3435
use OCP\BackgroundJob\TimedJob;
@@ -45,10 +46,14 @@ class UploadCleanup extends TimedJob {
4546
/** @var IJobList */
4647
private $jobList;
4748

48-
public function __construct(ITimeFactory $time, IRootFolder $rootFolder, IJobList $jobList) {
49+
/** @var CustomPropertiesService */
50+
private $customPropertiesService;
51+
52+
public function __construct(ITimeFactory $time, IRootFolder $rootFolder, IJobList $jobList, CustomPropertiesService $customPropertiesService) {
4953
parent::__construct($time);
5054
$this->rootFolder = $rootFolder;
5155
$this->jobList = $jobList;
56+
$this->customPropertiesService = $customPropertiesService;
5257

5358
// Run once a day
5459
$this->setInterval(60 * 60 * 24);
@@ -72,6 +77,8 @@ protected function run($argument) {
7277

7378
$files = $uploadFolder->getDirectoryListing();
7479

80+
$davPath = 'uploads/' . $uid . '/' . $uploadFolder->getName();
81+
7582
// Remove if all files have an mtime of more than a day
7683
$time = $this->time->getTime() - 60 * 60 * 24;
7784

@@ -83,6 +90,7 @@ protected function run($argument) {
8390
}, $initial);
8491

8592
if ($expire) {
93+
$this->customPropertiesService->delete($uid, $davPath);
8694
$uploadFolder->delete();
8795
$this->jobList->remove(self::class, $argument);
8896
}

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

+1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
use OCA\DAV\Connector\Sabre\Exception\FileLocked;
3939
use OCA\DAV\Connector\Sabre\Exception\Forbidden;
4040
use OCA\DAV\Connector\Sabre\Exception\InvalidPath;
41+
use OCA\DAV\Upload\FutureFile;
4142
use OCP\Files\FileInfo;
4243
use OCP\Files\ForbiddenException;
4344
use OCP\Files\InvalidPathException;

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

+8
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,14 @@ public function getInternalFileId() {
248248
return $this->info->getId();
249249
}
250250

251+
public function getInternalPath(): string {
252+
return $this->info->getInternalPath();
253+
}
254+
255+
public function getAbsoluteInternalPath(): string {
256+
return $this->info->getPath();
257+
}
258+
251259
/**
252260
* @param string $user
253261
* @return int

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

+2
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
use OC\Files\Node\Folder;
3636
use OCA\DAV\AppInfo\PluginManager;
3737
use OCA\DAV\Files\BrowserErrorPagePlugin;
38+
use OCA\DAV\Service\CustomPropertiesService;
3839
use OCP\Files\Mount\IMountManager;
3940
use OCP\IConfig;
4041
use OCP\IDBConnection;
@@ -204,6 +205,7 @@ public function createServer($baseUri,
204205
new \OCA\DAV\DAV\CustomPropertiesBackend(
205206
$objectTree,
206207
$this->databaseConnection,
208+
\OC::$server->get(CustomPropertiesService::class),
207209
$this->userSession->getUser()
208210
)
209211
)

apps/dav/lib/DAV/CustomPropertiesBackend.php

+9-6
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
namespace OCA\DAV\DAV;
2727

2828
use OCA\DAV\Connector\Sabre\Node;
29+
use OCA\DAV\Service\CustomPropertiesService;
2930
use OCP\IDBConnection;
3031
use OCP\IUser;
3132
use Sabre\DAV\PropertyStorage\Backend\BackendInterface;
@@ -63,6 +64,11 @@ class CustomPropertiesBackend implements BackendInterface {
6364
*/
6465
private $connection;
6566

67+
/**
68+
* @var CustomPropertiesService
69+
*/
70+
private $customPropertiesService;
71+
6672
/**
6773
* @var IUser
6874
*/
@@ -83,9 +89,11 @@ class CustomPropertiesBackend implements BackendInterface {
8389
public function __construct(
8490
Tree $tree,
8591
IDBConnection $connection,
92+
CustomPropertiesService $customPropertiesService,
8693
IUser $user) {
8794
$this->tree = $tree;
8895
$this->connection = $connection;
96+
$this->customPropertiesService = $customPropertiesService;
8997
$this->user = $user;
9098
}
9199

@@ -155,12 +163,7 @@ public function propPatch($path, PropPatch $propPatch) {
155163
* @param string $path path of node for which to delete properties
156164
*/
157165
public function delete($path) {
158-
$statement = $this->connection->prepare(
159-
'DELETE FROM `*PREFIX*properties` WHERE `userid` = ? AND `propertypath` = ?'
160-
);
161-
$statement->execute([$this->user->getUID(), $this->formatPath($path)]);
162-
$statement->closeCursor();
163-
166+
$this->customPropertiesService->delete($this->user->getUID(), $path);
164167
unset($this->cache[$path]);
165168
}
166169

apps/dav/lib/Server.php

+4
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,10 @@
6464
use OCA\DAV\Files\BrowserErrorPagePlugin;
6565
use OCA\DAV\Files\LazySearchBackend;
6666
use OCA\DAV\Provisioning\Apple\AppleProvisioningPlugin;
67+
use OCA\DAV\Service\CustomPropertiesService;
6768
use OCA\DAV\SystemTag\SystemTagPlugin;
6869
use OCA\DAV\Upload\ChunkingPlugin;
70+
use OCA\DAV\Upload\ChunkingV2Plugin;
6971
use OCP\EventDispatcher\IEventDispatcher;
7072
use OCP\IRequest;
7173
use OCP\SabrePluginEvent;
@@ -203,6 +205,7 @@ public function __construct(IRequest $request, $baseUri) {
203205
));
204206

205207
$this->server->addPlugin(new CopyEtagHeaderPlugin());
208+
$this->server->addPlugin(new ChunkingV2Plugin());
206209
$this->server->addPlugin(new ChunkingPlugin());
207210

208211
// allow setup of additional plugins
@@ -248,6 +251,7 @@ public function __construct(IRequest $request, $baseUri) {
248251
new CustomPropertiesBackend(
249252
$this->server->tree,
250253
\OC::$server->getDatabaseConnection(),
254+
\OC::$server->get(CustomPropertiesService::class),
251255
\OC::$server->getUserSession()->getUser()
252256
)
253257
)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
<?php
2+
/*
3+
* @copyright Copyright (c) 2021 Julius Härtl <jus@bitgrid.net>
4+
*
5+
* @author Julius Härtl <jus@bitgrid.net>
6+
*
7+
* @license GNU AGPL version 3 or any later version
8+
*
9+
* This program is free software: you can redistribute it and/or modify
10+
* it under the terms of the GNU Affero General Public License as
11+
* published by the Free Software Foundation, either version 3 of the
12+
* License, or (at your option) any later version.
13+
*
14+
* This program is distributed in the hope that it will be useful,
15+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
16+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
17+
* GNU Affero General Public License for more details.
18+
*
19+
* You should have received a copy of the GNU Affero General Public License
20+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
21+
*
22+
*/
23+
24+
declare(strict_types=1);
25+
26+
27+
namespace OCA\DAV\Service;
28+
29+
use OCP\IDBConnection;
30+
31+
class CustomPropertiesService {
32+
33+
/** @var IDBConnection */
34+
private $connection;
35+
36+
public function __construct(IDBConnection $connection) {
37+
$this->connection = $connection;
38+
}
39+
40+
public function delete(string $userId, string $path): void {
41+
$statement = $this->connection->prepare(
42+
'DELETE FROM `*PREFIX*properties` WHERE `userid` = ? AND `propertypath` = ?'
43+
);
44+
$result = $statement->execute([$userId, $this->formatPath($path)]);
45+
$result->closeCursor();
46+
}
47+
48+
/**
49+
* long paths are hashed to ensure they fit in the database
50+
*
51+
* @param string $path
52+
* @return string
53+
*/
54+
private function formatPath(string $path): string {
55+
if (strlen($path) > 250) {
56+
return sha1($path);
57+
}
58+
return $path;
59+
}
60+
}

0 commit comments

Comments
 (0)