From b393b6f6a144c199a7aaa8ab41fcc33f9efa9ec4 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 27 Jan 2026 19:13:13 +0100 Subject: [PATCH 1/2] fix: add repair step for cleanup shares with excess (2) Signed-off-by: Robin Appelman --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/Repair/CleanupShareTarget.php | 128 ++++++++++++++++++ lib/private/Repair.php | 2 + 4 files changed, 132 insertions(+) create mode 100644 apps/files_sharing/lib/Repair/CleanupShareTarget.php diff --git a/apps/files_sharing/composer/composer/autoload_classmap.php b/apps/files_sharing/composer/composer/autoload_classmap.php index 6de418dce0101..6750f63164d97 100644 --- a/apps/files_sharing/composer/composer/autoload_classmap.php +++ b/apps/files_sharing/composer/composer/autoload_classmap.php @@ -91,6 +91,7 @@ 'OCA\\Files_Sharing\\Notification\\Notifier' => $baseDir . '/../lib/Notification/Notifier.php', 'OCA\\Files_Sharing\\OpenMetrics\\SharesCountMetric' => $baseDir . '/../lib/OpenMetrics/SharesCountMetric.php', 'OCA\\Files_Sharing\\OrphanHelper' => $baseDir . '/../lib/OrphanHelper.php', + 'OCA\\Files_Sharing\\Repair\\CleanupShareTarget' => $baseDir . '/../lib/Repair/CleanupShareTarget.php', 'OCA\\Files_Sharing\\ResponseDefinitions' => $baseDir . '/../lib/ResponseDefinitions.php', 'OCA\\Files_Sharing\\Scanner' => $baseDir . '/../lib/Scanner.php', 'OCA\\Files_Sharing\\Settings\\Personal' => $baseDir . '/../lib/Settings/Personal.php', diff --git a/apps/files_sharing/composer/composer/autoload_static.php b/apps/files_sharing/composer/composer/autoload_static.php index fb0da77fb4095..922bc6d7fd3a3 100644 --- a/apps/files_sharing/composer/composer/autoload_static.php +++ b/apps/files_sharing/composer/composer/autoload_static.php @@ -106,6 +106,7 @@ class ComposerStaticInitFiles_Sharing 'OCA\\Files_Sharing\\Notification\\Notifier' => __DIR__ . '/..' . '/../lib/Notification/Notifier.php', 'OCA\\Files_Sharing\\OpenMetrics\\SharesCountMetric' => __DIR__ . '/..' . '/../lib/OpenMetrics/SharesCountMetric.php', 'OCA\\Files_Sharing\\OrphanHelper' => __DIR__ . '/..' . '/../lib/OrphanHelper.php', + 'OCA\\Files_Sharing\\Repair\\CleanupShareTarget' => __DIR__ . '/..' . '/../lib/Repair/CleanupShareTarget.php', 'OCA\\Files_Sharing\\ResponseDefinitions' => __DIR__ . '/..' . '/../lib/ResponseDefinitions.php', 'OCA\\Files_Sharing\\Scanner' => __DIR__ . '/..' . '/../lib/Scanner.php', 'OCA\\Files_Sharing\\Settings\\Personal' => __DIR__ . '/..' . '/../lib/Settings/Personal.php', diff --git a/apps/files_sharing/lib/Repair/CleanupShareTarget.php b/apps/files_sharing/lib/Repair/CleanupShareTarget.php new file mode 100644 index 0000000000000..ef1a573a4c681 --- /dev/null +++ b/apps/files_sharing/lib/Repair/CleanupShareTarget.php @@ -0,0 +1,128 @@ +countProblemShares(); + if ($count === 0) { + return; + } + $output->startProgress($count); + + $lastUser = ''; + $userMounts = []; + + foreach ($this->getProblemShares() as $shareInfo) { + $recipient = $this->userManager->getExistingUser($shareInfo['share_with']); + $share = $this->shareProviderFactory + ->getProviderForType((int)$shareInfo['share_type']) + ->getShareById($shareInfo['id'], $recipient->getUID()); + + // since we ordered the share by user, we can reuse the last data until we get to the next user + if ($lastUser !== $recipient->getUID()) { + $lastUser = $recipient->getUID(); + + $this->setupManager->tearDown(); + $this->setupManager->setupForUser($recipient); + $userMounts = $this->mountManager->getAll(); + } + + $oldTarget = $share->getTarget(); + $newTarget = $this->cleanTarget($oldTarget); + $share->setTarget($newTarget); + $this->shareManager->moveShare($share, $recipient->getUID()); + + $this->shareTargetValidator->verifyMountPoint( + $recipient, + $share, + $userMounts, + [$share], + ); + + $oldMountPoint = "/{$recipient->getUID()}/files$oldTarget/"; + $newMountPoint = "/{$recipient->getUID()}/files$newTarget/"; + $userMounts[$newMountPoint] = $userMounts[$oldMountPoint]; + unset($userMounts[$oldMountPoint]); + + $output->advance(); + } + $output->finishProgress(); + $output->info("Fixed $count shares"); + } + + private function countProblemShares(): int { + $query = $this->connection->getQueryBuilder(); + $query->select($query->func()->count('id')) + ->from('share') + ->where($query->expr()->like('file_target', $query->createNamedParameter('% (_) (_)%'))) + ->andWhere($query->expr()->in('share_type', $query->createNamedParameter(self::USER_SHARE_TYPES, IQueryBuilder::PARAM_INT_ARRAY), IQueryBuilder::PARAM_INT_ARRAY)); + return (int)$query->executeQuery()->fetchOne(); + } + + /** + * @return \Traversable + */ + private function getProblemShares(): \Traversable { + $query = $this->connection->getQueryBuilder(); + $query->select('id', 'share_type', 'share_with') + ->from('share') + ->where($query->expr()->like('file_target', $query->createNamedParameter('% (_) (_)%'))) + ->andWhere($query->expr()->in('share_type', $query->createNamedParameter(self::USER_SHARE_TYPES, IQueryBuilder::PARAM_INT_ARRAY), IQueryBuilder::PARAM_INT_ARRAY)) + ->orderBy('share_with') + ->addOrderBy('id'); + $result = $query->executeQuery(); + /** @var \Traversable $rows */ + $rows = $result->iterateAssociative(); + return $rows; + } + + private function cleanTarget(string $target): string { + return preg_replace('/( \([2-9]\)){2,}/', '', $target); + } +} diff --git a/lib/private/Repair.php b/lib/private/Repair.php index 73f2ae0617818..ba1a659958a44 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -60,6 +60,7 @@ use OC\Template\JSCombiner; use OCA\DAV\Migration\DeleteSchedulingObjects; use OCA\DAV\Migration\RemoveObjectProperties; +use OCA\Files_Sharing\Repair\CleanupShareTarget; use OCP\AppFramework\QueryException; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Collaboration\Resources\IManager; @@ -221,6 +222,7 @@ public static function getExpensiveRepairSteps() { ), \OCP\Server::get(DeleteSchedulingObjects::class), \OC::$server->get(RemoveObjectProperties::class), + \OCP\Server::get(CleanupShareTarget::class), ]; } From d881686e3a42706d5798c5f7f444dc04f62cc14c Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 28 Jan 2026 13:31:41 +0100 Subject: [PATCH 2/2] test: add test for share target repair Signed-off-by: Robin Appelman --- .../tests/Repair/CleanupShareTargetTest.php | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 apps/files_sharing/tests/Repair/CleanupShareTargetTest.php diff --git a/apps/files_sharing/tests/Repair/CleanupShareTargetTest.php b/apps/files_sharing/tests/Repair/CleanupShareTargetTest.php new file mode 100644 index 0000000000000..8c752c52a0c25 --- /dev/null +++ b/apps/files_sharing/tests/Repair/CleanupShareTargetTest.php @@ -0,0 +1,114 @@ +cleanupShareTarget = Server::get(CleanupShareTarget::class); + } + + private function createUserShare(string $by, string $target = self::TEST_FOLDER_NAME): IShare { + $userFolder = $this->rootFolder->getUserFolder($by); + + try { + $node = $userFolder->get(self::TEST_FOLDER_NAME); + } catch (NotFoundException $e) { + $node = $userFolder->newFolder(self::TEST_FOLDER_NAME); + } + $share1 = $this->shareManager->newShare(); + $share1->setNode($node) + ->setSharedBy($by) + ->setSharedWith(self::TEST_FILES_SHARING_API_USER2) + ->setShareType(IShare::TYPE_USER) + ->setPermissions(31); + $share = $this->shareManager->createShare($share1); + $share->setStatus(IShare::STATUS_ACCEPTED); + $this->shareManager->updateShare($share); + + $share->setTarget($target); + $this->shareManager->moveShare($share, self::TEST_FILES_SHARING_API_USER2); + + $share = $this->shareManager->getShareById($share->getFullId()); + $this->assertEquals($target, $share->getTarget()); + + return $share; + } + + public function testBasicRepair() { + $share = $this->createUserShare(self::TEST_FILES_SHARING_API_USER1, self::TEST_FOLDER_NAME . ' (2) (2) (2) (2)'); + + $this->cleanupShareTarget->run(new NullOutput()); + + $share = $this->shareManager->getShareById($share->getFullId()); + $this->assertEquals(self::TEST_FOLDER_NAME, $share->getTarget()); + } + + public function testRepairConflictFile() { + $share = $this->createUserShare(self::TEST_FILES_SHARING_API_USER1, self::TEST_FOLDER_NAME . ' (2) (2) (2) (2)'); + + $userFolder2 = $this->rootFolder->getUserFolder(self::TEST_FILES_SHARING_API_USER2); + $folder = $userFolder2->newFolder(self::TEST_FOLDER_NAME); + + $this->cleanupShareTarget->run(new NullOutput()); + $folder->delete(); + + $share = $this->shareManager->getShareById($share->getFullId()); + $this->assertEquals(self::TEST_FOLDER_NAME . ' (2)', $share->getTarget()); + } + + public function testRepairConflictShare() { + $share = $this->createUserShare(self::TEST_FILES_SHARING_API_USER1, self::TEST_FOLDER_NAME . ' (2) (2) (2) (2)'); + + $share2 = $this->createUserShare(self::TEST_FILES_SHARING_API_USER3); + + $this->cleanupShareTarget->run(new NullOutput()); + + $share2 = $this->shareManager->getShareById($share2->getFullId()); + $this->assertEquals(self::TEST_FOLDER_NAME, $share2->getTarget()); + $share = $this->shareManager->getShareById($share->getFullId()); + $this->assertEquals(self::TEST_FOLDER_NAME . ' (2)', $share->getTarget()); + } + + public function testRepairMultipleConflicting() { + $share = $this->createUserShare(self::TEST_FILES_SHARING_API_USER1, self::TEST_FOLDER_NAME . ' (2) (2) (2) (2)'); + $share2 = $this->createUserShare(self::TEST_FILES_SHARING_API_USER3, self::TEST_FOLDER_NAME . ' (2) (2) (2) (2) (2)'); + + $this->cleanupShareTarget->run(new NullOutput()); + + $share = $this->shareManager->getShareById($share->getFullId()); + $share2 = $this->shareManager->getShareById($share2->getFullId()); + + // there is no guarantee for what order the 2 shares got repaired by + $targets = [ + $share->getTarget(), + $share2->getTarget(), + ]; + sort($targets); + $this->assertEquals([ + self::TEST_FOLDER_NAME, + self::TEST_FOLDER_NAME . ' (2)' + ], $targets); + } +}