diff --git a/app/Console/Commands/Deprecated/StatusDedupe.php b/app/Console/Commands/Deprecated/StatusDedupe.php index 8a69221a6..778057397 100644 --- a/app/Console/Commands/Deprecated/StatusDedupe.php +++ b/app/Console/Commands/Deprecated/StatusDedupe.php @@ -46,13 +46,17 @@ class StatusDedupe extends Command return; } + // Deterministically keep the earliest-fetched status per uri via + // MIN(id). Selecting a non-aggregated id under GROUP BY is + // nondeterministic and cannot be influenced by ORDER BY, so the + // previous query could keep an arbitrary duplicate. DB::table('statuses') - ->selectRaw('id, uri, count(uri) as occurences') + ->selectRaw('MIN(id) as id, uri, count(uri) as occurences') ->whereNull('deleted_at') ->whereNotNull('uri') ->groupBy('uri') - ->orderBy('created_at') ->having('occurences', '>', 1) + ->orderBy('uri') ->chunk(50, function ($statuses) { foreach ($statuses as $status) { $this->info("Found duplicate: $status->uri"); diff --git a/tests/Feature/Console/StatusDedupeTest.php b/tests/Feature/Console/StatusDedupeTest.php new file mode 100644 index 000000000..c0c7025be --- /dev/null +++ b/tests/Feature/Console/StatusDedupeTest.php @@ -0,0 +1,70 @@ +dropUnique('statuses_uri_unique'); + }); +}); + +/* +|-------------------------------------------------------------------------- +| status:dedup deterministic keep +|-------------------------------------------------------------------------- +| +| The dedupe command must deterministically keep the earliest (MIN id) status +| per uri and delete the rest. A non-aggregated id under GROUP BY is +| nondeterministic and could keep an arbitrary duplicate. +| +*/ + +it('keeps the lowest-id status per uri and deletes the duplicates', function () { + Bus::fake(); + + $author = User::factory()->create(); + $author->refresh(); + + $uri = 'https://remote.example/users/bob/statuses/42'; + + // Snowflake ids are time-ordered, so the first created has the min id. + $first = Status::factory()->create(['profile_id' => $author->profile_id, 'type' => 'photo', 'uri' => $uri]); + $second = Status::factory()->create(['profile_id' => $author->profile_id, 'type' => 'photo', 'uri' => $uri]); + $third = Status::factory()->create(['profile_id' => $author->profile_id, 'type' => 'photo', 'uri' => $uri]); + + $ids = collect([$first->id, $second->id, $third->id]); + $keepId = $ids->min(); + $deleteIds = $ids->reject(fn ($id) => $id === $keepId)->values(); + + $this->artisan('status:dedup')->assertExitCode(0); + + $statusIdOf = function ($job) { + $ref = new ReflectionProperty($job, 'status'); + $ref->setAccessible(true); + + return $ref->getValue($job)->id; + }; + + // The two higher-id duplicates are dispatched for deletion. + Bus::assertDispatched(StatusDelete::class, function ($job) use ($deleteIds, $statusIdOf) { + return $deleteIds->contains($statusIdOf($job)); + }); + + // The kept (min id) status is never dispatched for deletion. + Bus::assertNotDispatched(StatusDelete::class, function ($job) use ($keepId, $statusIdOf) { + return $statusIdOf($job) === $keepId; + }); + + // Exactly two deletions (the two duplicates). + Bus::assertDispatchedTimes(StatusDelete::class, 2); +});