Ai slop cleanup

internal/config/cache_config.go

@@ -1,83 +1,50 @@
 package config
 
 import (
-	"errors"
 	"fmt"
-	"strings"
+	"slices"
 	"time"
 
-	"gopkg.in/yaml.v3"
+	multierror "github.com/hashicorp/go-multierror"
 )
 
-// ErrInvalidCacheConfig is returned when the cache-config YAML value is not a mapping.
-var ErrInvalidCacheConfig = errors.New("expected a mapping for cache-config")
-
 type CacheGlobs []string
 
 func (g CacheGlobs) Clone() CacheGlobs {
-	if g == nil {
-		return nil
-	}
-
-	cacheGlobs := make(CacheGlobs, 0, len(g))
-	cacheGlobs = append(cacheGlobs, g...)
-
-	return cacheGlobs
+	return slices.Clone(g)
 }
 
 type CacheConfig struct {
-	ExpirationTime time.Duration `yaml:"-"`
+	ExpirationTime time.Duration `yaml:"expiration-time"`
 	MaxSize        int64         `yaml:"max-size"`
 	Methods        []string      `yaml:"methods"`
 }
 
 func (c *CacheConfig) Clone() *CacheConfig {
-	var methods []string
-	if c.Methods != nil {
-		methods = append(methods, c.Methods...)
-	}
-
 	return &CacheConfig{
 		ExpirationTime: c.ExpirationTime,
 		MaxSize:        c.MaxSize,
-		Methods:        methods,
+		Methods:        slices.Clone(c.Methods),
 	}
 }
 
-// UnmarshalYAML implements custom decoding so that the "expiration-time" field
-// can be expressed as a human-readable duration string (e.g. "30m", "1h").
-// Other fields are decoded by the standard yaml.v3 machinery.
-// Only fields present in the YAML node are updated; existing values (defaults)
-// are preserved for absent keys.
-func (c *CacheConfig) UnmarshalYAML(value *yaml.Node) error {
-	if value.Kind != yaml.MappingNode {
-		return ErrInvalidCacheConfig
-	}
+func (c *CacheConfig) Validate(field string) error {
+	var errs *multierror.Error
 
-	for i := 0; i+1 < len(value.Content); i += 2 {
-		keyNode := value.Content[i]
-		valNode := value.Content[i+1]
+	errs = multierror.Append(errs, ValidateDuration(joinPath(field, "expiration-time"), c.ExpirationTime, false))
 
-		switch keyNode.Value {
-		case "expiration-time":
-			dur, err := time.ParseDuration(strings.ReplaceAll(valNode.Value, " ", ""))
-			if err != nil {
-				return fmt.Errorf("invalid expiration-time %q: %w", valNode.Value, err)
-			}
+	if c.MaxSize <= 0 {
+		msg := fmt.Sprintf("%s must be greater than 0", joinPath(field, "max-size"))
+		errs = multierror.Append(errs, &ValidationError{msg})
+	}
+
+	if len(c.Methods) == 0 {
+		errs = multierror.Append(errs, &ValidationError{"methods must not be empty"})
+	}
 
-			c.ExpirationTime = dur
-		case "max-size":
-			err := valNode.Decode(&c.MaxSize)
-			if err != nil {
-				return err
-			}
-		case "methods":
-			err := valNode.Decode(&c.Methods)
-			if err != nil {
-				return err
-			}
-		}
+	for i, method := range c.Methods {
+		errs = multierror.Append(errs, ValidateMethod(joinPath(field, "methods", index(i)), method, false))
 	}
 
-	return nil
+	return joinErrors(errs)
 }

internal/config/cache_config_test.go

@@ -8,68 +8,8 @@ import (
 	"github.com/evg4b/uncors/internal/config"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"gopkg.in/yaml.v3"
 )
 
-func TestCacheConfigUnmarshalYAML(t *testing.T) {
-	t.Run("decodes all fields", func(t *testing.T) {
-		const input = `
-expiration-time: 30m
-max-size: 52428800
-methods:
-  - GET
-  - POST
-`
-
-		var actual config.CacheConfig
-
-		require.NoError(t, yaml.Unmarshal([]byte(input), &actual))
-		assert.Equal(t, config.CacheConfig{
-			ExpirationTime: 30 * time.Minute,
-			MaxSize:        52428800,
-			Methods:        []string{http.MethodGet, http.MethodPost},
-		}, actual)
-	})
-
-	t.Run("parses expiration-time with embedded spaces", func(t *testing.T) {
-		const input = `expiration-time: "1h 30m"`
-
-		var actual config.CacheConfig
-
-		require.NoError(t, yaml.Unmarshal([]byte(input), &actual))
-		assert.Equal(t, 90*time.Minute, actual.ExpirationTime)
-	})
-
-	t.Run("absent fields keep zero values", func(t *testing.T) {
-		const input = `max-size: 1024`
-
-		var actual config.CacheConfig
-
-		require.NoError(t, yaml.Unmarshal([]byte(input), &actual))
-		assert.Equal(t, int64(1024), actual.MaxSize)
-		assert.Zero(t, actual.ExpirationTime)
-		assert.Nil(t, actual.Methods)
-	})
-
-	t.Run("returns ErrInvalidCacheConfig for non-mapping node", func(t *testing.T) {
-		const input = `- item1`
-
-		var actual config.CacheConfig
-
-		err := yaml.Unmarshal([]byte(input), &actual)
-
-		assert.ErrorIs(t, err, config.ErrInvalidCacheConfig)
-	})
-
-	t.Run("returns error for invalid expiration-time", func(t *testing.T) {
-		const input = `expiration-time: not-a-duration`
-
-		var actual config.CacheConfig
-
-		assert.Error(t, yaml.Unmarshal([]byte(input), &actual))
-	})
-}
-
 func TestCacheGlobsClone(t *testing.T) {
 	globs := config.CacheGlobs{
 		"/api/**",
@@ -110,3 +50,68 @@ func TestCacheConfigClone(t *testing.T) {
 		assert.NotSame(t, &cacheConfig.Methods, &clonedCacheConfig.Methods)
 	})
 }
+
+func TestCacheConfigValidator(t *testing.T) {
+	const field = "test"
+
+	t.Run("should not register errors for", func(t *testing.T) {
+		err := (&config.CacheConfig{
+			ExpirationTime: 5 * time.Minute,
+			MaxSize:        100 * 1024 * 1024,
+			Methods:        []string{http.MethodGet, http.MethodPost},
+		}).Validate(field)
+		assert.NoError(t, err)
+	})
+
+	t.Run("should register errors for", func(t *testing.T) {
+		tests := []struct {
+			name  string
+			value config.CacheConfig
+			error string
+		}{
+			{
+				name:  "empty expiration time",
+				value: config.CacheConfig{MaxSize: 100 * 1024 * 1024, Methods: []string{http.MethodGet}},
+				error: "test.expiration-time must be greater than 0",
+			},
+			{
+				name:  "zero max size",
+				value: config.CacheConfig{ExpirationTime: 5 * time.Minute, MaxSize: 0, Methods: []string{http.MethodGet}},
+				error: "test.max-size must be greater than 0",
+			},
+			{
+				name:  "negative max size",
+				value: config.CacheConfig{ExpirationTime: 5 * time.Minute, MaxSize: -1, Methods: []string{http.MethodGet}},
+				error: "test.max-size must be greater than 0",
+			},
+			{
+				name:  "empty methods",
+				value: config.CacheConfig{ExpirationTime: 5 * time.Minute, MaxSize: 100 * 1024 * 1024},
+				error: "methods must not be empty",
+			},
+			{
+				name: "invalid method",
+				value: config.CacheConfig{
+					ExpirationTime: 5 * time.Minute,
+					MaxSize:        100 * 1024 * 1024,
+					Methods:        []string{"invalid"},
+				},
+				error: "test.methods[0] must be one of GET, HEAD, POST, PUT, PATCH, DELETE, CONNECT, OPTIONS, TRACE",
+			},
+			{
+				name: "invalid second method",
+				value: config.CacheConfig{
+					ExpirationTime: 5 * time.Minute,
+					MaxSize:        100 * 1024 * 1024,
+					Methods:        []string{http.MethodGet, "invalid", http.MethodPost},
+				},
+				error: "test.methods[1] must be one of GET, HEAD, POST, PUT, PATCH, DELETE, CONNECT, OPTIONS, TRACE",
+			},
+		}
+		for _, test := range tests {
+			t.Run(test.name, func(t *testing.T) {
+				require.EqualError(t, test.value.Validate(field), test.error)
+			})
+		}
+	})
+}

internal/config/config.go

@@ -3,12 +3,12 @@ package config
 import (
 	"fmt"
 
+	multierror "github.com/hashicorp/go-multierror"
 	"github.com/spf13/afero"
 	"github.com/spf13/pflag"
 	"gopkg.in/yaml.v3"
 )
 
-// UncorsConfig is the root configuration for the uncors proxy.
 type UncorsConfig struct {
 	Mappings    Mappings    `yaml:"mappings"`
 	Proxy       string      `yaml:"proxy"`
@@ -17,9 +17,6 @@ type UncorsConfig struct {
 	Interactive bool        `yaml:"-"`
 }
 
-// LoadConfiguration parses CLI arguments and optionally reads a YAML config file.
-// CLI flags take precedence over config file values.
-// Returns the loaded config, the active config file path (empty if none), and any error.
 func LoadConfiguration(fs afero.Fs, args []string) (*UncorsConfig, string, error) {
 	flags := defineFlags()
 
@@ -32,9 +29,9 @@ func LoadConfiguration(fs afero.Fs, args []string) (*UncorsConfig, string, error
 	configPath, _ := flags.GetString("config")
 
 	if configPath != "" {
-		readErr := readYAMLFile(fs, cfg, configPath)
-		if readErr != nil {
-			return nil, "", readErr
+		err := readYAMLFile(fs, cfg, configPath)
+		if err != nil {
+			return nil, "", err
 		}
 	}
 
@@ -45,11 +42,14 @@ func LoadConfiguration(fs afero.Fs, args []string) (*UncorsConfig, string, error
 
 	cfg.Mappings = NormaliseMappings(cfg.Mappings)
 
+	err = cfg.Validate(fs)
+	if err != nil {
+		return nil, "", err
+	}
+
 	return cfg, configPath, nil
 }
 
-// readYAMLFile opens a YAML config file and decodes it directly into cfg,
-// preserving any existing default values for keys absent in the file.
 func readYAMLFile(fs afero.Fs, cfg *UncorsConfig, path string) error {
 	file, err := fs.Open(path)
 	if err != nil {
@@ -66,8 +66,6 @@ func readYAMLFile(fs afero.Fs, cfg *UncorsConfig, path string) error {
 	return nil
 }
 
-// applyFlagOverrides applies CLI flag values to cfg, overriding any config file values.
-// Only flags explicitly set on the command line are applied.
 func applyFlagOverrides(cfg *UncorsConfig, flags *pflag.FlagSet) error {
 	if flags.Changed("proxy") {
 		cfg.Proxy, _ = flags.GetString("proxy")
@@ -86,3 +84,20 @@ func applyFlagOverrides(cfg *UncorsConfig, flags *pflag.FlagSet) error {
 
 	return mergeURLMappings(cfg, from, to)
 }
+
+func (cfg *UncorsConfig) Validate(fs afero.Fs) error {
+	if len(cfg.Mappings) == 0 {
+		return &ValidationError{"mappings must not be empty"}
+	}
+
+	var errs *multierror.Error
+
+	for i, mapping := range cfg.Mappings {
+		errs = multierror.Append(errs, mapping.Validate(joinPath("mappings", index(i)), fs))
+	}
+
+	errs = multierror.Append(errs, ValidateProxy("proxy", cfg.Proxy))
+	errs = multierror.Append(errs, cfg.CacheConfig.Validate("cache-config"))
+
+	return joinErrors(errs)
+}