From bae75dd89831a599bd402020df4dab2126e40473 Mon Sep 17 00:00:00 2001 From: Dat Date: Fri, 4 Sep 2026 11:54:22 +0200 Subject: [PATCH 1/3] fix(api): skip deleted wikis before resolving backend URLs Bug: T433575 --- app/Jobs/PlatformStatsSummaryJob.php | 4 +- tests/Jobs/PlatformStatsSummaryJobTest.php | 68 ++++++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/app/Jobs/PlatformStatsSummaryJob.php b/app/Jobs/PlatformStatsSummaryJob.php index f201e0c2..f8c66dc4 100644 --- a/app/Jobs/PlatformStatsSummaryJob.php +++ b/app/Jobs/PlatformStatsSummaryJob.php @@ -80,14 +80,14 @@ public function prepareStats(array $allStats, $wikis): array { $currentTime = CarbonImmutable::now(); foreach ($wikis as $wiki) { - $this->apiUrl = $this->mwHostResolver->getBackendUrlForDomain($wiki->domain) . '/w/api.php'; // used in PageFetcher::fetchPagesInNamespace - if (!is_null($wiki->deleted_at)) { $deletedWikis[] = $wiki; continue; } + $this->apiUrl = $this->mwHostResolver->getBackendUrlForDomain($wiki->domain) . '/w/api.php'; // used in PageFetcher::fetchPagesInNamespace + // add items and properties counts of the wiki to the corresponded arrays try { $nextItemCount = count($this->fetchPagesInNamespace($wiki->domain, MediawikiNamespace::item)); diff --git a/tests/Jobs/PlatformStatsSummaryJobTest.php b/tests/Jobs/PlatformStatsSummaryJobTest.php index f7284eb5..be63dd9d 100644 --- a/tests/Jobs/PlatformStatsSummaryJobTest.php +++ b/tests/Jobs/PlatformStatsSummaryJobTest.php @@ -234,6 +234,74 @@ public function testGroupings() { ); } + public function testSkipDeletedWikisBeforeResolvingBackendUrl() { + $deletedWiki = Wiki::factory()->create(['deleted_at' => CarbonImmutable::now()->subDay(), 'domain' => 'deleted.cloud']); + WikiDb::create([ + 'name' => 'deleted_db', + 'user' => 'asdasd', + 'password' => 'asdasfasfasf', + 'version' => 'version', + 'prefix' => 'asdasd', + 'wiki_id' => $deletedWiki->id, + ]); + + $activeWiki = Wiki::factory()->create(['deleted_at' => null, 'domain' => 'active.cloud']); + WikiDb::create([ + 'name' => 'active_db', + 'user' => 'asdasd', + 'password' => 'asdasfasfasf', + 'version' => 'version', + 'prefix' => 'asdasd', + 'wiki_id' => $activeWiki->id, + ]); + + Http::fake([ + $this->mwBackendHost . '/w/api.php?action=query&list=allpages&apnamespace=122&apcontinue=&aplimit=max&format=json' => Http::response([ + 'query' => ['allpages' => []], + ], 200), + $this->mwBackendHost . '/w/api.php?action=query&list=allpages&apnamespace=120&apcontinue=&aplimit=max&format=json' => Http::response([ + 'query' => ['allpages' => []], + ], 200), + ]); + + $this->mockMwHostResolver + ->expects($this->once()) + ->method('getBackendUrlForDomain') + ->with('active.cloud') + ->willReturn($this->mwBackendHost); + + $job = new PlatformStatsSummaryJob(); + (function ($resolver): void { + $this->mwHostResolver = $resolver; + })->call($job, $this->mockMwHostResolver); + + $groups = $job->prepareStats([ + [ + 'wiki' => 'active.cloud', + 'edits' => 1, + 'pages' => 1, + 'users' => 1, + 'active_users' => 1, + 'lastEdit' => MWTimestampHelper::getMWTimestampFromCarbon(CarbonImmutable::now()), + 'first100UsingOauth' => '0', + 'platform_summary_version' => 'v1', + ], + [ + 'wiki' => 'deleted.cloud', + 'edits' => 1, + 'pages' => 1, + 'users' => 1, + 'active_users' => 1, + 'lastEdit' => MWTimestampHelper::getMWTimestampFromCarbon(CarbonImmutable::now()), + 'first100UsingOauth' => '0', + 'platform_summary_version' => 'v1', + ], + ], [$deletedWiki, $activeWiki]); + + $this->assertSame(1, $groups['deleted']); + $this->assertSame(1, $groups['edited_last_90_days']); + } + public function testCreationStats() { $this->markTestSkipped('Pollutes the deleted wiki list'); $mockJob = $this->createMock(Job::class); From 3dbf9ea63c2dfe6e5049297f59de5b45ef8c490b Mon Sep 17 00:00:00 2001 From: Ollie Date: Sat, 12 Sep 2026 16:01:39 +0100 Subject: [PATCH 2/3] minor updates to test after pair review --- tests/Jobs/PlatformStatsSummaryJobTest.php | 39 +++++++++------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/tests/Jobs/PlatformStatsSummaryJobTest.php b/tests/Jobs/PlatformStatsSummaryJobTest.php index be63dd9d..7ec1fdff 100644 --- a/tests/Jobs/PlatformStatsSummaryJobTest.php +++ b/tests/Jobs/PlatformStatsSummaryJobTest.php @@ -235,34 +235,23 @@ public function testGroupings() { } public function testSkipDeletedWikisBeforeResolvingBackendUrl() { - $deletedWiki = Wiki::factory()->create(['deleted_at' => CarbonImmutable::now()->subDay(), 'domain' => 'deleted.cloud']); - WikiDb::create([ - 'name' => 'deleted_db', - 'user' => 'asdasd', - 'password' => 'asdasfasfasf', - 'version' => 'version', - 'prefix' => 'asdasd', - 'wiki_id' => $deletedWiki->id, - ]); + $deletedWiki = Wiki::factory()->create(['deleted_at' => CarbonImmutable::yesterday(), 'domain' => 'deleted.cloud']); + WikiDb::factory()->for($deletedWiki)->create(['name' => 'deleted_db']); $activeWiki = Wiki::factory()->create(['deleted_at' => null, 'domain' => 'active.cloud']); - WikiDb::create([ - 'name' => 'active_db', - 'user' => 'asdasd', - 'password' => 'asdasfasfasf', - 'version' => 'version', - 'prefix' => 'asdasd', - 'wiki_id' => $activeWiki->id, - ]); + WikiDb::factory()->for($activeWiki)->create(['name' => 'active_db']); + // TODO: investigate if this is needed or not Http::fake([ - $this->mwBackendHost . '/w/api.php?action=query&list=allpages&apnamespace=122&apcontinue=&aplimit=max&format=json' => Http::response([ - 'query' => ['allpages' => []], - ], 200), - $this->mwBackendHost . '/w/api.php?action=query&list=allpages&apnamespace=120&apcontinue=&aplimit=max&format=json' => Http::response([ - 'query' => ['allpages' => []], - ], 200), + "{$this->mwBackendHost}/w/api.php?action=query&list=allpages&apnamespace=122&apcontinue=&aplimit=max&format=json" + => Http::response(['query' => ['allpages' => []]], 200), + "{$this->mwBackendHost}/w/api.php?action=query&list=allpages&apnamespace=120&apcontinue=&aplimit=max&format=json" + => Http::response(['query' => ['allpages' => []]], 200), ]); + // this passed + Http::assertNothingSent(); + // this fails + Http::assertSentCount(1); $this->mockMwHostResolver ->expects($this->once()) @@ -271,6 +260,10 @@ public function testSkipDeletedWikisBeforeResolvingBackendUrl() { ->willReturn($this->mwBackendHost); $job = new PlatformStatsSummaryJob(); + // This is a hack to override the `private` `PlatformStatsSummaryJob::mwHostResolver` property. + // See https://www.php.net/manual/en/closure.call.php for more details on how this works. + // TODO: figure out how to stub the `DatabaseManager` correctly and/or refactor the Job so that + // we can more easily inject dependencies in the tests. (function ($resolver): void { $this->mwHostResolver = $resolver; })->call($job, $this->mockMwHostResolver); From 6e5fa9a935004218ba52cdc905f44b4354fe77ff Mon Sep 17 00:00:00 2001 From: Ollie Date: Sat, 12 Sep 2026 16:12:38 +0100 Subject: [PATCH 3/3] remove superfluous call to Http::fake() --- tests/Jobs/PlatformStatsSummaryJobTest.php | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/tests/Jobs/PlatformStatsSummaryJobTest.php b/tests/Jobs/PlatformStatsSummaryJobTest.php index 7ec1fdff..c0b310c1 100644 --- a/tests/Jobs/PlatformStatsSummaryJobTest.php +++ b/tests/Jobs/PlatformStatsSummaryJobTest.php @@ -241,18 +241,6 @@ public function testSkipDeletedWikisBeforeResolvingBackendUrl() { $activeWiki = Wiki::factory()->create(['deleted_at' => null, 'domain' => 'active.cloud']); WikiDb::factory()->for($activeWiki)->create(['name' => 'active_db']); - // TODO: investigate if this is needed or not - Http::fake([ - "{$this->mwBackendHost}/w/api.php?action=query&list=allpages&apnamespace=122&apcontinue=&aplimit=max&format=json" - => Http::response(['query' => ['allpages' => []]], 200), - "{$this->mwBackendHost}/w/api.php?action=query&list=allpages&apnamespace=120&apcontinue=&aplimit=max&format=json" - => Http::response(['query' => ['allpages' => []]], 200), - ]); - // this passed - Http::assertNothingSent(); - // this fails - Http::assertSentCount(1); - $this->mockMwHostResolver ->expects($this->once()) ->method('getBackendUrlForDomain')