Merge pull request #36690 from nextcloud/bugfix/noid/rollback-hook

4 files changed, 34 insertions, 21 deletions

diff --git a/apps/files_versions/lib/Storage.php b/apps/files_versions/lib/Storage.php
index 75ca7e4bcee..8fddf589a44 100644
--- a/apps/files_versions/lib/Storage.php
+++ b/apps/files_versions/lib/Storage.php
@@ -417,12 +417,6 @@ class Storage {
 
 			$node = $userFolder->get($file);
 
-			// TODO: move away from those legacy hooks!
-			\OC_Hook::emit('\OCP\Versions', 'rollback', [
-				'path' => $filename,
-				'revision' => $revision,
-				'node' => $node,
-			]);
 			return true;
 		} elseif ($versionCreated) {
 			self::deleteVersion($users_view, $version);
diff --git a/apps/files_versions/lib/Versions/LegacyVersionsBackend.php b/apps/files_versions/lib/Versions/LegacyVersionsBackend.php
index cbfbc001e0c..0ad0eb8a439 100644
--- a/apps/files_versions/lib/Versions/LegacyVersionsBackend.php
+++ b/apps/files_versions/lib/Versions/LegacyVersionsBackend.php
@@ -69,13 +69,17 @@ class LegacyVersionsBackend implements IVersionBackend, INameableVersionBackend,
 		if ($storage->instanceOfStorage(SharedStorage::class)) {
 			$owner = $storage->getOwner('');
 			$user = $this->userManager->get($owner);
-		}
 
-		$userFolder = $this->rootFolder->getUserFolder($user->getUID());
-		$nodes = $userFolder->getById($file->getId());
-		$file2 = array_pop($nodes);
+			$userFolder = $this->rootFolder->getUserFolder($user->getUID());
+			$nodes = $userFolder->getById($file->getId());
+			$file = array_pop($nodes);
+
+			if (!$file) {
+				throw new NotFoundException("version file not found for share owner");
+			}
+		}
 
-		$versions = $this->getVersionsForFileFromDB($file2, $user);
+		$versions = $this->getVersionsForFileFromDB($file, $user);
 
 		if (count($versions) > 0) {
 			return $versions;
@@ -83,18 +87,18 @@ class LegacyVersionsBackend implements IVersionBackend, INameableVersionBackend,
 
 		// Insert the entry in the DB for the current version.
 		$versionEntity = new VersionEntity();
-		$versionEntity->setFileId($file2->getId());
-		$versionEntity->setTimestamp($file2->getMTime());
-		$versionEntity->setSize($file2->getSize());
-		$versionEntity->setMimetype($this->mimeTypeLoader->getId($file2->getMimetype()));
+		$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);
 
 		// Insert entries in the DB for existing versions.
-		$versionsOnFS = Storage::getVersions($user->getUID(), $userFolder->getRelativePath($file2->getPath()));
+		$versionsOnFS = Storage::getVersions($user->getUID(), $userFolder->getRelativePath($file->getPath()));
 		foreach ($versionsOnFS as $version) {
 			$versionEntity = new VersionEntity();
-			$versionEntity->setFileId($file2->getId());
+			$versionEntity->setFileId($file->getId());
 			$versionEntity->setTimestamp((int)$version['version']);
 			$versionEntity->setSize((int)$version['size']);
 			$versionEntity->setMimetype($this->mimeTypeLoader->getId($version['mimetype']));
@@ -102,13 +106,13 @@ class LegacyVersionsBackend implements IVersionBackend, INameableVersionBackend,
 			$this->versionsMapper->insert($versionEntity);
 		}
 
-		return $this->getVersionsForFileFromDB($file2, $user);
+		return $this->getVersionsForFileFromDB($file, $user);
 	}
 
 	/**
 	 * @return IVersion[]
 	 */
-	private function getVersionsForFileFromDB(Node $file, IUser $user): array {
+	private function getVersionsForFileFromDB(FileInfo $file, IUser $user): array {
 		$userFolder = $this->rootFolder->getUserFolder($user->getUID());
 
 		return array_map(
diff --git a/apps/files_versions/lib/Versions/VersionManager.php b/apps/files_versions/lib/Versions/VersionManager.php
index bfae0937df8..b3b6d6f85e8 100644
--- a/apps/files_versions/lib/Versions/VersionManager.php
+++ b/apps/files_versions/lib/Versions/VersionManager.php
@@ -94,7 +94,16 @@ class VersionManager implements IVersionManager, INameableVersionBackend, IDelet
 
 	public function rollback(IVersion $version) {
 		$backend = $version->getBackend();
-		return $backend->rollback($version);
+		$result = $backend->rollback($version);
+		// rollback doesn't have a return type yet and some implementations don't return anything
+		if ($result === null || $result === true) {
+			\OC_Hook::emit('\OCP\Versions', 'rollback', [
+				'path' => $version->getVersionPath(),
+				'revision' => $version->getRevisionId(),
+				'node' => $version->getSourceFile(),
+			]);
+		}
+		return $result;
 	}
 
 	public function read(IVersion $version) {
diff --git a/apps/files_versions/tests/VersioningTest.php b/apps/files_versions/tests/VersioningTest.php
index 4f171031ab3..f454ba30b86 100644
--- a/apps/files_versions/tests/VersioningTest.php
+++ b/apps/files_versions/tests/VersioningTest.php
@@ -37,6 +37,7 @@ namespace OCA\Files_Versions\Tests;
 use OC\Files\Storage\Temporary;
 use OCA\Files_Versions\Db\VersionEntity;
 use OCA\Files_Versions\Db\VersionsMapper;
+use OCA\Files_Versions\Versions\IVersionManager;
 use OCP\Files\IMimeTypeLoader;
 use OCP\IConfig;
 use OCP\IUser;
@@ -822,7 +823,12 @@ class VersioningTest extends \Test\TestCase {
 		$params = [];
 		$this->connectMockHooks('rollback', $params);
 
-		$this->assertTrue(\OCA\Files_Versions\Storage::rollback('sub/test.txt', $t2, $this->user1));
+		$versionManager = \OCP\Server::get(IVersionManager::class);
+		$versions = $versionManager->getVersionsForFile($this->user1, $info1);
+		$version = array_filter($versions, function ($version) use ($t2) {
+			return $version->getRevisionId() === $t2;
+		});
+		$this->assertTrue($versionManager->rollback(current($version)));
 		$expectedParams = [
 			'path' => '/sub/test.txt',
 		];