Cleaned up documentation and removed stubs and TODOs throughout the application

sinks/builtins.go

@@ -1,11 +1,6 @@
 package sinks
 
-import (
-	"fmt"
-	"strings"
-
-	"gitea.maximumdirect.net/ejr/feedkit/config"
-)
+import "gitea.maximumdirect.net/ejr/feedkit/config"
 
 // RegisterBuiltins registers sink drivers included in this binary.
 //
@@ -17,11 +12,6 @@ func RegisterBuiltins(r *Registry) {
 		return NewStdoutSink(cfg.Name), nil
 	})
 
-	// File sink: writes/archives events somewhere on disk.
-	r.Register("file", func(cfg config.SinkConfig) (Sink, error) {
-		return NewFileSinkFromConfig(cfg)
-	})
-
 	// Postgres sink: persists events durably.
 	r.Register("postgres", func(cfg config.SinkConfig) (Sink, error) {
 		return NewPostgresSinkFromConfig(cfg)
@@ -32,24 +22,3 @@ func RegisterBuiltins(r *Registry) {
 		return NewNATSSinkFromConfig(cfg)
 	})
 }
-
-// ---- helpers for validating sink params ----
-//
-// These helpers live in sinks (not config) on purpose:
-// - config is domain-agnostic and should not embed driver-specific validation helpers.
-// - sinks are adapters; validating their own params here keeps the logic near the driver.
-
-func requireStringParam(cfg config.SinkConfig, key string) (string, error) {
-	v, ok := cfg.Params[key]
-	if !ok {
-		return "", fmt.Errorf("sink %q: params.%s is required", cfg.Name, key)
-	}
-	s, ok := v.(string)
-	if !ok {
-		return "", fmt.Errorf("sink %q: params.%s must be a string", cfg.Name, key)
-	}
-	if strings.TrimSpace(s) == "" {
-		return "", fmt.Errorf("sink %q: params.%s cannot be empty", cfg.Name, key)
-	}
-	return s, nil
-}

sinks/doc.go

@@ -1,18 +1,27 @@
-// Package sinks provides sink abstractions, a sink driver registry, and several
-// built-in sink drivers.
+// Package sinks defines the feedkit sink interface, sink driver registry, and
+// built-in infrastructure sinks.
+//
+// External API surface:
+//   - Sink: adapter interface that consumes event.Event values
+//   - Registry / NewRegistry: named sink factory registry
+//   - RegisterBuiltins: registers the built-in sink drivers in this binary
 //
 // Built-in drivers:
 //   - stdout
 //   - nats
 //   - postgres
 //
+// Optional helpers from helpers.go:
+//   - RegisterPostgresSchemaForConfiguredSinks: registers one Postgres schema
+//     for each configured sink using driver=postgres
+//
 // # NATS built-in overview
 //
 // The NATS sink publishes each event as JSON to a configured subject.
 //
 // Required params:
 //   - url: NATS server URL (for example, nats://localhost:4222)
-//   - exchange: NATS subject to publish to
+//   - subject: NATS subject to publish to
 //
 // Example config:
 //
@@ -21,7 +30,7 @@
 //	    driver: nats
 //	    params:
 //	      url: nats://localhost:4222
-//	      exchange: feedkit.events
+//	      subject: feedkit.events
 //
 // # Postgres built-in overview
 //

sinks/file.go

@@ -1,30 +0,0 @@
-package sinks
-
-import (
-	"context"
-	"fmt"
-
-	"gitea.maximumdirect.net/ejr/feedkit/config"
-	"gitea.maximumdirect.net/ejr/feedkit/event"
-)
-
-type FileSink struct {
-	name string
-	path string
-}
-
-func NewFileSinkFromConfig(cfg config.SinkConfig) (Sink, error) {
-	path, err := requireStringParam(cfg, "path")
-	if err != nil {
-		return nil, err
-	}
-	return &FileSink{name: cfg.Name, path: path}, nil
-}
-
-func (s *FileSink) Name() string { return s.name }
-
-func (s *FileSink) Consume(ctx context.Context, e event.Event) error {
-	_ = ctx
-	_ = e
-	return fmt.Errorf("file sink: TODO implement (path=%s)", s.path)
-}

sinks/helpers.go

@@ -7,6 +7,25 @@ import (
 	"gitea.maximumdirect.net/ejr/feedkit/config"
 )
 
+// requireStringParam returns a non-empty string sink param.
+//
+// This helper is intentionally local to sinks rather than config so
+// driver-specific validation stays close to the adapters that use it.
+func requireStringParam(cfg config.SinkConfig, key string) (string, error) {
+	v, ok := cfg.Params[key]
+	if !ok {
+		return "", fmt.Errorf("sink %q: params.%s is required", cfg.Name, key)
+	}
+	s, ok := v.(string)
+	if !ok {
+		return "", fmt.Errorf("sink %q: params.%s must be a string", cfg.Name, key)
+	}
+	if strings.TrimSpace(s) == "" {
+		return "", fmt.Errorf("sink %q: params.%s cannot be empty", cfg.Name, key)
+	}
+	return s, nil
+}
+
 // RegisterPostgresSchemaForConfiguredSinks registers one Postgres schema for each
 // configured sink using driver=postgres.
 func RegisterPostgresSchemaForConfiguredSinks(cfg *config.Config, schema PostgresSchema) error {

sinks/nats.go

@@ -13,9 +13,9 @@ import (
 )
 
 type NATSSink struct {
-	name     string
-	url      string
-	exchange string
+	name    string
+	url     string
+	subject string
 
 	mu   sync.Mutex
 	conn *nats.Conn
@@ -26,11 +26,11 @@ func NewNATSSinkFromConfig(cfg config.SinkConfig) (Sink, error) {
 	if err != nil {
 		return nil, err
 	}
-	ex, err := requireStringParam(cfg, "exchange")
+	subject, err := requireStringParam(cfg, "subject")
 	if err != nil {
 		return nil, err
 	}
-	return &NATSSink{name: cfg.Name, url: url, exchange: ex}, nil
+	return &NATSSink{name: cfg.Name, url: url, subject: subject}, nil
 }
 
 func (r *NATSSink) Name() string { return r.name }
@@ -59,7 +59,7 @@ func (r *NATSSink) Consume(ctx context.Context, e event.Event) error {
 	if err := ctx.Err(); err != nil {
 		return err
 	}
-	if err := conn.Publish(r.exchange, b); err != nil {
+	if err := conn.Publish(r.subject, b); err != nil {
 		return fmt.Errorf("NATS sink: publish: %w", err)
 	}
 	return nil

sinks/nats_test.go

@@ -0,0 +1,47 @@
+package sinks
+
+import (
+	"strings"
+	"testing"
+
+	"gitea.maximumdirect.net/ejr/feedkit/config"
+)
+
+func TestNewNATSSinkFromConfigRequiresSubject(t *testing.T) {
+	sink, err := NewNATSSinkFromConfig(config.SinkConfig{
+		Name:   "nats-main",
+		Driver: "nats",
+		Params: map[string]any{
+			"url":     "nats://localhost:4222",
+			"subject": "feedkit.events",
+		},
+	})
+	if err != nil {
+		t.Fatalf("NewNATSSinkFromConfig() error = %v", err)
+	}
+
+	natsSink, ok := sink.(*NATSSink)
+	if !ok {
+		t.Fatalf("sink type = %T, want *NATSSink", sink)
+	}
+	if natsSink.subject != "feedkit.events" {
+		t.Fatalf("subject = %q, want feedkit.events", natsSink.subject)
+	}
+}
+
+func TestNewNATSSinkFromConfigRejectsLegacyExchange(t *testing.T) {
+	_, err := NewNATSSinkFromConfig(config.SinkConfig{
+		Name:   "nats-main",
+		Driver: "nats",
+		Params: map[string]any{
+			"url":      "nats://localhost:4222",
+			"exchange": "feedkit.events",
+		},
+	})
+	if err == nil {
+		t.Fatalf("NewNATSSinkFromConfig() expected error")
+	}
+	if !strings.Contains(err.Error(), "params.subject is required") {
+		t.Fatalf("error = %q, want params.subject is required", err)
+	}
+}

sinks/registry.go

@@ -2,6 +2,7 @@ package sinks
 
 import (
 	"fmt"
+	"strings"
 
 	"gitea.maximumdirect.net/ejr/feedkit/config"
 )
@@ -21,13 +22,40 @@ func NewRegistry() *Registry {
 }
 
 func (r *Registry) Register(driver string, f Factory) {
+	if r == nil {
+		panic("sinks.Registry.Register: registry cannot be nil")
+	}
+	driver = strings.TrimSpace(driver)
+	if driver == "" {
+		panic("sinks.Registry.Register: driver cannot be empty")
+	}
+	if f == nil {
+		panic(fmt.Sprintf("sinks.Registry.Register: factory cannot be nil (driver=%q)", driver))
+	}
+	if r.byDriver == nil {
+		r.byDriver = map[string]Factory{}
+	}
+	if _, exists := r.byDriver[driver]; exists {
+		panic(fmt.Sprintf("sinks.Registry.Register: driver %q already registered", driver))
+	}
 	r.byDriver[driver] = f
 }
 
 func (r *Registry) Build(cfg config.SinkConfig) (Sink, error) {
-	f, ok := r.byDriver[cfg.Driver]
-	if !ok {
-		return nil, fmt.Errorf("unknown sink driver: %q", cfg.Driver)
+	if r == nil {
+		return nil, fmt.Errorf("sinks registry is nil")
 	}
-	return f(cfg)
+	driver := strings.TrimSpace(cfg.Driver)
+	f, ok := r.byDriver[driver]
+	if !ok {
+		return nil, fmt.Errorf("unknown sink driver: %q", driver)
+	}
+	sink, err := f(cfg)
+	if err != nil {
+		return nil, fmt.Errorf("build sink %q: %w", driver, err)
+	}
+	if sink == nil {
+		return nil, fmt.Errorf("build sink %q: factory returned nil sink", driver)
+	}
+	return sink, nil
 }

sinks/registry_test.go

@@ -0,0 +1,126 @@
+package sinks
+
+import (
+	"context"
+	"errors"
+	"strings"
+	"testing"
+
+	"gitea.maximumdirect.net/ejr/feedkit/config"
+	"gitea.maximumdirect.net/ejr/feedkit/event"
+)
+
+type testSink struct{ name string }
+
+func (s testSink) Name() string { return s.name }
+
+func (s testSink) Consume(context.Context, event.Event) error { return nil }
+
+func TestRegistryRegisterPanicsOnNilRegistry(t *testing.T) {
+	var r *Registry
+	defer func() {
+		if recover() == nil {
+			t.Fatalf("Register() expected panic on nil registry")
+		}
+	}()
+	r.Register("stdout", func(config.SinkConfig) (Sink, error) { return testSink{name: "stdout"}, nil })
+}
+
+func TestRegistryRegisterPanicsOnEmptyDriver(t *testing.T) {
+	r := NewRegistry()
+	defer func() {
+		if recover() == nil {
+			t.Fatalf("Register() expected panic on empty driver")
+		}
+	}()
+	r.Register("   ", func(config.SinkConfig) (Sink, error) { return testSink{name: "x"}, nil })
+}
+
+func TestRegistryRegisterPanicsOnNilFactory(t *testing.T) {
+	r := NewRegistry()
+	defer func() {
+		if recover() == nil {
+			t.Fatalf("Register() expected panic on nil factory")
+		}
+	}()
+	r.Register("stdout", nil)
+}
+
+func TestRegistryRegisterPanicsOnDuplicateDriver(t *testing.T) {
+	r := NewRegistry()
+	r.Register("stdout", func(config.SinkConfig) (Sink, error) { return testSink{name: "a"}, nil })
+
+	defer func() {
+		if recover() == nil {
+			t.Fatalf("Register() expected panic on duplicate driver")
+		}
+	}()
+	r.Register("stdout", func(config.SinkConfig) (Sink, error) { return testSink{name: "b"}, nil })
+}
+
+func TestRegistryBuildNilRegistryFails(t *testing.T) {
+	var r *Registry
+	_, err := r.Build(config.SinkConfig{Driver: "stdout"})
+	if err == nil {
+		t.Fatalf("Build() expected error for nil registry")
+	}
+	if !strings.Contains(err.Error(), "registry is nil") {
+		t.Fatalf("Build() error = %q, want registry is nil", err)
+	}
+}
+
+func TestRegistryBuildTrimsDriver(t *testing.T) {
+	r := NewRegistry()
+	r.Register("stdout", func(config.SinkConfig) (Sink, error) { return testSink{name: "stdout"}, nil })
+
+	sink, err := r.Build(config.SinkConfig{Name: "sink1", Driver: " stdout "})
+	if err != nil {
+		t.Fatalf("Build() error = %v", err)
+	}
+	if sink.Name() != "stdout" {
+		t.Fatalf("Build() sink name = %q, want stdout", sink.Name())
+	}
+}
+
+func TestRegistryBuildWrapsFactoryError(t *testing.T) {
+	r := NewRegistry()
+	r.Register("broken", func(config.SinkConfig) (Sink, error) { return nil, errors.New("boom") })
+
+	_, err := r.Build(config.SinkConfig{Driver: "broken"})
+	if err == nil {
+		t.Fatalf("Build() expected error")
+	}
+	if !strings.Contains(err.Error(), `build sink "broken": boom`) {
+		t.Fatalf("Build() error = %q", err)
+	}
+}
+
+func TestRegistryBuildRejectsNilSink(t *testing.T) {
+	r := NewRegistry()
+	r.Register("nil_sink", func(config.SinkConfig) (Sink, error) { return nil, nil })
+
+	_, err := r.Build(config.SinkConfig{Driver: "nil_sink"})
+	if err == nil {
+		t.Fatalf("Build() expected nil sink error")
+	}
+	if !strings.Contains(err.Error(), "factory returned nil sink") {
+		t.Fatalf("Build() error = %q", err)
+	}
+}
+
+func TestRegisterBuiltinsExposesExpectedDrivers(t *testing.T) {
+	r := NewRegistry()
+	RegisterBuiltins(r)
+
+	if len(r.byDriver) != 3 {
+		t.Fatalf("len(byDriver) = %d, want 3", len(r.byDriver))
+	}
+	for _, driver := range []string{"stdout", "nats", "postgres"} {
+		if _, ok := r.byDriver[driver]; !ok {
+			t.Fatalf("builtins missing driver %q", driver)
+		}
+	}
+	if _, ok := r.byDriver["file"]; ok {
+		t.Fatalf("builtins unexpectedly registered file driver")
+	}
+}