fix: don't rely on share providers being avaiable in CleanupShareTarget

Signed-off-by: Robin Appelman <robin@icewind.nl>

Author: icewind1991

apps/files_sharing/lib/Repair/CleanupShareTarget.php

@@ -13,14 +13,16 @@
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\Files\Config\ICachedMountInfo;
 use OCP\Files\Config\IUserMountCache;
+use OCP\Files\IRootFolder;
 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;
 
+/**
+ * @psalm-type ShareInfo = array{id: string|int, share_type: string, share_with: string, file_source: string, file_target: string};
+ */
 class CleanupShareTarget implements IRepairStep {
 	/** we only care about shares with a user target,
 	 *  since the underling group/deck/talk share doesn't get moved
@@ -34,12 +36,11 @@ class CleanupShareTarget implements IRepairStep {
 
 	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,
+		private readonly IRootFolder $rootFolder,
 	) {
 	}
 
@@ -58,12 +59,10 @@ public function run(IOutput $output) {
 
 		$lastUser = '';
 		$userMounts = [];
+		$userFolder = null;
 
 		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()) {
@@ -74,19 +73,23 @@ public function run(IOutput $output) {
 				$mounts = $this->userMountCache->getMountsForUser($recipient);
 				$mountPoints = array_map(fn (ICachedMountInfo $mount) => $mount->getMountPoint(), $mounts);
 				$userMounts = array_combine($mountPoints, $mounts);
+				$userFolder = $this->rootFolder->getUserFolder($recipient->getUID());
 			}
 
-			$oldTarget = $share->getTarget();
+			$oldTarget = $shareInfo['file_target'];
 			$newTarget = $this->cleanTarget($oldTarget);
-			$share->setTarget($newTarget);
-			$this->shareManager->moveShare($share, $recipient->getUID());
+			$absoluteNewTarget = $userFolder->getFullPath($newTarget);
+			$targetParentNode = $this->rootFolder->get(dirname($absoluteNewTarget));
 
-			$this->shareTargetValidator->verifyMountPoint(
-				$recipient,
-				$share,
+			$absoluteNewTarget = $this->shareTargetValidator->generateUniqueTarget(
+				$shareInfo['file_source'],
+				$absoluteNewTarget,
+				$targetParentNode->getMountPoint(),
 				$userMounts,
-				[$share],
 			);
+			$newTarget = $userFolder->getRelativePath($absoluteNewTarget);
+
+			$this->moveShare((string)$shareInfo['id'], $newTarget);
 
 			$oldMountPoint = "/{$recipient->getUID()}/files$oldTarget/";
 			$newMountPoint = "/{$recipient->getUID()}/files$newTarget/";
@@ -108,12 +111,22 @@ private function countProblemShares(): int {
 		return (int)$query->executeQuery()->fetchOne();
 	}
 
+	private function moveShare(string $id, string $target) {
+		// since we only process user-specific shares, we can just move them
+		// without having to check if we need to create a user-specific override
+		$query = $this->connection->getQueryBuilder();
+		$query->update('share')
+			->set('file_target', $query->createNamedParameter($target))
+			->where($query->expr()->eq('id', $query->createNamedParameter($id)))
+			->executeStatement();
+	}
+
 	/**
-	 * @return \Traversable<array{id: string, share_type: string, share_with: string}>
+	 * @return \Traversable<ShareInfo>
 	 */
 	private function getProblemShares(): \Traversable {
 		$query = $this->connection->getQueryBuilder();
-		$query->select('id', 'share_type', 'share_with')
+		$query->select('id', 'share_type', 'share_with', 'file_source', 'file_target')
 			->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))

apps/files_sharing/lib/ShareTargetValidator.php

@@ -85,7 +85,7 @@ public function verifyMountPoint(
 		}
 
 		$newAbsoluteMountPoint = $this->generateUniqueTarget(
-			$share,
+			$share->getNodeId(),
 			Filesystem::normalizePath($absoluteParent . '/' . $mountPoint),
 			$parentMount,
 			$allCachedMounts,
@@ -108,8 +108,8 @@ public function verifyMountPoint(
 	/**
 	 * @param ICachedMountInfo[] $allCachedMounts
 	 */
-	private function generateUniqueTarget(
-		IShare $share,
+	public function generateUniqueTarget(
+		int $shareNodeId,
 		string $absolutePath,
 		IMountPoint $parentMount,
 		array $allCachedMounts,
@@ -122,7 +122,7 @@ private function generateUniqueTarget(
 		$i = 2;
 		$parentCache = $parentMount->getStorage()->getCache();
 		$internalPath = $parentMount->getInternalPath($absolutePath);
-		while ($parentCache->inCache($internalPath) || $this->hasConflictingMount($share, $allCachedMounts, $absolutePath)) {
+		while ($parentCache->inCache($internalPath) || $this->hasConflictingMount($shareNodeId, $allCachedMounts, $absolutePath)) {
 			$absolutePath = Filesystem::normalizePath($dir . '/' . $name . ' (' . $i . ')' . $ext);
 			$internalPath = $parentMount->getInternalPath($absolutePath);
 			$i++;
@@ -134,13 +134,13 @@ private function generateUniqueTarget(
 	/**
 	 * @param ICachedMountInfo[] $allCachedMounts
 	 */
-	private function hasConflictingMount(IShare $share, array $allCachedMounts, string $absolutePath): bool {
+	private function hasConflictingMount(int $shareNodeId, array $allCachedMounts, string $absolutePath): bool {
 		if (!isset($allCachedMounts[$absolutePath . '/'])) {
 			return false;
 		}
 
 		$mount = $allCachedMounts[$absolutePath . '/'];
-		if ($mount->getMountProvider() === MountProvider::class && $mount->getRootId() === $share->getNodeId()) {
+		if ($mount->getMountProvider() === MountProvider::class && $mount->getRootId() === $shareNodeId) {
 			// "conflicting" mount is a mount for the current share
 			return false;
 		}