Public: anyone can read this; the crew can change it.

GET /api/v1/changelog carries an ETag and answers a repeat with 304, as /changelog.atom already does #9123

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

`GET /api/v1/changelog` now carries a weak `etag`, and a repeat with a matching `If-None-Match` gets an empty 304. The 304 has the same `etag` and `cache-control`, and it is sent before the response body is built. The new tests and the changelog-api, changelog-atom, changelog and openapi-coverage tests pass here, and `bun scripts/gate.ts` and `bun x tsc --noEmit` are clean. I did not run the rest…

shipped production is serving it deploy 6365650

opened this

GET /api/v1/changelog now carries a weak etag, and a repeat with a matching If-None-Match gets an empty 304. The 304 has the same etag and cache-control, and it is sent before the response body is built. The new tests and the changelog-api, changelog-atom, changelog and openapi-coverage tests pass here, and bun scripts/gate.ts and bun x tsc --noEmit are clean. I did not run the rest of the suite; the forge's check does that on this push. I did not check the 304 against a running preview, and I did not look at the Server-Timing output, so the "before the entries are rendered" point rests on the code order, not a measurement.

  • Shared helper: changelogTag() in src/web/feeds.ts builds the tag from the live version, the rows' repo, number, title and time, and an optional extra scope. changelogAtom and changelogJson both call it.
  • JSON tag: it covers both live and onItsWay, because the JSON answers both. The extra scope is the raw ?repo= string plus the live count. So ?repo=all and the default get different tags even when the rows happen to match.
  • Atom tag: unchanged. With no scope the helper hashes the same input as before, so feed readers won't refetch.
  • Viewer-dependence: the JSON answer is the same for every viewer, because it only reads public repositories. So it stays public, max-age=5 and I did not add private or vary. The helper accepts a viewer in its scope if that ever changes, but nothing sends one today.
  • OpenAPI: getApiV1Changelog.ts now names the ETag, If-None-Match and the 304 (alsoStatus: [304]).
  • Live region: changelogPage builds its rows from readLandings, not from this endpoint, so the page's live region is unaffected. That is from reading the code, not from loading the page.
  • Tests: test/changelog-etag.test.ts covers a repeat giving a 304 with the same headers, ?repo=all against the default having different tags, and a newly shipped pull giving a 200 with a new tag. Two of the three fail without the src change (the 304 test and the ?repo=all test).

Closes #9112

commented

GET /api/v1/changelog now carries a weak etag, and a repeat with a matching If-None-Match gets an empty 304. The 304 has the same etag and cache-control, and it is sent before the response body is built. The new tests and the changelog-api, changelog-atom, changelog and openapi-coverage tests pass here, and bun scripts/gate.ts and bun x tsc --noEmit are clean. I did not run the rest of the suite; the forge's check does that on this push. I did not check the 304 against a running preview, and I did not look at the Server-Timing output, so the "before the entries are rendered" point rests on the code order, not a measurement.

  • Shared helper: changelogTag() in src/web/feeds.ts builds the tag from the live version, the rows' repo, number, title and time, and an optional extra scope. changelogAtom and changelogJson both call it.
  • JSON tag: it covers both live and onItsWay, because the JSON answers both. The extra scope is the raw ?repo= string plus the live count. So ?repo=all and the default get different tags even when the rows happen to match.
  • Atom tag: unchanged. With no scope the helper hashes the same input as before, so feed readers won't refetch.
  • Viewer-dependence: the JSON answer is the same for every viewer, because it only reads public repositories. So it stays public, max-age=5 and I did not add private or vary. The helper accepts a viewer in its scope if that ever changes, but nothing sends one today.
  • OpenAPI: getApiV1Changelog.ts now names the ETag, If-None-Match and the 304 (alsoStatus: [304]).
  • Live region: changelogPage builds its rows from readLandings, not from this endpoint, so the page's live region is unaffected. That is from reading the code, not from loading the page.
  • Tests: test/changelog-etag.test.ts covers a repeat giving a 304 with the same headers, ?repo=all against the default having different tags, and a newly shipped pull giving a 200 with a new tag. Two of the three fail without the src change (the 304 test and the ?repo=all test).
commented

The gate failed because no commit message had a Try: line. I added an empty commit that has one, and pushed it. bun scripts/gate.ts now runs clean here, but I haven't seen the forge's check re-run on the push. No code changed since the last push, so the tests are the ones that passed before.

The Try: line says to curl /api/v1/changelog on the preview twice. The first request returns an etag. The second sends it back in If-None-Match and should return a 304 with no body.

Changes requested — before the latest push

The preview works: the first request returns a weak etag, a repeat with If-None-Match gets an empty 304 with the same etag and cache-control, and ?repo=all has a different tag. The helper is shared with the Atom feed, and the JSON answer reads public repositories only, so staying public is right. One change is needed: the third test (a newly shipped pull gives a 200 with a new tag) would pass on main. On main there is no etag, so before is "" and the null etag is not equal to it, and the status is already 200. Assert that before matches /^W\//, then that after has an etag that differs from it. Then every behaviour added has a test that fails without it, and box 5 can be ticked.

commented

I changed the third test as @juniper asked and pushed it to the same branch. The three tests in test/changelog-etag.test.ts pass here. I didn't re-run the gate or tsc after this edit, and the forge's check hasn't reported on the push yet.

The "newly shipped pull" test now expects the tag from before shipping to match /^W\//. It then expects the answer after shipping to be a 200 whose tag also matches /^W\// and differs from the first. On main there is no etag, so the first assertion fails there.

The commit message has a Try: line, so the preview gate rule should pass. No source code changed.

Approved

Earlier ask (third test must fail on main) is met: it now asserts a tag before shipping and a different one after. Preview checked: 200 carries a weak etag, a repeat with If-None-Match is 304 with the same cache-control, and ?repo=all gets a different tag. Answer is viewer-independent (public repos only), so public is right and private/vary is not needed. Next, non-blocking: (1) add a Server-Timing assertion that the 304 skips rendering; (2) ?repo=owner/name case in the tag test.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs