Merge pull request #44175 from nextcloud/artonge/chore/use_new_version_metadata_API

apps/files_versions/lib/Db/VersionEntity.php

@@ -70,15 +70,6 @@ class VersionEntity extends Entity implements JsonSerializable {
 		];
 	}
 
-	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

apps/files_versions/lib/Listener/MetadataFileEvents.php

@@ -62,7 +62,7 @@ class MetadataFileEvents implements IEventListener {
 		// 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);
 		}
 	}
 }

apps/files_versions/lib/Sabre/Plugin.php

@@ -94,7 +94,7 @@ class Plugin extends ServerPlugin {
 
 	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 @@ class Plugin extends ServerPlugin {
 		$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));
 		}
 	}
 }

apps/files_versions/lib/Sabre/VersionFile.php

@@ -28,6 +28,7 @@ namespace OCA\Files_Versions\Sabre;
 
 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\Exception\NotFound;
 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 @@ class VersionFile implements IFile {
 		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 @@ class VersionFile implements IFile {
 	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;
 	}

apps/files_versions/lib/Storage.php

@@ -598,7 +598,7 @@ class Storage {
 				$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 @@ class Storage {
 					$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);

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

apps/files_versions/lib/Versions/LegacyVersionsBackend.php

@@ -46,25 +46,14 @@ use OCP\IUser;
 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 @@ class LegacyVersionsBackend implements IVersionBackend, INameableVersionBackend,
 				$file,
 				$this,
 				$user,
-				$versions['db']->getLabel(),
+				$versions['db']->getMetadata() ?? [],
 			);
 
 			array_push($davVersions, $version);
@@ -185,7 +174,7 @@ class LegacyVersionsBackend implements IVersionBackend, INameableVersionBackend,
 	}
 
 	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 @@ class LegacyVersionsBackend implements IVersionBackend, INameableVersionBackend,
 		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 @@ class LegacyVersionsBackend implements IVersionBackend, INameableVersionBackend,
 		$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 @@ class LegacyVersionsBackend implements IVersionBackend, INameableVersionBackend,
 		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);
 	}
 }

apps/files_versions/lib/Versions/Version.php

@@ -26,61 +26,21 @@ declare(strict_types=1);
 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 @@ class Version implements IVersion, INameableVersion, IMetadataVersion {
 		return $this->name;
 	}
 
-	public function getLabel(): string {
-		return $this->label;
-	}
-
 	public function getMimeType(): string {
 		return $this->mimetype;
 	}
@@ -124,9 +80,6 @@ class Version implements IVersion, INameableVersion, IMetadataVersion {
 	}
 
 	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;
 	}
 }

apps/files_versions/lib/Versions/VersionManager.php

@@ -36,7 +36,7 @@ use OCP\Files\Storage\IStorage;
 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 @@ class VersionManager implements IVersionManager, INameableVersionBackend, IDelet
 		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 @@ class VersionManager implements IVersionManager, INameableVersionBackend, IDelet
 		}
 	}
 
-	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;
 	}
 
 	/**