From 86ce4eb68cd0a4db85993b9bcf248cf3d2908a79 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Thu, 11 Jun 2026 02:13:38 +0000 Subject: [PATCH] Share HTTP config parsing for multi-document sources --- .../sources/internal/httpconfig/config.go | 58 +++++++++ .../internal/httpconfig/config_test.go | 115 ++++++++++++++++++ internal/sources/spc/convective_outlook.go | 49 ++------ 3 files changed, 185 insertions(+), 37 deletions(-) create mode 100644 internal/sources/internal/httpconfig/config.go create mode 100644 internal/sources/internal/httpconfig/config_test.go diff --git a/internal/sources/internal/httpconfig/config.go b/internal/sources/internal/httpconfig/config.go new file mode 100644 index 0000000..0a0647c --- /dev/null +++ b/internal/sources/internal/httpconfig/config.go @@ -0,0 +1,58 @@ +package httpconfig + +import ( + "fmt" + "strings" + "time" + + "gitea.maximumdirect.net/ejr/feedkit/config" + "gitea.maximumdirect.net/ejr/feedkit/transport" +) + +// Settings contains common HTTP client config for sources that fetch multiple documents. +type Settings struct { + Name string + UserAgent string + Timeout time.Duration + BodyLimitBytes int64 +} + +func Parse(driver string, cfg config.SourceConfig) (Settings, error) { + name := strings.TrimSpace(cfg.Name) + if name == "" { + return Settings{}, fmt.Errorf("%s: name is required", driver) + } + if cfg.Params == nil { + return Settings{}, fmt.Errorf("%s %q: params are required", driver, name) + } + + userAgent, ok := cfg.ParamString("user_agent", "userAgent") + if !ok { + return Settings{}, fmt.Errorf("%s %q: params.user_agent is required", driver, name) + } + + timeout := transport.DefaultHTTPTimeout + if _, exists := cfg.Params["http_timeout"]; exists { + var ok bool + timeout, ok = cfg.ParamDuration("http_timeout") + if !ok || timeout <= 0 { + return Settings{}, fmt.Errorf("source %q: params.http_timeout must be a positive duration", name) + } + } + + bodyLimit := transport.DefaultHTTPResponseBodyLimitBytes + if _, exists := cfg.Params["http_response_body_limit_bytes"]; exists { + rawLimit, ok := cfg.ParamInt("http_response_body_limit_bytes") + if !ok || rawLimit <= 0 { + return Settings{}, fmt.Errorf("source %q: params.http_response_body_limit_bytes must be a positive integer", name) + } + bodyLimit = int64(rawLimit) + } + + return Settings{ + Name: name, + UserAgent: userAgent, + Timeout: timeout, + BodyLimitBytes: bodyLimit, + }, nil +} diff --git a/internal/sources/internal/httpconfig/config_test.go b/internal/sources/internal/httpconfig/config_test.go new file mode 100644 index 0000000..53f60b1 --- /dev/null +++ b/internal/sources/internal/httpconfig/config_test.go @@ -0,0 +1,115 @@ +package httpconfig + +import ( + "strings" + "testing" + "time" + + "gitea.maximumdirect.net/ejr/feedkit/config" + "gitea.maximumdirect.net/ejr/feedkit/transport" +) + +func TestParseUsesRequiredValuesAndDefaults(t *testing.T) { + got, err := Parse("test_driver", config.SourceConfig{ + Name: " test-source ", + Params: map[string]any{ + "user_agent": "test-agent", + }, + }) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + if got.Name != "test-source" { + t.Fatalf("Name = %q, want test-source", got.Name) + } + if got.UserAgent != "test-agent" { + t.Fatalf("UserAgent = %q, want test-agent", got.UserAgent) + } + if got.Timeout != transport.DefaultHTTPTimeout { + t.Fatalf("Timeout = %s, want %s", got.Timeout, transport.DefaultHTTPTimeout) + } + if got.BodyLimitBytes != transport.DefaultHTTPResponseBodyLimitBytes { + t.Fatalf("BodyLimitBytes = %d, want %d", got.BodyLimitBytes, transport.DefaultHTTPResponseBodyLimitBytes) + } +} + +func TestParseUsesAliasesAndOverrides(t *testing.T) { + got, err := Parse("test_driver", config.SourceConfig{ + Name: "test-source", + Params: map[string]any{ + "userAgent": "test-agent", + "http_timeout": "2s", + "http_response_body_limit_bytes": 2048, + }, + }) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + if got.UserAgent != "test-agent" { + t.Fatalf("UserAgent = %q, want test-agent", got.UserAgent) + } + if got.Timeout != 2*time.Second { + t.Fatalf("Timeout = %s, want 2s", got.Timeout) + } + if got.BodyLimitBytes != 2048 { + t.Fatalf("BodyLimitBytes = %d, want 2048", got.BodyLimitBytes) + } +} + +func TestParseRejectsInvalidConfig(t *testing.T) { + tests := []struct { + name string + cfg config.SourceConfig + wantErr string + }{ + { + name: "missing name", + cfg: config.SourceConfig{Params: map[string]any{"user_agent": "test-agent"}}, + wantErr: "test_driver: name is required", + }, + { + name: "missing params", + cfg: config.SourceConfig{Name: "test-source"}, + wantErr: `test_driver "test-source": params are required`, + }, + { + name: "missing user agent", + cfg: config.SourceConfig{Name: "test-source", Params: map[string]any{}}, + wantErr: `test_driver "test-source": params.user_agent is required`, + }, + { + name: "invalid timeout", + cfg: config.SourceConfig{ + Name: "test-source", + Params: map[string]any{ + "user_agent": "test-agent", + "http_timeout": "0s", + }, + }, + wantErr: `source "test-source": params.http_timeout must be a positive duration`, + }, + { + name: "invalid body limit", + cfg: config.SourceConfig{ + Name: "test-source", + Params: map[string]any{ + "user_agent": "test-agent", + "http_response_body_limit_bytes": 0, + }, + }, + wantErr: `source "test-source": params.http_response_body_limit_bytes must be a positive integer`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := Parse("test_driver", tt.cfg) + if err == nil { + t.Fatalf("Parse() error = nil, want %q", tt.wantErr) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("Parse() error = %q, want %q", err, tt.wantErr) + } + }) + } +} diff --git a/internal/sources/spc/convective_outlook.go b/internal/sources/spc/convective_outlook.go index 492e622..f78881f 100644 --- a/internal/sources/spc/convective_outlook.go +++ b/internal/sources/spc/convective_outlook.go @@ -16,6 +16,7 @@ import ( fksources "gitea.maximumdirect.net/ejr/feedkit/sources" "gitea.maximumdirect.net/ejr/feedkit/transport" spcprovider "gitea.maximumdirect.net/ejr/weatherfeeder/internal/providers/spc" + "gitea.maximumdirect.net/ejr/weatherfeeder/internal/sources/internal/httpconfig" "gitea.maximumdirect.net/ejr/weatherfeeder/standards" ) @@ -54,53 +55,27 @@ type ConvectiveOutlookSource struct { } func NewConvectiveOutlookSource(cfg config.SourceConfig) (*ConvectiveOutlookSource, error) { - name := strings.TrimSpace(cfg.Name) - if name == "" { - return nil, fmt.Errorf("%s: name is required", DriverConvectiveOutlook) - } - if cfg.Params == nil { - return nil, fmt.Errorf("%s %q: params are required", DriverConvectiveOutlook, name) - } - - userAgent, ok := cfg.ParamString("user_agent", "userAgent") - if !ok { - return nil, fmt.Errorf("%s %q: params.user_agent is required", DriverConvectiveOutlook, name) + httpSettings, err := httpconfig.Parse(DriverConvectiveOutlook, cfg) + if err != nil { + return nil, err } latitude, err := requireFloatParam(cfg, "latitude") if err != nil { - return nil, fmt.Errorf("%s %q: %w", DriverConvectiveOutlook, name, err) + return nil, fmt.Errorf("%s %q: %w", DriverConvectiveOutlook, httpSettings.Name, err) } longitude, err := requireFloatParam(cfg, "longitude") if err != nil { - return nil, fmt.Errorf("%s %q: %w", DriverConvectiveOutlook, name, err) - } - - timeout := transport.DefaultHTTPTimeout - if _, exists := cfg.Params["http_timeout"]; exists { - var ok bool - timeout, ok = cfg.ParamDuration("http_timeout") - if !ok || timeout <= 0 { - return nil, fmt.Errorf("source %q: params.http_timeout must be a positive duration", name) - } - } - - bodyLimit := transport.DefaultHTTPResponseBodyLimitBytes - if _, exists := cfg.Params["http_response_body_limit_bytes"]; exists { - rawLimit, ok := cfg.ParamInt("http_response_body_limit_bytes") - if !ok || rawLimit <= 0 { - return nil, fmt.Errorf("source %q: params.http_response_body_limit_bytes must be a positive integer", name) - } - bodyLimit = int64(rawLimit) + return nil, fmt.Errorf("%s %q: %w", DriverConvectiveOutlook, httpSettings.Name, err) } geoJSONProducts, err := configuredGeoJSONProducts(cfg) if err != nil { - return nil, fmt.Errorf("%s %q: %w", DriverConvectiveOutlook, name, err) + return nil, fmt.Errorf("%s %q: %w", DriverConvectiveOutlook, httpSettings.Name, err) } discussions, err := configuredDiscussionProducts(cfg) if err != nil { - return nil, fmt.Errorf("%s %q: %w", DriverConvectiveOutlook, name, err) + return nil, fmt.Errorf("%s %q: %w", DriverConvectiveOutlook, httpSettings.Name, err) } rssURL := "" @@ -112,14 +87,14 @@ func NewConvectiveOutlookSource(cfg config.SourceConfig) (*ConvectiveOutlookSour locationName, _ := cfg.ParamString("location_name", "locationName") return &ConvectiveOutlookSource{ - name: name, - userAgent: userAgent, + name: httpSettings.Name, + userAgent: httpSettings.UserAgent, locationID: locationID, locationName: locationName, latitude: latitude, longitude: longitude, - client: transport.NewHTTPClient(timeout), - bodyLimit: bodyLimit, + client: transport.NewHTTPClient(httpSettings.Timeout), + bodyLimit: httpSettings.BodyLimitBytes, geoJSONProducts: geoJSONProducts, discussions: discussions, rssURL: rssURL,