From e6450138c225f6174d15396a61b0c53d7981b5a0 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Thu, 13 Aug 2026 03:38:49 +0000 Subject: [PATCH] Reuse weather API readiness response --- docs/integrations/weatherapi.md | 16 +-- docs/internal/collect.md | 4 + internal/adapters/weatherapi/client.go | 105 +++++++++++--------- internal/adapters/weatherapi/client_test.go | 11 +- internal/collect/collect_test.go | 18 +++- 5 files changed, 95 insertions(+), 59 deletions(-) diff --git a/docs/integrations/weatherapi.md b/docs/integrations/weatherapi.md index dc94896..2098c9b 100644 --- a/docs/integrations/weatherapi.md +++ b/docs/integrations/weatherapi.md @@ -17,16 +17,18 @@ configured timeout on every HTTP attempt. Every request sends `format` and, except where noted below, `units`. The configured format must be `json`. -Before retrieving sources, Weatherreporter warms up -`/conditions/current` with the same `format`, `units`, and `precision` query -parameters used for current conditions. The warmup only requires a readable -2xx response; its body is not decoded. Failure after its internal retry budget -stops the fetch before source requests begin. +Before retrieving sources, Weatherreporter requests `/conditions/current` with +the same `format`, `units`, and `precision` query parameters used for current +conditions. After a readable 2xx response, it retains that response for the +normal current-conditions source step rather than making a second identical +request. Failure after the readiness request's internal retry budget stops the +fetch before source requests begin. ## Endpoints And Query Parameters -The adapter makes one source request for each endpoint after a successful -warmup, subject to retry on transient failures. +The adapter makes one source request for each endpoint, subject to retry on +transient failures. A successful readiness request supplies the current +conditions source response. | Source | Endpoint | Query parameters | Availability | | --- | --- | --- | --- | diff --git a/docs/internal/collect.md b/docs/internal/collect.md index dd50e5e..71b5798 100644 --- a/docs/internal/collect.md +++ b/docs/internal/collect.md @@ -11,6 +11,10 @@ semantics belong in [weather-data internals](weather-data.md). `config.Config`. It constructs the Weather API adapter from that configuration, calls `FetchBundle`, and returns `Result{Bundle: *weatherdata.Bundle}`. +The adapter's successful readiness request for current conditions is reused as +that normalized source; collection does not trigger a second identical current +conditions request. + The package wraps adapter construction failures, including an invalid Weather API base URL, as weather-collection setup errors and fetch failures as bundle-collection errors. It does not retry, persist, select reports, derive diff --git a/internal/adapters/weatherapi/client.go b/internal/adapters/weatherapi/client.go index 099bc99..be540e9 100644 --- a/internal/adapters/weatherapi/client.go +++ b/internal/adapters/weatherapi/client.go @@ -24,9 +24,10 @@ import ( const ( convectiveOutlooksEndpoint = "/outlooks/convective" + currentConditionsEndpoint = "/conditions/current" sourceSPCConvectiveOutlooks = config.MissingSourceSPCConvectiveOutlooks - defaultWarmupEndpoint = "/conditions/current" + defaultWarmupEndpoint = currentConditionsEndpoint defaultWarmupAttempts = 3 defaultWarmupDelay = time.Second defaultFetchAttempts = 2 @@ -113,7 +114,8 @@ func New(cfg config.Config, opts ...Option) (*Client, error) { } func (c *Client) FetchBundle(ctx context.Context) (*weatherdata.Bundle, error) { - if err := c.warmup(ctx); err != nil { + warmup, err := c.warmup(ctx) + if err != nil { return nil, err } @@ -127,7 +129,7 @@ func (c *Client) FetchBundle(ctx context.Context) (*weatherdata.Bundle, error) { if err := builder.fetchObservation(ctx); err != nil { return nil, err } - if err := builder.fetchCurrent(ctx); err != nil { + if err := builder.fetchCurrent(ctx, warmup); err != nil { return nil, err } if err := builder.fetchHourly(ctx); err != nil { @@ -172,6 +174,13 @@ type fetchedSource struct { source weatherdata.Source } +type warmupResponse struct { + endpoint string + requestURL *url.URL + body []byte + fetchedAt time.Time +} + func (b *bundleBuilder) fetchObservation(ctx context.Context) error { var observation weatherdata.Observation fetched, ok, err := b.fetchDecodedSource(ctx, sourceRequest{ @@ -190,14 +199,28 @@ func (b *bundleBuilder) fetchObservation(ctx context.Context) error { return nil } -func (b *bundleBuilder) fetchCurrent(ctx context.Context) error { - var current weatherdata.Current - fetched, ok, err := b.fetchDecodedSource(ctx, sourceRequest{ +func currentConditionsRequest() sourceRequest { + return sourceRequest{ name: config.MissingSourceCurrent, - endpoint: "/conditions/current", + endpoint: currentConditionsEndpoint, query: queryOptions{precision: true}, missingMessage: "current conditions data is missing", - }, ¤t) + } +} + +func (b *bundleBuilder) fetchCurrent(ctx context.Context, warmup warmupResponse) error { + var current weatherdata.Current + request := currentConditionsRequest() + var ( + fetched fetchedSource + ok bool + err error + ) + if warmup.endpoint == request.endpoint { + fetched, ok, err = b.fetchWarmupSource(warmup, request, ¤t) + } else { + fetched, ok, err = b.fetchDecodedSource(ctx, request, ¤t) + } if err != nil || !ok { return err } @@ -356,6 +379,21 @@ func (b *bundleBuilder) fetchDecodedSource(ctx context.Context, request sourceRe if err != nil || !ok { return fetchedSource{}, false, err } + return b.decodeFetchedSource(fetched, request, target) +} + +func (b *bundleBuilder) fetchWarmupSource(warmup warmupResponse, request sourceRequest, target any) (fetchedSource, bool, error) { + raw, source, err := b.client.decodeSourceResponse(request.name, request.endpoint, request.query, warmup.requestURL, warmup.body, warmup.fetchedAt) + if err != nil { + return fetchedSource{}, false, err + } + if raw == nil { + return fetchedSource{}, false, b.handleMissing(&source, request.missingMessage, request.required) + } + return b.decodeFetchedSource(fetchedSource{raw: raw, source: source}, request, target) +} + +func (b *bundleBuilder) decodeFetchedSource(fetched fetchedSource, request sourceRequest, target any) (fetchedSource, bool, error) { if err := decodeSource(fetched.raw, target); err != nil { return fetchedSource{}, false, b.handleMalformed(&fetched.source, err, request) } @@ -441,7 +479,10 @@ func (c *Client) fetch(ctx context.Context, sourceName string, endpoint string, if err != nil { return nil, weatherdata.Source{}, err } + return c.decodeSourceResponse(sourceName, endpoint, opts, reqURL, body, c.now()) +} +func (c *Client) decodeSourceResponse(sourceName string, endpoint string, opts queryOptions, reqURL *url.URL, body []byte, fetchedAt time.Time) (json.RawMessage, weatherdata.Source, error) { var env envelope if err := json.Unmarshal(body, &env); err != nil { return nil, weatherdata.Source{}, fmt.Errorf("decode %s envelope: %w", endpoint, err) @@ -451,7 +492,7 @@ func (c *Client) fetch(ctx context.Context, sourceName string, endpoint string, Name: sourceName, Endpoint: endpoint, Query: queryMap(reqURL.Query()), - FetchedAt: c.now(), + FetchedAt: fetchedAt, } if len(env.Data) == 0 || (isJSONNull(env.Data) && !opts.allowNull) { source.Missing = true @@ -465,7 +506,7 @@ func (c *Client) fetch(ctx context.Context, sourceName string, endpoint string, return env.Data, source, nil } -func (c *Client) warmup(ctx context.Context) error { +func (c *Client) warmup(ctx context.Context) (warmupResponse, error) { endpoint := c.warmupEndpoint if strings.TrimSpace(endpoint) == "" { endpoint = defaultWarmupEndpoint @@ -475,56 +516,30 @@ func (c *Client) warmup(ctx context.Context) error { var lastRetryable bool for attempt := 1; attempt <= attempts; attempt++ { if err := ctx.Err(); err != nil { - return fmt.Errorf("warm up weather API via %s: %w", endpoint, err) + return warmupResponse{}, fmt.Errorf("warm up weather API via %s: %w", endpoint, err) } - if err := c.warmupOnce(ctx, endpoint); err != nil { + reqURL, body, err := c.warmupOnce(ctx, endpoint) + if err != nil { lastErr = err lastRetryable = isRetryableRequestError(err) } else { - return nil + return warmupResponse{endpoint: endpoint, requestURL: reqURL, body: body, fetchedAt: c.now()}, nil } if !lastRetryable || attempt == attempts { break } if err := waitForRetry(ctx, c.warmupDelay); err != nil { - return fmt.Errorf("warm up weather API via %s after %d attempt(s): %w", endpoint, attempt, err) + return warmupResponse{}, fmt.Errorf("warm up weather API via %s after %d attempt(s): %w", endpoint, attempt, err) } } if !lastRetryable { - return lastErr + return warmupResponse{}, lastErr } - return fmt.Errorf("warm up weather API via %s failed after %d attempts: %w", endpoint, attempts, lastErr) + return warmupResponse{}, fmt.Errorf("warm up weather API via %s failed after %d attempts: %w", endpoint, attempts, lastErr) } -func (c *Client) warmupOnce(ctx context.Context, endpoint string) error { - reqURL := c.endpointURL(endpoint, queryOptions{precision: true}) - req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL.String(), nil) - if err != nil { - return fmt.Errorf("create request for %s: %w", endpoint, err) - } - - resp, err := c.httpClient.Do(req) - if err != nil { - err = fmt.Errorf("fetch %s: %w", endpoint, err) - if ctx.Err() != nil { - return err - } - return retryableRequestError{err: err} - } - defer resp.Body.Close() - - _, err = readResponseBody(resp.Body) - if err != nil { - return responseReadError(ctx, endpoint, err) - } - if resp.StatusCode < 200 || resp.StatusCode >= 300 { - err := fmt.Errorf("fetch %s: unexpected HTTP status %d", endpoint, resp.StatusCode) - if isRetryableHTTPStatus(resp.StatusCode) { - return retryableRequestError{err: err} - } - return err - } - return nil +func (c *Client) warmupOnce(ctx context.Context, endpoint string) (*url.URL, []byte, error) { + return c.fetchHTTPOnce(ctx, endpoint, queryOptions{precision: true}) } func (c *Client) fetchHTTP(ctx context.Context, endpoint string, opts queryOptions) (*url.URL, []byte, error) { diff --git a/internal/adapters/weatherapi/client_test.go b/internal/adapters/weatherapi/client_test.go index 0b36f89..3d224b1 100644 --- a/internal/adapters/weatherapi/client_test.go +++ b/internal/adapters/weatherapi/client_test.go @@ -87,8 +87,8 @@ func TestFetchBundleFromFixtures(t *testing.T) { "/weatherstories/latest", convectiveOutlooksEndpoint, } - if len(requested) != len(wantPaths)+1 { - t.Fatalf("requested paths = %v, want warmup plus %d source endpoints", requested, len(wantPaths)) + if len(requested) != len(wantPaths) { + t.Fatalf("requested paths = %v, want %d source endpoints", requested, len(wantPaths)) } if !strings.HasPrefix(requested[0], defaultWarmupEndpoint+"?") && requested[0] != defaultWarmupEndpoint { t.Fatalf("first requested path = %q, want warmup endpoint %s", requested[0], defaultWarmupEndpoint) @@ -98,6 +98,9 @@ func TestFetchBundleFromFixtures(t *testing.T) { t.Fatalf("requested paths = %v, want %s", requested, want) } } + if got := countPath(requested, currentConditionsEndpoint); got != 1 { + t.Fatalf("conditions/current requests = %d, want 1; requested paths = %v", got, requested) + } if !containsPath(requested, "/forecast/hourly") || containsPath(requested, "/forecast/hourly/today") { t.Fatalf("requested paths = %v, want full hourly endpoint only", requested) } @@ -258,8 +261,8 @@ func TestWarmupRetriesBeforeFetchBundle(t *testing.T) { if bundle.Current == nil { t.Fatal("Current = nil, want successful fetch after warmup retry") } - if warmupCalls != 3 { - t.Fatalf("conditions/current calls = %d, want failed warmup, successful warmup, and current source fetch", warmupCalls) + if warmupCalls != 2 { + t.Fatalf("conditions/current calls = %d, want failed and successful warmup attempts", warmupCalls) } if len(requested) < 2 || !containsPath(requested[:2], defaultWarmupEndpoint) { t.Fatalf("initial requests = %v, want warmup endpoint retries", requested) diff --git a/internal/collect/collect_test.go b/internal/collect/collect_test.go index 9e00869..e0c151c 100644 --- a/internal/collect/collect_test.go +++ b/internal/collect/collect_test.go @@ -5,13 +5,19 @@ import ( "net/http" "net/http/httptest" "strings" + "sync/atomic" "testing" "gitea.maximumdirect.net/eric/weatherreporter/internal/config" ) func TestRunFetchesBundle(t *testing.T) { - server := collectionTestServer(t, nil) + var currentRequests atomic.Int32 + server := collectionTestServer(t, nil, func(r *http.Request) { + if r.URL.Path == "/conditions/current" { + currentRequests.Add(1) + } + }) defer server.Close() cfg := config.Defaults() @@ -30,6 +36,9 @@ func TestRunFetchesBundle(t *testing.T) { if result.Bundle.WeatherStory == nil || result.Bundle.WeatherStory.Title != "Several Chances for Rain Through Monday" { t.Fatalf("WeatherStory = %#v, want fetched weather story", result.Bundle.WeatherStory) } + if got := currentRequests.Load(); got != 1 { + t.Fatalf("conditions/current requests = %d, want 1", got) + } } func TestRunWrapsAdapterConstructionError(t *testing.T) { @@ -50,7 +59,7 @@ func TestRunWrapsAdapterConstructionError(t *testing.T) { } func TestRunWrapsFetchError(t *testing.T) { - server := collectionTestServer(t, map[string]int{"/observations": http.StatusBadRequest}) + server := collectionTestServer(t, map[string]int{"/observations": http.StatusBadRequest}, nil) defer server.Close() cfg := config.Defaults() @@ -68,9 +77,12 @@ func TestRunWrapsFetchError(t *testing.T) { } } -func collectionTestServer(t *testing.T, statusByPath map[string]int) *httptest.Server { +func collectionTestServer(t *testing.T, statusByPath map[string]int, onRequest func(*http.Request)) *httptest.Server { t.Helper() return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if onRequest != nil { + onRequest(r) + } if status := statusByPath[r.URL.Path]; status != 0 { http.Error(w, "upstream failure", status) return