From f13f43cf56d51d7779e3e3f198b9f473ffe46630 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Thu, 15 Jan 2026 20:40:53 -0600 Subject: [PATCH] refactor(providers): centralize provider-specific parsing and invariants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Introduce internal/providers/nws with shared timestamp parsing used by both NWS sources and normalizers - Migrate NWS observation source + normalizer to use the shared provider helper for consistent RFC3339/RFC3339Nano handling - Introduce internal/providers/openweather with a shared URL invariant helper enforcing units=metric - Remove duplicated OpenWeather URL validation logic from the observation source - Align provider layering: move provider “contract/quirk” logic out of normalizers and into internal/providers - Update normalizer and standards documentation to clearly distinguish: provider helpers (internal/providers) vs canonical mapping logic (internal/normalizers) This refactor reduces duplication between sources and normalizers, clarifies layering boundaries, and establishes a scalable pattern for future forecast and alert implementations. --- internal/normalizers/doc.go | 4 +++ internal/normalizers/nws/observation.go | 5 ++-- internal/normalizers/openmeteo/common.go | 19 ------------- internal/normalizers/openmeteo/observation.go | 3 ++- internal/providers/nws/doc.go | 8 ++++++ internal/providers/nws/time.go | 27 +++++++++++++++++++ internal/providers/openweather/doc.go | 8 ++++++ internal/providers/openweather/url.go | 26 ++++++++++++++++++ internal/sources/nws/observation.go | 5 ++-- internal/sources/openweather/observation.go | 24 +++-------------- internal/standards/doc.go | 3 ++- 11 files changed, 86 insertions(+), 46 deletions(-) delete mode 100644 internal/normalizers/openmeteo/common.go create mode 100644 internal/providers/nws/doc.go create mode 100644 internal/providers/nws/time.go create mode 100644 internal/providers/openweather/doc.go create mode 100644 internal/providers/openweather/url.go diff --git a/internal/normalizers/doc.go b/internal/normalizers/doc.go index 0548d6d..84f9c9e 100644 --- a/internal/normalizers/doc.go +++ b/internal/normalizers/doc.go @@ -36,6 +36,10 @@ // 2. Provider-level shared helpers live under the provider directory: // internal/normalizers// // +// Use this for provider-specific quirks that should be shared by BOTH sources +// and normalizers (time parsing, URL/unit invariants, ID normalization, etc.). +// Keep these helpers pure (no I/O) and easy to test. +// // You may use multiple helper files (recommended) when it improves clarity: // - types.go (provider JSON structs) // - common.go (provider-shared helpers) diff --git a/internal/normalizers/nws/observation.go b/internal/normalizers/nws/observation.go index 36891c4..7a1ea16 100644 --- a/internal/normalizers/nws/observation.go +++ b/internal/normalizers/nws/observation.go @@ -10,6 +10,7 @@ import ( "gitea.maximumdirect.net/ejr/feedkit/event" "gitea.maximumdirect.net/ejr/weatherfeeder/internal/model" normcommon "gitea.maximumdirect.net/ejr/weatherfeeder/internal/normalizers/common" + nwscommon "gitea.maximumdirect.net/ejr/weatherfeeder/internal/providers/nws" "gitea.maximumdirect.net/ejr/weatherfeeder/internal/standards" ) @@ -46,11 +47,11 @@ func buildObservation(parsed nwsObservationResponse) (model.WeatherObservation, // Timestamp (RFC3339) var ts time.Time if s := strings.TrimSpace(parsed.Properties.Timestamp); s != "" { - t, err := time.Parse(time.RFC3339, s) + t, err := nwscommon.ParseTime(s) if err != nil { return model.WeatherObservation{}, time.Time{}, fmt.Errorf("invalid timestamp %q: %w", s, err) } - ts = t + ts = t.UTC() } cloudLayers := make([]model.CloudLayer, 0, len(parsed.Properties.CloudLayers)) diff --git a/internal/normalizers/openmeteo/common.go b/internal/normalizers/openmeteo/common.go deleted file mode 100644 index c2b630b..0000000 --- a/internal/normalizers/openmeteo/common.go +++ /dev/null @@ -1,19 +0,0 @@ -// FILE: ./internal/normalizers/openmeteo/common.go -package openmeteo - -import ( - "time" - - openmeteo "gitea.maximumdirect.net/ejr/weatherfeeder/internal/providers/openmeteo" -) - -// parseOpenMeteoTime parses Open-Meteo timestamps. -// -// The actual parsing logic lives in internal/providers/openmeteo so both the -// source (envelope EffectiveAt / event ID) and normalizer (canonical payload) -// can share identical timestamp behavior. -// -// We keep this thin wrapper to avoid churn in the normalizer package. -func parseOpenMeteoTime(s string, tz string, utcOffsetSeconds int) (time.Time, error) { - return openmeteo.ParseTime(s, tz, utcOffsetSeconds) -} diff --git a/internal/normalizers/openmeteo/observation.go b/internal/normalizers/openmeteo/observation.go index 2d2fca4..59483c0 100644 --- a/internal/normalizers/openmeteo/observation.go +++ b/internal/normalizers/openmeteo/observation.go @@ -10,6 +10,7 @@ import ( "gitea.maximumdirect.net/ejr/feedkit/event" "gitea.maximumdirect.net/ejr/weatherfeeder/internal/model" normcommon "gitea.maximumdirect.net/ejr/weatherfeeder/internal/normalizers/common" + omcommon "gitea.maximumdirect.net/ejr/weatherfeeder/internal/providers/openmeteo" "gitea.maximumdirect.net/ejr/weatherfeeder/internal/standards" ) @@ -54,7 +55,7 @@ func buildObservation(parsed omResponse) (model.WeatherObservation, time.Time, e // Parse current.time. var ts time.Time if s := strings.TrimSpace(parsed.Current.Time); s != "" { - t, err := parseOpenMeteoTime(s, parsed.Timezone, parsed.UTCOffsetSeconds) + t, err := omcommon.ParseTime(s, parsed.Timezone, parsed.UTCOffsetSeconds) if err != nil { return model.WeatherObservation{}, time.Time{}, fmt.Errorf("parse time %q: %w", s, err) } diff --git a/internal/providers/nws/doc.go b/internal/providers/nws/doc.go new file mode 100644 index 0000000..1324d06 --- /dev/null +++ b/internal/providers/nws/doc.go @@ -0,0 +1,8 @@ +// Package nws contains provider-specific helper code for the National Weather Service +// used by both sources and normalizers. +// +// Rules: +// - No network I/O here (sources fetch; normalizers transform). +// - Keep helpers deterministic and easy to unit test. +// - Prefer putting provider quirks/parsing here when sources + normalizers both need it. +package nws diff --git a/internal/providers/nws/time.go b/internal/providers/nws/time.go new file mode 100644 index 0000000..23c9b46 --- /dev/null +++ b/internal/providers/nws/time.go @@ -0,0 +1,27 @@ +package nws + +import ( + "fmt" + "strings" + "time" +) + +// ParseTime parses NWS timestamps. +// +// NWS observation timestamps are typically RFC3339, sometimes with fractional seconds. +// We accept RFC3339Nano first, then RFC3339. +func ParseTime(s string) (time.Time, error) { + s = strings.TrimSpace(s) + if s == "" { + return time.Time{}, fmt.Errorf("empty time") + } + + if t, err := time.Parse(time.RFC3339Nano, s); err == nil { + return t, nil + } + if t, err := time.Parse(time.RFC3339, s); err == nil { + return t, nil + } + + return time.Time{}, fmt.Errorf("unsupported NWS timestamp format: %q", s) +} diff --git a/internal/providers/openweather/doc.go b/internal/providers/openweather/doc.go new file mode 100644 index 0000000..f2f7a6b --- /dev/null +++ b/internal/providers/openweather/doc.go @@ -0,0 +1,8 @@ +// Package openweather contains provider-specific helper code for OpenWeather used by +// both sources and normalizers. +// +// Rules: +// - No network I/O here. +// - Keep helpers deterministic and easy to unit test. +// - Put provider invariants here (e.g., units=metric enforcement). +package openweather diff --git a/internal/providers/openweather/url.go b/internal/providers/openweather/url.go new file mode 100644 index 0000000..2078bd8 --- /dev/null +++ b/internal/providers/openweather/url.go @@ -0,0 +1,26 @@ +package openweather + +import ( + "fmt" + "net/url" + "strings" +) + +// RequireMetricUnits enforces weatherfeeder's OpenWeather invariant: +// the request URL must include units=metric (otherwise temperatures/winds/pressure differ). +func RequireMetricUnits(rawURL string) error { + u, err := url.Parse(strings.TrimSpace(rawURL)) + if err != nil { + return fmt.Errorf("invalid url %q: %w", rawURL, err) + } + + units := strings.ToLower(strings.TrimSpace(u.Query().Get("units"))) + if units != "metric" { + if units == "" { + units = "(missing; defaults to standard)" + } + return fmt.Errorf("url must include units=metric (got units=%s)", units) + } + + return nil +} diff --git a/internal/sources/nws/observation.go b/internal/sources/nws/observation.go index d9625ce..740421f 100644 --- a/internal/sources/nws/observation.go +++ b/internal/sources/nws/observation.go @@ -9,6 +9,7 @@ import ( "gitea.maximumdirect.net/ejr/feedkit/config" "gitea.maximumdirect.net/ejr/feedkit/event" + nwscommon "gitea.maximumdirect.net/ejr/weatherfeeder/internal/providers/nws" "gitea.maximumdirect.net/ejr/weatherfeeder/internal/sources/common" "gitea.maximumdirect.net/ejr/weatherfeeder/internal/standards" ) @@ -85,8 +86,8 @@ func (s *ObservationSource) fetchRaw(ctx context.Context) (json.RawMessage, obse tsStr := strings.TrimSpace(meta.Properties.Timestamp) if tsStr != "" { - if t, err := time.Parse(time.RFC3339, tsStr); err == nil { - meta.ParsedTimestamp = t + if t, err := nwscommon.ParseTime(tsStr); err == nil { + meta.ParsedTimestamp = t.UTC() } } diff --git a/internal/sources/openweather/observation.go b/internal/sources/openweather/observation.go index 6ec3b96..8de2c03 100644 --- a/internal/sources/openweather/observation.go +++ b/internal/sources/openweather/observation.go @@ -5,12 +5,11 @@ import ( "context" "encoding/json" "fmt" - "net/url" - "strings" "time" "gitea.maximumdirect.net/ejr/feedkit/config" "gitea.maximumdirect.net/ejr/feedkit/event" + owcommon "gitea.maximumdirect.net/ejr/weatherfeeder/internal/providers/openweather" "gitea.maximumdirect.net/ejr/weatherfeeder/internal/sources/common" "gitea.maximumdirect.net/ejr/weatherfeeder/internal/standards" ) @@ -27,7 +26,7 @@ func NewObservationSource(cfg config.SourceConfig) (*ObservationSource, error) { return nil, err } - if err := requireMetricUnits(hs.URL); err != nil { + if err := owcommon.RequireMetricUnits(hs.URL); err != nil { return nil, fmt.Errorf("%s %q: %w", hs.Driver, hs.Name, err) } @@ -39,7 +38,7 @@ func (s *ObservationSource) Name() string { return s.http.Name } func (s *ObservationSource) Kind() event.Kind { return event.Kind("observation") } func (s *ObservationSource) Poll(ctx context.Context) ([]event.Event, error) { - if err := requireMetricUnits(s.http.URL); err != nil { + if err := owcommon.RequireMetricUnits(s.http.URL); err != nil { return nil, fmt.Errorf("%s %q: %w", s.http.Driver, s.http.Name, err) } @@ -93,20 +92,3 @@ func (s *ObservationSource) fetchRaw(ctx context.Context) (json.RawMessage, open return raw, meta, nil } - -func requireMetricUnits(rawURL string) error { - u, err := url.Parse(strings.TrimSpace(rawURL)) - if err != nil { - return fmt.Errorf("invalid url %q: %w", rawURL, err) - } - - units := strings.ToLower(strings.TrimSpace(u.Query().Get("units"))) - if units != "metric" { - if units == "" { - units = "(missing; defaults to standard)" - } - return fmt.Errorf("url must include units=metric (got units=%s)", units) - } - - return nil -} diff --git a/internal/standards/doc.go b/internal/standards/doc.go index c323f61..1764688 100644 --- a/internal/standards/doc.go +++ b/internal/standards/doc.go @@ -10,5 +10,6 @@ // provider-specific logic and free of dependencies on internal/sources/* or // internal/normalizers/* to avoid import cycles. // -// Provider-specific decoding and mapping lives in internal/normalizers/. +// Provider-specific decoding helpers and quirks live in internal/providers/. +// Normalizer implementations and canonical mapping logic live in internal/normalizers/. package standards