Merge main back into dev to reconcile the squash-promotion divergence #91
2 changed files with 164 additions and 14 deletions
|
|
@ -19,6 +19,7 @@ import (
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
"thermograph/frontend/internal/config"
|
"thermograph/frontend/internal/config"
|
||||||
"thermograph/frontend/internal/contentapi"
|
"thermograph/frontend/internal/contentapi"
|
||||||
|
|
@ -474,6 +475,116 @@ func TestMonthCtx(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// --- fetchWithCity -----------------------------------------------------------
|
||||||
|
|
||||||
|
func TestFetchWithCityHappyPath(t *testing.T) {
|
||||||
|
api := &fakeAPI{
|
||||||
|
city: func(slug, _ string) (*contentapi.CityPayload, error) {
|
||||||
|
return &contentapi.CityPayload{City: contentapi.CityInfo{Slug: slug, Name: "Testville"}}, nil
|
||||||
|
},
|
||||||
|
}
|
||||||
|
primary, city, err := fetchWithCity(api, "testville", func() (string, error) { return "primary-value", nil })
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if primary != "primary-value" {
|
||||||
|
t.Errorf("primary = %q", primary)
|
||||||
|
}
|
||||||
|
if city.Slug != "testville" || city.Name != "Testville" {
|
||||||
|
t.Errorf("city = %+v", city)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// When only primary fails, its error must win even though City() also ran
|
||||||
|
// (and, here, succeeded) -- matching the old sequential code's behavior,
|
||||||
|
// where City() was never even called once primary had already failed.
|
||||||
|
func TestFetchWithCityPrimaryErrorWins(t *testing.T) {
|
||||||
|
primaryErr := notFound("no such month")
|
||||||
|
api := &fakeAPI{
|
||||||
|
city: func(slug, _ string) (*contentapi.CityPayload, error) {
|
||||||
|
return &contentapi.CityPayload{City: contentapi.CityInfo{Slug: slug}}, nil
|
||||||
|
},
|
||||||
|
}
|
||||||
|
_, _, err := fetchWithCity(api, "testville", func() (string, error) { return "", primaryErr })
|
||||||
|
if err != primaryErr {
|
||||||
|
t.Errorf("err = %v, want the primary error", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// When primary succeeds but City() fails, City's error must surface -- same
|
||||||
|
// as the old sequential code (it called City() second and returned whatever
|
||||||
|
// that call did).
|
||||||
|
func TestFetchWithCityCityErrorSurfaces(t *testing.T) {
|
||||||
|
cityErr := notFound("no such city")
|
||||||
|
api := &fakeAPI{
|
||||||
|
city: func(_, _ string) (*contentapi.CityPayload, error) { return nil, cityErr },
|
||||||
|
}
|
||||||
|
_, _, err := fetchWithCity(api, "testville", func() (string, error) { return "primary-value", nil })
|
||||||
|
if err != cityErr {
|
||||||
|
t.Errorf("err = %v, want the city error", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// When BOTH fail, primary's error still wins -- the same priority as the
|
||||||
|
// single-failure case above, just with both goroutines erroring.
|
||||||
|
func TestFetchWithCityBothErrorPrimaryWins(t *testing.T) {
|
||||||
|
primaryErr, cityErr := notFound("primary"), notFound("city")
|
||||||
|
api := &fakeAPI{
|
||||||
|
city: func(_, _ string) (*contentapi.CityPayload, error) { return nil, cityErr },
|
||||||
|
}
|
||||||
|
_, _, err := fetchWithCity(api, "testville", func() (string, error) { return "", primaryErr })
|
||||||
|
if err != primaryErr {
|
||||||
|
t.Errorf("err = %v, want the primary error (city error must not win)", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Proves the two calls actually run CONCURRENTLY rather than sequentially --
|
||||||
|
// deterministically, via a rendezvous, not by timing (which would be flaky
|
||||||
|
// under CI load). Each fake blocks until BOTH have started; if fetchWithCity
|
||||||
|
// ran them sequentially, the second would never start until the first
|
||||||
|
// returned, and this would deadlock and fail on the test's own timeout.
|
||||||
|
func TestFetchWithCityRunsConcurrently(t *testing.T) {
|
||||||
|
started := make(chan struct{}, 2)
|
||||||
|
release := make(chan struct{})
|
||||||
|
rendezvous := func() {
|
||||||
|
started <- struct{}{}
|
||||||
|
<-release
|
||||||
|
}
|
||||||
|
|
||||||
|
api := &fakeAPI{
|
||||||
|
city: func(_, _ string) (*contentapi.CityPayload, error) {
|
||||||
|
rendezvous()
|
||||||
|
return &contentapi.CityPayload{}, nil
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
done := make(chan struct{})
|
||||||
|
go func() {
|
||||||
|
fetchWithCity(api, "testville", func() (string, error) {
|
||||||
|
rendezvous()
|
||||||
|
return "", nil
|
||||||
|
})
|
||||||
|
close(done)
|
||||||
|
}()
|
||||||
|
|
||||||
|
// Both goroutines must reach the rendezvous before either can proceed --
|
||||||
|
// only possible if they were launched concurrently.
|
||||||
|
for range 2 {
|
||||||
|
select {
|
||||||
|
case <-started:
|
||||||
|
case <-time.After(2 * time.Second):
|
||||||
|
t.Fatal("timed out waiting for both calls to start -- they are not running concurrently")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
close(release)
|
||||||
|
|
||||||
|
select {
|
||||||
|
case <-done:
|
||||||
|
case <-time.After(2 * time.Second):
|
||||||
|
t.Fatal("fetchWithCity did not return after release")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestMonthCtxUnknownLabelIsError(t *testing.T) {
|
func TestMonthCtxUnknownLabelIsError(t *testing.T) {
|
||||||
h := newHandlers(t, fixtureAPI(t))
|
h := newHandlers(t, fixtureAPI(t))
|
||||||
j := loadFixture[contentapi.MonthPayload](t, "month")
|
j := loadFixture[contentapi.MonthPayload](t, "month")
|
||||||
|
|
|
||||||
|
|
@ -7,6 +7,7 @@ import (
|
||||||
"html/template"
|
"html/template"
|
||||||
"net/http"
|
"net/http"
|
||||||
"strings"
|
"strings"
|
||||||
|
"sync"
|
||||||
|
|
||||||
"thermograph/frontend/internal/contentapi"
|
"thermograph/frontend/internal/contentapi"
|
||||||
"thermograph/frontend/internal/contentdata"
|
"thermograph/frontend/internal/contentdata"
|
||||||
|
|
@ -165,6 +166,49 @@ func (h *Handlers) cityCtx(j *contentapi.CityPayload) (map[string]any, format.Un
|
||||||
return ctx, unit, nil
|
return ctx, unit, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// fetchWithCity runs a page's primary API call and the shared City lookup
|
||||||
|
// concurrently instead of sequentially: both are independent single-slug
|
||||||
|
// fetches (City never depends on primary's result), so waiting for one to
|
||||||
|
// fully round-trip before even starting the other was pure latency with
|
||||||
|
// nothing to show for it. Concurrent instead cuts that leg to roughly
|
||||||
|
// whichever call is slower.
|
||||||
|
//
|
||||||
|
// If primary fails, its error wins even when City also fails or hasn't
|
||||||
|
// finished — matching the old sequential code's behavior exactly (it never
|
||||||
|
// even called City() once primary had already failed). The one real
|
||||||
|
// trade-off: City() is now ALWAYS launched, even on a request that's about
|
||||||
|
// to 404 from primary, so an invalid slug costs one extra (wasted, cheap)
|
||||||
|
// backend lookup it previously skipped. Worth it: a bad slug is the rare
|
||||||
|
// path; a good one is the common path this speeds up.
|
||||||
|
func fetchWithCity[T any](api API, slug string, primary func() (T, error)) (T, contentapi.CityInfo, error) {
|
||||||
|
var (
|
||||||
|
pVal T
|
||||||
|
pErr error
|
||||||
|
cPay *contentapi.CityPayload
|
||||||
|
cErr error
|
||||||
|
group sync.WaitGroup
|
||||||
|
)
|
||||||
|
group.Add(2)
|
||||||
|
go func() {
|
||||||
|
defer group.Done()
|
||||||
|
pVal, pErr = primary()
|
||||||
|
}()
|
||||||
|
go func() {
|
||||||
|
defer group.Done()
|
||||||
|
cPay, cErr = api.City(slug, "") // no origin: the Python called city(slug) bare here too
|
||||||
|
}()
|
||||||
|
group.Wait()
|
||||||
|
|
||||||
|
if pErr != nil {
|
||||||
|
return pVal, contentapi.CityInfo{}, pErr
|
||||||
|
}
|
||||||
|
if cErr != nil {
|
||||||
|
var zero T
|
||||||
|
return zero, contentapi.CityInfo{}, cErr
|
||||||
|
}
|
||||||
|
return pVal, cPay.City, nil
|
||||||
|
}
|
||||||
|
|
||||||
// --- month page --------------------------------------------------------------
|
// --- month page --------------------------------------------------------------
|
||||||
|
|
||||||
// MonthPage renders /climate/{slug}/{month} (content.py's month_page).
|
// MonthPage renders /climate/{slug}/{month} (content.py's month_page).
|
||||||
|
|
@ -176,17 +220,14 @@ func (h *Handlers) cityCtx(j *contentapi.CityPayload) (map[string]any, format.Un
|
||||||
// Jinja iterated (label, value) tuples).
|
// Jinja iterated (label, value) tuples).
|
||||||
func (h *Handlers) MonthPage(w http.ResponseWriter, r *http.Request) {
|
func (h *Handlers) MonthPage(w http.ResponseWriter, r *http.Request) {
|
||||||
slug, month := r.PathValue("slug"), r.PathValue("month")
|
slug, month := r.PathValue("slug"), r.PathValue("month")
|
||||||
j, err := h.api.CityMonth(slug, month)
|
j, city, err := fetchWithCity(h.api, slug, func() (*contentapi.MonthPayload, error) {
|
||||||
|
return h.api.CityMonth(slug, month)
|
||||||
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
h.apiError(w, err)
|
h.apiError(w, err)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
cp, err := h.api.City(slug, "") // no origin: the Python called city(slug) bare here
|
ctx, unit, err := h.monthCtx(j, city)
|
||||||
if err != nil {
|
|
||||||
h.apiError(w, err)
|
|
||||||
return
|
|
||||||
}
|
|
||||||
ctx, unit, err := h.monthCtx(j, cp.City)
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
h.serverError(w, err)
|
h.serverError(w, err)
|
||||||
return
|
return
|
||||||
|
|
@ -308,17 +349,15 @@ func fmtPeriodSide(unit format.Unit, s contentapi.PeriodSide) map[string]any {
|
||||||
// temperature metrics, nil otherwise, as the Python set them.
|
// temperature metrics, nil otherwise, as the Python set them.
|
||||||
func (h *Handlers) RecordsPage(w http.ResponseWriter, r *http.Request) {
|
func (h *Handlers) RecordsPage(w http.ResponseWriter, r *http.Request) {
|
||||||
slug := r.PathValue("slug")
|
slug := r.PathValue("slug")
|
||||||
j, err := h.api.CityRecords(slug, origin(r))
|
o := origin(r)
|
||||||
|
j, city, err := fetchWithCity(h.api, slug, func() (*contentapi.RecordsPayload, error) {
|
||||||
|
return h.api.CityRecords(slug, o)
|
||||||
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
h.apiError(w, err)
|
h.apiError(w, err)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
cp, err := h.api.City(slug, "") // no origin, same as month_page
|
ctx, unit, err := h.recordsCtx(j, city)
|
||||||
if err != nil {
|
|
||||||
h.apiError(w, err)
|
|
||||||
return
|
|
||||||
}
|
|
||||||
ctx, unit, err := h.recordsCtx(j, cp.City)
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
h.serverError(w, err)
|
h.serverError(w, err)
|
||||||
return
|
return
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue