Merge pull request #40296 from nextcloud/artonge/feat/enable_files_versions_for_groupfolders

Enable new versions feature for groupfolders

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

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

apps/files_versions/composer/composer/installed.php

@@ -3,7 +3,7 @@
         'name' => '__root__',
         'pretty_version' => 'dev-master',
         'version' => 'dev-master',
-        'reference' => 'b1797842784b250fb01ed5e3bf130705eb94751b',
+        'reference' => 'a820e3d036741ad1194361eca11bc1cbcdda0a47',
         'type' => 'library',
         'install_path' => __DIR__ . '/../',
         'aliases' => array(),
@@ -13,7 +13,7 @@
         '__root__' => array(
             'pretty_version' => 'dev-master',
             'version' => 'dev-master',
-            'reference' => 'b1797842784b250fb01ed5e3bf130705eb94751b',
+            'reference' => 'a820e3d036741ad1194361eca11bc1cbcdda0a47',
             'type' => 'library',
             'install_path' => __DIR__ . '/../',
             'aliases' => array(),

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<int, 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);
+		}
 	}
 
 	/**

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

apps/files_versions/lib/Versions/INeedSyncVersionBackend.php

@@ -0,0 +1,35 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023 Louis Chmn <louis@chmn.me>
+ *
+ * @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/>.
+ *
+ */
+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;
+}

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());
+	}
 }

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.
 	 *

apps/files_versions/src/components/Version.vue

@@ -257,12 +257,12 @@ export default {
 
 		/** @return {boolean} */
 		enableLabeling() {
-			return this.capabilities.files.version_labeling === true && this.fileInfo.mountType !== 'group'
+			return this.capabilities.files.version_labeling === true
 		},
 
 		/** @return {boolean} */
 		enableDeletion() {
-			return this.capabilities.files.version_deletion === true && this.fileInfo.mountType !== 'group'
+			return this.capabilities.files.version_deletion === true
 		},
 	},
 	methods: {

apps/files_versions/src/utils/versions.js

@@ -29,7 +29,6 @@ import { encodeFilePath } from '../../../files/src/utils/fileUtils.js'
 import client from '../utils/davClient.js'
 import davRequest from '../utils/davRequest.js'
 import logger from '../utils/logger.js'
-import path from 'path'
 
 /**
  * @typedef {object} Version
@@ -101,16 +100,13 @@ export async function restoreVersion(version) {
 function formatVersion(version, fileInfo) {
 	const mtime = moment(version.lastmod).unix() * 1000
 	let previewUrl = ''
-	let filename = ''
 
 	if (mtime === fileInfo.mtime) { // Version is the current one
-		filename = path.join('files', getCurrentUser()?.uid ?? '', fileInfo.path, fileInfo.name)
 		previewUrl = generateUrl('/core/preview?fileId={fileId}&c={fileEtag}&x=250&y=250&forceIcon=0&a=0', {
 			fileId: fileInfo.id,
 			fileEtag: fileInfo.etag,
 		})
 	} else {
-		filename = version.filename
 		previewUrl = generateUrl('/apps/files_versions/preview?file={file}&version={fileVersion}', {
 			file: joinPaths(fileInfo.path, fileInfo.name),
 			fileVersion: version.basename,
@@ -120,7 +116,7 @@ function formatVersion(version, fileInfo) {
 	return {
 		fileId: fileInfo.id,
 		label: version.props['version-label'],
-		filename,
+		filename: version.filename,
 		basename: moment(mtime).format('LLL'),
 		mime: version.mime,
 		etag: `${version.props.getetag}`,
@@ -130,8 +126,8 @@ function formatVersion(version, fileInfo) {
 		permissions: 'R',
 		hasPreview: version.props['has-preview'] === 1,
 		previewUrl,
-		url: joinPaths('/remote.php/dav', filename),
-		source: generateRemoteUrl('dav') + encodeFilePath(filename),
+		url: joinPaths('/remote.php/dav', version.filename),
+		source: generateRemoteUrl('dav') + encodeFilePath(version.filename),
 		fileVersion: version.basename,
 	}
 }

apps/files_versions/src/views/VersionTab.vue

@@ -35,11 +35,15 @@
 </template>
 
 <script>
+import path from 'path'
+
 import { showError, showSuccess } from '@nextcloud/dialogs'
 import isMobile from '@nextcloud/vue/dist/Mixins/isMobile.js'
+import { emit, subscribe, unsubscribe } from '@nextcloud/event-bus'
+import { getCurrentUser } from '@nextcloud/auth'
+
 import { fetchVersions, deleteVersion, restoreVersion, setVersionLabel } from '../utils/versions.js'
 import Version from '../components/Version.vue'
-import { emit, subscribe, unsubscribe } from '@nextcloud/event-bus'
 
 export default {
 	name: 'VersionTab',
@@ -249,7 +253,13 @@ export default {
 
 			// Versions previews are too small for our use case, so we override hasPreview and previewUrl
 			// which makes the viewer render the original file.
-			const versions = this.versions.map(version => ({ ...version, hasPreview: false, previewUrl: undefined }))
+			// We also point to the original filename if the version is the current one.
+			const versions = this.versions.map(version => ({
+				...version,
+				filename: version.mtime === this.fileInfo.mtime ? path.join('files', getCurrentUser()?.uid ?? '', fileInfo.path, fileInfo.name) : version.filename,
+				hasPreview: false,
+				previewUrl: undefined,
+			}))
 
 			OCA.Viewer.open({
 				fileInfo: versions.find(v => v.source === version.source),