From 4d93b8f9bc1b8419f0016a92dcaf135a3afc03ed Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Wed, 6 Sep 2023 15:29:34 +0200 Subject: [PATCH] Move module versions management logic out of main logic Signed-off-by: Louis Chemineau --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/Listener/FileEventsListener.php | 54 ++++++++++--------- apps/files_versions/lib/Storage.php | 2 +- .../lib/Versions/INeedSyncVersionBackend.php | 35 ++++++++++++ .../lib/Versions/LegacyVersionsBackend.php | 36 ++++++++++++- .../lib/Versions/VersionManager.php | 24 ++++++++- 7 files changed, 124 insertions(+), 29 deletions(-) create mode 100644 apps/files_versions/lib/Versions/INeedSyncVersionBackend.php diff --git a/apps/files_versions/composer/composer/autoload_classmap.php b/apps/files_versions/composer/composer/autoload_classmap.php index 25a1e1c4913..ecdba54b17d 100644 --- a/apps/files_versions/composer/composer/autoload_classmap.php +++ b/apps/files_versions/composer/composer/autoload_classmap.php @@ -34,6 +34,7 @@ return array( 'OCA\\Files_Versions\\Versions\\IDeletableVersionBackend' => $baseDir . '/../lib/Versions/IDeletableVersionBackend.php', 'OCA\\Files_Versions\\Versions\\INameableVersion' => $baseDir . '/../lib/Versions/INameableVersion.php', 'OCA\\Files_Versions\\Versions\\INameableVersionBackend' => $baseDir . '/../lib/Versions/INameableVersionBackend.php', + 'OCA\\Files_Versions\\Versions\\INeedSyncVersionBackend' => $baseDir . '/../lib/Versions/INeedSyncVersionBackend.php', 'OCA\\Files_Versions\\Versions\\IVersion' => $baseDir . '/../lib/Versions/IVersion.php', 'OCA\\Files_Versions\\Versions\\IVersionBackend' => $baseDir . '/../lib/Versions/IVersionBackend.php', 'OCA\\Files_Versions\\Versions\\IVersionManager' => $baseDir . '/../lib/Versions/IVersionManager.php', diff --git a/apps/files_versions/composer/composer/autoload_static.php b/apps/files_versions/composer/composer/autoload_static.php index ee445fe9636..9cf94d3a2aa 100644 --- a/apps/files_versions/composer/composer/autoload_static.php +++ b/apps/files_versions/composer/composer/autoload_static.php @@ -49,6 +49,7 @@ class ComposerStaticInitFiles_Versions 'OCA\\Files_Versions\\Versions\\IDeletableVersionBackend' => __DIR__ . '/..' . '/../lib/Versions/IDeletableVersionBackend.php', 'OCA\\Files_Versions\\Versions\\INameableVersion' => __DIR__ . '/..' . '/../lib/Versions/INameableVersion.php', 'OCA\\Files_Versions\\Versions\\INameableVersionBackend' => __DIR__ . '/..' . '/../lib/Versions/INameableVersionBackend.php', + 'OCA\\Files_Versions\\Versions\\INeedSyncVersionBackend' => __DIR__ . '/..' . '/../lib/Versions/INeedSyncVersionBackend.php', 'OCA\\Files_Versions\\Versions\\IVersion' => __DIR__ . '/..' . '/../lib/Versions/IVersion.php', 'OCA\\Files_Versions\\Versions\\IVersionBackend' => __DIR__ . '/..' . '/../lib/Versions/IVersionBackend.php', 'OCA\\Files_Versions\\Versions\\IVersionManager' => __DIR__ . '/..' . '/../lib/Versions/IVersionManager.php', diff --git a/apps/files_versions/lib/Listener/FileEventsListener.php b/apps/files_versions/lib/Listener/FileEventsListener.php index fe638364b9d..d90283bacc8 100644 --- a/apps/files_versions/lib/Listener/FileEventsListener.php +++ b/apps/files_versions/lib/Listener/FileEventsListener.php @@ -36,9 +36,9 @@ use OC\Files\Filesystem; use OC\Files\Mount\MoveableMount; use OC\Files\Node\NonExistingFile; use OC\Files\View; -use OCA\Files_Versions\Db\VersionEntity; -use OCA\Files_Versions\Db\VersionsMapper; use OCA\Files_Versions\Storage; +use OCA\Files_Versions\Versions\INeedSyncVersionBackend; +use OCA\Files_Versions\Versions\IVersionManager; use OCP\AppFramework\Db\DoesNotExistException; use OCP\DB\Exception; use OCP\EventDispatcher\Event; @@ -54,6 +54,7 @@ use OCP\Files\Events\Node\NodeDeletedEvent; use OCP\Files\Events\Node\NodeRenamedEvent; use OCP\Files\Events\Node\NodeTouchedEvent; use OCP\Files\Events\Node\NodeWrittenEvent; +use OCP\Files\File; use OCP\Files\Folder; use OCP\Files\IMimeTypeLoader; use OCP\Files\IRootFolder; @@ -62,7 +63,7 @@ use Psr\Log\LoggerInterface; class FileEventsListener implements IEventListener { private IRootFolder $rootFolder; - private VersionsMapper $versionsMapper; + private IVersionManager $versionManager; /** * @var array */ @@ -80,12 +81,12 @@ class FileEventsListener implements IEventListener { public function __construct( IRootFolder $rootFolder, - VersionsMapper $versionsMapper, + IVersionManager $versionManager, IMimeTypeLoader $mimeTypeLoader, LoggerInterface $logger, ) { $this->rootFolder = $rootFolder; - $this->versionsMapper = $versionsMapper; + $this->versionManager = $versionManager; $this->mimeTypeLoader = $mimeTypeLoader; $this->logger = $logger; } @@ -160,11 +161,10 @@ class FileEventsListener implements IEventListener { unset($this->nodesTouched[$node->getId()]); try { - // We update the timestamp of the version entity associated with the previousNode. - $versionEntity = $this->versionsMapper->findVersionForFileId($previousNode->getId(), $previousNode->getMTime()); - // Create a version in the DB for the current content. - $versionEntity->setTimestamp($node->getMTime()); - $this->versionsMapper->update($versionEntity); + if ($node instanceof File && $this->versionManager instanceof INeedSyncVersionBackend) { + // We update the timestamp of the version entity associated with the previousNode. + $this->versionManager->updateVersionEntity($node, $previousNode->getMTime(), ['timestamp' => $node->getMTime()]); + } } catch (DbalException $ex) { // Ignore UniqueConstraintViolationException, as we are probably in the middle of a rollback // Where the previous node would temporary have the mtime of the old version, so the rollback touches it to fix it. @@ -179,17 +179,9 @@ class FileEventsListener implements IEventListener { public function created(Node $node): void { // Do not handle folders. - if ($node instanceof Folder) { - return; + if ($node instanceof File && $this->versionManager instanceof INeedSyncVersionBackend) { + $this->versionManager->createVersionEntity($node); } - - $versionEntity = new VersionEntity(); - $versionEntity->setFileId($node->getId()); - $versionEntity->setTimestamp($node->getMTime()); - $versionEntity->setSize($node->getSize()); - $versionEntity->setMimetype($this->mimeTypeLoader->getId($node->getMimetype())); - $versionEntity->setMetadata([]); - $this->versionsMapper->insert($versionEntity); } /** @@ -242,11 +234,17 @@ class FileEventsListener implements IEventListener { try { // If no new version was stored in the FS, no new version should be added in the DB. // So we simply update the associated version. - $currentVersionEntity = $this->versionsMapper->findVersionForFileId($node->getId(), $writeHookInfo['previousNode']->getMtime()); - $currentVersionEntity->setTimestamp($node->getMTime()); - $currentVersionEntity->setSize($node->getSize()); - $currentVersionEntity->setMimetype($this->mimeTypeLoader->getId($node->getMimetype())); - $this->versionsMapper->update($currentVersionEntity); + if ($node instanceof File && $this->versionManager instanceof INeedSyncVersionBackend) { + $this->versionManager->updateVersionEntity( + $node, + $writeHookInfo['previousNode']->getMtime(), + [ + 'timestamp' => $node->getMTime(), + 'size' => $node->getSize(), + 'mimetype' => $this->mimeTypeLoader->getId($node->getMimetype()), + ], + ); + } } catch (Exception $e) { $this->logger->error('Failed to update existing version for ' . $node->getPath(), [ 'exception' => $e, @@ -283,7 +281,11 @@ class FileEventsListener implements IEventListener { $relativePath = $this->getPathForNode($node); unset($this->versionsDeleted[$path]); Storage::delete($relativePath); - $this->versionsMapper->deleteAllVersionsForFileId($node->getId()); + // If no new version was stored in the FS, no new version should be added in the DB. + // So we simply update the associated version. + if ($node instanceof File && $this->versionManager instanceof INeedSyncVersionBackend) { + $this->versionManager->deleteVersionsEntity($node); + } } /** diff --git a/apps/files_versions/lib/Storage.php b/apps/files_versions/lib/Storage.php index 426d5931ba9..30f5c8073a1 100644 --- a/apps/files_versions/lib/Storage.php +++ b/apps/files_versions/lib/Storage.php @@ -350,7 +350,7 @@ class Storage { // move each version one by one to the target directory $rootView->$operation( '/' . $sourceOwner . '/files_versions/' . $sourcePath.'.v' . $v['version'], - '/' . $targetOwner . '/files_versions/' . $targetPath.'.v'.$v['version'] + '/' . $targetOwner . '/files_versions/' . $targetPath.'.v' . $v['version'] ); } } diff --git a/apps/files_versions/lib/Versions/INeedSyncVersionBackend.php b/apps/files_versions/lib/Versions/INeedSyncVersionBackend.php new file mode 100644 index 00000000000..f5112ab11dd --- /dev/null +++ b/apps/files_versions/lib/Versions/INeedSyncVersionBackend.php @@ -0,0 +1,35 @@ + + * + * @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 . + * + */ +namespace OCA\Files_Versions\Versions; + +use OCP\Files\File; + +/** + * @since 28.0.0 + */ +interface INeedSyncVersionBackend { + public function createVersionEntity(File $file): void; + public function updateVersionEntity(File $sourceFile, int $revision, array $properties): void; + public function deleteVersionsEntity(File $file): void; +} diff --git a/apps/files_versions/lib/Versions/LegacyVersionsBackend.php b/apps/files_versions/lib/Versions/LegacyVersionsBackend.php index a9ea09247c2..e99ab7d6870 100644 --- a/apps/files_versions/lib/Versions/LegacyVersionsBackend.php +++ b/apps/files_versions/lib/Versions/LegacyVersionsBackend.php @@ -42,7 +42,7 @@ use OCP\Files\Storage\IStorage; use OCP\IUser; use OCP\IUserManager; -class LegacyVersionsBackend implements IVersionBackend, INameableVersionBackend, IDeletableVersionBackend { +class LegacyVersionsBackend implements IVersionBackend, INameableVersionBackend, IDeletableVersionBackend, INeedSyncVersionBackend { private IRootFolder $rootFolder; private IUserManager $userManager; private VersionsMapper $versionsMapper; @@ -99,6 +99,8 @@ class LegacyVersionsBackend implements IVersionBackend, INameableVersionBackend, $versions = $this->getVersionsForFileFromDB($file, $user); + // Early exit if we find any version in the database. + // Else we continue to populate the DB from what's on disk. if (count($versions) > 0) { return $versions; } @@ -221,4 +223,36 @@ class LegacyVersionsBackend implements IVersionBackend, INameableVersionBackend, ); $this->versionsMapper->delete($versionEntity); } + + public function createVersionEntity(File $file): void { + $versionEntity = new VersionEntity(); + $versionEntity->setFileId($file->getId()); + $versionEntity->setTimestamp($file->getMTime()); + $versionEntity->setSize($file->getSize()); + $versionEntity->setMimetype($this->mimeTypeLoader->getId($file->getMimetype())); + $versionEntity->setMetadata([]); + $this->versionsMapper->insert($versionEntity); + } + + public function updateVersionEntity(File $sourceFile, int $revision, array $properties): void { + $versionEntity = $this->versionsMapper->findVersionForFileId($sourceFile->getId(), $revision); + + if (isset($properties['timestamp'])) { + $versionEntity->setTimestamp($properties['timestamp']); + } + + if (isset($properties['size'])) { + $versionEntity->setSize($properties['size']); + } + + if (isset($properties['mimetype'])) { + $versionEntity->setMimetype($properties['mimetype']); + } + + $this->versionsMapper->update($versionEntity); + } + + public function deleteVersionsEntity(File $file): void { + $this->versionsMapper->deleteAllVersionsForFileId($file->getId()); + } } diff --git a/apps/files_versions/lib/Versions/VersionManager.php b/apps/files_versions/lib/Versions/VersionManager.php index 3e769000da7..abbbe69e586 100644 --- a/apps/files_versions/lib/Versions/VersionManager.php +++ b/apps/files_versions/lib/Versions/VersionManager.php @@ -31,11 +31,12 @@ use OCP\Files\IRootFolder; use OCP\Files\Lock\ILock; use OCP\Files\Lock\ILockManager; use OCP\Files\Lock\LockContext; +use OCP\Files\NotFoundException; use OCP\Files\Storage\IStorage; use OCP\IUser; use OCP\Lock\ManuallyLockedException; -class VersionManager implements IVersionManager, INameableVersionBackend, IDeletableVersionBackend { +class VersionManager implements IVersionManager, INameableVersionBackend, IDeletableVersionBackend, INeedSyncVersionBackend { /** @var (IVersionBackend[])[] */ private $backends = []; @@ -139,6 +140,27 @@ class VersionManager implements IVersionManager, INameableVersionBackend, IDelet } } + public function createVersionEntity(File $file): void { + $backend = $this->getBackendForStorage($file->getStorage()); + if ($backend instanceof INeedSyncVersionBackend) { + $backend->createVersionEntity($file); + } + } + + public function updateVersionEntity(File $sourceFile, int $revision, array $properties): void { + $backend = $this->getBackendForStorage($sourceFile->getStorage()); + if ($backend instanceof INeedSyncVersionBackend) { + $backend->updateVersionEntity($sourceFile, $revision, $properties); + } + } + + public function deleteVersionsEntity(File $file): void { + $backend = $this->getBackendForStorage($file->getStorage()); + if ($backend instanceof INeedSyncVersionBackend) { + $backend->deleteVersionsEntity($file); + } + } + /** * Catch ManuallyLockedException and retry in app context if possible. * -- 2.39.5