fix(notifications)!: take each provider's config as a typed struct

notifications/README.md

@@ -10,36 +10,30 @@ A multi-channel notification router with a pluggable provider model. Your applic
 
 ## Usage
 
-```go
-import notification "github.com/OpenNSW/core/notifications"
+Each provider takes its own typed config in its constructor and is handed, ready to send, to `NewManager`:
 
-manager, err := notification.NewManager(
-    notification.Config{
-        Providers: map[notification.ChannelType]map[string]any{
-            notification.ChannelEmail: {
-                "api_key":      "sg-xxxxx",
-                "from_address": "noreply@example.com",
-            },
-            notification.ChannelSMS: {
-                "account_sid": "ACxxxxx",
-                "auth_token":  "xxxxx",
-                "from_number": "+61400000000",
-            },
-        },
-    },
-    myEmailProvider,
-    mySMSProvider,
+```go
+import (
+    notification "github.com/OpenNSW/core/notifications"
+    "github.com/OpenNSW/core/notifications/providers"
 )
 
+sms, err := providers.NewSMSProvider(cfg.Notification.SMS)
+email, err := providers.NewEmailProvider(cfg.Notification.Email)
+
+manager, err := notification.NewManager(sms, email)
+
 err = manager.Send(ctx, notification.Request{
-    Channel: notification.ChannelEmail,
-    To:      "applicant@example.com",
-    Subject: "Application received",
-    Body:    "Your application #12345 has been received and is under review.",
+    Channel:  notification.ChannelEmail,
+    To:       "applicant@example.com",
+    Subject:  "Application received",
+    Body:     "Your application #12345 has been received and is under review.",
     HTMLBody: "<p>Your application <strong>#12345</strong> has been received.</p>",
 })
 ```
 
+`NewManager` returns an error when it gets no providers, a nil one, or two for the same channel.
+
 ## Channels
 
 | Constant                    | Value     |
@@ -53,48 +47,52 @@ Implement `notification.Provider`:
 
 ```go
 type Provider interface {
-    Type()                          ChannelType
-    Configure(cfg json.RawMessage)  error
+    Type() ChannelType
     Send(ctx context.Context, req Request) error
 }
 ```
 
 - `Type()` declares which channel this provider handles.
-- `Configure` is called at startup with the provider's block from `Config.Providers`, re-marshaled to JSON (so existing `Provider` implementations are unaffected by how the block was sourced).
 - `Send` delivers the message.
 
-```go
-type MyEmailProvider struct {
-    apiKey string
-}
+Give the provider its own exported config type, with `yaml` tags, and a constructor that validates it and returns a ready provider, as `providers.NewSMSProvider` and `providers.NewEmailProvider` do:
 
-func (p *MyEmailProvider) Type() notification.ChannelType { return notification.ChannelEmail }
-
-func (p *MyEmailProvider) Configure(cfg json.RawMessage) error {
-    var c struct{ APIKey string `json:"api_key"` }
-    if err := json.Unmarshal(cfg, &c); err != nil { return err }
-    p.apiKey = c.APIKey
-    return nil
+```go
+type MyEmailConfig struct {
+    APIKey string `yaml:"apiKey"`
 }
 
-func (p *MyEmailProvider) Send(ctx context.Context, req notification.Request) error {
-    // send via your email API
-    return nil
+func NewMyEmailProvider(cfg MyEmailConfig) (*MyEmailProvider, error) {
+    if cfg.APIKey == "" {
+        return nil, errors.New("apiKey is required")
+    }
+    return &MyEmailProvider{apiKey: cfg.APIKey}, nil
 }
 ```
 
 ## Provider configuration
 
-`Config.Providers` holds provider-specific configuration keyed by channel type — no standalone config file is needed. It carries a `yaml` struct tag (`providers`), so it can be embedded in a larger application config struct and populated generically, e.g. via [`configyaml.LoadAndExpand`](../configyaml/README.md) so a provider's API key can be sourced from an env var or a mounted file instead of living in the checked-in config:
+An application embeds the provider configs in its own config struct and loads it with [`configyaml.LoadAndExpand`](../configyaml/README.md), so a provider's credentials can come from an env var or a mounted file instead of the checked-in config:
+
+```go
+type AppConfig struct {
+    Notification struct {
+        SMS   providers.SMSConfig   `yaml:"sms"`
+        Email providers.EmailConfig `yaml:"email"`
+    } `yaml:"notification"`
+}
+```
 
 ```yaml
 notification:
-  providers:
-    email:
-      api_key: "{{env:SENDGRID_API_KEY}}"
-      from_address: noreply@example.com
-    sms:
-      account_sid: ACxxxxx
-      auth_token: "{{env:SMS_AUTH_TOKEN}}"
-      from_number: "+61400000000"
+  sms:
+    baseURL: https://sms.example.com
+    userName: nsw
+    password: "{{env:SMS_PASSWORD}}"
+    sidCode: NSW
+  email:
+    baseURL: https://email.example.com
+    token: "{{env:EMAIL_TOKEN}}"
 ```
+
+The config fields are typed, so a value decodes into its field's type: a secret that only looks like a number (a password of `12345678`, a SID code of `0123`) stays the string it was. The email `token` is the bearer token itself; configyaml resolves the placeholder, and the provider uses it as written.

notifications/go.mod

@@ -4,5 +4,7 @@ go 1.26
 
 require (
 	github.com/OpenNSW/core/remote v0.8.0
-	github.com/OpenNSW/core/secret v0.2.0
+	gopkg.in/yaml.v3 v3.0.1
 )
+
+require github.com/OpenNSW/core/secret v0.2.0 // indirect

notifications/go.sum

@@ -8,5 +8,7 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
 github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

notifications/manager.go

@@ -5,7 +5,6 @@ package notification
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"log/slog"
@@ -17,13 +16,16 @@ type Manager struct {
 	providers map[ChannelType]Provider
 }
 
-// NewManager configures each provider from its block in cfg.Providers and
-// returns a ready Manager. Returns an error if cfg is invalid, a provider's
-// channel is missing from cfg.Providers, or any provider's Configure call
-// fails.
-func NewManager(cfg Config, providers ...Provider) (*Manager, error) {
-	if err := cfg.Validate(); err != nil {
-		return nil, fmt.Errorf("invalid notification config: %w", err)
+// ErrNoProviders is returned by NewManager when it is given no providers.
+var ErrNoProviders = errors.New("notification: at least one provider is required")
+
+// NewManager returns a Manager that routes each request to the provider for
+// its channel. The providers are already configured by their constructors.
+// Returns an error if none is given, one is nil, or two handle the same
+// channel.
+func NewManager(providers ...Provider) (*Manager, error) {
+	if len(providers) == 0 {
+		return nil, ErrNoProviders
 	}
 
 	m := &Manager{
@@ -34,20 +36,6 @@ func NewManager(cfg Config, providers ...Provider) (*Manager, error) {
 		if p == nil {
 			return nil, errors.New("nil provider passed to NewManager")
 		}
-		block, ok := cfg.Providers[p.Type()]
-		if !ok {
-			return nil, fmt.Errorf("no config for %q provider", p.Type())
-		}
-		// Providers still configure from json.RawMessage (unchanged interface,
-		// so every existing Provider keeps working); the block just arrives
-		// from cfg.Providers now instead of a standalone JSON file.
-		raw, err := json.Marshal(block)
-		if err != nil {
-			return nil, fmt.Errorf("marshal %q provider config: %w", p.Type(), err)
-		}
-		if err := p.Configure(raw); err != nil {
-			return nil, fmt.Errorf("configure %q provider: %w", p.Type(), err)
-		}
 		if _, dup := m.providers[p.Type()]; dup {
 			return nil, fmt.Errorf("duplicate notification provider for channel %q", p.Type())
 		}

notifications/manager_test.go

@@ -5,27 +5,19 @@ package notification
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
 	"testing"
 )
 
 // stubProvider is a test double for Provider.
 type stubProvider struct {
-	channelType     ChannelType
-	configureErr    error
-	sendErr         error
-	configureCalled bool
-	sendCalled      bool
+	channelType ChannelType
+	sendErr     error
+	sendCalled  bool
 }
 
 func (s *stubProvider) Type() ChannelType { return s.channelType }
 
-func (s *stubProvider) Configure(_ json.RawMessage) error {
-	s.configureCalled = true
-	return s.configureErr
-}
-
 func (s *stubProvider) Send(_ context.Context, _ Request) error {
 	s.sendCalled = true
 	return s.sendErr
@@ -34,70 +26,39 @@ func (s *stubProvider) Send(_ context.Context, _ Request) error {
 func TestNewManager(t *testing.T) {
 	t.Parallel()
 
-	validReq := func(ch ChannelType) Request {
-		return Request{Channel: ch, To: "a@b.com", Body: "hi"}
-	}
-
-	t.Run("happy path — single provider", func(t *testing.T) {
+	t.Run("routes to the provider for the channel", func(t *testing.T) {
 		t.Parallel()
-		cfg := Config{Providers: map[ChannelType]map[string]any{
-			ChannelEmail: {"host": "localhost"},
-		}}
 		p := &stubProvider{channelType: ChannelEmail}
-		m, err := NewManager(cfg, p)
+		m, err := NewManager(p)
 		if err != nil {
-			t.Fatalf("unexpected error: %v", err)
-		}
-		if !p.configureCalled {
-			t.Error("Configure was not called")
+			t.Fatalf("NewManager: %v", err)
 		}
-		if err := m.Send(context.Background(), validReq(ChannelEmail)); err != nil {
+		if err := m.Send(context.Background(), Request{Channel: ChannelEmail, To: "a@b.com", Body: "hi"}); err != nil {
 			t.Errorf("Send: %v", err)
 		}
 		if !p.sendCalled {
 			t.Error("Send was not called on provider")
 		}
 	})
 
-	t.Run("missing config key for provider", func(t *testing.T) {
-		t.Parallel()
-		cfg := Config{Providers: map[ChannelType]map[string]any{
-			ChannelSMS: {"host": "localhost"},
-		}}
-		p := &stubProvider{channelType: ChannelEmail}
-		_, err := NewManager(cfg, p)
-		if err == nil {
-			t.Fatal("expected error, got nil")
-		}
-	})
-
-	t.Run("Configure failure propagates", func(t *testing.T) {
+	t.Run("no providers", func(t *testing.T) {
 		t.Parallel()
-		cfg := Config{Providers: map[ChannelType]map[string]any{ChannelEmail: {}}}
-		configErr := errors.New("bad config")
-		p := &stubProvider{channelType: ChannelEmail, configureErr: configErr}
-		_, err := NewManager(cfg, p)
-		if !errors.Is(err, configErr) {
-			t.Errorf("got %v, want wrapping %v", err, configErr)
+		if _, err := NewManager(); !errors.Is(err, ErrNoProviders) {
+			t.Errorf("got %v, want ErrNoProviders", err)
 		}
 	})
 
-	t.Run("invalid Config: no providers", func(t *testing.T) {
+	t.Run("nil provider", func(t *testing.T) {
 		t.Parallel()
-		_, err := NewManager(Config{})
-		if !errors.Is(err, ErrProvidersRequired) {
-			t.Errorf("got %v, want ErrProvidersRequired", err)
+		if _, err := NewManager(nil); err == nil {
+			t.Fatal("expected an error for a nil provider, got nil")
 		}
 	})
 
-	t.Run("duplicate channel type returns error", func(t *testing.T) {
+	t.Run("two providers for one channel", func(t *testing.T) {
 		t.Parallel()
-		cfg := Config{Providers: map[ChannelType]map[string]any{ChannelEmail: {"a": 1}}}
-		p1 := &stubProvider{channelType: ChannelEmail}
-		p2 := &stubProvider{channelType: ChannelEmail}
-		_, err := NewManager(cfg, p1, p2)
-		if err == nil {
-			t.Fatal("expected error for duplicate channel type, got nil")
+		if _, err := NewManager(&stubProvider{channelType: ChannelEmail}, &stubProvider{channelType: ChannelEmail}); err == nil {
+			t.Fatal("expected an error for a duplicate channel, got nil")
 		}
 	})
 }
@@ -107,8 +68,7 @@ func TestManager_Send(t *testing.T) {
 
 	makeManager := func(t *testing.T, p *stubProvider) *Manager {
 		t.Helper()
-		cfg := Config{Providers: map[ChannelType]map[string]any{p.channelType: {}}}
-		m, err := NewManager(cfg, p)
+		m, err := NewManager(p)
 		if err != nil {
 			t.Fatalf("NewManager: %v", err)
 		}