Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions lib/Service/Source/AppStoreSource.php
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,8 @@ private function fetchAppPayloadUncached(string $appId): ?array {
if (!is_array($decoded)) {
return null;
}
// The whole catalogue arrived regardless of the filter; keep it.
$this->cacheCatalogueEntries($decoded);
$appPayload = $this->extractAppPayload($decoded, $appId);
if (is_array($appPayload)) {
return $appPayload;
Expand Down Expand Up @@ -357,6 +359,9 @@ private function fetchAppPayloadUncached(string $appId): ?array {
if (!is_array($decoded)) {
continue;
}
// Same reasoning as the filtered endpoint above: this response
// is the whole platform catalogue, so index all of it.
$this->cacheCatalogueEntries($decoded);
$appPayload = $this->extractAppPayload($decoded, $appId);
if (is_array($appPayload)) {
return $appPayload;
Expand Down Expand Up @@ -416,6 +421,57 @@ private function arrayField(array $payload, string $key): ?array {
* @param array<array-key, mixed> $entries
* @return array<array-key, mixed>|null
*/
/**
* Caches EVERY app in a freshly-downloaded catalogue, not just the one that
* was asked for.
*
* The App Store's `apps.json` IGNORES its `filter` parameter — measured
* 2026-08-21, `?filter=notes` returned all 755 entries and 31.7 MB — so a
* lookup for one app already pays for the whole catalogue. Keeping one
* entry and discarding 754 meant a full advisory sweep over 88 enabled
* apps downloaded ~31.7 MB per app, and did it twice per app because
* `listAdvisories()` and `listVersions()` each resolve a payload.
*
* Indexing the whole response makes the FIRST lookup pay for the download
* and every subsequent app in the same sweep a cache hit. Nothing else
* changes: entries are written through the same per-app cache with the same
* TTL, so a caller asking for one app in isolation behaves exactly as before.
*
* @spec openspec/specs/security-advisory-correlation/spec.md
* @param array<array-key, mixed> $decoded A decoded catalogue response.
*/
private function cacheCatalogueEntries(array $decoded): void {
$entries = null;
$data = $this->arrayField($decoded, 'data');
if ($data !== null && array_is_list($data)) {
$entries = $data;
} elseif (array_is_list($decoded)) {
$entries = $decoded;
} else {
$apps = $this->arrayField($decoded, 'apps');
if ($apps !== null && array_is_list($apps)) {
$entries = $apps;
}
}

if ($entries === null) {
return;
}

/** @var mixed $entry */
foreach ($entries as $entry) {
if (!is_array($entry)) {
continue;
}
/** @var mixed $id */
$id = $entry['id'] ?? null;
if (!is_string($id) || $id === '') {
continue;
}
$this->writeCachedPayload($id, $entry);
}
}

private function findById(array $entries, string $appId): ?array {
/** @var mixed $entry */
foreach ($entries as $entry) {
Expand Down
62 changes: 62 additions & 0 deletions tests/unit/Service/Source/AppStoreSourceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -277,4 +277,66 @@ function (string $app, string $key, string $default = '') use ($body): string {

$this->assertSame('2.3.0', $result['versions'][0]['version'], 'stale cache must serve during an upstream outage');
}

/**
* ONE DOWNLOAD SERVES THE WHOLE SWEEP.
*
* The App Store ignores `?filter=`, so a lookup for one app already
* downloads every app (measured: 755 entries, 31.7 MB). Before this, the
* other 754 were discarded, so an advisory sweep over 88 enabled apps
* re-downloaded the catalogue per app.
*
* The assertion is on HTTP calls, not on elapsed time: the second app must
* be answered without touching the network at all.
*/
public function testCachesEveryAppInTheCatalogueSoASweepDownloadsItOnce(): void {
$catalogue = ['data' => [
['id' => 'notes', 'releases' => [['version' => '4.13.0']]],
['id' => 'calendar', 'releases' => [['version' => '5.4.0']]],
['id' => 'deck', 'releases' => [['version' => '1.14.0']]],
]];

$response = $this->createMock(IResponse::class);
$response->method('getStatusCode')->willReturn(200);
$response->method('getBody')->willReturn(json_encode($catalogue, JSON_THROW_ON_ERROR));

$client = $this->createMock(IClient::class);
// THE ASSERTION: exactly one GET for three apps.
$client->expects($this->once())->method('get')->willReturn($response);

$clientService = $this->createMock(IClientService::class);
$clientService->method('newClient')->willReturn($client);

// An in-memory app config, so a cache write by one lookup is visible to
// the next — which is the whole mechanism under test.
$store = [];
$config = $this->createMock(IConfig::class);
$config->method('getSystemValueString')->willReturn('28.0.0');
$config->method('setAppValue')->willReturnCallback(
static function (string $app, string $key, string $value) use (&$store): void {
$store[$key] = $value;
},
);
$config->method('getAppValue')->willReturnCallback(
static function (string $app, string $key, string $default = '') use (&$store): string {
return $store[$key] ?? $default;
},
);

$l10nFactory = $this->createMock(IFactory::class);
$l10nFactory->method('findLanguage')->willReturn('en');

$source = new AppStoreSource($clientService, $config, $l10nFactory);

$first = $source->listVersions('notes', $this->binding());
$second = $source->listVersions('calendar', $this->binding());
$third = $source->listVersions('deck', $this->binding());

// Each app must still get ITS OWN payload — a shared cache that returned
// the first app's data for every lookup would also satisfy the call
// count above, so assert the versions differ.
$this->assertSame('4.13.0', $first['versions'][0]['version']);
$this->assertSame('5.4.0', $second['versions'][0]['version'], 'the second app must be served from cache, with its own payload');
$this->assertSame('1.14.0', $third['versions'][0]['version'], 'the third app must be served from cache, with its own payload');
}
}
Loading