diff --git a/apps/files_versions/lib/Storage.php b/apps/files_versions/lib/Storage.php index 2938accddcc44..ac440650d6263 100644 --- a/apps/files_versions/lib/Storage.php +++ b/apps/files_versions/lib/Storage.php @@ -404,12 +404,6 @@ public static function rollback(string $file, int $revision, IUser $user) { $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 4ea0985e11346..f19dae723e324 100644 --- a/apps/files_versions/lib/Versions/LegacyVersionsBackend.php +++ b/apps/files_versions/lib/Versions/LegacyVersionsBackend.php @@ -58,6 +58,14 @@ public function getVersionsForFile(IUser $user, FileInfo $file): array { if ($storage->instanceOfStorage(SharedStorage::class)) { $owner = $storage->getOwner(''); $user = $this->userManager->get($owner); + + $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"); + } } $userFolder = $this->rootFolder->getUserFolder($user->getUID()); diff --git a/apps/files_versions/lib/Versions/VersionManager.php b/apps/files_versions/lib/Versions/VersionManager.php index 7642537e0b6e7..205f5c8aebf59 100644 --- a/apps/files_versions/lib/Versions/VersionManager.php +++ b/apps/files_versions/lib/Versions/VersionManager.php @@ -99,7 +99,16 @@ public function createVersion(IUser $user, FileInfo $file) { public function rollback(IVersion $version) { $backend = $version->getBackend(); - return self::handleAppLocks(fn(): ?bool => $backend->rollback($version)); + $result = self::handleAppLocks(fn(): ?bool => $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 b8f58ecff2b14..5fc4c9132b428 100644 --- a/apps/files_versions/tests/VersioningTest.php +++ b/apps/files_versions/tests/VersioningTest.php @@ -38,6 +38,8 @@ use OCP\IConfig; use OCP\IUser; use OCP\Share\IShare; +use OCA\Files_Versions\Versions\IVersionManager; +use Test\Traits\MountProviderTrait; /** * Class Test_Files_versions @@ -49,6 +51,7 @@ class VersioningTest extends \Test\TestCase { public const TEST_VERSIONS_USER = 'test-versions-user'; public const TEST_VERSIONS_USER2 = 'test-versions-user2'; public const USERS_VERSIONS_ROOT = '/test-versions-user/files_versions'; + use MountProviderTrait; /** * @var \OC\Files\View @@ -646,7 +649,8 @@ public function testRestoreSameStorage() { public function testRestoreCrossStorage() { $storage2 = new Temporary([]); - \OC\Files\Filesystem::mount($storage2, [], self::TEST_VERSIONS_USER . '/files/sub'); + $this->registerMount(self::TEST_VERSIONS_USER, $storage2, self::TEST_VERSIONS_USER . '/files/sub'); + $this->loginAsUser(self::TEST_VERSIONS_USER); $this->doTestRestore(); } @@ -789,7 +793,12 @@ private function doTestRestore() { $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', ]; diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index 5669f2a4b4599..beb2ac60d559e 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -326,7 +326,7 @@ protected function getAppDataDirectoryName(): string { * @param int $id * @return array */ - protected function getByIdInRootMount(int $id): array { + protected function getByIdInRootMount(int $id): array { if (!method_exists($this->root, 'createNode')) { // Always expected to be false. Being a method of Folder, this is // always implemented. For it is an internal method and should not