Update for legacy guids

This commit is contained in:
DariusIII
2026-08-14 14:05:48 +02:00
parent b04a8c5992
commit b49a0cd4d7
8 changed files with 196 additions and 46 deletions
+65 -22
View File
@@ -17,10 +17,15 @@ use Illuminate\Support\Str;
* repairs the one class of problem that can be fixed without side effects.
*
* `leftguid` is derived data that nothing outside the database depends on, so it is
* rewritten in place. Invalid and duplicated `guid` values are only reported: a guid
* rewritten in place. Oversized and duplicated `guid` values are only reported: a guid
* determines the on-disk NZB path and is published in download links, so replacing one
* is a migration of file and link state rather than a database repair.
*
* The migration narrows `guid` to `CHAR(40) ascii`, so legacy 40 character nZEDb SHA1
* guids are still valid and are reported as informational only. Only values longer than
* 40 characters, values containing non-printable-ASCII bytes, and case-insensitive
* duplicates actually block the rebuild.
*
* Every scan here is either a single aggregate or a keyset (`id > ?`) walk. Never use
* `chunk()`: its `LIMIT/OFFSET` paging degrades quadratically, and on a multi-million
* row `releases` table the later batches re-read millions of rows each.
@@ -31,6 +36,11 @@ use Illuminate\Support\Str;
#[Description('Report release guid problems and resync leftguid so the releases normalization migration can run')]
class ReleasesNormalizeGuids extends Command
{
/** Values that survive the narrowing to `CHAR(40) ascii`. */
private const string SUPPORTED_PATTERN = '/^[\x20-\x7E]{1,40}$/D';
private const string SUPPORTED_REGEXP = '^[ -~]{1,40}$';
private const string UUID_PATTERN = '/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/iD';
private const string UUID_REGEXP = '^[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}$';
@@ -53,24 +63,34 @@ class ReleasesNormalizeGuids extends Command
}
$leftGuids = $this->syncLeftGuids();
$invalid = $this->countInvalidGuids();
$unsupported = $this->countUnsupportedGuids();
$duplicates = $this->countDuplicateGuids();
$legacy = $this->countLegacyGuids();
$this->table(['Issue', 'Releases'], [
['leftguid out of sync with guid', (string) $leftGuids],
['guid is not a UUID', (string) $invalid],
['guid duplicated case-insensitively', (string) $duplicates],
$this->table(['Issue', 'Releases', 'Blocks migration'], [
['leftguid out of sync with guid', (string) $leftGuids, 'no (repaired)'],
['guid longer than 40 chars or not ASCII', (string) $unsupported, 'yes'],
['guid duplicated case-insensitively', (string) $duplicates, 'yes'],
['guid is a legacy non-UUID', (string) $legacy, 'no'],
]);
if ($invalid + $duplicates === 0) {
if ($unsupported + $duplicates === 0) {
if ($legacy > 0) {
$this->line(
$legacy.' releases still carry a legacy non-UUID guid. These fit CHAR(40) ascii and are kept '
.'as-is so their NZB paths and published download links keep working. Only newly created '
.'releases get a UUID, so this count decays on its own.'
);
}
$this->info($this->dryRun
? 'No guid problems found. Re-run without --dry-run to apply the leftguid sync.'
? 'No blocking guid problems found. Re-run without --dry-run to apply the leftguid sync.'
: 'Release guids are consistent.');
return self::SUCCESS;
}
$this->reportBlockingReleases($invalid + $duplicates);
$this->reportBlockingReleases($unsupported + $duplicates);
return self::FAILURE;
}
@@ -116,15 +136,39 @@ class ReleasesNormalizeGuids extends Command
* On MySQL this is a single aggregate. Elsewhere it is a keyset walk that
* applies the pattern in PHP, keeping memory flat regardless of table size.
*/
private function countInvalidGuids(): int
private function countUnsupportedGuids(): int
{
if ($this->isMySql()) {
return $this->invalidGuidQuery()->count();
return $this->unsupportedGuidQuery()->count();
}
$count = 0;
$this->walkByKey(function (object $row) use (&$count): bool {
if ($this->isInvalidGuid($row->guid)) {
if ($this->isUnsupportedGuid($row->guid)) {
$count++;
}
return true;
});
return $count;
}
/**
* Legacy guids fit the narrowed column, so this is reported for visibility only.
* It is the count of releases whose download links predate UUID generation.
*/
private function countLegacyGuids(): int
{
if ($this->isMySql()) {
return DB::table('releases')
->whereRaw("`guid` NOT REGEXP '".self::UUID_REGEXP."'")
->count();
}
$count = 0;
$this->walkByKey(function (object $row) use (&$count): bool {
if (! is_string($row->guid) || preg_match(self::UUID_PATTERN, $row->guid) !== 1) {
$count++;
}
@@ -160,7 +204,6 @@ class ReleasesNormalizeGuids extends Command
.'row that references the old value. Resolve them deliberately: delete the affected releases, or assign '
.'new guids and relocate the matching NZB files yourself.'
);
$sample = $this->sampleBlockingReleases();
if ($sample === []) {
return;
@@ -181,23 +224,23 @@ class ReleasesNormalizeGuids extends Command
private function sampleBlockingReleases(): array
{
if ($this->isMySql()) {
$invalid = $this->invalidGuidQuery()
$unsupported = $this->unsupportedGuidQuery()
->select(['id', 'guid', 'name'])
->orderBy('id')
->limit(self::SAMPLE_SIZE)
->get()
->all();
if (count($invalid) >= self::SAMPLE_SIZE) {
return $invalid;
if (count($unsupported) >= self::SAMPLE_SIZE) {
return $unsupported;
}
return [...$invalid, ...$this->sampleDuplicateGuids(self::SAMPLE_SIZE - count($invalid), $invalid)];
return [...$unsupported, ...$this->sampleDuplicateGuids(self::SAMPLE_SIZE - count($unsupported), $unsupported)];
}
$sample = [];
$this->walkByKey(function (object $row) use (&$sample): bool {
if ($this->isInvalidGuid($row->guid)) {
if ($this->isUnsupportedGuid($row->guid)) {
$sample[] = $row;
}
@@ -259,17 +302,17 @@ class ReleasesNormalizeGuids extends Command
} while ($rows->count() === $chunk);
}
private function invalidGuidQuery(): Builder
private function unsupportedGuidQuery(): Builder
{
return DB::table('releases')->where(function ($query): void {
$query->whereNull('guid')
->orWhereRaw("`guid` NOT REGEXP '".self::UUID_REGEXP."'");
->orWhereRaw("`guid` NOT REGEXP '".self::SUPPORTED_REGEXP."'");
});
}
private function isInvalidGuid(mixed $guid): bool
private function isUnsupportedGuid(mixed $guid): bool
{
return ! is_string($guid) || preg_match(self::UUID_PATTERN, $guid) !== 1;
return ! is_string($guid) || preg_match(self::SUPPORTED_PATTERN, $guid) !== 1;
}
private function isMySql(): bool
@@ -63,6 +63,16 @@ return new class extends Migration
'additional_pp_claim_token',
];
/**
* `guid` is narrowed to `CHAR(40) ascii`, so the only values that cannot survive
* the rebuild are ones longer than 40 characters or containing bytes outside
* printable ASCII. Legacy 40 character nZEDb SHA1 guids are explicitly still
* valid; only newly generated guids are UUIDs.
*/
private const string GUID_REGEXP = '^[ -~]{1,40}$';
private const string GUID_PATTERN = '/^[\x20-\x7E]{1,40}$/D';
public function up(): void
{
if (! Schema::hasTable('releases')) {
@@ -286,7 +296,7 @@ return new class extends Migration
}
}
$specifications[] = 'MODIFY `guid` CHAR(36) CHARACTER SET ascii COLLATE ascii_general_ci NOT NULL';
$specifications[] = 'MODIFY `guid` CHAR(40) CHARACTER SET ascii COLLATE ascii_general_ci NOT NULL';
$specifications[] = "MODIFY `leftguid` CHAR(1) CHARACTER SET ascii COLLATE ascii_general_ci NOT NULL COMMENT 'The first letter of the release guid'";
foreach (self::CLAIM_TOKEN_COLUMNS as $column) {
if (Schema::hasColumn('releases', $column)) {
@@ -560,12 +570,12 @@ return new class extends Migration
if ($this->isMariaDbOrMySql()) {
$invalidGuids = DB::table('releases')
->whereNull('guid')
->orWhereRaw("guid NOT REGEXP '^[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}$'")
->orWhereRaw("`guid` NOT REGEXP '".self::GUID_REGEXP."'")
->count();
} else {
$invalidGuids = DB::table('releases')->pluck('guid')->filter(
static fn (mixed $guid): bool => ! is_string($guid)
|| preg_match('/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/iD', $guid) !== 1,
|| preg_match(self::GUID_PATTERN, $guid) !== 1,
)->count();
}
@@ -579,7 +589,7 @@ return new class extends Migration
$blockers = array_filter([
'duplicate release IDs' => $duplicateIds,
'invalid release GUIDs' => $invalidGuids,
'release GUIDs that do not fit CHAR(40) ascii' => $invalidGuids,
'case-insensitive duplicate release GUIDs' => $duplicateGuids,
'mismatched release leftguid values' => $leftGuidMismatches,
]);
@@ -23,6 +23,11 @@ use Illuminate\Support\Facades\Schema;
* 2. Both claim tokens narrowed to `CHAR(32) ascii`. They only ever hold
* `bin2hex(random_bytes(16))`, so `VARCHAR(64)` utf8mb4 reserved four bytes
* per character for hex digits.
* 3. `guid` widened from `CHAR(36)` to `CHAR(40) ascii`. The original rebuild
* assumed every guid was a UUID, but legacy nZEDb installs still hold 40
* character SHA1 guids that `CHAR(36)` would silently truncate. Note that this
* only restores the column shape: an install that already ran the `CHAR(36)`
* version has lost those four characters and must restore them from a backup.
*
* Both now happen inside the normalization rebuild's single `ALTER`, because the
* claim token type change is `ALGORITHM=COPY` only and would otherwise force a
@@ -52,8 +57,9 @@ return new class extends Migration
$indexNeedsSize = ! $this->ppClaimIndexCoversSize();
$columnsToNarrow = $this->claimTokenColumnsToNarrow();
$guidNeedsWidening = $this->guidNeedsWidening();
if (! $indexNeedsSize && $columnsToNarrow === []) {
if (! $indexNeedsSize && $columnsToNarrow === [] && ! $guidNeedsWidening) {
return;
}
@@ -69,6 +75,9 @@ return new class extends Migration
foreach ($columnsToNarrow as $column) {
$specifications[] = 'MODIFY `'.$column.'` CHAR(32) CHARACTER SET ascii COLLATE ascii_general_ci NULL';
}
if ($guidNeedsWidening) {
$specifications[] = 'MODIFY `guid` CHAR(40) CHARACTER SET ascii COLLATE ascii_general_ci NOT NULL';
}
DB::statement('ALTER TABLE '.$this->table('releases').' '.implode(', ', $specifications));
@@ -130,6 +139,25 @@ return new class extends Migration
return $pending;
}
/**
* True only for an install left on the `CHAR(36)` guid shape, which would
* truncate the legacy 40 character SHA1 guids that predate UUID generation.
*/
private function guidNeedsWidening(): bool
{
if (! $this->isMariaDbOrMySql()) {
return false;
}
foreach (Schema::getColumns('releases') as $column) {
if ($column['name'] === 'guid') {
return str_starts_with(strtolower((string) ($column['type'] ?? '')), 'char(36)');
}
}
return false;
}
private function indexExists(string $name): bool
{
foreach (Schema::getIndexes('releases') as $index) {
+1 -1
View File
@@ -1299,7 +1299,7 @@ CREATE TABLE `releases` (
`size` bigint(20) unsigned NOT NULL DEFAULT 0,
`postdate` datetime DEFAULT NULL,
`adddate` datetime DEFAULT NULL,
`guid` char(36) CHARACTER SET ascii COLLATE ascii_general_ci NOT NULL,
`guid` char(40) CHARACTER SET ascii COLLATE ascii_general_ci NOT NULL,
`leftguid` char(1) CHARACTER SET ascii COLLATE ascii_general_ci NOT NULL COMMENT 'The first letter of the release guid',
`fromname` varchar(255) DEFAULT NULL,
`completion` double NOT NULL DEFAULT 0,
@@ -128,14 +128,14 @@ final class ReleaseSchemaOptimizationMigrationTest extends TestCase
public function test_preflight_blocks_bad_identifiers_and_the_opt_out_bypasses_it(): void
{
DB::table('releases')->insert([
['id' => 1, 'guid' => 'not-a-uuid', 'leftguid' => 'z', 'comments' => 0],
['id' => 1, 'guid' => str_repeat('a', 41), 'leftguid' => 'a', 'comments' => 0],
]);
try {
$this->invoke('assertReleaseIdentifiersAreSafe');
$this->fail('The migration accepted an invalid guid.');
$this->fail('The migration accepted a guid that does not fit the narrowed column.');
} catch (RuntimeException $e) {
$this->assertStringContainsString('invalid release GUIDs', $e->getMessage());
$this->assertStringContainsString('release GUIDs that do not fit CHAR(40) ascii', $e->getMessage());
$this->assertStringContainsString('releases:normalize-guids', $e->getMessage());
}
@@ -144,6 +144,18 @@ final class ReleaseSchemaOptimizationMigrationTest extends TestCase
$this->assertSame(1, DB::table('releases')->count());
}
public function test_preflight_accepts_legacy_sha1_guids(): void
{
DB::table('releases')->insert([
['id' => 1, 'guid' => '0c5c002220e26542a4c9dae845a58a38b3e7e63a', 'leftguid' => '0', 'comments' => 0],
['id' => 2, 'guid' => '01234567-89ab-cdef-0123-456789abcdef', 'leftguid' => '0', 'comments' => 0],
]);
$this->invoke('assertReleaseIdentifiersAreSafe');
$this->assertSame(2, DB::table('releases')->count());
}
private function invoke(string $method): void
{
/** @var Migration $migration */
+33 -7
View File
@@ -70,7 +70,7 @@ final class ReleasesNormalizeGuidsTest extends TestCase
public function test_dry_run_reports_without_writing(): void
{
$this->insertRelease(1, '01234567-89ab-cdef-0123-456789abcdef', 'f');
$this->insertRelease(2, 'd41d8cd98f00b204e9800998ecf8427e', 'd');
$this->insertRelease(2, str_repeat('a', 41), 'x');
$status = Artisan::call('releases:normalize-guids', ['--dry-run' => true]);
$output = Artisan::output();
@@ -78,21 +78,47 @@ final class ReleasesNormalizeGuidsTest extends TestCase
$this->assertSame(1, $status);
$this->assertStringContainsString('Dry run: no rows are modified.', $output);
$this->assertSame('f', DB::table('releases')->where('id', 1)->value('leftguid'));
$this->assertSame('d41d8cd98f00b204e9800998ecf8427e', DB::table('releases')->where('id', 2)->value('guid'));
$this->assertSame(str_repeat('a', 41), DB::table('releases')->where('id', 2)->value('guid'));
}
public function test_invalid_guid_is_reported_and_never_rewritten(): void
public function test_legacy_sha1_guid_fits_the_narrowed_column_and_does_not_block(): void
{
$this->insertRelease(1, 'd41d8cd98f00b204e9800998ecf8427e', 'd', ['name' => 'Legacy nZEDb release']);
$sha1 = '0c5c002220e26542a4c9dae845a58a38b3e7e63a';
$this->insertRelease(1, $sha1, '0');
$status = Artisan::call('releases:normalize-guids');
$output = Artisan::output();
$this->assertSame(0, $status);
$this->assertStringContainsString('Release guids are consistent.', $output);
$this->assertStringContainsString('1 releases still carry a legacy non-UUID guid', $output);
$this->assertSame($sha1, DB::table('releases')->where('id', 1)->value('guid'));
}
public function test_oversized_guid_is_reported_and_never_rewritten(): void
{
$tooLong = str_repeat('b', 41);
$this->insertRelease(1, $tooLong, 'b', ['name' => 'Oversized guid release']);
$status = Artisan::call('releases:normalize-guids');
$output = Artisan::output();
$this->assertSame(1, $status);
$this->assertStringContainsString('1 releases have a guid that blocks the normalization migration.', $output);
$this->assertStringContainsString('d41d8cd98f00b204e9800998ecf8427e', $output);
$this->assertStringContainsString('Legacy nZEDb release', $output);
$this->assertSame('d41d8cd98f00b204e9800998ecf8427e', DB::table('releases')->where('id', 1)->value('guid'));
$this->assertStringContainsString('Oversized guid release', $output);
$this->assertSame($tooLong, DB::table('releases')->where('id', 1)->value('guid'));
}
public function test_non_ascii_guid_blocks_the_migration(): void
{
$this->insertRelease(1, 'd41d8cd98f00b204e9800998ecf842ü', 'd', ['name' => 'Non ASCII guid']);
$status = Artisan::call('releases:normalize-guids');
$output = Artisan::output();
$this->assertSame(1, $status);
$this->assertStringContainsString('1 releases have a guid that blocks', $output);
$this->assertStringContainsString('Non ASCII guid', $output);
}
public function test_case_insensitive_duplicates_report_every_id_but_the_lowest(): void
@@ -69,7 +69,7 @@ final class ReleaseSchemaOptimizationMigrationMariaDbTest extends TestCase
$this->assertSame('temporary', DB::table('release_nzb_creation_failures')->where('releases_id', 1)->value('last_error'));
$this->assertSame(['id'], $this->primaryKeyColumns());
$this->assertSame('ascii_general_ci', $this->columnCollation('guid'));
$this->assertSame('char(36)', strtolower($this->columnType('guid')));
$this->assertSame('char(40)', strtolower($this->columnType('guid')));
$this->assertSame('ascii_general_ci', $this->columnCollation('leftguid'));
$this->assertSame(1, (int) DB::table('releases')->where('id', 1)->value('comments'));
$this->assertFalse(Schema::hasColumn('releases', 'nzb_password'));
@@ -113,6 +113,23 @@ final class ReleaseSchemaOptimizationMigrationMariaDbTest extends TestCase
$this->assertDatabaseHas('release_files', ['releases_id' => 1, 'name' => 'kept.nzb']);
}
#[Test]
public function rebuild_keeps_legacy_sha1_guids_intact(): void
{
$sha1 = '0c5c002220e26542a4c9dae845a58a38b3e7e63a';
DB::table('releases')->insert([
'id' => 9001,
'guid' => $sha1,
'leftguid' => '0',
'name' => 'Legacy nZEDb release',
]);
$this->migration()->up();
$this->assertSame('char(40)', strtolower($this->columnType('guid')));
$this->assertSame($sha1, DB::table('releases')->where('id', 9001)->value('guid'));
}
#[Test]
public function rebuild_narrows_claim_tokens_and_covers_size_in_one_pass(): void
{
@@ -46,7 +46,7 @@ final class ReleasesNormalizeGuidsMariaDbTest extends TestCase
DB::statement(
'CREATE TABLE `'.$this->table('releases').'` (
`id` INT UNSIGNED NOT NULL AUTO_INCREMENT,
`guid` VARCHAR(40) NOT NULL,
`guid` VARCHAR(64) NOT NULL,
`leftguid` CHAR(1) NOT NULL DEFAULT "",
`name` VARCHAR(255) NOT NULL DEFAULT "",
PRIMARY KEY (`id`),
@@ -77,19 +77,33 @@ final class ReleasesNormalizeGuidsMariaDbTest extends TestCase
}
#[Test]
public function legacy_md5_guids_are_detected_by_the_regexp_check(): void
public function legacy_sha1_guids_fit_the_narrowed_column_and_do_not_block(): void
{
$this->insert(1, '01234567-89ab-cdef-0123-456789abcdef', '0');
$this->insert(2, 'd41d8cd98f00b204e9800998ecf8427e', 'd', 'Legacy nZEDb release');
$this->insert(3, 'not-a-uuid-at-all-------------------x', 'n');
$this->insert(2, '0c5c002220e26542a4c9dae845a58a38b3e7e63a', '0', 'Legacy nZEDb release');
$status = Artisan::call('releases:normalize-guids');
$output = Artisan::output();
$this->assertSame(0, $status);
$this->assertStringContainsString('Release guids are consistent.', $output);
$this->assertStringContainsString('1 releases still carry a legacy non-UUID guid', $output);
$this->assertSame('0c5c002220e26542a4c9dae845a58a38b3e7e63a', DB::table('releases')->where('id', 2)->value('guid'));
}
#[Test]
public function oversized_guids_are_detected_by_the_regexp_check(): void
{
$this->insert(1, '01234567-89ab-cdef-0123-456789abcdef', '0');
$this->insert(2, str_repeat('a', 41), 'a', 'Oversized guid release');
$status = Artisan::call('releases:normalize-guids');
$output = Artisan::output();
$this->assertSame(1, $status);
$this->assertStringContainsString('2 releases have a guid that blocks', $output);
$this->assertStringContainsString('Legacy nZEDb release', $output);
$this->assertSame('d41d8cd98f00b204e9800998ecf8427e', DB::table('releases')->where('id', 2)->value('guid'));
$this->assertStringContainsString('1 releases have a guid that blocks', $output);
$this->assertStringContainsString('Oversized guid release', $output);
$this->assertSame(str_repeat('a', 41), DB::table('releases')->where('id', 2)->value('guid'));
}
#[Test]