Skip to content

Commit 6f1e914

Browse files
authored
Merge pull request #56079 from nextcloud/fix/systemtags
fix(systemtags): remove duplicates, cleanup existing tags
2 parents e4739c4 + 84a8e5b commit 6f1e914

13 files changed

Lines changed: 601 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: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
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+
10+
namespace OCA\Settings\SetupChecks;
11+
12+
use OC\Repair\RepairSanitizeSystemTags;
13+
use OCP\IL10N;
14+
use OCP\SetupCheck\ISetupCheck;
15+
use OCP\SetupCheck\SetupResult;
16+
17+
class RepairSanitizeSystemTagsAvailable implements ISetupCheck {
18+
19+
public function __construct(
20+
private RepairSanitizeSystemTags $repairSanitizeSystemTags,
21+
private IL10N $l10n,
22+
) {
23+
}
24+
25+
#[\Override]
26+
public function getCategory(): string {
27+
return 'system';
28+
}
29+
30+
#[\Override]
31+
public function getName(): string {
32+
return $this->l10n->t('Sanitize and merge duplicate system tags available');
33+
}
34+
35+
#[\Override]
36+
public function run(): SetupResult {
37+
if ($this->repairSanitizeSystemTags->migrationsAvailable()) {
38+
return SetupResult::warning(
39+
$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.'),
40+
);
41+
} else {
42+
return SetupResult::success('None');
43+
}
44+
}
45+
}

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

0 commit comments

Comments
 (0)