From 9361a305ba363070af0cd35a4e696b12e4d94a31 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Wed, 13 Mar 2024 14:54:15 +0100 Subject: [PATCH] chore(files_versions): Use new metadata API for versions Signed-off-by: Louis Chemineau --- apps/files_versions/lib/Db/VersionEntity.php | 9 --- .../lib/Listener/MetadataFileEvents.php | 2 +- apps/files_versions/lib/Sabre/Plugin.php | 4 +- apps/files_versions/lib/Sabre/VersionFile.php | 33 ++++---- apps/files_versions/lib/Storage.php | 4 +- .../lib/Versions/IMetadataVersionBackend.php | 12 +-- .../lib/Versions/LegacyVersionsBackend.php | 78 ++++--------------- apps/files_versions/lib/Versions/Version.php | 71 +++-------------- .../lib/Versions/VersionManager.php | 23 +----- 9 files changed, 53 insertions(+), 183 deletions(-) diff --git a/apps/files_versions/lib/Db/VersionEntity.php b/apps/files_versions/lib/Db/VersionEntity.php index 6d1df79c122e8..dda88817b5563 100644 --- a/apps/files_versions/lib/Db/VersionEntity.php +++ b/apps/files_versions/lib/Db/VersionEntity.php @@ -70,15 +70,6 @@ public function jsonSerialize(): array { ]; } - public function getLabel(): string { - return $this->metadata['label'] ?? ''; - } - - public function setLabel(string $label): void { - $this->metadata['label'] = $label; - $this->markFieldUpdated('metadata'); - } - /** * @abstract given a key, return the value associated with the key in the metadata column * if nothing is found, we return an empty string diff --git a/apps/files_versions/lib/Listener/MetadataFileEvents.php b/apps/files_versions/lib/Listener/MetadataFileEvents.php index 652c9f9a1cef5..c8a38da588da3 100644 --- a/apps/files_versions/lib/Listener/MetadataFileEvents.php +++ b/apps/files_versions/lib/Listener/MetadataFileEvents.php @@ -62,7 +62,7 @@ public function post_write_hook(Node $node): void { // check if our version manager supports setting the metadata if ($this->versionManager instanceof IMetadataVersionBackend) { $author = $user->getUID(); - $this->versionManager->setMetadataValue($node, 'author', $author); + $this->versionManager->setMetadataValue($node, $node->getMTime(), 'author', $author); } } } diff --git a/apps/files_versions/lib/Sabre/Plugin.php b/apps/files_versions/lib/Sabre/Plugin.php index b70b97b7485d4..c50d650c9589b 100644 --- a/apps/files_versions/lib/Sabre/Plugin.php +++ b/apps/files_versions/lib/Sabre/Plugin.php @@ -94,7 +94,7 @@ public function afterGet(RequestInterface $request, ResponseInterface $response) public function propFind(PropFind $propFind, INode $node): void { if ($node instanceof VersionFile) { - $propFind->handle(self::VERSION_LABEL, fn () => $node->getLabel()); + $propFind->handle(self::VERSION_LABEL, fn () => $node->getMetadataValue('label')); $propFind->handle(self::VERSION_AUTHOR, fn () => $node->getMetadataValue("author")); $propFind->handle(FilesPlugin::HAS_PREVIEW_PROPERTYNAME, fn () => $this->previewManager->isMimeSupported($node->getContentType())); } @@ -104,7 +104,7 @@ public function propPatch($path, PropPatch $propPatch): void { $node = $this->server->tree->getNodeForPath($path); if ($node instanceof VersionFile) { - $propPatch->handle(self::VERSION_LABEL, fn ($label) => $node->setLabel($label)); + $propPatch->handle(self::VERSION_LABEL, fn (string $label) => $node->setMetadataValue('label', $label)); } } } diff --git a/apps/files_versions/lib/Sabre/VersionFile.php b/apps/files_versions/lib/Sabre/VersionFile.php index 061cf285907a2..ef89a033c297e 100644 --- a/apps/files_versions/lib/Sabre/VersionFile.php +++ b/apps/files_versions/lib/Sabre/VersionFile.php @@ -28,6 +28,7 @@ use OCA\Files_Versions\Versions\IDeletableVersionBackend; use OCA\Files_Versions\Versions\IMetadataVersion; +use OCA\Files_Versions\Versions\IMetadataVersionBackend; use OCA\Files_Versions\Versions\INameableVersion; use OCA\Files_Versions\Versions\INameableVersionBackend; use OCA\Files_Versions\Versions\IVersion; @@ -38,15 +39,10 @@ use Sabre\DAV\IFile; class VersionFile implements IFile { - /** @var IVersion */ - private $version; - - /** @var IVersionManager */ - private $versionManager; - - public function __construct(IVersion $version, IVersionManager $versionManager) { - $this->version = $version; - $this->versionManager = $versionManager; + public function __construct( + private IVersion $version, + private IVersionManager $versionManager + ) { } public function put($data) { @@ -93,17 +89,14 @@ public function setName($name) { throw new Forbidden(); } - public function getLabel(): ?string { - if ($this->version instanceof INameableVersion && $this->version->getSourceFile()->getSize() > 0) { - return $this->version->getLabel(); - } else { - return null; - } - } + public function setMetadataValue(string $key, string $value): bool { + $backend = $this->version->getBackend(); - public function setLabel($label): bool { - if ($this->versionManager instanceof INameableVersionBackend) { - $this->versionManager->setVersionLabel($this->version, $label); + if ($backend instanceof IMetadataVersionBackend) { + $backend->setMetadataValue($this->version->getSourceFile(), $this->version->getRevisionId(), $key, $value); + return true; + } elseif ($key === 'label' && $backend instanceof INameableVersionBackend) { + $backend->setVersionLabel($this->version, $value); return true; } else { return false; @@ -113,6 +106,8 @@ public function setLabel($label): bool { public function getMetadataValue(string $key): ?string { if ($this->version instanceof IMetadataVersion) { return $this->version->getMetadataValue($key); + } elseif ($key === 'label' && $this->version instanceof INameableVersion) { + return $this->version->getLabel(); } return null; } diff --git a/apps/files_versions/lib/Storage.php b/apps/files_versions/lib/Storage.php index a35f151d956ef..44ce909e03ba8 100644 --- a/apps/files_versions/lib/Storage.php +++ b/apps/files_versions/lib/Storage.php @@ -598,7 +598,7 @@ public static function expireOlderThanMaxForUser($uid) { $versionEntity = $versionsMapper->findVersionForFileId($node->getId(), $version); $versionEntities[$info->getId()] = $versionEntity; - if ($versionEntity->getLabel() !== '') { + if ($versionEntity->getMetadataValue('label') !== null && $versionEntity->getMetadataValue('label') !== '') { return false; } } catch (NotFoundException $e) { @@ -929,7 +929,7 @@ public static function expire($filename, $uid) { $pathparts = pathinfo($path); $timestamp = (int)substr($pathparts['extension'] ?? '', 1); $versionEntity = $versionsMapper->findVersionForFileId($file->getId(), $timestamp); - if ($versionEntity->getLabel() !== '') { + if ($versionEntity->getMetadataValue('label') !== '') { continue; } $versionsMapper->delete($versionEntity); diff --git a/apps/files_versions/lib/Versions/IMetadataVersionBackend.php b/apps/files_versions/lib/Versions/IMetadataVersionBackend.php index e9225a14244a2..348596c050ca8 100644 --- a/apps/files_versions/lib/Versions/IMetadataVersionBackend.php +++ b/apps/files_versions/lib/Versions/IMetadataVersionBackend.php @@ -35,18 +35,10 @@ interface IMetadataVersionBackend { * Sets a key value pair in the metadata column corresponding to the node's version. * * @param Node $node the node that triggered the Metadata event listener, aka, the file version + * @param int $revision the key for the json value of the metadata column * @param string $key the key for the json value of the metadata column * @param string $value the value that corresponds to the key in the metadata column * @since 29.0.0 */ - public function setMetadataValue(Node $node, string $key, string $value): void; - - /** - * Retrieves a corresponding value from the metadata column using the key. - * - * @param Node $node the node that triggered the Metadata event listener, aka, the file version - * @param string $key the key for the json value of the metadata column - * @since 29.0.0 - */ - public function getMetadataValue(Node $node, string $key): ?string; + public function setMetadataValue(Node $node, int $revision, string $key, string $value): void; } diff --git a/apps/files_versions/lib/Versions/LegacyVersionsBackend.php b/apps/files_versions/lib/Versions/LegacyVersionsBackend.php index 6c052b3059183..0f752c2ea6848 100644 --- a/apps/files_versions/lib/Versions/LegacyVersionsBackend.php +++ b/apps/files_versions/lib/Versions/LegacyVersionsBackend.php @@ -46,25 +46,14 @@ use OCP\IUserManager; use OCP\IUserSession; -class LegacyVersionsBackend implements IVersionBackend, INameableVersionBackend, IDeletableVersionBackend, INeedSyncVersionBackend, IMetadataVersionBackend { - private IRootFolder $rootFolder; - private IUserManager $userManager; - private VersionsMapper $versionsMapper; - private IMimeTypeLoader $mimeTypeLoader; - private IUserSession $userSession; - +class LegacyVersionsBackend implements IVersionBackend, IDeletableVersionBackend, INeedSyncVersionBackend, IMetadataVersionBackend { public function __construct( - IRootFolder $rootFolder, - IUserManager $userManager, - VersionsMapper $versionsMapper, - IMimeTypeLoader $mimeTypeLoader, - IUserSession $userSession, + private IRootFolder $rootFolder, + private IUserManager $userManager, + private VersionsMapper $versionsMapper, + private IMimeTypeLoader $mimeTypeLoader, + private IUserSession $userSession, ) { - $this->rootFolder = $rootFolder; - $this->userManager = $userManager; - $this->versionsMapper = $versionsMapper; - $this->mimeTypeLoader = $mimeTypeLoader; - $this->userSession = $userSession; } public function useBackendForStorage(IStorage $storage): bool { @@ -160,7 +149,7 @@ public function getVersionsForFile(IUser $user, FileInfo $file): array { $file, $this, $user, - $versions['db']->getLabel(), + $versions['db']->getMetadata() ?? [], ); array_push($davVersions, $version); @@ -185,7 +174,7 @@ public function createVersion(IUser $user, FileInfo $file) { } public function rollback(IVersion $version) { - if (!$this->currentUserHasPermissions($version, \OCP\Constants::PERMISSION_UPDATE)) { + if (!$this->currentUserHasPermissions($version->getSourceFile(), \OCP\Constants::PERMISSION_UPDATE)) { throw new Forbidden('You cannot restore this version because you do not have update permissions on the source file.'); } @@ -234,24 +223,8 @@ public function getVersionFile(IUser $user, FileInfo $sourceFile, $revision): Fi return $file; } - public function setVersionLabel(IVersion $version, string $label): void { - if (!$this->currentUserHasPermissions($version, \OCP\Constants::PERMISSION_UPDATE)) { - throw new Forbidden('You cannot label this version because you do not have update permissions on the source file.'); - } - - $versionEntity = $this->versionsMapper->findVersionForFileId( - $version->getSourceFile()->getId(), - $version->getTimestamp(), - ); - if (trim($label) === '') { - $label = null; - } - $versionEntity->setLabel($label ?? ''); - $this->versionsMapper->update($versionEntity); - } - public function deleteVersion(IVersion $version): void { - if (!$this->currentUserHasPermissions($version, \OCP\Constants::PERMISSION_DELETE)) { + if (!$this->currentUserHasPermissions($version->getSourceFile(), \OCP\Constants::PERMISSION_DELETE)) { throw new Forbidden('You cannot delete this version because you do not have delete permissions on the source file.'); } @@ -295,8 +268,7 @@ public function deleteVersionsEntity(File $file): void { $this->versionsMapper->deleteAllVersionsForFileId($file->getId()); } - private function currentUserHasPermissions(IVersion $version, int $permissions): bool { - $sourceFile = $version->getSourceFile(); + private function currentUserHasPermissions(FileInfo $sourceFile, int $permissions): bool { $currentUserId = $this->userSession->getUser()?->getUID(); if ($currentUserId === null) { @@ -314,32 +286,14 @@ private function currentUserHasPermissions(IVersion $version, int $permissions): return ($sourceFile->getPermissions() & $permissions) === $permissions; } - public function setMetadataValue(Node $node, string $key, string $value): void { - // Do not handle folders. - if ($node instanceof File) { - - try { - $versionEntity = $this->versionsMapper->findVersionForFileId($node->getId(), $node->getMTime()); - } catch (\Exception $e) { - throw $e; // the version does not exist or too many versions exist - } - - $currentMetadata = $versionEntity->getMetadata() ?? []; - - $currentMetadata[$key] = $value; - $versionEntity->setMetadata($currentMetadata); - $this->versionsMapper->update($versionEntity); + public function setMetadataValue(Node $node, int $revision, string $key, string $value): void { + if (!$this->currentUserHasPermissions($node, \OCP\Constants::PERMISSION_UPDATE)) { + throw new Forbidden('You cannot update the version\'s metadata because you do not have update permissions on the source file.'); } - } + $versionEntity = $this->versionsMapper->findVersionForFileId($node->getId(), $revision); - public function getMetadataValue(Node $node, string $key): ?string { - try { - $versionEntity = $this->versionsMapper->findVersionForFileId($node->getId(), $node->getMTime()); - return $versionEntity->getMetadataValue($key); - } catch (\InvalidArgumentException $e) { - // we tried to find a version or key that doesn't exist - return null; - } + $versionEntity->setMetadataValue($key, $value); + $this->versionsMapper->update($versionEntity); } } diff --git a/apps/files_versions/lib/Versions/Version.php b/apps/files_versions/lib/Versions/Version.php index b3b06812f75c2..40f2507433d83 100644 --- a/apps/files_versions/lib/Versions/Version.php +++ b/apps/files_versions/lib/Versions/Version.php @@ -26,61 +26,21 @@ namespace OCA\Files_Versions\Versions; use OCP\Files\FileInfo; -use OCP\Files\Node; use OCP\IUser; -class Version implements IVersion, INameableVersion, IMetadataVersion { - /** @var int */ - private $timestamp; - - /** @var int|string */ - private $revisionId; - - /** @var string */ - private $name; - - private string $label; - - /** @var int|float */ - private $size; - - /** @var string */ - private $mimetype; - - /** @var string */ - private $path; - - /** @var FileInfo */ - private $sourceFileInfo; - - /** @var IVersionBackend */ - private $backend; - - /** @var IUser */ - private $user; - +class Version implements IVersion, IMetadataVersion { public function __construct( - int $timestamp, - $revisionId, - string $name, - int|float $size, - string $mimetype, - string $path, - FileInfo $sourceFileInfo, - IVersionBackend $backend, - IUser $user, - string $label = '' + private int $timestamp, + private int|string $revisionId, + private string $name, + private int|float $size, + private string $mimetype, + private string $path, + private FileInfo $sourceFileInfo, + private IVersionBackend $backend, + private IUser $user, + private array $metadata = [], ) { - $this->timestamp = $timestamp; - $this->revisionId = $revisionId; - $this->name = $name; - $this->label = $label; - $this->size = $size; - $this->mimetype = $mimetype; - $this->path = $path; - $this->sourceFileInfo = $sourceFileInfo; - $this->backend = $backend; - $this->user = $user; } public function getBackend(): IVersionBackend { @@ -107,10 +67,6 @@ public function getSourceFileName(): string { return $this->name; } - public function getLabel(): string { - return $this->label; - } - public function getMimeType(): string { return $this->mimetype; } @@ -124,9 +80,6 @@ public function getUser(): IUser { } public function getMetadataValue(string $key): ?string { - if ($this->backend instanceof IMetadataVersionBackend && $this->sourceFileInfo instanceof Node) { - return $this->backend->getMetadataValue($this->sourceFileInfo, "author"); - } - return null; + return $this->metadata[$key] ?? null; } } diff --git a/apps/files_versions/lib/Versions/VersionManager.php b/apps/files_versions/lib/Versions/VersionManager.php index d884ffb0f7be3..f12b5679fbce8 100644 --- a/apps/files_versions/lib/Versions/VersionManager.php +++ b/apps/files_versions/lib/Versions/VersionManager.php @@ -36,7 +36,7 @@ use OCP\IUser; use OCP\Lock\ManuallyLockedException; -class VersionManager implements IVersionManager, INameableVersionBackend, IDeletableVersionBackend, INeedSyncVersionBackend, IMetadataVersionBackend { +class VersionManager implements IVersionManager, IDeletableVersionBackend, INeedSyncVersionBackend, IMetadataVersionBackend { /** @var (IVersionBackend[])[] */ private $backends = []; @@ -126,15 +126,8 @@ public function useBackendForStorage(IStorage $storage): bool { return false; } - public function setVersionLabel(IVersion $version, string $label): void { - $backend = $this->getBackendForStorage($version->getSourceFile()->getStorage()); - if ($backend instanceof INameableVersionBackend) { - $backend->setVersionLabel($version, $label); - } - } - public function deleteVersion(IVersion $version): void { - $backend = $this->getBackendForStorage($version->getSourceFile()->getStorage()); + $backend = $version->getBackend(); if ($backend instanceof IDeletableVersionBackend) { $backend->deleteVersion($version); } @@ -161,19 +154,11 @@ public function deleteVersionsEntity(File $file): void { } } - public function setMetadataValue(Node $node, string $key, string $value): void { - $backend = $this->getBackendForStorage($node->getStorage()); - if ($backend instanceof IMetadataVersionBackend) { - $backend->setMetadataValue($node, $key, $value); - } - } - - public function getMetadataValue(Node $node, string $key): ?string { + public function setMetadataValue(Node $node, int $revision, string $key, string $value): void { $backend = $this->getBackendForStorage($node->getStorage()); if ($backend instanceof IMetadataVersionBackend) { - return $backend->getMetadataValue($node, $key); + $backend->setMetadataValue($node, $revision, $key, $value); } - return null; } /**