Skip to content

Commit b0b8d8e

Browse files
committed
fix(Migrations): Introduce url_hash again in case the first migration failed but was marked as done
see #2475 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
1 parent 0046dc2 commit b0b8d8e

3 files changed

Lines changed: 183 additions & 9 deletions

File tree

lib/Migration/Version016002000Date20260218134723.php

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,17 +38,22 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array
3838
->where($setQb->expr()->eq('id', $setQb->createParameter('id')));
3939
$i = 1;
4040
$this->db->beginTransaction();
41-
while ($row = $result->fetch()) {
42-
if ($i++ % 1000 === 0) {
43-
$this->db->commit();
44-
$this->db->beginTransaction();
41+
try {
42+
while ($row = $result->fetch()) {
43+
if ($i++ % 1000 === 0) {
44+
$this->db->commit();
45+
$this->db->beginTransaction();
46+
}
47+
$setQb->setParameter('url_hash', hash('xxh128', $row['url']));
48+
$setQb->setParameter('id', $row['id']);
49+
$setQb->executeStatement();
50+
$output->advance();
4551
}
46-
$setQb->setParameter('url_hash', hash('xxh128', $row['url']));
47-
$setQb->setParameter('id', $row['id']);
48-
$setQb->executeStatement();
49-
$output->advance();
52+
$this->db->commit();
53+
} catch (\Throwable $e) {
54+
$this->db->rollBack();
55+
throw $e;
5056
}
51-
$this->db->commit();
5257
$output->finishProgress();
5358
$result->closeCursor();
5459
}
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
<?php
2+
3+
/*
4+
* Copyright (c) 2020-2024. The Nextcloud Bookmarks contributors.
5+
*
6+
* This file is licensed under the Affero General Public License version 3 or later. See the COPYING file.
7+
*/
8+
9+
namespace OCA\Bookmarks\Migration;
10+
11+
use Closure;
12+
use Doctrine\DBAL\Schema\SchemaException;
13+
use OCP\DB\ISchemaWrapper;
14+
use OCP\IDBConnection;
15+
use OCP\Migration\IOutput;
16+
17+
/**
18+
* Recovery migration for instances where the url_hash migration
19+
* (Version016002000Date20260201124723) was recorded in oc_migrations but never
20+
* actually applied to the schema — e.g. an upgrade that failed in
21+
* postSchemaChange on a unique-constraint violation and was then unstuck by a
22+
* DB snapshot restore or by manually marking the version as executed.
23+
* See https://github.com/nextcloud/bookmarks/issues/2475 and #2374.
24+
*
25+
* The original migration guards column creation with a `hasColumn` check and,
26+
* being recorded as executed, never runs again — so there is otherwise no code
27+
* path that will ever create the column. This migration re-establishes the
28+
* `url_hash` column and backfills it. The unique index is added by the
29+
* following migration (Version016002000Date20260717134723), which must run
30+
* only after duplicates have been merged and hashes written here, so that the
31+
* index cannot fail on colliding hashes.
32+
*
33+
* All steps are idempotent: on a healthy instance the column already exists,
34+
* there are no NULL hashes to backfill, and this migration is effectively a
35+
* no-op.
36+
*/
37+
class Version016002000Date20260717124723 extends Version016002000Date20260218124723 {
38+
public function __construct(
39+
private IDBConnection $db,
40+
) {
41+
parent::__construct($db);
42+
}
43+
44+
/**
45+
* @throws SchemaException
46+
*/
47+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) {
48+
/** @var ISchemaWrapper $schema */
49+
$schema = $schemaClosure();
50+
if ($schema->hasTable('bookmarks')) {
51+
$table = $schema->getTable('bookmarks');
52+
if (!$table->hasColumn('url_hash')) {
53+
$output->info('Recovering missing url_hash column on the bookmarks table');
54+
$table->addColumn('url_hash', 'string', [
55+
'notnull' => false,
56+
'length' => 32,
57+
]);
58+
}
59+
}
60+
61+
return $schema;
62+
}
63+
64+
public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) {
65+
// Only do any work if there are bookmarks without a hash. On healthy
66+
// instances this short-circuits before the (expensive) dedup scan.
67+
$countQb = $this->db->getQueryBuilder();
68+
$countQb->select($countQb->func()->count('id'))
69+
->from('bookmarks')
70+
->where($countQb->expr()->isNull('url_hash'));
71+
$result = $countQb->executeQuery();
72+
$count = (int)$result->fetchOne();
73+
$result->closeCursor();
74+
75+
if ($count === 0) {
76+
return;
77+
}
78+
79+
// Merge per-user duplicate URLs before hashing, otherwise duplicates
80+
// would produce colliding (user_id, url_hash) pairs and break the
81+
// unique index added by the next migration.
82+
$this->deduplicateAll($output);
83+
84+
$output->info('Hashing URLs of n=' . $count . ' bookmarks');
85+
$output->startProgress($count);
86+
87+
$qb = $this->db->getQueryBuilder();
88+
$qb->select('id', 'url')
89+
->from('bookmarks')
90+
->where($qb->expr()->isNull('url_hash'));
91+
$result = $qb->executeQuery();
92+
93+
$setQb = $this->db->getQueryBuilder();
94+
$setQb->update('bookmarks')
95+
->set('url_hash', $setQb->createParameter('url_hash'))
96+
->where($setQb->expr()->eq('id', $setQb->createParameter('id')));
97+
98+
$i = 1;
99+
$this->db->beginTransaction();
100+
try {
101+
while ($row = $result->fetch()) {
102+
if ($i++ % 1000 === 0) {
103+
$this->db->commit();
104+
$this->db->beginTransaction();
105+
}
106+
$setQb->setParameter('url_hash', hash('xxh128', $row['url']));
107+
$setQb->setParameter('id', $row['id']);
108+
$setQb->executeStatement();
109+
$output->advance();
110+
}
111+
$this->db->commit();
112+
} catch (\Throwable $e) {
113+
$this->db->rollBack();
114+
throw $e;
115+
}
116+
$output->finishProgress();
117+
$result->closeCursor();
118+
}
119+
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php
2+
3+
/*
4+
* Copyright (c) 2020-2024. The Nextcloud Bookmarks contributors.
5+
*
6+
* This file is licensed under the Affero General Public License version 3 or later. See the COPYING file.
7+
*/
8+
9+
namespace OCA\Bookmarks\Migration;
10+
11+
use Closure;
12+
use Doctrine\DBAL\Schema\SchemaException;
13+
use OCP\DB\ISchemaWrapper;
14+
use OCP\IDBConnection;
15+
use OCP\Migration\IOutput;
16+
use OCP\Migration\SimpleMigrationStep;
17+
18+
/**
19+
* Recovery migration (part 2) for instances that lost the url_hash unique
20+
* index. See Version016002000Date20260717124723 and
21+
* https://github.com/nextcloud/bookmarks/issues/2475.
22+
*
23+
* Runs after the column has been (re-)created and hashes have been backfilled
24+
* and de-duplicated, so the unique index cannot fail on colliding hashes.
25+
* Idempotent: on a healthy instance the index already exists and this is a
26+
* no-op.
27+
*/
28+
class Version016002000Date20260717134723 extends SimpleMigrationStep {
29+
public function __construct(
30+
private IDBConnection $db,
31+
) {
32+
}
33+
34+
/**
35+
* @throws SchemaException
36+
*/
37+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) {
38+
/** @var ISchemaWrapper $schema */
39+
$schema = $schemaClosure();
40+
if ($schema->hasTable('bookmarks')) {
41+
$table = $schema->getTable('bookmarks');
42+
if ($table->hasColumn('url_hash') && !$table->hasIndex('bookmarks_uniq_url')) {
43+
$output->info('Recovering missing bookmarks_uniq_url unique index');
44+
$table->addUniqueIndex(['user_id', 'url_hash'], 'bookmarks_uniq_url');
45+
}
46+
}
47+
48+
return $schema;
49+
}
50+
}

0 commit comments

Comments
 (0)