The client compares names while Android installs by versionCode, and the wire had no way to close that gap: the sidecar was one positional line and the endpoint returned a name only. This is the plumbing that makes the ordering key decidable by the client at all. The sidecar is now JSON rather than a grown positional string. That shape was chosen against a specific failure: the obvious growth path was "<name> <code>", which a first-space split silently mangles the moment a third field appears — the code stops parsing as an integer and the reader falls back to name comparison WITHOUT erroring. JSON cannot mistake a new field for an old one. code is a POINTER on both sides, and omitempty on the wire. Absent has to stay distinguishable from zero: a build published before ordering keys were recorded genuinely has no code, and zero would claim it is infinitely old rather than unknown. A malformed sidecar now fails loudly instead of serving a blank version. If an unreadable file produced an empty name, every client would compare against nothing, conclude it was current, and go quiet — "I cannot read this" and "there is nothing newer" would return the same answer, which is the failure mode nobody reports because nobody is offered anything to report. The non-tag :latest path no longer RECONSTRUCTS the bundled APK's version. android-release now publishes the sidecar as a release asset beside the APK, and the image build downloads it. The old reconstruction duplicated a derivation formula across two files, and could only ever recover the name — the ordering key is build-time minutes and exists nowhere once that build ends. Releases predating the sidecar report their name with a null code, which is the honest answer rather than a guessed one. image-release also drops to a shallow checkout: it needed full history and tags only to re-derive versions from the tagged commit, and now touches git for nothing. MINSTREL_VERSION comes from GITHUB_REF. Two things checked rather than assumed. The Android Json sets ignoreUnknownKeys, so the added fields cannot break already-installed apps. It also sets coerceInputValues, which will silently turn a null code into 0 if step 4 declares the field non-nullable — recorded on task #3811, because reading the field declaration alone would never reveal it. Also fixes a stale comment block describing "the Flutter client", deleted in v2026.08.18. Step 3 of 5 — Scribe task #3810, milestone #390. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
260 lines
9.0 KiB
Go
260 lines
9.0 KiB
Go
package api
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"io"
|
|
"log/slog"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"os"
|
|
"path/filepath"
|
|
"testing"
|
|
|
|
"github.com/jackc/pgx/v5/pgtype"
|
|
|
|
"git.fabledsword.com/bvandeusen/minstrel/internal/auth"
|
|
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
|
)
|
|
|
|
// authedRequest builds a request with a fake user in context, mirroring
|
|
// what the auth.RequireUser middleware injects in production.
|
|
// userIDByte fills every byte of the user's UUID, so two requests with
|
|
// different userIDByte values get distinct rate-limit slots.
|
|
func authedRequest(method, path string, userIDByte byte) *http.Request {
|
|
user := dbq.User{
|
|
ID: pgtype.UUID{
|
|
Bytes: [16]byte{userIDByte, userIDByte, userIDByte, userIDByte,
|
|
userIDByte, userIDByte, userIDByte, userIDByte,
|
|
userIDByte, userIDByte, userIDByte, userIDByte,
|
|
userIDByte, userIDByte, userIDByte, userIDByte},
|
|
Valid: true,
|
|
},
|
|
Username: "tester",
|
|
}
|
|
r := httptest.NewRequest(method, path, nil)
|
|
ctx := context.WithValue(r.Context(), auth.UserCtxKeyForTest(), user)
|
|
return r.WithContext(ctx)
|
|
}
|
|
|
|
// withClientAPKDir points the handlers at a fresh temp dir for each
|
|
// test and restores the env var on cleanup. Returns the dir.
|
|
func withClientAPKDir(t *testing.T) string {
|
|
t.Helper()
|
|
dir := t.TempDir()
|
|
prev, hadPrev := os.LookupEnv("MINSTREL_CLIENT_APK_DIR")
|
|
t.Setenv("MINSTREL_CLIENT_APK_DIR", dir)
|
|
t.Cleanup(func() {
|
|
if hadPrev {
|
|
t.Setenv("MINSTREL_CLIENT_APK_DIR", prev)
|
|
}
|
|
// t.Setenv auto-restores the prior empty/unset state at end of
|
|
// test, so the !hadPrev branch needs no explicit Unsetenv.
|
|
})
|
|
return dir
|
|
}
|
|
|
|
func TestClientVersion_404WhenNoAPK(t *testing.T) {
|
|
withClientAPKDir(t)
|
|
h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
|
|
rr := httptest.NewRecorder()
|
|
h.handleClientVersion(rr, httptest.NewRequest(http.MethodGet, "/api/client/version", nil))
|
|
if rr.Code != http.StatusNotFound {
|
|
t.Errorf("want 404, got %d", rr.Code)
|
|
}
|
|
}
|
|
|
|
func TestClientVersion_404WhenAPKButNoVersion(t *testing.T) {
|
|
dir := withClientAPKDir(t)
|
|
if err := os.WriteFile(filepath.Join(dir, clientAPKFilename), []byte("fake apk"), 0o644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
|
|
rr := httptest.NewRecorder()
|
|
h.handleClientVersion(rr, httptest.NewRequest(http.MethodGet, "/api/client/version", nil))
|
|
if rr.Code != http.StatusNotFound {
|
|
t.Errorf("want 404, got %d", rr.Code)
|
|
}
|
|
}
|
|
|
|
// writeClientAssets stages an APK plus a raw sidecar body, and returns the
|
|
// APK's size so callers can assert size_bytes without recomputing it.
|
|
func writeClientAssets(t *testing.T, sidecar string) int64 {
|
|
t.Helper()
|
|
dir := withClientAPKDir(t)
|
|
body := []byte("fake apk content")
|
|
if err := os.WriteFile(filepath.Join(dir, clientAPKFilename), body, 0o644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.WriteFile(filepath.Join(dir, clientVersionFile), []byte(sidecar), 0o644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
return int64(len(body))
|
|
}
|
|
|
|
func getClientVersion(t *testing.T) *httptest.ResponseRecorder {
|
|
t.Helper()
|
|
h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
|
|
rr := httptest.NewRecorder()
|
|
h.handleClientVersion(rr, httptest.NewRequest(http.MethodGet, "/api/client/version", nil))
|
|
return rr
|
|
}
|
|
|
|
func TestClientVersion_200WithBothFiles(t *testing.T) {
|
|
size := writeClientAssets(t, `{"name":"2026.09.10.1432","code":3523847,"channel":"stable"}`+"\n")
|
|
rr := getClientVersion(t)
|
|
if rr.Code != http.StatusOK {
|
|
t.Fatalf("want 200, got %d (body: %s)", rr.Code, rr.Body.String())
|
|
}
|
|
var resp clientVersionResponse
|
|
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if resp.Version != "2026.09.10.1432" {
|
|
t.Errorf("version: want 2026.09.10.1432, got %q", resp.Version)
|
|
}
|
|
if resp.Code == nil {
|
|
t.Fatal("code: want 3523847, got absent — the client decides on this, so absent means it silently falls back to name comparison")
|
|
}
|
|
if *resp.Code != 3523847 {
|
|
t.Errorf("code: want 3523847, got %d", *resp.Code)
|
|
}
|
|
if resp.Channel != "stable" {
|
|
t.Errorf("channel: want stable, got %q", resp.Channel)
|
|
}
|
|
if resp.APKURL != "/api/client/apk" {
|
|
t.Errorf("apk_url: want /api/client/apk, got %q", resp.APKURL)
|
|
}
|
|
if resp.SizeBytes != size {
|
|
t.Errorf("size_bytes: want %d, got %d", size, resp.SizeBytes)
|
|
}
|
|
}
|
|
|
|
// A release published before ordering keys were recorded has a name and
|
|
// genuinely no code. That must arrive as ABSENT, not as 0 — zero would claim
|
|
// the build is infinitely old and offer an update to everyone forever.
|
|
func TestClientVersion_CodeAbsentIsOmittedNotZero(t *testing.T) {
|
|
writeClientAssets(t, `{"name":"2026.09.09","code":null,"channel":"stable"}`)
|
|
rr := getClientVersion(t)
|
|
if rr.Code != http.StatusOK {
|
|
t.Fatalf("want 200, got %d (body: %s)", rr.Code, rr.Body.String())
|
|
}
|
|
var resp clientVersionResponse
|
|
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if resp.Code != nil {
|
|
t.Errorf("code: want absent, got %d", *resp.Code)
|
|
}
|
|
// The wire must omit the key entirely, so a client can distinguish
|
|
// "this server reports no code" from "this build's code is 0".
|
|
var raw map[string]any
|
|
if err := json.Unmarshal(rr.Body.Bytes(), &raw); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if _, present := raw["code"]; present {
|
|
t.Errorf("code key should be omitted entirely, body was %s", rr.Body.String())
|
|
}
|
|
}
|
|
|
|
// The failure this guards is the one nobody reports: if an unreadable sidecar
|
|
// produced an empty version, every client would compare against nothing,
|
|
// decide it was current, and go quiet. "I cannot read this" and "there is
|
|
// nothing newer" must not be the same answer.
|
|
func TestClientVersion_MalformedSidecarErrorsRatherThanReportingNothing(t *testing.T) {
|
|
for _, sidecar := range []string{
|
|
"2026.09.10.1432", // the OLD plain-text format
|
|
`{"name":"x",`, // truncated JSON
|
|
`{"code":123,"channel":"dev"}`, // valid JSON, no name
|
|
"",
|
|
} {
|
|
writeClientAssets(t, sidecar)
|
|
rr := getClientVersion(t)
|
|
if rr.Code == http.StatusOK {
|
|
t.Errorf("sidecar %q: want an error status, got 200 with body %s", sidecar, rr.Body.String())
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestClientAPK_401WhenUnauthenticated(t *testing.T) {
|
|
withClientAPKDir(t)
|
|
testResetClientAPKRateLimit()
|
|
h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
|
|
rr := httptest.NewRecorder()
|
|
// No user in context — middleware would block in prod; handler also
|
|
// rejects defensively in case the route ever lands outside the
|
|
// authed group by accident.
|
|
h.handleClientAPK(rr, httptest.NewRequest(http.MethodGet, "/api/client/apk", nil))
|
|
if rr.Code != http.StatusUnauthorized {
|
|
t.Errorf("want 401, got %d", rr.Code)
|
|
}
|
|
}
|
|
|
|
func TestClientAPK_404WhenMissing(t *testing.T) {
|
|
withClientAPKDir(t)
|
|
testResetClientAPKRateLimit()
|
|
h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
|
|
rr := httptest.NewRecorder()
|
|
h.handleClientAPK(rr, authedRequest(http.MethodGet, "/api/client/apk", 0x01))
|
|
if rr.Code != http.StatusNotFound {
|
|
t.Errorf("want 404, got %d", rr.Code)
|
|
}
|
|
}
|
|
|
|
func TestClientAPK_StreamsWithCorrectContentType(t *testing.T) {
|
|
dir := withClientAPKDir(t)
|
|
testResetClientAPKRateLimit()
|
|
body := []byte("PK\x03\x04 fake apk bytes")
|
|
if err := os.WriteFile(filepath.Join(dir, clientAPKFilename), body, 0o644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
|
|
rr := httptest.NewRecorder()
|
|
h.handleClientAPK(rr, authedRequest(http.MethodGet, "/api/client/apk", 0x02))
|
|
if rr.Code != http.StatusOK {
|
|
t.Fatalf("want 200, got %d", rr.Code)
|
|
}
|
|
if got := rr.Header().Get("Content-Type"); got != "application/vnd.android.package-archive" {
|
|
t.Errorf("Content-Type: want application/vnd.android.package-archive, got %q", got)
|
|
}
|
|
if rr.Body.Len() != len(body) {
|
|
t.Errorf("body length: want %d, got %d", len(body), rr.Body.Len())
|
|
}
|
|
if got := rr.Body.Bytes(); string(got) != string(body) {
|
|
t.Errorf("body bytes mismatch")
|
|
}
|
|
}
|
|
|
|
func TestClientAPK_RateLimit_429OnRapidSecondCall(t *testing.T) {
|
|
dir := withClientAPKDir(t)
|
|
testResetClientAPKRateLimit()
|
|
if err := os.WriteFile(filepath.Join(dir, clientAPKFilename), []byte("apk"), 0o644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
|
|
|
|
// First call from user 0x03 — succeeds.
|
|
rr1 := httptest.NewRecorder()
|
|
h.handleClientAPK(rr1, authedRequest(http.MethodGet, "/api/client/apk", 0x03))
|
|
if rr1.Code != http.StatusOK {
|
|
t.Fatalf("first call: want 200, got %d", rr1.Code)
|
|
}
|
|
|
|
// Immediate second call from same user — rate-limited.
|
|
rr2 := httptest.NewRecorder()
|
|
h.handleClientAPK(rr2, authedRequest(http.MethodGet, "/api/client/apk", 0x03))
|
|
if rr2.Code != http.StatusTooManyRequests {
|
|
t.Fatalf("second call: want 429, got %d", rr2.Code)
|
|
}
|
|
if got := rr2.Header().Get("Retry-After"); got == "" {
|
|
t.Errorf("expected Retry-After header on 429 response")
|
|
}
|
|
|
|
// Different user — not rate-limited.
|
|
rr3 := httptest.NewRecorder()
|
|
h.handleClientAPK(rr3, authedRequest(http.MethodGet, "/api/client/apk", 0x04))
|
|
if rr3.Code != http.StatusOK {
|
|
t.Errorf("different user: want 200, got %d", rr3.Code)
|
|
}
|
|
}
|