Move dav property deletion to dedicated service to cleanup on background job

Signed-off-by: Julius Härtl <jus@bitgrid.net>

Author: juliusknorr

apps/dav/composer/composer/autoload_classmap.php

@@ -248,6 +248,7 @@
     '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',

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',

apps/dav/lib/BackgroundJob/UploadCleanup.php

@@ -29,6 +29,7 @@
 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 @@ protected function run($argument) {
 
 		$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;
 

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

@@ -35,6 +35,7 @@
 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 @@ public function createServer($baseUri,
 						new \OCA\DAV\DAV\CustomPropertiesBackend(
 							$objectTree,
 							$this->databaseConnection,
+							\OC::$server->get(CustomPropertiesService::class),
 							$this->userSession->getUser()
 						)
 					)

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 @@ public function propPatch($path, PropPatch $propPatch) {
 	 * @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]);
 	}
 

apps/dav/lib/Server.php

@@ -64,6 +64,7 @@
 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 @@ public function __construct(IRequest $request, $baseUri) {
 						new CustomPropertiesBackend(
 							$this->server->tree,
 							\OC::$server->getDatabaseConnection(),
+							\OC::$server->get(CustomPropertiesService::class),
 							\OC::$server->getUserSession()->getUser()
 						)
 					)

apps/dav/lib/Service/CustomPropertiesService.php

@@ -0,0 +1,60 @@
+<?php
+/*
+ * @copyright Copyright (c) 2021 Julius Härtl <jus@bitgrid.net>
+ *
+ * @author Julius Härtl <jus@bitgrid.net>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+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;
+	}
+}

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

@@ -37,6 +37,7 @@
 
 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 @@ protected function setUp(): void {
 		$this->plugin = new \OCA\DAV\DAV\CustomPropertiesBackend(
 			$this->tree,
 			\OC::$server->getDatabaseConnection(),
+			$this->createMock(CustomPropertiesService::class),
 			$this->user
 		);
 	}

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 @@
  */
 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 @@ protected function setUp(): void {
 			->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 @@ protected function getProps(string $user, string $path) {
 
 	public function testPropFindNoDbCalls() {
 		$db = $this->createMock(IDBConnection::class);
+		$service = new CustomPropertiesService($db);
 		$backend = new CustomPropertiesBackend(
 			$this->tree,
 			$db,
+			$service,
 			$this->user
 		);