All checks were successful
PR build (required check) / changes (pull_request) Successful in 6s
secrets-guard / encrypted (pull_request) Successful in 7s
PR build (required check) / build-backend (pull_request) Has been skipped
shell-lint / shellcheck (pull_request) Successful in 7s
PR build (required check) / validate-observability (pull_request) Has been skipped
PR build (required check) / build-frontend (pull_request) Successful in 1m0s
PR build (required check) / gate (pull_request) Successful in 2s
Ports frontend/ (Jinja2/FastAPI, ~1180 LOC) to Go with html/template. No climate math, no DB, no auth here -- every route fetches from the backend's /content/* API, so this is I/O-bound glue with no hard-porting wall; the risk was always in reproducing the rendering exactly, not the language. Verified with a golden-HTML diff, not just unit tests: both the Python original and the Go rewrite were run against the same committed fixtures (frontend/tests/fixtures/) and every one of the 11 routes compared byte-for-byte. The only surviving differences after that process are insignificant inter-tag whitespace and one attribute where Go's stricter escaper HTML-encodes an apostrophe Jinja left literal (functionally identical in every browser) -- confirmed programmatically by normalizing whitespace and unescaping before diffing, not by eyeballing. That process caught defects unit tests alone would have missed, because map[string]any has no compile-time field check: - Render-context keys were snake_case throughout (content.py's Jinja convention, ported verbatim) while the templates -- written independently -- read PascalCase fields. A missing map key doesn't error in html/template, it silently renders empty, so this was invisible in every status code and every "it built" signal: title, meta description, canonical URL, OpenGraph tags, the homepage's entire ranked list, and the brand-tag/nav-active state were all blank across every page. Fixed by renaming every key to match each template's own header comment (the authoritative per-page field contract) and, where an API struct's exported fields already matched what a template needed (contentapi.CityInfo, Crumb, HomeRanked, HubCountry, ...), passing the struct straight through instead of hand-rewrapping it in a map -- removes a whole layer of future drift risk, not just this instance of it. - Three pages 500'd outright: `.ToolHref` needed a fully-composed href string, not the bare "lat,lon" fragment the handlers were building; the all-time-records table needed the raw contentapi.AllTimeRecords struct, not a re-wrapped map. - JSON-LD was being double-encoded: `<script type="application/ld+json">` is JAVASCRIPT context to html/template's contextual escaper regardless of the script's `type` attribute, so a template.HTML-typed value placed there gets re-escaped as a quoted JS string instead of emitted raw -- the entire structured-data payload shipped as a JSON string containing JSON, which no crawler would parse as the intended object. Needed template.JS instead, the type that actually means "trusted JS source." The glossary term page's JSON-LD was simply never built at all (the Jinja original assembled it inline in the template rather than through content.py's context dict, and that got lost in translation) -- added. - html/template silently strips literal HTML comments AND JavaScript comments from the parsed output (verified in isolation, zero template actions involved) -- confirmed as real engine behavior, not a bug in either port, so both need a FuncMap function returning template.HTML / template.JS respectively to survive parsing rather than a literal `<!-- -->` or `//` in the template source. Packaging: multi-stage Go build, final image alpine (not distroless -- the Swarm stack's env-entrypoint.sh shim needs bash), 187MB -> 22.6MB. Two defects caught before they reached a host: - The Swarm stack overrides `entrypoint:` with no `command:`, which drops the image's own CMD entirely (Docker/Swarm semantics, not merged) -- env-entrypoint.sh then fell through to its hardcoded `exec uvicorn app:app` fallback, which doesn't exist in this image. Every deploy would have exited 127. Fixed with an explicit `command:` on the stack's frontend service, and corrected the shim's stale comment claiming CMD passes through automatically. - `COPY --chown=thermograph` resolves the group by NAME at copy time; Alpine's `adduser -S` with no `-G` doesn't create a same-named group, so the classic (non-BuildKit) Docker builder -- which this CI runner falls back to, since it installs plain `docker.io` with no buildx plugin -- failed outright. Fixed with an explicit group and numeric --chown. Verification: go build/vet/test -race clean across all packages; the Docker image builds and passes its embedded go test step under both BuildKit and the classic builder; shellcheck 0 findings on the one script touched; rebased onto current main (the ERA5 lake stack landed on both main and dev during this work -- confirmed additive, no overlap with frontend/daemon).
251 lines
7.9 KiB
Go
251 lines
7.9 KiB
Go
package contentapi
|
|
|
|
import (
|
|
"errors"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"sync"
|
|
"sync/atomic"
|
|
"testing"
|
|
"time"
|
|
)
|
|
|
|
func newTestClient(t *testing.T, handler http.Handler, opts Options) (*Client, *httptest.Server) {
|
|
t.Helper()
|
|
srv := httptest.NewServer(handler)
|
|
t.Cleanup(srv.Close)
|
|
opts.BaseURL = srv.URL
|
|
return New(opts), srv
|
|
}
|
|
|
|
func TestHappyPathAndCaching(t *testing.T) {
|
|
var hits atomic.Int64
|
|
c, _ := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
hits.Add(1)
|
|
if r.URL.Path != "/api/v2/content/hub" {
|
|
t.Errorf("path = %q", r.URL.Path)
|
|
}
|
|
w.Header().Set("Content-Type", "application/json")
|
|
w.Write([]byte(`{"n_cities": 2, "n_countries": 1,
|
|
"countries": [{"country": "France", "cities": [
|
|
{"slug": "paris-fr", "name": "Paris", "display": "Paris, France"}]}]}`))
|
|
}), Options{})
|
|
|
|
h, err := c.Hub()
|
|
if err != nil {
|
|
t.Fatalf("Hub: %v", err)
|
|
}
|
|
if h.NCities != 2 || len(h.Countries) != 1 || h.Countries[0].Cities[0].Slug != "paris-fr" {
|
|
t.Errorf("unexpected payload: %+v", h)
|
|
}
|
|
// Second call inside the TTL is served from cache — no new backend hit.
|
|
if _, err := c.Hub(); err != nil {
|
|
t.Fatalf("Hub (cached): %v", err)
|
|
}
|
|
if got := hits.Load(); got != 1 {
|
|
t.Errorf("backend hits = %d, want 1 (TTL cache)", got)
|
|
}
|
|
}
|
|
|
|
func TestNon2xxIsStatusError(t *testing.T) {
|
|
c, _ := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
w.WriteHeader(http.StatusNotFound)
|
|
w.Write([]byte(`{"detail":"Unknown city."}`))
|
|
}), Options{})
|
|
|
|
_, err := c.City("nowhere", "")
|
|
var se *StatusError
|
|
if !errors.As(err, &se) {
|
|
t.Fatalf("want *StatusError, got %v", err)
|
|
}
|
|
if se.StatusCode != 404 {
|
|
t.Errorf("StatusCode = %d, want 404", se.StatusCode)
|
|
}
|
|
if se.Body != `{"detail":"Unknown city."}` {
|
|
t.Errorf("Body = %q", se.Body)
|
|
}
|
|
// Errors are never cached: the next call must reach the backend again.
|
|
if _, err := c.City("nowhere", ""); err == nil {
|
|
t.Fatal("expected the error to repeat, not a cached success")
|
|
}
|
|
}
|
|
|
|
func TestNotModifiedIsStatusError(t *testing.T) {
|
|
// This client never sends If-None-Match (the Python didn't either), so a
|
|
// 304 is an out-of-contract response: it must surface as an error, never
|
|
// be cached as data.
|
|
c, _ := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
if inm := r.Header.Get("If-None-Match"); inm != "" {
|
|
t.Errorf("client sent If-None-Match %q; it must not send conditional requests", inm)
|
|
}
|
|
w.WriteHeader(http.StatusNotModified)
|
|
}), Options{})
|
|
|
|
_, err := c.Home()
|
|
var se *StatusError
|
|
if !errors.As(err, &se) {
|
|
t.Fatalf("want *StatusError, got %v", err)
|
|
}
|
|
if se.StatusCode != 304 {
|
|
t.Errorf("StatusCode = %d, want 304", se.StatusCode)
|
|
}
|
|
}
|
|
|
|
func TestTimeoutSurfacesAsTransportError(t *testing.T) {
|
|
block := make(chan struct{})
|
|
defer close(block)
|
|
c, _ := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
<-block
|
|
}), Options{HTTPClient: &http.Client{Timeout: 50 * time.Millisecond}})
|
|
|
|
_, err := c.Home()
|
|
if err == nil {
|
|
t.Fatal("expected a timeout error")
|
|
}
|
|
var se *StatusError
|
|
if errors.As(err, &se) {
|
|
t.Fatalf("timeout must be a transport error (503 to callers), not StatusError %d", se.StatusCode)
|
|
}
|
|
}
|
|
|
|
func TestTTLExpiry(t *testing.T) {
|
|
var hits atomic.Int64
|
|
c, _ := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
hits.Add(1)
|
|
w.Write([]byte(`{"key":"abc123"}`))
|
|
}), Options{TTL: 30 * time.Millisecond})
|
|
|
|
if k, err := c.IndexNowKey(); err != nil || k != "abc123" {
|
|
t.Fatalf("IndexNowKey = %q, %v", k, err)
|
|
}
|
|
c.IndexNowKey() // cached
|
|
time.Sleep(40 * time.Millisecond)
|
|
c.IndexNowKey() // expired -> refetch
|
|
if got := hits.Load(); got != 2 {
|
|
t.Errorf("backend hits = %d, want 2 (one refetch after TTL)", got)
|
|
}
|
|
}
|
|
|
|
func TestLRUEvictionBounded(t *testing.T) {
|
|
c := New(Options{BaseURL: "http://unused", MaxCacheEntries: 3})
|
|
for _, k := range []string{"k0", "k1", "k2"} {
|
|
c.cachePut(k, k)
|
|
}
|
|
// Touch k0 so it's most-recently-used; k1 becomes the eviction candidate.
|
|
if c.cacheGet("k0") == nil {
|
|
t.Fatal("k0 should be present")
|
|
}
|
|
c.cachePut("k3", "k3")
|
|
|
|
if c.cacheGet("k1") != nil {
|
|
t.Error("k1 should have been evicted (least-recently-used)")
|
|
}
|
|
for _, k := range []string{"k0", "k2", "k3"} {
|
|
if c.cacheGet(k) == nil {
|
|
t.Errorf("%s should have survived", k)
|
|
}
|
|
}
|
|
if n := len(c.cache); n != 3 {
|
|
t.Errorf("cache size = %d, want 3", n)
|
|
}
|
|
}
|
|
|
|
func TestSingleFlightDedupesConcurrentMisses(t *testing.T) {
|
|
// N concurrent misses on the same key must reach the backend once, not N
|
|
// times — every caller gets the same result either way.
|
|
var hits atomic.Int64
|
|
release := make(chan struct{})
|
|
c, _ := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
hits.Add(1)
|
|
<-release
|
|
w.Write([]byte(`{"unusual": null, "stale": false, "ranked": [], "cities": []}`))
|
|
}), Options{})
|
|
|
|
const n = 5
|
|
var wg sync.WaitGroup
|
|
errs := make([]error, n)
|
|
for i := range n {
|
|
wg.Add(1)
|
|
go func() {
|
|
defer wg.Done()
|
|
_, errs[i] = c.Home()
|
|
}()
|
|
}
|
|
time.Sleep(100 * time.Millisecond) // let every goroutine reach the flight lock
|
|
close(release)
|
|
wg.Wait()
|
|
|
|
if got := hits.Load(); got != 1 {
|
|
t.Errorf("backend hits = %d, want exactly 1 for %d concurrent misses", got, n)
|
|
}
|
|
for i, err := range errs {
|
|
if err != nil {
|
|
t.Errorf("caller %d: %v", i, err)
|
|
}
|
|
}
|
|
c.inflightMu.Lock()
|
|
inflight := len(c.inflight)
|
|
c.inflightMu.Unlock()
|
|
if inflight != 0 {
|
|
t.Errorf("inflight map should be empty after completion, has %d entries", inflight)
|
|
}
|
|
}
|
|
|
|
func TestOriginForwardedAsHostAndProto(t *testing.T) {
|
|
var gotHost, gotProto string
|
|
c, _ := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
gotHost, gotProto = r.Host, r.Header.Get("X-Forwarded-Proto")
|
|
w.Write([]byte(`{"city":{"slug":"x","name":"X","admin1":"","country":"","country_code":"GB",` +
|
|
`"lat":1,"lon":2,"population":3},"display":"X","title":"t","year_range":[1980,2025],` +
|
|
`"n_years":45,"months":[],"warmest_month_slug":"","coldest_month_slug":"",` +
|
|
`"wettest_month_slug":"","all_time_records":{},"today_vs_normal":null,"flavor":null,` +
|
|
`"event":null,"default_unit":"C","breadcrumb":[],"canonical_path":"/climate/x",` +
|
|
`"page_title":"","page_description":"","jsonld":{}}`))
|
|
}), Options{})
|
|
|
|
if _, err := c.City("x", "https://thermograph.org"); err != nil {
|
|
t.Fatalf("City: %v", err)
|
|
}
|
|
if gotHost != "thermograph.org" {
|
|
t.Errorf("Host = %q, want thermograph.org", gotHost)
|
|
}
|
|
if gotProto != "https" {
|
|
t.Errorf("X-Forwarded-Proto = %q, want https", gotProto)
|
|
}
|
|
}
|
|
|
|
func TestOriginIsPartOfCacheKey(t *testing.T) {
|
|
var hits atomic.Int64
|
|
body := `{"city":{"slug":"x","name":"X","admin1":"","country":"","country_code":"GB",` +
|
|
`"lat":1,"lon":2,"population":3},"display":"X","title":"t","year_range":[1980,2025],` +
|
|
`"n_years":45,"months":[],"warmest_month_slug":"","coldest_month_slug":"",` +
|
|
`"wettest_month_slug":"","all_time_records":{},"today_vs_normal":null,"flavor":null,` +
|
|
`"event":null,"default_unit":"C","breadcrumb":[],"canonical_path":"/climate/x",` +
|
|
`"page_title":"","page_description":"","jsonld":{}}`
|
|
c, _ := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
hits.Add(1)
|
|
w.Write([]byte(body))
|
|
}), Options{})
|
|
|
|
c.City("x", "https://a.example")
|
|
c.City("x", "https://b.example") // distinct origin -> distinct cache entry
|
|
c.City("x", "https://a.example") // cached
|
|
if got := hits.Load(); got != 2 {
|
|
t.Errorf("backend hits = %d, want 2 (origin folded into the cache key)", got)
|
|
}
|
|
}
|
|
|
|
func TestBasePrefixPrependedToEveryPath(t *testing.T) {
|
|
var gotPath string
|
|
c, _ := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
gotPath = r.URL.Path
|
|
w.Write([]byte(`{"key":"k"}`))
|
|
}), Options{BasePrefix: "/thermograph"})
|
|
|
|
if _, err := c.IndexNowKey(); err != nil {
|
|
t.Fatalf("IndexNowKey: %v", err)
|
|
}
|
|
if gotPath != "/thermograph/api/v2/content/indexnow-key" {
|
|
t.Errorf("path = %q, want the backend's own THERMOGRAPH_BASE prefix applied", gotPath)
|
|
}
|
|
}
|