Skip to content

Commit 0b25353

Browse files
committed
fix(systemtags): remove duplicates, prevent and sanitize existing tags
Signed-off-by: skjnldsv <skjnldsv@protonmail.com> Assisted-by: ClaudeCode:claude-opus-4-8
1 parent 5ae39da commit 0b25353

12 files changed

Lines changed: 384 additions & 3 deletions

File tree

‎apps/dav/lib/SystemTag/SystemTagPlugin.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ private function createTag($data, $contentType = 'application/json') {
164164
throw new BadRequest('Missing "name" attribute');
165165
}
166166

167-
$tagName = $data['name'];
167+
$tagName = Util::sanitizeWordsAndEmojis($data['name']);
168168
$userVisible = true;
169169
$userAssignable = true;
170170

‎apps/settings/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@
130130
'OCA\\Settings\\SetupChecks\\PushService' => $baseDir . '/../lib/SetupChecks/PushService.php',
131131
'OCA\\Settings\\SetupChecks\\RandomnessSecure' => $baseDir . '/../lib/SetupChecks/RandomnessSecure.php',
132132
'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => $baseDir . '/../lib/SetupChecks/ReadOnlyConfig.php',
133+
'OCA\\Settings\\SetupChecks\\RepairSanitizeSystemTagsAvailable' => $baseDir . '/../lib/SetupChecks/RepairSanitizeSystemTagsAvailable.php',
133134
'OCA\\Settings\\SetupChecks\\SchedulingTableSize' => $baseDir . '/../lib/SetupChecks/SchedulingTableSize.php',
134135
'OCA\\Settings\\SetupChecks\\SecurityHeaders' => $baseDir . '/../lib/SetupChecks/SecurityHeaders.php',
135136
'OCA\\Settings\\SetupChecks\\ServerIdConfig' => $baseDir . '/../lib/SetupChecks/ServerIdConfig.php',

‎apps/settings/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ class ComposerStaticInitSettings
145145
'OCA\\Settings\\SetupChecks\\PushService' => __DIR__ . '/..' . '/../lib/SetupChecks/PushService.php',
146146
'OCA\\Settings\\SetupChecks\\RandomnessSecure' => __DIR__ . '/..' . '/../lib/SetupChecks/RandomnessSecure.php',
147147
'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => __DIR__ . '/..' . '/../lib/SetupChecks/ReadOnlyConfig.php',
148+
'OCA\\Settings\\SetupChecks\\RepairSanitizeSystemTagsAvailable' => __DIR__ . '/..' . '/../lib/SetupChecks/RepairSanitizeSystemTagsAvailable.php',
148149
'OCA\\Settings\\SetupChecks\\SchedulingTableSize' => __DIR__ . '/..' . '/../lib/SetupChecks/SchedulingTableSize.php',
149150
'OCA\\Settings\\SetupChecks\\SecurityHeaders' => __DIR__ . '/..' . '/../lib/SetupChecks/SecurityHeaders.php',
150151
'OCA\\Settings\\SetupChecks\\ServerIdConfig' => __DIR__ . '/..' . '/../lib/SetupChecks/ServerIdConfig.php',

‎apps/settings/lib/AppInfo/Application.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
use OCA\Settings\SetupChecks\PushService;
6868
use OCA\Settings\SetupChecks\RandomnessSecure;
6969
use OCA\Settings\SetupChecks\ReadOnlyConfig;
70+
use OCA\Settings\SetupChecks\RepairSanitizeSystemTagsAvailable;
7071
use OCA\Settings\SetupChecks\SchedulingTableSize;
7172
use OCA\Settings\SetupChecks\SecurityHeaders;
7273
use OCA\Settings\SetupChecks\ServerIdConfig;
@@ -211,6 +212,7 @@ public function register(IRegistrationContext $context): void {
211212
$context->registerSetupCheck(PhpOutputBuffering::class);
212213
$context->registerSetupCheck(RandomnessSecure::class);
213214
$context->registerSetupCheck(ReadOnlyConfig::class);
215+
$context->registerSetupCheck(RepairSanitizeSystemTagsAvailable::class);
214216
$context->registerSetupCheck(SecurityHeaders::class);
215217
$context->registerSetupCheck(ServerIdConfig::class);
216218
$context->registerSetupCheck(SchedulingTableSize::class);
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
namespace OCA\Settings\SetupChecks;
10+
11+
use OC\Repair\RepairSanitizeSystemTags;
12+
use OCP\IL10N;
13+
use OCP\SetupCheck\ISetupCheck;
14+
use OCP\SetupCheck\SetupResult;
15+
16+
class RepairSanitizeSystemTagsAvailable implements ISetupCheck {
17+
18+
public function __construct(
19+
private RepairSanitizeSystemTags $repairSanitizeSystemTags,
20+
private IL10N $l10n,
21+
) {
22+
}
23+
24+
public function getCategory(): string {
25+
return 'system';
26+
}
27+
28+
public function getName(): string {
29+
return $this->l10n->t('Sanitize and merge duplicate system tags available');
30+
}
31+
32+
public function run(): SetupResult {
33+
if ($this->repairSanitizeSystemTags->migrationsAvailable()) {
34+
return SetupResult::warning(
35+
$this->l10n->t('One or more system tags need to be sanitized or merged. This can take a long time on larger instances so this is not done automatically during upgrades. Use the command `occ maintenance:repair --include-expensive` to perform the migrations.'),
36+
);
37+
} else {
38+
return SetupResult::success('None');
39+
}
40+
}
41+
}

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2131,6 +2131,7 @@
21312131
'OC\\Repair\\RepairInvalidShares' => $baseDir . '/lib/private/Repair/RepairInvalidShares.php',
21322132
'OC\\Repair\\RepairLogoDimension' => $baseDir . '/lib/private/Repair/RepairLogoDimension.php',
21332133
'OC\\Repair\\RepairMimeTypes' => $baseDir . '/lib/private/Repair/RepairMimeTypes.php',
2134+
'OC\\Repair\\RepairSanitizeSystemTags' => $baseDir . '/lib/private/Repair/RepairSanitizeSystemTags.php',
21342135
'OC\\RichObjectStrings\\RichTextFormatter' => $baseDir . '/lib/private/RichObjectStrings/RichTextFormatter.php',
21352136
'OC\\RichObjectStrings\\Validator' => $baseDir . '/lib/private/RichObjectStrings/Validator.php',
21362137
'OC\\Route\\CachingRouter' => $baseDir . '/lib/private/Route/CachingRouter.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2172,6 +2172,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
21722172
'OC\\Repair\\RepairInvalidShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairInvalidShares.php',
21732173
'OC\\Repair\\RepairLogoDimension' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairLogoDimension.php',
21742174
'OC\\Repair\\RepairMimeTypes' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairMimeTypes.php',
2175+
'OC\\Repair\\RepairSanitizeSystemTags' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairSanitizeSystemTags.php',
21752176
'OC\\RichObjectStrings\\RichTextFormatter' => __DIR__ . '/../../..' . '/lib/private/RichObjectStrings/RichTextFormatter.php',
21762177
'OC\\RichObjectStrings\\Validator' => __DIR__ . '/../../..' . '/lib/private/RichObjectStrings/Validator.php',
21772178
'OC\\Route\\CachingRouter' => __DIR__ . '/../../..' . '/lib/private/Route/CachingRouter.php',

‎lib/private/Repair.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
use OC\Repair\RepairInvalidShares;
5959
use OC\Repair\RepairLogoDimension;
6060
use OC\Repair\RepairMimeTypes;
61+
use OC\Repair\RepairSanitizeSystemTags;
6162
use OCP\EventDispatcher\IEventDispatcher;
6263
use OCP\IConfig;
6364
use OCP\IDBConnection;
@@ -204,6 +205,7 @@ public static function getRepairSteps(bool $includeExpensive = false): array {
204205
Server::get(OldGroupMembershipShares::class),
205206
Server::get(RemoveBrokenProperties::class),
206207
Server::get(RepairMimeTypes::class),
208+
Server::get(RepairSanitizeSystemTags::class),
207209
];
208210
$repairSteps = array_merge($repairSteps, $expensiveSteps);
209211
}
Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
<?php
2+
3+
/**
4+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: AGPL-3.0-only
6+
*/
7+
8+
declare(strict_types=1);
9+
10+
namespace OC\Repair;
11+
12+
use OC\Migration\NullOutput;
13+
use OCP\DB\QueryBuilder\IQueryBuilder;
14+
use OCP\IDBConnection;
15+
use OCP\Migration\IOutput;
16+
use OCP\Migration\IRepairStep;
17+
use OCP\Util;
18+
19+
class RepairSanitizeSystemTags implements IRepairStep {
20+
private bool $dryRun = false;
21+
private int $changeCount = 0;
22+
23+
public function __construct(
24+
protected IDBConnection $connection,
25+
) {
26+
}
27+
28+
public function getName(): string {
29+
return 'Sanitize and merge duplicate system tags';
30+
}
31+
32+
public function migrationsAvailable(): bool {
33+
$this->dryRun = true;
34+
$this->sanitizeAndMergeTags(new NullOutput());
35+
$this->dryRun = false;
36+
return $this->changeCount > 0;
37+
}
38+
39+
public function run(IOutput $output): void {
40+
$this->dryRun = false;
41+
$this->sanitizeAndMergeTags($output);
42+
}
43+
44+
private function sanitizeAndMergeTags(IOutput $output): void {
45+
$output->info('Starting sanitization of system tags...');
46+
47+
// This is a manually triggered expensive repair step, so we load all
48+
// tags in memory: we need the full set to group duplicates by their
49+
// sanitized name anyway. Each tag already carries its object count.
50+
$tags = $this->getAllTags();
51+
52+
// Group tags by sanitized name
53+
$sanitizedMap = [];
54+
foreach ($tags as $tag) {
55+
$sanitizedMap[$tag['sanitizedName']][] = $tag;
56+
}
57+
58+
$output->info(count($tags) . ' tags found with ' . count($sanitizedMap) . ' unique sanitized names.');
59+
60+
// Process each sanitized name group
61+
foreach ($sanitizedMap as $sanitizedName => $group) {
62+
// Single tag, no duplicates found
63+
if (count($group) === 1) {
64+
$tag = $group[0];
65+
if ($tag['originalName'] !== $sanitizedName) {
66+
if (!$this->dryRun) {
67+
$qb = $this->connection->getQueryBuilder();
68+
$qb->update('systemtag')
69+
->set('name', $qb->createNamedParameter($sanitizedName))
70+
->where($qb->expr()->eq('id', $qb->createNamedParameter($tag['id'])))
71+
->executeStatement();
72+
}
73+
$this->changeCount++;
74+
$output->info("Sanitized tag ID {$tag['id']}: '{$tag['originalName']}' → '$sanitizedName'");
75+
}
76+
continue;
77+
}
78+
79+
// Multiple tags with same sanitized name - merge them
80+
$this->mergeTagGroup($group, $sanitizedName, $output);
81+
}
82+
83+
$output->info('System tag sanitization and merge completed.');
84+
}
85+
86+
private function mergeTagGroup(array $group, string $sanitizedName, IOutput $output): void {
87+
// Validate that all tags in the group have the same visibility and editable settings
88+
$firstTag = $group[0];
89+
$visibility = $firstTag['visibility'];
90+
$editable = $firstTag['editable'];
91+
92+
foreach ($group as $tag) {
93+
if ($tag['visibility'] !== $visibility || $tag['editable'] !== $editable) {
94+
$output->warning(
95+
"Cannot merge tag group '$sanitizedName': tags have different visibility or editable settings. "
96+
. 'Manual verification required. Tag IDs: ' . implode(', ', array_column($group, 'id'))
97+
);
98+
return;
99+
}
100+
}
101+
102+
// Determine which tag to keep (most object mappings, then lowest ID as tiebreaker)
103+
$keepTag = null;
104+
$maxCount = -1;
105+
106+
foreach ($group as $tag) {
107+
$count = $tag['objectCount'];
108+
if ($count > $maxCount || ($count === $maxCount && ($keepTag === null || $tag['id'] < $keepTag['id']))) {
109+
$maxCount = $count;
110+
$keepTag = $tag;
111+
}
112+
}
113+
114+
$keepId = $keepTag['id'];
115+
if ($keepTag === null) {
116+
$output->warning("Cannot merge tag group '$sanitizedName': unable to determine which tag to keep");
117+
return;
118+
}
119+
120+
$duplicateIds = array_filter(array_column($group, 'id'), fn ($id) => $id !== $keepId);
121+
if (empty($duplicateIds)) {
122+
return;
123+
}
124+
125+
if (!$this->dryRun) {
126+
$this->connection->beginTransaction();
127+
try {
128+
// Step 1: Delete ALL mappings from duplicate tags that conflict with keepId
129+
// This must happen FIRST before any updates to avoid unique constraint violations
130+
$this->deleteConflictingMappings($duplicateIds, $keepId);
131+
132+
// Step 2: Update all remaining mappings from duplicates to keepId
133+
// These won't conflict because we just deleted the conflicts
134+
$qb = $this->connection->getQueryBuilder();
135+
$qb->update('systemtag_object_mapping')
136+
->set('systemtagid', $qb->createNamedParameter($keepId))
137+
->where($qb->expr()->in('systemtagid', $qb->createNamedParameter($duplicateIds, IQueryBuilder::PARAM_INT_ARRAY)))
138+
->executeStatement();
139+
140+
// Step 3: Delete duplicate tags in bulk (safe now that mappings are gone)
141+
$qb = $this->connection->getQueryBuilder();
142+
$qb->delete('systemtag')
143+
->where($qb->expr()->in('id', $qb->createNamedParameter($duplicateIds, IQueryBuilder::PARAM_INT_ARRAY)))
144+
->executeStatement();
145+
146+
// Step 4: Sanitize the kept tag name if needed
147+
// This is safe because we've already deleted all duplicates with the same sanitized name
148+
if ($keepTag['originalName'] !== $sanitizedName) {
149+
$qb = $this->connection->getQueryBuilder();
150+
$qb->update('systemtag')
151+
->set('name', $qb->createNamedParameter($sanitizedName))
152+
->where($qb->expr()->eq('id', $qb->createNamedParameter($keepId)))
153+
->executeStatement();
154+
}
155+
156+
$this->connection->commit();
157+
} catch (\Exception $e) {
158+
$this->connection->rollBack();
159+
$output->warning("Failed to merge tag group '$sanitizedName': " . $e->getMessage());
160+
return;
161+
}
162+
}
163+
164+
$this->changeCount += count($duplicateIds);
165+
if ($keepTag['originalName'] !== $sanitizedName) {
166+
$this->changeCount++;
167+
}
168+
169+
$duplicateIdsList = implode(', ', $duplicateIds);
170+
$output->info("Merged tags [$duplicateIdsList] into ID $keepId (sanitized: '$sanitizedName')");
171+
}
172+
173+
/**
174+
* Delete mappings from duplicate tags where the same object is already mapped to keepId
175+
* This prevents unique constraint violations when updating systemtagid
176+
*/
177+
private function deleteConflictingMappings(array $duplicateIds, int $keepId): void {
178+
$batchSize = 1000;
179+
$batch = [];
180+
181+
// Stream keepId mappings and process in batches
182+
$qb = $this->connection->getQueryBuilder();
183+
$qb->select('objectid', 'objecttype')
184+
->from('systemtag_object_mapping')
185+
->where($qb->expr()->eq('systemtagid', $qb->createNamedParameter($keepId)));
186+
187+
$result = $qb->executeQuery();
188+
189+
while ($mapping = $result->fetch()) {
190+
$batch[] = $mapping;
191+
192+
// When batch is full, delete conflicts for this batch
193+
if (count($batch) >= $batchSize) {
194+
$this->deleteBatchConflicts($batch, $duplicateIds);
195+
$batch = []; // Clear batch
196+
}
197+
}
198+
199+
$result->closeCursor();
200+
201+
// Process remaining mappings in the last batch
202+
if (!empty($batch)) {
203+
$this->deleteBatchConflicts($batch, $duplicateIds);
204+
}
205+
}
206+
207+
/**
208+
* Delete mappings in a batch that conflict with keepId mappings
209+
*/
210+
private function deleteBatchConflicts(array $batch, array $duplicateIds): void {
211+
$qb = $this->connection->getQueryBuilder();
212+
$qb->delete('systemtag_object_mapping')
213+
->where($qb->expr()->in('systemtagid', $qb->createNamedParameter($duplicateIds, IQueryBuilder::PARAM_INT_ARRAY)));
214+
215+
$orX = $qb->expr()->orX();
216+
foreach ($batch as $mapping) {
217+
$orX->add($qb->expr()->andX(
218+
$qb->expr()->eq('objectid', $qb->createNamedParameter($mapping['objectid'])),
219+
$qb->expr()->eq('objecttype', $qb->createNamedParameter($mapping['objecttype']))
220+
));
221+
}
222+
$qb->andWhere($orX);
223+
$qb->executeStatement();
224+
}
225+
226+
/**
227+
* Fetch all tags together with their object mapping count in a single query.
228+
*
229+
* @return list<array{id: int, originalName: string, sanitizedName: string, visibility: int, editable: int, objectCount: int}>
230+
*/
231+
private function getAllTags(): array {
232+
$qb = $this->connection->getQueryBuilder();
233+
$qb->select('t.id', 't.name', 't.visibility', 't.editable')
234+
->selectAlias($qb->func()->count('m.systemtagid'), 'object_count')
235+
->from('systemtag', 't')
236+
->leftJoin('t', 'systemtag_object_mapping', 'm', $qb->expr()->eq('t.id', 'm.systemtagid'))
237+
->groupBy('t.id', 't.name', 't.visibility', 't.editable')
238+
->orderBy('t.name')
239+
->addOrderBy('t.id');
240+
241+
$tags = [];
242+
$result = $qb->executeQuery();
243+
while ($row = $result->fetch()) {
244+
$tags[] = [
245+
'id' => (int)$row['id'],
246+
'originalName' => $row['name'],
247+
'sanitizedName' => Util::sanitizeWordsAndEmojis($row['name']),
248+
'visibility' => (int)$row['visibility'],
249+
'editable' => (int)$row['editable'],
250+
'objectCount' => (int)$row['object_count'],
251+
];
252+
}
253+
$result->closeCursor();
254+
return $tags;
255+
}
256+
}

0 commit comments

Comments
 (0)