From cf46bd691566833ee60e8939872d16425f48a64e Mon Sep 17 00:00:00 2001 From: Julius Härtl Date: Mon, 13 Feb 2023 14:30:40 +0100 Subject: fix: Make sure that rollback hook is triggered on all version backends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/files_versions/lib/Storage.php | 6 ------ apps/files_versions/lib/Versions/VersionManager.php | 8 +++++++- 2 files changed, 7 insertions(+), 7 deletions(-) (limited to 'apps/files_versions') 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/VersionManager.php b/apps/files_versions/lib/Versions/VersionManager.php index bfae0937df8..c11b429202b 100644 --- a/apps/files_versions/lib/Versions/VersionManager.php +++ b/apps/files_versions/lib/Versions/VersionManager.php @@ -94,7 +94,13 @@ class VersionManager implements IVersionManager, INameableVersionBackend, IDelet public function rollback(IVersion $version) { $backend = $version->getBackend(); - return $backend->rollback($version); + $result = $backend->rollback($version); + \OC_Hook::emit('\OCP\Versions', 'rollback', [ + 'path' => \OC\Files\Filesystem::getView()->getRelativePath($version->getSourceFile()->getPath()), + 'revision' => $version->getRevisionId(), + 'node' => $version->getSourceFile(), + ]); + return $result; } public function read(IVersion $version) { -- cgit v1.2.3 From c22d51c1ad939e1bf2a2b78f8fb3329a5269c9e7 Mon Sep 17 00:00:00 2001 From: Julius Härtl Date: Tue, 14 Feb 2023 12:38:51 +0100 Subject: tests(files_versions): Tear down fs to clear mount cache before testing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/files_versions/lib/Versions/VersionManager.php | 2 +- apps/files_versions/tests/VersioningTest.php | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) (limited to 'apps/files_versions') diff --git a/apps/files_versions/lib/Versions/VersionManager.php b/apps/files_versions/lib/Versions/VersionManager.php index c11b429202b..5a2480d6233 100644 --- a/apps/files_versions/lib/Versions/VersionManager.php +++ b/apps/files_versions/lib/Versions/VersionManager.php @@ -96,7 +96,7 @@ class VersionManager implements IVersionManager, INameableVersionBackend, IDelet $backend = $version->getBackend(); $result = $backend->rollback($version); \OC_Hook::emit('\OCP\Versions', 'rollback', [ - 'path' => \OC\Files\Filesystem::getView()->getRelativePath($version->getSourceFile()->getPath()), + 'path' => $version->getVersionPath(), 'revision' => $version->getRevisionId(), 'node' => $version->getSourceFile(), ]); diff --git a/apps/files_versions/tests/VersioningTest.php b/apps/files_versions/tests/VersioningTest.php index 4f171031ab3..547f1eca4a6 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; @@ -661,6 +662,7 @@ class VersioningTest extends \Test\TestCase { public function testRestoreCrossStorage() { $storage2 = new Temporary([]); \OC\Files\Filesystem::mount($storage2, [], self::TEST_VERSIONS_USER . '/files/sub'); + \OC\Files\Filesystem::tearDown(); $this->doTestRestore(); } @@ -822,7 +824,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', ]; -- cgit v1.2.3 From 62e6a32899dcd079b1ff3623d7363241892011ca Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 10 Mar 2023 17:10:06 +0100 Subject: don't re-get fileinfo for versioned file if it's not shared Signed-off-by: Robin Appelman --- .../lib/Versions/LegacyVersionsBackend.php | 30 ++++++++++++---------- apps/files_versions/tests/VersioningTest.php | 1 - 2 files changed, 17 insertions(+), 14 deletions(-) (limited to 'apps/files_versions') 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/tests/VersioningTest.php b/apps/files_versions/tests/VersioningTest.php index 547f1eca4a6..f454ba30b86 100644 --- a/apps/files_versions/tests/VersioningTest.php +++ b/apps/files_versions/tests/VersioningTest.php @@ -662,7 +662,6 @@ class VersioningTest extends \Test\TestCase { public function testRestoreCrossStorage() { $storage2 = new Temporary([]); \OC\Files\Filesystem::mount($storage2, [], self::TEST_VERSIONS_USER . '/files/sub'); - \OC\Files\Filesystem::tearDown(); $this->doTestRestore(); } -- cgit v1.2.3 From f00f4244d4b4bfb98ccb9d828d6a1033c8c94524 Mon Sep 17 00:00:00 2001 From: Julius Härtl Date: Mon, 13 Mar 2023 10:57:12 +0100 Subject: fix: Check return type on rollback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/files_versions/lib/Versions/VersionManager.php | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'apps/files_versions') diff --git a/apps/files_versions/lib/Versions/VersionManager.php b/apps/files_versions/lib/Versions/VersionManager.php index 5a2480d6233..b3b6d6f85e8 100644 --- a/apps/files_versions/lib/Versions/VersionManager.php +++ b/apps/files_versions/lib/Versions/VersionManager.php @@ -95,11 +95,14 @@ class VersionManager implements IVersionManager, INameableVersionBackend, IDelet public function rollback(IVersion $version) { $backend = $version->getBackend(); $result = $backend->rollback($version); - \OC_Hook::emit('\OCP\Versions', 'rollback', [ - 'path' => $version->getVersionPath(), - 'revision' => $version->getRevisionId(), - 'node' => $version->getSourceFile(), - ]); + // 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; } -- cgit v1.2.3