Skip to content

Commit 7702b3c

Browse files
AndyScherzingerAndyScherzinger
authored
Merge pull request #57858 from nextcloud/share-target-repair-33
[stable33] Add repair step for share targets with excess (2)
2 parents eb482a6 + d881686 commit 7702b3c

File tree

5 files changed

+246
-0
lines changed

5 files changed

+246
-0
lines changed

‎apps/files_sharing/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@
9191
'OCA\\Files_Sharing\\Notification\\Notifier' => $baseDir . '/../lib/Notification/Notifier.php',
9292
'OCA\\Files_Sharing\\OpenMetrics\\SharesCountMetric' => $baseDir . '/../lib/OpenMetrics/SharesCountMetric.php',
9393
'OCA\\Files_Sharing\\OrphanHelper' => $baseDir . '/../lib/OrphanHelper.php',
94+
'OCA\\Files_Sharing\\Repair\\CleanupShareTarget' => $baseDir . '/../lib/Repair/CleanupShareTarget.php',
9495
'OCA\\Files_Sharing\\ResponseDefinitions' => $baseDir . '/../lib/ResponseDefinitions.php',
9596
'OCA\\Files_Sharing\\Scanner' => $baseDir . '/../lib/Scanner.php',
9697
'OCA\\Files_Sharing\\Settings\\Personal' => $baseDir . '/../lib/Settings/Personal.php',

‎apps/files_sharing/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ class ComposerStaticInitFiles_Sharing
106106
'OCA\\Files_Sharing\\Notification\\Notifier' => __DIR__ . '/..' . '/../lib/Notification/Notifier.php',
107107
'OCA\\Files_Sharing\\OpenMetrics\\SharesCountMetric' => __DIR__ . '/..' . '/../lib/OpenMetrics/SharesCountMetric.php',
108108
'OCA\\Files_Sharing\\OrphanHelper' => __DIR__ . '/..' . '/../lib/OrphanHelper.php',
109+
'OCA\\Files_Sharing\\Repair\\CleanupShareTarget' => __DIR__ . '/..' . '/../lib/Repair/CleanupShareTarget.php',
109110
'OCA\\Files_Sharing\\ResponseDefinitions' => __DIR__ . '/..' . '/../lib/ResponseDefinitions.php',
110111
'OCA\\Files_Sharing\\Scanner' => __DIR__ . '/..' . '/../lib/Scanner.php',
111112
'OCA\\Files_Sharing\\Settings\\Personal' => __DIR__ . '/..' . '/../lib/Settings/Personal.php',

‎apps/files_sharing/lib/Repair/CleanupShareTarget.php‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
9+
namespace OCA\Files_Sharing\Repair;
10+
11+
use OC\Files\SetupManager;
12+
use OCA\Files_Sharing\ShareTargetValidator;
13+
use OCP\DB\QueryBuilder\IQueryBuilder;
14+
use OCP\Files\Mount\IMountManager;
15+
use OCP\IDBConnection;
16+
use OCP\IUserManager;
17+
use OCP\Migration\IOutput;
18+
use OCP\Migration\IRepairStep;
19+
use OCP\Share\IManager;
20+
use OCP\Share\IProviderFactory;
21+
use OCP\Share\IShare;
22+
23+
class CleanupShareTarget implements IRepairStep {
24+
/** we only care about shares with a user target,
25+
* since the underling group/deck/talk share doesn't get moved
26+
*/
27+
private const USER_SHARE_TYPES = [
28+
IShare::TYPE_USER,
29+
IShare::TYPE_USERGROUP,
30+
IShare::TYPE_DECK_USER,
31+
11 // TYPE_USERROOM
32+
];
33+
34+
public function __construct(
35+
private readonly IDBConnection $connection,
36+
private readonly IManager $shareManager,
37+
private readonly IProviderFactory $shareProviderFactory,
38+
private readonly ShareTargetValidator $shareTargetValidator,
39+
private readonly IUserManager $userManager,
40+
private readonly SetupManager $setupManager,
41+
private readonly IMountManager $mountManager,
42+
) {
43+
}
44+
45+
#[\Override]
46+
public function getName() {
47+
return 'Cleanup share names with false conflicts';
48+
}
49+
50+
#[\Override]
51+
public function run(IOutput $output) {
52+
$count = $this->countProblemShares();
53+
if ($count === 0) {
54+
return;
55+
}
56+
$output->startProgress($count);
57+
58+
$lastUser = '';
59+
$userMounts = [];
60+
61+
foreach ($this->getProblemShares() as $shareInfo) {
62+
$recipient = $this->userManager->getExistingUser($shareInfo['share_with']);
63+
$share = $this->shareProviderFactory
64+
->getProviderForType((int)$shareInfo['share_type'])
65+
->getShareById($shareInfo['id'], $recipient->getUID());
66+
67+
// since we ordered the share by user, we can reuse the last data until we get to the next user
68+
if ($lastUser !== $recipient->getUID()) {
69+
$lastUser = $recipient->getUID();
70+
71+
$this->setupManager->tearDown();
72+
$this->setupManager->setupForUser($recipient);
73+
$userMounts = $this->mountManager->getAll();
74+
}
75+
76+
$oldTarget = $share->getTarget();
77+
$newTarget = $this->cleanTarget($oldTarget);
78+
$share->setTarget($newTarget);
79+
$this->shareManager->moveShare($share, $recipient->getUID());
80+
81+
$this->shareTargetValidator->verifyMountPoint(
82+
$recipient,
83+
$share,
84+
$userMounts,
85+
[$share],
86+
);
87+
88+
$oldMountPoint = "/{$recipient->getUID()}/files$oldTarget/";
89+
$newMountPoint = "/{$recipient->getUID()}/files$newTarget/";
90+
$userMounts[$newMountPoint] = $userMounts[$oldMountPoint];
91+
unset($userMounts[$oldMountPoint]);
92+
93+
$output->advance();
94+
}
95+
$output->finishProgress();
96+
$output->info("Fixed $count shares");
97+
}
98+
99+
private function countProblemShares(): int {
100+
$query = $this->connection->getQueryBuilder();
101+
$query->select($query->func()->count('id'))
102+
->from('share')
103+
->where($query->expr()->like('file_target', $query->createNamedParameter('% (_) (_)%')))
104+
->andWhere($query->expr()->in('share_type', $query->createNamedParameter(self::USER_SHARE_TYPES, IQueryBuilder::PARAM_INT_ARRAY), IQueryBuilder::PARAM_INT_ARRAY));
105+
return (int)$query->executeQuery()->fetchOne();
106+
}
107+
108+
/**
109+
* @return \Traversable<array{id: string, share_type: string, share_with: string}>
110+
*/
111+
private function getProblemShares(): \Traversable {
112+
$query = $this->connection->getQueryBuilder();
113+
$query->select('id', 'share_type', 'share_with')
114+
->from('share')
115+
->where($query->expr()->like('file_target', $query->createNamedParameter('% (_) (_)%')))
116+
->andWhere($query->expr()->in('share_type', $query->createNamedParameter(self::USER_SHARE_TYPES, IQueryBuilder::PARAM_INT_ARRAY), IQueryBuilder::PARAM_INT_ARRAY))
117+
->orderBy('share_with')
118+
->addOrderBy('id');
119+
$result = $query->executeQuery();
120+
/** @var \Traversable<array{id: string, share_type: string, share_with: string}> $rows */
121+
$rows = $result->iterateAssociative();
122+
return $rows;
123+
}
124+
125+
private function cleanTarget(string $target): string {
126+
return preg_replace('/( \([2-9]\)){2,}/', '', $target);
127+
}
128+
}

‎apps/files_sharing/tests/Repair/CleanupShareTargetTest.php‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
<?php
2+
3+
/**
4+
* SPDX-FileCopyrightText: 2026
5+
* SPDX-License-Identifier: AGPL-3.0-only
6+
*/
7+
namespace OCA\Files_Sharing\Tests\Repair;
8+
9+
use OC\Migration\NullOutput;
10+
use OCA\Files_Sharing\Repair\CleanupShareTarget;
11+
use OCA\Files_Sharing\Tests\TestCase;
12+
use OCP\Files\NotFoundException;
13+
use OCP\Server;
14+
use OCP\Share\IShare;
15+
use PHPUnit\Framework\Attributes\Group;
16+
17+
/**
18+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
19+
* SPDX-License-Identifier: AGPL-3.0-or-later
20+
*/
21+
#[Group(name: 'DB')]
22+
class CleanupShareTargetTest extends TestCase {
23+
public const TEST_FOLDER_NAME = '/folder_share_api_test';
24+
25+
private CleanupShareTarget $cleanupShareTarget;
26+
27+
protected function setUp(): void {
28+
parent::setUp();
29+
$this->cleanupShareTarget = Server::get(CleanupShareTarget::class);
30+
}
31+
32+
private function createUserShare(string $by, string $target = self::TEST_FOLDER_NAME): IShare {
33+
$userFolder = $this->rootFolder->getUserFolder($by);
34+
35+
try {
36+
$node = $userFolder->get(self::TEST_FOLDER_NAME);
37+
} catch (NotFoundException $e) {
38+
$node = $userFolder->newFolder(self::TEST_FOLDER_NAME);
39+
}
40+
$share1 = $this->shareManager->newShare();
41+
$share1->setNode($node)
42+
->setSharedBy($by)
43+
->setSharedWith(self::TEST_FILES_SHARING_API_USER2)
44+
->setShareType(IShare::TYPE_USER)
45+
->setPermissions(31);
46+
$share = $this->shareManager->createShare($share1);
47+
$share->setStatus(IShare::STATUS_ACCEPTED);
48+
$this->shareManager->updateShare($share);
49+
50+
$share->setTarget($target);
51+
$this->shareManager->moveShare($share, self::TEST_FILES_SHARING_API_USER2);
52+
53+
$share = $this->shareManager->getShareById($share->getFullId());
54+
$this->assertEquals($target, $share->getTarget());
55+
56+
return $share;
57+
}
58+
59+
public function testBasicRepair() {
60+
$share = $this->createUserShare(self::TEST_FILES_SHARING_API_USER1, self::TEST_FOLDER_NAME . ' (2) (2) (2) (2)');
61+
62+
$this->cleanupShareTarget->run(new NullOutput());
63+
64+
$share = $this->shareManager->getShareById($share->getFullId());
65+
$this->assertEquals(self::TEST_FOLDER_NAME, $share->getTarget());
66+
}
67+
68+
public function testRepairConflictFile() {
69+
$share = $this->createUserShare(self::TEST_FILES_SHARING_API_USER1, self::TEST_FOLDER_NAME . ' (2) (2) (2) (2)');
70+
71+
$userFolder2 = $this->rootFolder->getUserFolder(self::TEST_FILES_SHARING_API_USER2);
72+
$folder = $userFolder2->newFolder(self::TEST_FOLDER_NAME);
73+
74+
$this->cleanupShareTarget->run(new NullOutput());
75+
$folder->delete();
76+
77+
$share = $this->shareManager->getShareById($share->getFullId());
78+
$this->assertEquals(self::TEST_FOLDER_NAME . ' (2)', $share->getTarget());
79+
}
80+
81+
public function testRepairConflictShare() {
82+
$share = $this->createUserShare(self::TEST_FILES_SHARING_API_USER1, self::TEST_FOLDER_NAME . ' (2) (2) (2) (2)');
83+
84+
$share2 = $this->createUserShare(self::TEST_FILES_SHARING_API_USER3);
85+
86+
$this->cleanupShareTarget->run(new NullOutput());
87+
88+
$share2 = $this->shareManager->getShareById($share2->getFullId());
89+
$this->assertEquals(self::TEST_FOLDER_NAME, $share2->getTarget());
90+
$share = $this->shareManager->getShareById($share->getFullId());
91+
$this->assertEquals(self::TEST_FOLDER_NAME . ' (2)', $share->getTarget());
92+
}
93+
94+
public function testRepairMultipleConflicting() {
95+
$share = $this->createUserShare(self::TEST_FILES_SHARING_API_USER1, self::TEST_FOLDER_NAME . ' (2) (2) (2) (2)');
96+
$share2 = $this->createUserShare(self::TEST_FILES_SHARING_API_USER3, self::TEST_FOLDER_NAME . ' (2) (2) (2) (2) (2)');
97+
98+
$this->cleanupShareTarget->run(new NullOutput());
99+
100+
$share = $this->shareManager->getShareById($share->getFullId());
101+
$share2 = $this->shareManager->getShareById($share2->getFullId());
102+
103+
// there is no guarantee for what order the 2 shares got repaired by
104+
$targets = [
105+
$share->getTarget(),
106+
$share2->getTarget(),
107+
];
108+
sort($targets);
109+
$this->assertEquals([
110+
self::TEST_FOLDER_NAME,
111+
self::TEST_FOLDER_NAME . ' (2)'
112+
], $targets);
113+
}
114+
}

‎lib/private/Repair.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
use OC\Template\JSCombiner;
6161
use OCA\DAV\Migration\DeleteSchedulingObjects;
6262
use OCA\DAV\Migration\RemoveObjectProperties;
63+
use OCA\Files_Sharing\Repair\CleanupShareTarget;
6364
use OCP\AppFramework\QueryException;
6465
use OCP\AppFramework\Utility\ITimeFactory;
6566
use OCP\Collaboration\Resources\IManager;
@@ -221,6 +222,7 @@ public static function getExpensiveRepairSteps() {
221222
),
222223
\OCP\Server::get(DeleteSchedulingObjects::class),
223224
\OC::$server->get(RemoveObjectProperties::class),
225+
\OCP\Server::get(CleanupShareTarget::class),
224226
];
225227
}
226228

0 commit comments

Comments
 (0)