From 78d3c9f37eaa310c7b1ca0d45c673e4efecd564f Mon Sep 17 00:00:00 2001 From: Dat Date: Wed, 23 Sep 2026 14:24:43 +0200 Subject: [PATCH 1/6] Add test to catch duplicate keys errors --- tests/Jobs/UpdateWikiDailyMetricJobTest.php | 35 +++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/Jobs/UpdateWikiDailyMetricJobTest.php b/tests/Jobs/UpdateWikiDailyMetricJobTest.php index e3c601c5..91df1bc5 100644 --- a/tests/Jobs/UpdateWikiDailyMetricJobTest.php +++ b/tests/Jobs/UpdateWikiDailyMetricJobTest.php @@ -72,4 +72,39 @@ public function testRunJobForAllWikisIncludingDeletedWikis() { 'entity_schema_count' => 0, ]); } + + public function testRunningJobTwiceForSameWikiWithChangedValuesCreatesOnlyOneDailyRecord() { + $wiki = Wiki::factory()->create([ + 'domain' => 'duplicate.wikibase.cloud', + ]); + + $manager = $this->app->make('db'); + $job = new ProvisionWikiDbJob(); + $job->handle($manager); + + $wikiDb = WikiDb::whereDoesntHave('wiki')->first(); + $wikiDb->update(['wiki_id' => $wiki->id]); + + $wiki->wikiSiteStats()->create([ + 'pages' => 10, + 'users' => 3, + ]); + + $dailyMetricJob = new UpdateWikiDailyMetricJob(); + $dailyMetricJob->handle(); + + $wiki->wikiSiteStats()->first()->update([ + 'pages' => 12, + 'users' => 5, + ]); + + $dailyMetricJob->handle(); + + $this->assertDatabaseCount('wiki_daily_metrics', 1) + ->assertDatabaseHas('wiki_daily_metrics', [ + 'wiki_id' => $wiki->id, + 'date' => Carbon::today()->toDateString(), + 'pages' => 12, + ]); + } } From 4336d097bdea47c72fcd62960d4db7e50ac61f3f Mon Sep 17 00:00:00 2001 From: Dat Date: Wed, 23 Sep 2026 14:25:18 +0200 Subject: [PATCH 2/6] Add withoutOverlapping() to UpdateWikiDailyMetricJob --- app/Console/Kernel.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Console/Kernel.php b/app/Console/Kernel.php index cca1f77a..90d7a0f9 100644 --- a/app/Console/Kernel.php +++ b/app/Console/Kernel.php @@ -57,7 +57,7 @@ protected function schedule(Schedule $schedule): void { $schedule->job(new SendEmptyWikiNotificationsJob())->dailyAt('21:00'); - $schedule->job(new UpdateWikiDailyMetricJob())->dailyAt('23:00'); + $schedule->job(new UpdateWikiDailyMetricJob())->dailyAt('23:00')->withoutOverlapping(); $schedule->job(new UpdateQueryserviceAllowList())->weeklyOn(Schedule::MONDAY, '01:00'); From ba8716dfab9d3749be5a1c3bf5b324369a49ea08 Mon Sep 17 00:00:00 2001 From: Ollie Date: Tue, 6 Oct 2026 22:15:31 +0100 Subject: [PATCH 3/6] Squash messy git history --- app/Metrics/App/WikiMetrics.php | 12 ++++++++++++ tests/Jobs/UpdateWikiDailyMetricJobTest.php | 20 ++++++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/app/Metrics/App/WikiMetrics.php b/app/Metrics/App/WikiMetrics.php index e0915e93..dd5ca72d 100644 --- a/app/Metrics/App/WikiMetrics.php +++ b/app/Metrics/App/WikiMetrics.php @@ -25,6 +25,18 @@ public function saveMetrics(Wiki $wiki): void { $this->wiki = $wiki; $today = now()->format('Y-m-d'); + + // Skip expensive metrics collection if today's record already exists. + $recordExists = WikiDailyMetrics::where('wiki_id', $wiki->id) + ->where('date', $today) + ->exists(); + + if ($recordExists) { + Log::warning("Daily metric already exists for Wiki ID {$wiki->id} on {$today}; skipping metrics collection."); + + return; + } + $tripleCount = $this->getNumOfTriples(); $todayPageCount = $wiki->wikiSiteStats()->first()->pages ?? 0; $isDeleted = (bool) $wiki->deleted_at; diff --git a/tests/Jobs/UpdateWikiDailyMetricJobTest.php b/tests/Jobs/UpdateWikiDailyMetricJobTest.php index 91df1bc5..e0699546 100644 --- a/tests/Jobs/UpdateWikiDailyMetricJobTest.php +++ b/tests/Jobs/UpdateWikiDailyMetricJobTest.php @@ -8,12 +8,20 @@ use App\WikiDb; use Carbon\Carbon; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Queue; use Tests\TestCase; +use TiMacDonald\Log\LogEntry; +use TiMacDonald\Log\LogFake; class UpdateWikiDailyMetricJobTest extends TestCase { use RefreshDatabase; + protected function setUp(): void { + parent::setUp(); + Log::swap(new LogFake()); + } + public function testDispatchJob() { Queue::fake(); @@ -73,7 +81,7 @@ public function testRunJobForAllWikisIncludingDeletedWikis() { ]); } - public function testRunningJobTwiceForSameWikiWithChangedValuesCreatesOnlyOneDailyRecord() { + public function testRunningJobTwiceForSameWikiWithChangedValuesSkipsSecondRunWhenRecordExistsForToday() { $wiki = Wiki::factory()->create([ 'domain' => 'duplicate.wikibase.cloud', ]); @@ -104,7 +112,15 @@ public function testRunningJobTwiceForSameWikiWithChangedValuesCreatesOnlyOneDai ->assertDatabaseHas('wiki_daily_metrics', [ 'wiki_id' => $wiki->id, 'date' => Carbon::today()->toDateString(), - 'pages' => 12, + 'pages' => 10, ]); + + Log::assertLogged(function (LogEntry $log) use ($wiki) { + if ($log->level !== 'warning') { + return false; + } + + return str_contains($log->message, "Daily metric already exists for Wiki ID {$wiki->id} on " . Carbon::today()->toDateString()); + }); } } From 4206fd56affc2d4dc05ddbabd7fa9e6368db8d97 Mon Sep 17 00:00:00 2001 From: Dat Date: Wed, 30 Sep 2026 12:12:07 +0200 Subject: [PATCH 4/6] Remove withoutOverlapping() --- app/Console/Kernel.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Console/Kernel.php b/app/Console/Kernel.php index 90d7a0f9..cca1f77a 100644 --- a/app/Console/Kernel.php +++ b/app/Console/Kernel.php @@ -57,7 +57,7 @@ protected function schedule(Schedule $schedule): void { $schedule->job(new SendEmptyWikiNotificationsJob())->dailyAt('21:00'); - $schedule->job(new UpdateWikiDailyMetricJob())->dailyAt('23:00')->withoutOverlapping(); + $schedule->job(new UpdateWikiDailyMetricJob())->dailyAt('23:00'); $schedule->job(new UpdateQueryserviceAllowList())->weeklyOn(Schedule::MONDAY, '01:00'); From 9478ff3bf43c05dcfd90d3b8fd2e3389be730546 Mon Sep 17 00:00:00 2001 From: Ollie Date: Tue, 6 Oct 2026 02:11:36 +0100 Subject: [PATCH 5/6] Improvements during code review --- app/Metrics/App/WikiMetrics.php | 12 ++++-------- tests/Jobs/UpdateWikiDailyMetricJobTest.php | 18 ++++++------------ 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/app/Metrics/App/WikiMetrics.php b/app/Metrics/App/WikiMetrics.php index dd5ca72d..69e485e6 100644 --- a/app/Metrics/App/WikiMetrics.php +++ b/app/Metrics/App/WikiMetrics.php @@ -25,13 +25,10 @@ public function saveMetrics(Wiki $wiki): void { $this->wiki = $wiki; $today = now()->format('Y-m-d'); + $previousRecord = WikiDailyMetrics::where('wiki_id', $wiki->id)->latest('date')->first(); - // Skip expensive metrics collection if today's record already exists. - $recordExists = WikiDailyMetrics::where('wiki_id', $wiki->id) - ->where('date', $today) - ->exists(); - - if ($recordExists) { + // Skip expensive metrics collection if a record for today already exists. + if ($previousRecord?->date === $today) { Log::warning("Daily metric already exists for Wiki ID {$wiki->id} on {$today}; skipping metrics collection."); return; @@ -69,8 +66,7 @@ public function saveMetrics(Wiki $wiki): void { 'total_user_count' => $numberOfUsers, ]); - // compare current record to previous record and only save if there is a change - $previousRecord = WikiDailyMetrics::where('wiki_id', $wiki->id)->latest('date')->first(); + // compare current metrics to previous record and only save if there is a change if ($previousRecord?->areMetricsEqual($dailyMetrics)) { Log::info("Record unchanged for Wiki ID {$wiki->id}, no new record added."); diff --git a/tests/Jobs/UpdateWikiDailyMetricJobTest.php b/tests/Jobs/UpdateWikiDailyMetricJobTest.php index e0699546..3e246467 100644 --- a/tests/Jobs/UpdateWikiDailyMetricJobTest.php +++ b/tests/Jobs/UpdateWikiDailyMetricJobTest.php @@ -17,11 +17,6 @@ class UpdateWikiDailyMetricJobTest extends TestCase { use RefreshDatabase; - protected function setUp(): void { - parent::setUp(); - Log::swap(new LogFake()); - } - public function testDispatchJob() { Queue::fake(); @@ -81,14 +76,14 @@ public function testRunJobForAllWikisIncludingDeletedWikis() { ]); } - public function testRunningJobTwiceForSameWikiWithChangedValuesSkipsSecondRunWhenRecordExistsForToday() { + public function testWikiMetricsCollectionStopsEarlyWhenRecordExistsForToday() { + Log::swap(new LogFake()); + $wiki = Wiki::factory()->create([ 'domain' => 'duplicate.wikibase.cloud', ]); - $manager = $this->app->make('db'); - $job = new ProvisionWikiDbJob(); - $job->handle($manager); + dispatch(new ProvisionWikiDbJob()); $wikiDb = WikiDb::whereDoesntHave('wiki')->first(); $wikiDb->update(['wiki_id' => $wiki->id]); @@ -98,15 +93,14 @@ public function testRunningJobTwiceForSameWikiWithChangedValuesSkipsSecondRunWhe 'users' => 3, ]); - $dailyMetricJob = new UpdateWikiDailyMetricJob(); - $dailyMetricJob->handle(); + UpdateWikiDailyMetricJob::dispatch(); $wiki->wikiSiteStats()->first()->update([ 'pages' => 12, 'users' => 5, ]); - $dailyMetricJob->handle(); + UpdateWikiDailyMetricJob::dispatch(); $this->assertDatabaseCount('wiki_daily_metrics', 1) ->assertDatabaseHas('wiki_daily_metrics', [ From 0877388c6cd489ced0642d7aa5e2e4c5e6d3f431 Mon Sep 17 00:00:00 2001 From: Ollie Date: Tue, 6 Oct 2026 21:41:36 +0100 Subject: [PATCH 6/6] Move test to WikiMetricsTest --- tests/Jobs/UpdateWikiDailyMetricJobTest.php | 45 ----------------- tests/Metrics/WikiMetricsTest.php | 53 +++++++++++++++++++++ 2 files changed, 53 insertions(+), 45 deletions(-) diff --git a/tests/Jobs/UpdateWikiDailyMetricJobTest.php b/tests/Jobs/UpdateWikiDailyMetricJobTest.php index 3e246467..e3c601c5 100644 --- a/tests/Jobs/UpdateWikiDailyMetricJobTest.php +++ b/tests/Jobs/UpdateWikiDailyMetricJobTest.php @@ -8,11 +8,8 @@ use App\WikiDb; use Carbon\Carbon; use Illuminate\Foundation\Testing\RefreshDatabase; -use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Queue; use Tests\TestCase; -use TiMacDonald\Log\LogEntry; -use TiMacDonald\Log\LogFake; class UpdateWikiDailyMetricJobTest extends TestCase { use RefreshDatabase; @@ -75,46 +72,4 @@ public function testRunJobForAllWikisIncludingDeletedWikis() { 'entity_schema_count' => 0, ]); } - - public function testWikiMetricsCollectionStopsEarlyWhenRecordExistsForToday() { - Log::swap(new LogFake()); - - $wiki = Wiki::factory()->create([ - 'domain' => 'duplicate.wikibase.cloud', - ]); - - dispatch(new ProvisionWikiDbJob()); - - $wikiDb = WikiDb::whereDoesntHave('wiki')->first(); - $wikiDb->update(['wiki_id' => $wiki->id]); - - $wiki->wikiSiteStats()->create([ - 'pages' => 10, - 'users' => 3, - ]); - - UpdateWikiDailyMetricJob::dispatch(); - - $wiki->wikiSiteStats()->first()->update([ - 'pages' => 12, - 'users' => 5, - ]); - - UpdateWikiDailyMetricJob::dispatch(); - - $this->assertDatabaseCount('wiki_daily_metrics', 1) - ->assertDatabaseHas('wiki_daily_metrics', [ - 'wiki_id' => $wiki->id, - 'date' => Carbon::today()->toDateString(), - 'pages' => 10, - ]); - - Log::assertLogged(function (LogEntry $log) use ($wiki) { - if ($log->level !== 'warning') { - return false; - } - - return str_contains($log->message, "Daily metric already exists for Wiki ID {$wiki->id} on " . Carbon::today()->toDateString()); - }); - } } diff --git a/tests/Metrics/WikiMetricsTest.php b/tests/Metrics/WikiMetricsTest.php index 6648594f..82796349 100644 --- a/tests/Metrics/WikiMetricsTest.php +++ b/tests/Metrics/WikiMetricsTest.php @@ -15,7 +15,11 @@ use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Schema; +use Log; +use Psr\Log\LogLevel; use Tests\TestCase; +use TiMacDonald\Log\LogEntry; +use TiMacDonald\Log\LogFake; class WikiMetricsTest extends TestCase { use RefreshDatabase; @@ -70,6 +74,55 @@ public function testNoDuplicateRecordsWithOnlyDateChange() { ]); } + public function testPreventMetricsCollectionWhenRecordExistsForToday(): void { + Log::swap(new LogFake()); + + $wiki = Wiki::factory()->create(); + WikiDb::first()->update(['wiki_id' => $wiki->id]); + + $wiki->wikiSiteStats()->create([ + 'pages' => 10, + 'users' => 3, + ]); + + $wikiMetrics = new WikiMetrics(); + $wikiMetrics->saveMetrics($wiki); + + $this->assertDatabaseCount('wiki_daily_metrics', 1) + ->assertDatabaseHas('wiki_daily_metrics', [ + 'wiki_id' => $wiki->id, + 'date' => now()->toDateString(), + 'pages' => 10, + 'total_user_count' => 3, + ]); + + $wiki->wikiSiteStats()->first()->update([ + 'pages' => 12, + 'users' => 5, + ]); + + $wikiMetrics->saveMetrics($wiki); + + $this->assertDatabaseCount('wiki_daily_metrics', 1) + ->assertDatabaseHas('wiki_daily_metrics', [ + 'wiki_id' => $wiki->id, + 'date' => now()->toDateString(), + 'pages' => 10, + 'total_user_count' => 3, + ]); + + Log::assertLogged(function (LogEntry $log) use ($wiki) { + if ($log->level !== LogLevel::WARNING) { + return false; + } + + return str_contains( + $log->message, + "Daily metric already exists for Wiki ID {$wiki->id} on " . now()->toDateString() + ); + }); + } + public function testRecordCreatedWhenWikiFirstDeleted() { $wiki = Wiki::factory()->create([ 'domain' => 'thisfake.wikibase.cloud',