Merge pull request #57881 from nextcloud/share-target-repair

Add repair step for share targets with excess (2)

apps/files_sharing/composer/composer/autoload_classmap.php

@@ -92,6 +92,7 @@ return array(
     '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',

apps/files_sharing/composer/composer/autoload_static.php

@@ -107,6 +107,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',

apps/files_sharing/lib/Repair/CleanupShareTarget.php

@@ -0,0 +1,131 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Files_Sharing\Repair;
+
+use OC\Files\SetupManager;
+use OCA\Files_Sharing\ShareTargetValidator;
+use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\Files\Config\ICachedMountInfo;
+use OCP\Files\Config\IUserMountCache;
+use OCP\IDBConnection;
+use OCP\IUserManager;
+use OCP\Migration\IOutput;
+use OCP\Migration\IRepairStep;
+use OCP\Share\IManager;
+use OCP\Share\IProviderFactory;
+use OCP\Share\IShare;
+
+class CleanupShareTarget implements IRepairStep {
+	/** we only care about shares with a user target,
+	 *  since the underling group/deck/talk share doesn't get moved
+	 */
+	private const USER_SHARE_TYPES = [
+		IShare::TYPE_USER,
+		IShare::TYPE_USERGROUP,
+		IShare::TYPE_DECK_USER,
+		11 // TYPE_USERROOM
+	];
+
+	public function __construct(
+		private readonly IDBConnection $connection,
+		private readonly IManager $shareManager,
+		private readonly IProviderFactory $shareProviderFactory,
+		private readonly ShareTargetValidator $shareTargetValidator,
+		private readonly IUserManager $userManager,
+		private readonly SetupManager $setupManager,
+		private readonly IUserMountCache $userMountCache,
+	) {
+	}
+
+	#[\Override]
+	public function getName() {
+		return 'Cleanup share names with false conflicts';
+	}
+
+	#[\Override]
+	public function run(IOutput $output) {
+		$count = $this->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);
+				$mounts = $this->userMountCache->getMountsForUser($recipient);
+				$mountPoints = array_map(fn (ICachedMountInfo $mount) => $mount->getMountPoint(), $mounts);
+				$userMounts = array_combine($mountPoints, $mounts);
+			}
+
+			$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<array{id: string, share_type: string, share_with: string}>
+	 */
+	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<array{id: string, share_type: string, share_with: string}> $rows */
+		$rows = $result->iterateAssociative();
+		return $rows;
+	}
+
+	private function cleanTarget(string $target): string {
+		return preg_replace('/( \([2-9]\)){2,}/', '', $target);
+	}
+}

apps/files_sharing/tests/Repair/CleanupShareTargetTest.php

@@ -0,0 +1,115 @@
+<?php
+
+/**
+ * SPDX-FileCopyrightText: 2026
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+namespace OCA\Files_Sharing\Tests\Repair;
+
+use OC\Migration\NullOutput;
+use OCA\Files_Sharing\Repair\CleanupShareTarget;
+use OCA\Files_Sharing\Tests\TestCase;
+use OCP\Files\NotFoundException;
+use OCP\Server;
+use OCP\Share\IShare;
+use PHPUnit\Framework\Attributes\Group;
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+#[Group(name: 'DB')]
+class CleanupShareTargetTest extends TestCase {
+	public const TEST_FOLDER_NAME = '/folder_share_api_test';
+
+	private CleanupShareTarget $cleanupShareTarget;
+
+	protected function setUp(): void {
+		parent::setUp();
+		$this->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)');
+
+		$this->loginHelper(self::TEST_FILES_SHARING_API_USER2);
+		$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);
+	}
+}

lib/private/Repair.php

@@ -60,6 +60,7 @@ use OC\Repair\RepairMimeTypes;
 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 @@ class Repair implements IOutput {
 			),
 			\OCP\Server::get(DeleteSchedulingObjects::class),
 			\OC::$server->get(RemoveObjectProperties::class),
+			\OCP\Server::get(CleanupShareTarget::class),
 		];
 	}