Skip to content

perf(appstore): index the whole catalogue on fetch instead of discarding it - #167

Merged
rubenvdlinde merged 2 commits into
developmentfrom
perf/appstore-catalogue-once
Aug 21, 2026
Merged

perf(appstore): index the whole catalogue on fetch instead of discarding it#167
rubenvdlinde merged 2 commits into
developmentfrom
perf/appstore-catalogue-once

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

First of three following up #160/#166. Self-contained.

The measurement

The App Store's apps.json ignores its filter parameter. Against the endpoint the code actually calls (garm3.nextcloud.com/api/v1, not the apps.nextcloud.com mirror):

GET /apps.json?filter=notes   ->  200, 755 entries, 31,701,904 bytes

So a lookup for one app already downloads every app. extractAppPayload() then picked out the requested id and discarded the other 754 — and since listAdvisories() and listVersions() each resolve a payload, an advisory sweep over 88 enabled apps re-downloaded that catalogue per app.

That is the real cost behind #160. Not "176 external calls" — 176 calls each pulling a ~31.7 MB body.

The change

Write every entry of a downloaded catalogue through the same per-app cache, with the same TTL and the same shape. The first lookup in a sweep pays for the download; every subsequent app is a cache hit. A caller asking for a single app in isolation behaves exactly as before.

Both fetch paths are covered — the filtered endpoint and the /platform/{version}/apps.json fallback.

Why the test asserts what it does

It asserts on HTTP call count, not elapsed time — three apps, exactly one GET. Timing assertions on a network-mocked test measure nothing.

It also asserts each app receives its own payload, because a cache that returned the first app's data for every id would satisfy the call-count assertion just as well. Both halves are needed for the test to mean anything.

Control

With the two cacheCatalogueEntries() call sites removed, the test fails:

the second app must be served from cache, with its own payload
Failed asserting that null is identical to '5.4.0'.

Restored, it passes. A performance test that cannot fail is not a test.

Verification

  • 506 unit tests, 1024 assertions; no failure outside tests/unit/Command (19 errors there are a missing symfony/console in my local vendor copy, not code)
  • psalm: no issue in the changed file
  • gate-16 spec-coverage: count=0

🤖 Generated with Claude Code

…ing it

The App Store's `apps.json` IGNORES its `filter` parameter. Measured
2026-08-21 against the endpoint the code actually calls
(garm3.nextcloud.com/api/v1):

    GET /apps.json?filter=notes  ->  200, 755 entries, 31,701,904 bytes

So a lookup for ONE app already downloads every app. The response was then
searched for the requested id and the other 754 entries thrown away — and
because `listAdvisories()` and `listVersions()` each resolve a payload, an
advisory sweep over 88 enabled apps re-downloaded that catalogue per app.

That is the real cost behind #160: not 176 calls, but 176 calls each pulling
a ~31.7 MB body.

This writes every entry in a downloaded catalogue through the SAME per-app
cache, with the same TTL and the same shape. The first lookup in a sweep pays
for the download; every later app is a cache hit. A caller asking for a single
app in isolation behaves exactly as before.

The test asserts on HTTP CALL COUNT, not elapsed time: three apps, one GET.
It also asserts each app gets ITS OWN payload, because a cache that returned
the first app's data for every id would satisfy the call-count assertion too.

Control: with the two `cacheCatalogueEntries()` call sites removed, the test
fails with `Failed asserting that null is identical to '5.4.0'` — the second
app cannot be served at all. Restored, it passes.

506 unit tests, 1024 assertions, no failure outside tests/unit/Command (19
errors there are a missing symfony/console in the local vendor copy). psalm
clean on the changed file; gate-16 spec-coverage count=0.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/app-versions @ 226709a

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
composer ✅ 29/29
npm ✅ 282/282
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman ⏭️
Playwright
Hydra gates

Quality workflow — 2026-08-21 05:37 UTC

Download the full PDF report from the workflow artifacts.

psalm UnusedReturnValue: the count was documented and returned but no caller
used it, and AppStoreSource has no logger to report it through. Returning
void says what the method actually does rather than leaving a value that
exists only to be discarded.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/app-versions @ 9cc35e6

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
composer ✅ 29/29
npm ✅ 282/282
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman ⏭️
Playwright
Hydra gates

Quality workflow — 2026-08-21 07:54 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit 507d06b into development Aug 21, 2026
40 checks passed
@rubenvdlinde
rubenvdlinde deleted the perf/appstore-catalogue-once branch August 21, 2026 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant