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

Signed-off-by: Julius Härtl <jus@bitgrid.net>
This commit is contained in:
Julius Härtl 2021-05-21 13:17:30 +02:00
parent 37c5f0a11b
commit c62afe0128
No known key found for this signature in database
GPG Key ID: 4C614C6ED2CDE6DF
9 changed files with 98 additions and 10 deletions

View File

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

View File

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

View File

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

View File

@ -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()
)
)

View File

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

View File

@ -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()
)
)

View File

@ -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;
}
}

View File

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

View File

@ -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
);