Files
pixa/internal/config/config.go
clawbot 63fbc98e63
Some checks failed
check / check (push) Has been cancelled
feat: cache size management and LRU eviction (closes #51) (#55)
Implements #51 per the issue DoD and the owner direction comment (issuecomment-44068).

## Behavior

**Config: `cache_max_bytes`** (integrates with the #52/#53 validation framework)

- Strict int64 parsing via a new `getInt64`/`int64Val` getter in the existing strict-loader pattern; the key is registered in the known-keys list. A SET but invalid value — negative, float, null, non-numeric string, boolean, list — aborts startup with exit 1 naming the key and the offending value.
- Explicit values are used exactly as given, any non-negative amount, no floor. `cache_max_bytes: 0` is a valid value that disables the disk cache entirely.
- Omitted: after `state_dir` validation, the default resolves to `max(75% of free bytes on the filesystem containing <state_dir>/cache/, 500 MiB)`. The cache directory is created first and statfs runs on that actual path, so the measurement hits the right filesystem. The probe is injectable (`FreeSpaceProbeFunc`) so tests do not depend on the host disk. The effective limit (and disabled state) is logged at startup.

**Size accounting** (no directory scans on the hot path)

- Migration `002` adds a `variant_content` table — processed variants were previously untracked anywhere — and a `last_accessed_at` column on `source_content`, both indexed. Total usage is two SUM queries.
- Stores record accounting rows; cache hits touch the LRU timestamps (same cost class as the existing per-request stats UPDATEs). The variant accounting insert is best-effort with a warning: the reconciliation pass (below) adopts any file that missed its row, and this keeps the pre-migration inline test schema working.

**Eviction policy: global LRU across both content classes**

- Candidates are the least-recently-used entries from `variant_content` and `source_content` (batched, 100 per class per pass, merged oldest-first by `COALESCE(last_accessed_at, fetched_at)`), evicted until usage is at or below the limit.
- Why global LRU: recency of actual use is the best cheap predictor of future use for a CDN-style cache, and treating both classes in one ordering avoids pathologies of class-priority schemes (e.g. evicting every variant before any cold source blob, which would tank hit rate, or the reverse, which would hoard stale sources). Byte-for-byte, the coldest data goes first regardless of what kind it is. LFU-style schemes need more bookkeeping for marginal gain at this scale.
- Reference safety (the multi-reference DoD case): evicting a source blob deletes ALL `source_metadata` rows referencing it plus its `source_content` row in a single transaction BEFORE the file is unlinked. A blob referenced by multiple source paths is only ever removed together with all of its references, and DB rows never point at deleted files (the crash window leaves at worst an orphaned file, which reconciliation sweeps). The JSON metadata sidecars for removed rows are deleted as well.

**Triggers, off the request path**

- A background goroutine (started in the handlers OnStart hook, stopped in OnStop) runs an eviction pass on a periodic ticker (5 min) and on write pressure: every store sends a non-blocking notification on a capacity-1 channel. Requests never wait on eviction.
- On startup the goroutine first reconciles accounting with the disk (off the hot path): adopts untracked variant files (size/mtime from disk, content type from the `.meta` sidecar), drops accounting rows whose files are missing, removes source blob files the DB does not know (unreachable, since lookups go through `source_metadata`), removes rows whose files are gone, and sweeps `.tmp-*` files older than an hour.

**`cache_max_bytes: 0` disables the disk cache**

- No cache directories are created, lookups always miss, `StoreSource`/`StoreVariant` are no-ops, no evictor runs; every request fetches and processes uncached. Verified end-to-end (below).

## Notes for review

- At the `imgcache.CacheConfig` layer, disabling is an explicit `DisableDiskCache` flag rather than `MaxBytes == 0`, because existing test fixtures construct `CacheConfig` without `MaxBytes` and rely on the legacy "no limit" behavior; per repo rules those tests were not touched. The config layer maps `cache_max_bytes: 0` to the flag in `handlers`. `MaxBytes == 0` at that layer means "no limit enforced" and is unreachable from production config (the computed default is always at least 500 MiB).
- The negative cache stays active in disabled mode: it is DB-backed (TTL-expired rows in SQLite), not part of the disk cache this issue bounds, and it protects against hammering failing upstreams. Flagging explicitly since the direction said "no cache reads, no cache writes" — I read that as the disk cache; happy to disable it too if intended.
- One deviation from pure red/green: after the red commit I extended the new-test fixture helper (`newEvictionTestCache`) to pass `DisableDiskCache: maxBytes == 0`, mirroring the production mapping, when the flag design emerged. Assertions were not touched; no pre-existing tests were modified.
- Sidecar files (`.meta`, metadata JSON) are not counted in usage; they are bounded by entry counts and small (tens of bytes to ~1 KiB per entry) while content bytes dominate. Documented here for transparency.
- Discovered while working: `Cache.Stats` reads the never-populated `output_content`/`request_cache` tables, so `TotalItems`/`TotalSizeBytes` are always 0. Out of scope here; filing as a separate issue.

## Verification

- TDD: commit `3963ec3` adds the failing tests first (18 new tests covering strict parsing, default computation with injected probe including floor and 75% branches, explicit-no-floor, zero-disables, size accounting, dedup accounting, LRU order, multi-reference blob eviction with the no-dangling-references invariant, under-limit no-op, write-pressure trigger, periodic trigger, reconciliation); implementation follows in `8cb09b6`/`bdd86a4` until green.
- `make check` green (all tests, lint 0 issues, fmt-check) at HEAD.
- Pinned CI lint gate: `docker build --target lint .` green (golangci-lint v2.10.1).
- End-to-end with the built binary:
  - omitted key: startup logs `computed default cache size limit from free space` and `effective cache size limit` (75% of the test host's free space);
  - `cache_max_bytes: banana`: exit 1 with `config key "cache_max_bytes": value "banana" is not an integer`;
  - `cache_max_bytes: 0`: `cache_disabled=true` logged, two identical requests both fetch upstream (2 upstream fetches logged), 200 `image/jpeg` responses, no `cache/` directory created, only `state.sqlite3` in the state dir;
  - enabled: second request served from cache (1 upstream fetch), `variant_content` and `source_content` rows match the on-disk file sizes.

Co-authored-by: sneak <sneak@sneak.berlin>
Reviewed-on: #55
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
2026-08-09 13:22:51 +02:00

720 lines
19 KiB
Go

// Package config provides application configuration using smartconfig.
package config
import (
"fmt"
"log/slog"
"math"
"net/url"
"os"
"path/filepath"
"sort"
"strconv"
"strings"
"git.eeqj.de/sneak/smartconfig"
"go.uber.org/fx"
"sneak.berlin/go/pixa/internal/globals"
"sneak.berlin/go/pixa/internal/logger"
)
// Default configuration values.
const (
DefaultPort = 8080
DefaultStateDir = "/var/lib/pixa"
DefaultUpstreamConnectionsPerHost = 20
)
// Params defines dependencies for Config.
type Params struct {
fx.In
Globals *globals.Globals
Logger *logger.Logger
}
// Config holds application configuration values.
type Config struct {
Debug bool
MaintenanceMode bool
MetricsPassword string
MetricsUsername string
Port int
SentryDSN string
StateDir string
DBURL string
// Image proxy settings
SigningKey string // HMAC signing key for URL signatures
AllowlistHosts []string // Hosts that don't require signatures
AllowHTTP bool // Allow non-TLS upstream (testing only)
UpstreamConnectionsPerHost int // Max concurrent connections per upstream host
// CacheMaxBytes is the disk cache size limit in bytes. Zero
// disables the disk cache entirely. When cache_max_bytes is
// omitted from the configuration, this holds the computed default
// (75% of free space on the filesystem containing
// <state_dir>/cache/, floored at DefaultCacheMaxBytesFloor).
CacheMaxBytes int64
// cacheMaxBytesExplicit records whether cache_max_bytes was
// explicitly set in the configuration file. Explicit values are
// used exactly as given; only an omitted key gets the computed
// default (and its floor) in resolveCacheMaxBytes.
cacheMaxBytesExplicit bool
}
// New creates a new Config instance by loading configuration from file.
func New(_ fx.Lifecycle, params Params) (*Config, error) {
log := params.Logger.Get()
name := params.Globals.Appname
sc, err := loadConfigFile(log, name)
if err != nil {
return nil, err
}
if sc == nil {
log.Info("no config file found, using defaults")
}
c, err := newFromSmartConfig(sc)
if err != nil {
return nil, err
}
if err := c.ensureStateDirWritable(); err != nil {
return nil, err
}
if err := c.resolveCacheMaxBytes(log, defaultFreeSpaceProbe); err != nil {
return nil, err
}
if c.Debug {
params.Logger.EnableDebugLogging()
}
return c, nil
}
// newFromSmartConfig constructs a Config from a loaded smartconfig
// instance and validates it. A nil sc means no config file was found,
// in which case every option takes its default value. A key that is
// present but unparseable or invalid is an error: defaults apply only
// to omitted keys, never to invalid explicit values.
func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
if sc != nil {
if err := validateKnownKeys(sc); err != nil {
return nil, err
}
if err := validateAllowlistHostsValue(sc); err != nil {
return nil, err
}
}
loader := &strictLoader{sc: sc}
c := &Config{
Debug: loader.boolVal("debug", false),
MaintenanceMode: loader.boolVal("maintenance_mode", false),
Port: loader.intVal("port", DefaultPort),
StateDir: loader.stringVal("state_dir", DefaultStateDir),
SentryDSN: loader.stringVal("sentry_dsn", ""),
MetricsUsername: loader.stringVal("metrics.username", ""),
MetricsPassword: loader.stringVal("metrics.password", ""),
SigningKey: loader.stringVal("signing_key", ""),
AllowlistHosts: getStringSlice(sc, "allowlist_hosts"),
AllowHTTP: loader.boolVal("allow_http", false),
UpstreamConnectionsPerHost: loader.intVal(
"upstream_connections_per_host", DefaultUpstreamConnectionsPerHost),
CacheMaxBytes: loader.int64Val("cache_max_bytes", 0),
}
// The computed default for cache_max_bytes needs a validated
// state_dir, so it is resolved later (resolveCacheMaxBytes); here
// we only record whether the operator set the key explicitly.
if sc != nil {
if _, present := sc.Get("cache_max_bytes"); present {
c.cacheMaxBytesExplicit = true
}
}
// Build DBURL from StateDir if not explicitly set. The derived URL
// is a default: it applies only when db_url is omitted, never to an
// explicitly empty value.
c.DBURL = loader.stringVal("db_url", "")
if c.DBURL == "" && loader.err == nil {
if sc != nil {
if _, present := sc.Get("db_url"); present {
return nil, fmt.Errorf(
"config key %q: value must not be empty; omit the key to derive it from state_dir",
"db_url")
}
}
c.DBURL = fmt.Sprintf("file:%s/state.sqlite3?_journal_mode=WAL", c.StateDir)
}
if loader.err != nil {
return nil, loader.err
}
if err := c.validate(); err != nil {
return nil, err
}
return c, nil
}
// validateKnownKeys rejects configuration files containing keys the
// application does not understand, so typos fail at startup instead of
// being silently ignored, and rejects keys that are explicitly set to
// null: a null is a SET value, never an omission, so it must not
// silently take the default. The env section is permitted because
// smartconfig consumes it for environment variable injection.
func validateKnownKeys(sc *smartconfig.Config) error {
var unknown, nullKeys []string
for key, value := range sc.Data() {
if !isKnownConfigKey(key) {
unknown = append(unknown, key)
continue
}
if value == nil {
nullKeys = append(nullKeys, key)
continue
}
if key == "metrics" {
metricsMap, ok := value.(map[string]interface{})
if !ok {
return fmt.Errorf(
"config key %q: value %v is not a map of metrics settings",
"metrics", value)
}
for subkey, subvalue := range metricsMap {
if subkey != "username" && subkey != "password" {
unknown = append(unknown, "metrics."+subkey)
continue
}
if subvalue == nil {
nullKeys = append(nullKeys, "metrics."+subkey)
}
}
}
}
if len(unknown) > 0 {
sort.Strings(unknown)
return fmt.Errorf("unknown config keys: %s", strings.Join(unknown, ", "))
}
if len(nullKeys) > 0 {
sort.Strings(nullKeys)
if len(nullKeys) == 1 {
return errNullConfigValue(nullKeys[0])
}
return fmt.Errorf(
"config keys %s: value is null; omit a key entirely to use its default",
strings.Join(nullKeys, ", "))
}
return nil
}
// errNullConfigValue reports a config key that is explicitly set to
// null (including the bare "key:" form and the "~" alias). Silently
// applying the default would mask a truncated or typo'd config entry.
func errNullConfigValue(key string) error {
return fmt.Errorf(
"config key %q: value is null; omit the key entirely to use the default", key)
}
// isKnownConfigKey reports whether key is a permitted top-level
// configuration key.
func isKnownConfigKey(key string) bool {
switch key {
case "debug", "maintenance_mode", "port", "state_dir", "sentry_dsn",
"db_url", "metrics", "signing_key", "allowlist_hosts", "allow_http",
"upstream_connections_per_host", "cache_max_bytes", "env":
return true
}
return false
}
// ensureStateDirWritable verifies at startup that StateDir can be
// created and written to, so a misconfigured path aborts startup
// instead of failing later at first use.
func (c *Config) ensureStateDirWritable() error {
const stateDirPerms = 0o750
if err := os.MkdirAll(c.StateDir, stateDirPerms); err != nil {
return fmt.Errorf("config key %q: cannot create directory %q: %w",
"state_dir", c.StateDir, err)
}
probe, err := os.CreateTemp(c.StateDir, ".startup-write-probe-*")
if err != nil {
return fmt.Errorf("config key %q: directory %q is not writable: %w",
"state_dir", c.StateDir, err)
}
probePath := probe.Name()
if err := probe.Close(); err != nil {
return fmt.Errorf("config key %q: cannot close probe file %q: %w",
"state_dir", probePath, err)
}
//nolint:gosec // G703: probePath comes from os.CreateTemp inside the just-validated StateDir
if err := os.Remove(probePath); err != nil {
return fmt.Errorf("config key %q: cannot remove probe file %q: %w",
"state_dir", probePath, err)
}
return nil
}
// validate checks that all required configuration values are set and
// that every value is within its valid range.
func (c *Config) validate() error {
// The signing key value is never echoed in error messages.
if c.SigningKey == "" {
return fmt.Errorf("config key %q: a value is required", "signing_key")
}
// Minimum key length for security (32 bytes = 256 bits)
const minKeyLength = 32
if len(c.SigningKey) < minKeyLength {
return fmt.Errorf("config key %q: value must be at least %d characters, got %d",
"signing_key", minKeyLength, len(c.SigningKey))
}
const maxPort = 65535
if c.Port < 1 || c.Port > maxPort {
return fmt.Errorf("config key %q: value %d is outside the valid port range 1-%d",
"port", c.Port, maxPort)
}
if c.UpstreamConnectionsPerHost < 1 {
return fmt.Errorf("config key %q: value %d must be at least 1",
"upstream_connections_per_host", c.UpstreamConnectionsPerHost)
}
if c.StateDir == "" {
return fmt.Errorf("config key %q: value must not be empty", "state_dir")
}
// Zero is valid (it disables the disk cache); only negative
// values are rejected. No floor applies to explicit values.
if c.CacheMaxBytes < 0 {
return fmt.Errorf("config key %q: value %d must not be negative",
"cache_max_bytes", c.CacheMaxBytes)
}
for _, host := range c.AllowlistHosts {
if err := validateAllowlistHost(host); err != nil {
return err
}
}
if c.SentryDSN != "" {
parsed, err := url.Parse(c.SentryDSN)
if err != nil || parsed.Scheme == "" || parsed.Host == "" {
return fmt.Errorf("config key %q: value %q is not a valid URL",
"sentry_dsn", c.SentryDSN)
}
}
if (c.MetricsUsername == "") != (c.MetricsPassword == "") {
return fmt.Errorf("config keys %q and %q must be set together",
"metrics.username", "metrics.password")
}
return nil
}
// validateAllowlistHost checks that an allowlist_hosts entry is a bare
// hostname, optionally with a leading dot for suffix matching. URLs,
// paths, and whitespace indicate a misconfigured entry. An entry with
// no hostname labels (such as ".") is rejected: the allowlist matcher
// treats a leading dot as a suffix pattern, so a bare "." would match
// any upstream host written in FQDN trailing-dot form and effectively
// disable URL signing.
func validateAllowlistHost(host string) error {
if strings.Contains(host, "://") || strings.ContainsAny(host, "/ \t") {
return fmt.Errorf(
"config key %q: entry %q must be a bare hostname without scheme, path, or whitespace",
"allowlist_hosts", host)
}
if strings.Trim(host, ".") == "" {
return fmt.Errorf(
"config key %q: entry %q contains no hostname labels",
"allowlist_hosts", host)
}
return nil
}
// loadConfigFile loads configuration from PIXA_CONFIG_PATH env var or standard locations.
func loadConfigFile(log *slog.Logger, appName string) (*smartconfig.Config, error) {
// Check for explicit config path from environment
if envPath := os.Getenv("PIXA_CONFIG_PATH"); envPath != "" {
sc, err := smartconfig.NewFromConfigPath(envPath)
if err != nil {
return nil, fmt.Errorf("failed to load config from %s: %w", envPath, err)
}
log.Info("loaded config file", "path", envPath)
return sc, nil
}
// Try loading config from standard locations
configPaths := []string{
fmt.Sprintf("/etc/%s/config.yml", appName),
fmt.Sprintf("/etc/%s/config.yaml", appName),
filepath.Join(os.Getenv("HOME"), ".config", appName, "config.yml"),
filepath.Join(os.Getenv("HOME"), ".config", appName, "config.yaml"),
"config.yml",
"config.yaml",
}
for _, path := range configPaths {
cleanPath := filepath.Clean(path)
//nolint:gosec // G703: paths are hardcoded config locations
if _, statErr := os.Stat(cleanPath); statErr == nil {
// A config file that exists but does not parse is a fatal
// startup error, never something to skip over.
sc, err := smartconfig.NewFromConfigPath(path)
if err != nil {
return nil, fmt.Errorf("failed to parse config file %s: %w", path, err)
}
log.Info("loaded config file", "path", path)
return sc, nil
}
}
return nil, nil //nolint:nilnil // nil config is valid (use defaults)
}
// strictLoader accumulates the first error encountered while reading
// typed values out of a smartconfig instance, so Config construction
// can stay a single struct literal.
type strictLoader struct {
sc *smartconfig.Config
err error
}
func (l *strictLoader) stringVal(key, defaultVal string) string {
if l.err != nil {
return ""
}
val, err := getString(l.sc, key, defaultVal)
if err != nil {
l.err = err
}
return val
}
func (l *strictLoader) intVal(key string, defaultVal int) int {
if l.err != nil {
return 0
}
val, err := getInt(l.sc, key, defaultVal)
if err != nil {
l.err = err
}
return val
}
func (l *strictLoader) int64Val(key string, defaultVal int64) int64 {
if l.err != nil {
return 0
}
val, err := getInt64(l.sc, key, defaultVal)
if err != nil {
l.err = err
}
return val
}
func (l *strictLoader) boolVal(key string, defaultVal bool) bool {
if l.err != nil {
return false
}
val, err := getBool(l.sc, key, defaultVal)
if err != nil {
l.err = err
}
return val
}
// getString returns the string value for key, or defaultVal if the key
// is omitted. A present value that is not a string, or is explicitly
// null, is an error.
func getString(sc *smartconfig.Config, key, defaultVal string) (string, error) {
if sc == nil {
return defaultVal, nil
}
raw, ok := sc.Get(key)
if !ok {
return defaultVal, nil
}
if raw == nil {
return "", errNullConfigValue(key)
}
str, ok := raw.(string)
if !ok {
return "", fmt.Errorf("config key %q: value %v (%T) is not a string",
key, raw, raw)
}
return str, nil
}
// getInt returns the integer value for key, or defaultVal if the key is
// omitted. A present value that is not a whole number, or is explicitly
// null, is an error; fractional values are never truncated.
func getInt(sc *smartconfig.Config, key string, defaultVal int) (int, error) {
if sc == nil {
return defaultVal, nil
}
raw, ok := sc.Get(key)
if !ok {
return defaultVal, nil
}
if raw == nil {
return 0, errNullConfigValue(key)
}
switch val := raw.(type) {
case int:
return val, nil
case int64:
return int(val), nil
case float64:
if val != math.Trunc(val) {
return 0, fmt.Errorf("config key %q: value %v is not an integer", key, val)
}
return int(val), nil
case string:
parsed, err := strconv.Atoi(strings.TrimSpace(val))
if err != nil {
return 0, fmt.Errorf("config key %q: value %q is not an integer", key, val)
}
return parsed, nil
default:
return 0, fmt.Errorf("config key %q: value %v (%T) is not an integer",
key, raw, raw)
}
}
// getInt64 returns the 64-bit integer value for key, or defaultVal if
// the key is omitted. A present value that is not a whole number, or
// is explicitly null, is an error; fractional values are never
// truncated and out-of-range values are never clamped.
func getInt64(sc *smartconfig.Config, key string, defaultVal int64) (int64, error) {
if sc == nil {
return defaultVal, nil
}
raw, ok := sc.Get(key)
if !ok {
return defaultVal, nil
}
if raw == nil {
return 0, errNullConfigValue(key)
}
switch val := raw.(type) {
case int:
return int64(val), nil
case int64:
return val, nil
case uint64:
if val > math.MaxInt64 {
return 0, fmt.Errorf("config key %q: value %d overflows a 64-bit integer",
key, val)
}
return int64(val), nil //nolint:gosec // G115: bounds checked above
case float64:
if val != math.Trunc(val) {
return 0, fmt.Errorf("config key %q: value %v is not an integer", key, val)
}
return int64(val), nil
case string:
parsed, err := strconv.ParseInt(strings.TrimSpace(val), 10, 64)
if err != nil {
return 0, fmt.Errorf("config key %q: value %q is not an integer", key, val)
}
return parsed, nil
default:
return 0, fmt.Errorf("config key %q: value %v (%T) is not an integer",
key, raw, raw)
}
}
// getBool returns the boolean value for key, or defaultVal if the key
// is omitted. A present value that is not a boolean (or a ParseBool-able
// string), or is explicitly null, is an error; numbers are not accepted
// as booleans.
func getBool(sc *smartconfig.Config, key string, defaultVal bool) (bool, error) {
if sc == nil {
return defaultVal, nil
}
raw, ok := sc.Get(key)
if !ok {
return defaultVal, nil
}
if raw == nil {
return false, errNullConfigValue(key)
}
switch val := raw.(type) {
case bool:
return val, nil
case string:
parsed, err := strconv.ParseBool(strings.TrimSpace(val))
if err != nil {
return false, fmt.Errorf("config key %q: value %q is not a boolean", key, val)
}
return parsed, nil
default:
return false, fmt.Errorf("config key %q: value %v (%T) is not a boolean",
key, raw, raw)
}
}
// validateAllowlistHostsValue checks the raw shape of the
// allowlist_hosts value before the lenient extraction in getStringSlice
// runs: an explicitly null value, a value that is not a list of strings
// (or a comma-separated string), a non-string entry, or an empty entry
// is an error, never silently skipped.
func validateAllowlistHostsValue(sc *smartconfig.Config) error {
const key = "allowlist_hosts"
raw, ok := sc.Get(key)
if !ok {
return nil
}
if raw == nil {
return errNullConfigValue(key)
}
switch val := raw.(type) {
case []interface{}:
for _, item := range val {
str, ok := item.(string)
if !ok {
return fmt.Errorf(
"config key %q: list entry %v (%T) is not a string", key, item, item)
}
if strings.TrimSpace(str) == "" {
return fmt.Errorf("config key %q: list contains an empty entry", key)
}
}
case string:
if strings.TrimSpace(val) == "" {
return nil
}
for _, part := range strings.Split(val, ",") {
if strings.TrimSpace(part) == "" {
return fmt.Errorf(
"config key %q: value %q contains an empty entry", key, val)
}
}
default:
return fmt.Errorf("config key %q: value %v (%T) is not a list of strings",
key, raw, raw)
}
return nil
}
// getStringSlice returns the list of strings for key, or nil if the key
// is omitted. It accepts a YAML list of strings or a comma-separated
// string (backwards compatibility). Malformed entries are rejected
// beforehand by validateAllowlistHostsValue.
func getStringSlice(sc *smartconfig.Config, key string) []string {
if sc == nil {
return nil
}
val, ok := sc.Get(key)
if !ok || val == nil {
return nil
}
// Handle YAML list format
if slice, ok := val.([]interface{}); ok {
result := make([]string, 0, len(slice))
for _, item := range slice {
if str, ok := item.(string); ok {
trimmed := strings.TrimSpace(str)
if trimmed != "" {
result = append(result, trimmed)
}
}
}
return result
}
// Fall back to comma-separated string for backwards compatibility
if str, ok := val.(string); ok && str != "" {
parts := strings.Split(str, ",")
result := make([]string, 0, len(parts))
for _, part := range parts {
trimmed := strings.TrimSpace(part)
if trimmed != "" {
result = append(result, trimmed)
}
}
return result
}
return nil
}