From c62afe0128514dafb4936ecc28be0db19bcf77ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Fri, 21 May 2021 13:17:30 +0200 Subject: [PATCH] Move dav property deletion to dedicated service to cleanup on background job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- .../composer/composer/autoload_classmap.php | 1 + .../dav/composer/composer/autoload_static.php | 1 + apps/dav/lib/BackgroundJob/UploadCleanup.php | 10 +++- .../dav/lib/Connector/Sabre/ServerFactory.php | 2 + apps/dav/lib/DAV/CustomPropertiesBackend.php | 15 +++-- apps/dav/lib/Server.php | 2 + .../lib/Service/CustomPropertiesService.php | 60 +++++++++++++++++++ .../Sabre/CustomPropertiesBackendTest.php | 2 + .../unit/DAV/CustomPropertiesBackendTest.php | 15 ++++- 9 files changed, 98 insertions(+), 10 deletions(-) create mode 100644 apps/dav/lib/Service/CustomPropertiesService.php diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index 5c401081ec..35ebd7b085 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -248,6 +248,7 @@ return array( 'OCA\\DAV\\Search\\EventsSearchProvider' => $baseDir . '/../lib/Search/EventsSearchProvider.php', 'OCA\\DAV\\Search\\TasksSearchProvider' => $baseDir . '/../lib/Search/TasksSearchProvider.php', 'OCA\\DAV\\Server' => $baseDir . '/../lib/Server.php', + 'OCA\\DAV\\Service\\CustomPropertiesService' => $baseDir . '/../lib/Service/CustomPropertiesService.php', 'OCA\\DAV\\Settings\\CalDAVSettings' => $baseDir . '/../lib/Settings/CalDAVSettings.php', 'OCA\\DAV\\Storage\\PublicOwnerWrapper' => $baseDir . '/../lib/Storage/PublicOwnerWrapper.php', 'OCA\\DAV\\SystemTag\\SystemTagMappingNode' => $baseDir . '/../lib/SystemTag/SystemTagMappingNode.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index b83b70e3d0..c48633eee9 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -263,6 +263,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\Search\\EventsSearchProvider' => __DIR__ . '/..' . '/../lib/Search/EventsSearchProvider.php', 'OCA\\DAV\\Search\\TasksSearchProvider' => __DIR__ . '/..' . '/../lib/Search/TasksSearchProvider.php', 'OCA\\DAV\\Server' => __DIR__ . '/..' . '/../lib/Server.php', + 'OCA\\DAV\\Service\\CustomPropertiesService' => __DIR__ . '/..' . '/../lib/Service/CustomPropertiesService.php', 'OCA\\DAV\\Settings\\CalDAVSettings' => __DIR__ . '/..' . '/../lib/Settings/CalDAVSettings.php', 'OCA\\DAV\\Storage\\PublicOwnerWrapper' => __DIR__ . '/..' . '/../lib/Storage/PublicOwnerWrapper.php', 'OCA\\DAV\\SystemTag\\SystemTagMappingNode' => __DIR__ . '/..' . '/../lib/SystemTag/SystemTagMappingNode.php', diff --git a/apps/dav/lib/BackgroundJob/UploadCleanup.php b/apps/dav/lib/BackgroundJob/UploadCleanup.php index 104faf12b6..59a6e5c8b8 100644 --- a/apps/dav/lib/BackgroundJob/UploadCleanup.php +++ b/apps/dav/lib/BackgroundJob/UploadCleanup.php @@ -29,6 +29,7 @@ declare(strict_types=1); namespace OCA\DAV\BackgroundJob; use OC\User\NoUserException; +use OCA\DAV\Service\CustomPropertiesService; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\IJobList; use OCP\BackgroundJob\TimedJob; @@ -45,10 +46,14 @@ class UploadCleanup extends TimedJob { /** @var IJobList */ private $jobList; - public function __construct(ITimeFactory $time, IRootFolder $rootFolder, IJobList $jobList) { + /** @var CustomPropertiesService */ + private $customPropertiesService; + + public function __construct(ITimeFactory $time, IRootFolder $rootFolder, IJobList $jobList, CustomPropertiesService $customPropertiesService) { parent::__construct($time); $this->rootFolder = $rootFolder; $this->jobList = $jobList; + $this->customPropertiesService = $customPropertiesService; // Run once a day $this->setInterval(60 * 60 * 24); @@ -72,6 +77,9 @@ class UploadCleanup extends TimedJob { $files = $uploadFolder->getDirectoryListing(); + $davPath = 'uploads/' . $uid . '/' . $uploadFolder->getName(); + $this->customPropertiesService->delete($uid, $davPath); + // Remove if all files have an mtime of more than a day $time = $this->time->getTime() - 60 * 60 * 24; diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 8c0145973f..f6fce71e6d 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -35,6 +35,7 @@ namespace OCA\DAV\Connector\Sabre; use OC\Files\Node\Folder; use OCA\DAV\AppInfo\PluginManager; use OCA\DAV\Files\BrowserErrorPagePlugin; +use OCA\DAV\Service\CustomPropertiesService; use OCP\Files\Mount\IMountManager; use OCP\IConfig; use OCP\IDBConnection; @@ -204,6 +205,7 @@ class ServerFactory { new \OCA\DAV\DAV\CustomPropertiesBackend( $objectTree, $this->databaseConnection, + \OC::$server->get(CustomPropertiesService::class), $this->userSession->getUser() ) ) diff --git a/apps/dav/lib/DAV/CustomPropertiesBackend.php b/apps/dav/lib/DAV/CustomPropertiesBackend.php index b25c33add8..08062649b4 100644 --- a/apps/dav/lib/DAV/CustomPropertiesBackend.php +++ b/apps/dav/lib/DAV/CustomPropertiesBackend.php @@ -26,6 +26,7 @@ namespace OCA\DAV\DAV; use OCA\DAV\Connector\Sabre\Node; +use OCA\DAV\Service\CustomPropertiesService; use OCP\IDBConnection; use OCP\IUser; use Sabre\DAV\PropertyStorage\Backend\BackendInterface; @@ -63,6 +64,11 @@ class CustomPropertiesBackend implements BackendInterface { */ private $connection; + /** + * @var CustomPropertiesService + */ + private $customPropertiesService; + /** * @var IUser */ @@ -83,9 +89,11 @@ class CustomPropertiesBackend implements BackendInterface { public function __construct( Tree $tree, IDBConnection $connection, + CustomPropertiesService $customPropertiesService, IUser $user) { $this->tree = $tree; $this->connection = $connection; + $this->customPropertiesService = $customPropertiesService; $this->user = $user; } @@ -155,12 +163,7 @@ class CustomPropertiesBackend implements BackendInterface { * @param string $path path of node for which to delete properties */ public function delete($path) { - $statement = $this->connection->prepare( - 'DELETE FROM `*PREFIX*properties` WHERE `userid` = ? AND `propertypath` = ?' - ); - $statement->execute([$this->user->getUID(), $this->formatPath($path)]); - $statement->closeCursor(); - + $this->customPropertiesService->delete($this->user->getUID(), $path); unset($this->cache[$path]); } diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 1b4d390016..691bf7a109 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -64,6 +64,7 @@ use OCA\DAV\Events\SabrePluginAuthInitEvent; use OCA\DAV\Files\BrowserErrorPagePlugin; use OCA\DAV\Files\LazySearchBackend; use OCA\DAV\Provisioning\Apple\AppleProvisioningPlugin; +use OCA\DAV\Service\CustomPropertiesService; use OCA\DAV\SystemTag\SystemTagPlugin; use OCA\DAV\Upload\ChunkingPlugin; use OCA\DAV\Upload\ChunkingV2Plugin; @@ -250,6 +251,7 @@ class Server { new CustomPropertiesBackend( $this->server->tree, \OC::$server->getDatabaseConnection(), + \OC::$server->get(CustomPropertiesService::class), \OC::$server->getUserSession()->getUser() ) ) diff --git a/apps/dav/lib/Service/CustomPropertiesService.php b/apps/dav/lib/Service/CustomPropertiesService.php new file mode 100644 index 0000000000..cf5f9b8af8 --- /dev/null +++ b/apps/dav/lib/Service/CustomPropertiesService.php @@ -0,0 +1,60 @@ + + * + * @author Julius Härtl + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +declare(strict_types=1); + + +namespace OCA\DAV\Service; + +use OCP\IDBConnection; + +class CustomPropertiesService { + + /** @var IDBConnection */ + private $connection; + + public function __construct(IDBConnection $connection) { + $this->connection = $connection; + } + + public function delete(string $userId, string $path): void { + $statement = $this->connection->prepare( + 'DELETE FROM `*PREFIX*properties` WHERE `userid` = ? AND `propertypath` = ?' + ); + $result = $statement->execute([$userId, $this->formatPath($path)]); + $result->closeCursor(); + } + + /** + * long paths are hashed to ensure they fit in the database + * + * @param string $path + * @return string + */ + private function formatPath(string $path): string { + if (strlen($path) > 250) { + return sha1($path); + } + return $path; + } +} diff --git a/apps/dav/tests/unit/Connector/Sabre/CustomPropertiesBackendTest.php b/apps/dav/tests/unit/Connector/Sabre/CustomPropertiesBackendTest.php index 68bc60e170..93e26189ea 100644 --- a/apps/dav/tests/unit/Connector/Sabre/CustomPropertiesBackendTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/CustomPropertiesBackendTest.php @@ -37,6 +37,7 @@ namespace OCA\DAV\Tests\unit\Connector\Sabre; use OCA\DAV\Connector\Sabre\Directory; use OCA\DAV\Connector\Sabre\File; +use OCA\DAV\Service\CustomPropertiesService; use OCP\IUser; use Sabre\DAV\Tree; @@ -88,6 +89,7 @@ class CustomPropertiesBackendTest extends \Test\TestCase { $this->plugin = new \OCA\DAV\DAV\CustomPropertiesBackend( $this->tree, \OC::$server->getDatabaseConnection(), + $this->createMock(CustomPropertiesService::class), $this->user ); } diff --git a/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php b/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php index 855add1fc1..3ce042ca10 100644 --- a/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php +++ b/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php @@ -29,8 +29,10 @@ namespace OCA\DAV\Tests\DAV; use OCA\DAV\DAV\CustomPropertiesBackend; +use OCA\DAV\Service\CustomPropertiesService; use OCP\IDBConnection; use OCP\IUser; +use PHPUnit\Framework\MockObject\MockObject; use Sabre\DAV\PropFind; use Sabre\DAV\PropPatch; use Sabre\DAV\Tree; @@ -41,16 +43,19 @@ use Test\TestCase; */ class CustomPropertiesBackendTest extends TestCase { - /** @var Tree | \PHPUnit\Framework\MockObject\MockObject */ + /** @var Tree | MockObject */ private $tree; /** @var IDBConnection */ private $dbConnection; - /** @var IUser | \PHPUnit\Framework\MockObject\MockObject */ + /** @var CustomPropertiesService */ + private $customPropertiesService; + + /** @var IUser | MockObject */ private $user; - /** @var CustomPropertiesBackend | \PHPUnit\Framework\MockObject\MockObject */ + /** @var CustomPropertiesBackend */ private $backend; protected function setUp(): void { @@ -62,10 +67,12 @@ class CustomPropertiesBackendTest extends TestCase { ->with() ->willReturn('dummy_user_42'); $this->dbConnection = \OC::$server->getDatabaseConnection(); + $this->customPropertiesService = new CustomPropertiesService($this->dbConnection); $this->backend = new CustomPropertiesBackend( $this->tree, $this->dbConnection, + $this->customPropertiesService, $this->user ); } @@ -123,9 +130,11 @@ class CustomPropertiesBackendTest extends TestCase { public function testPropFindNoDbCalls() { $db = $this->createMock(IDBConnection::class); + $service = new CustomPropertiesService($db); $backend = new CustomPropertiesBackend( $this->tree, $db, + $service, $this->user );