diff --git a/conversion.go b/conversion.go index 6b775b8..e926a7c 100644 --- a/conversion.go +++ b/conversion.go @@ -81,31 +81,14 @@ func toEngineOverrides(values []Override) []engine.Override { func toEngineImportDefaults(value ImportDefaults) engine.ImportDefaults { return engine.ImportDefaults{ - Required: cloneImportDefaultBool(value.Required), - Follow: cloneImportDefaultBool(value.Follow), - Hidden: cloneImportDefaultBool(value.Hidden), - Recursive: cloneImportDefaultBool(value.Recursive), - Pattern: cloneImportDefaultStrings(value.Pattern), + Required: value.Required, + Follow: value.Follow, + Hidden: value.Hidden, + Recursive: value.Recursive, + Pattern: value.Pattern, } } -func cloneImportDefaultBool(value *bool) *bool { - if value == nil { - return nil - } - cloned := *value - return &cloned -} - -func cloneImportDefaultStrings(values []string) []string { - if values == nil { - return nil - } - cloned := make([]string, len(values)) - copy(cloned, values) - return cloned -} - func fromEngineSourceVersionCheck(value *engine.SourceVersionCheck) *SourceVersionCheck { if value == nil { return nil diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index c3ae64c..ca35214 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -151,7 +151,8 @@ building the package here; do not report that deferred check as passed. Finding: **F07**, defaults handoff. Owners: root `conversion.go`, `lifecycle.go`, `layered.go`; engine defaults/preflight/coordinator owners; `facade_import_defaults_test.go` -and `internal/engine/import_spec_test.go`. Status: **not started**. +and `internal/engine/import_spec_test.go`. Status: **complete**. Validation passed: +`env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'Import|Glob|Directory|SourceVersion'`, `env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test . -run 'ImportDefaults|FacadeImport|SourceVersion'`, `env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./...`, `env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test -race ./...`, and `git diff --check`. 1. Set engine `ImportDefaults` fields directly in root `toEngineOptions` and `toEngineLayeredOptions`. Retain `toEngineImportDefaults` as the explicit diff --git a/facade_import_defaults_test.go b/facade_import_defaults_test.go index 7cf6680..b914f59 100644 --- a/facade_import_defaults_test.go +++ b/facade_import_defaults_test.go @@ -6,8 +6,6 @@ import ( "path/filepath" "strings" "testing" - - "gitea.maximumdirect.net/eric/ConfigKit/internal/engine" ) func TestImportDepthChecksSuffixAfterSaturation(t *testing.T) { @@ -125,20 +123,61 @@ func TestImportDefaultsPreserveEmptyPatternAndRequiredness(t *testing.T) { if sources := loadError.Sources(); len(sources) != 1 || len(sources[0].ImportSelections) != 0 { t.Fatalf("required empty selection was recorded as optional: %#v", sources) } +} - recursiveMain := writeFacadeImportFile(t, directory, "recursive.yml", `configkit: +func TestImportDefaultsAreSnapshottedBeforeOrdinaryAndLayeredCallbacks(t *testing.T) { + type config struct { + Version string `yaml:"version"` + Imported string `yaml:"imported"` + } + for _, layered := range []bool{false, true} { + t.Run(map[bool]string{false: "ordinary", true: "layered"}[layered], func(t *testing.T) { + directory := t.TempDir() + mainPath := writeFacadeImportFile(t, directory, "main.yml", `version: v1 +configkit: imports: - - dir: recursive-files + - dir: parts `) - writeFacadeImportFile(t, directory, "recursive-files/nested/value.cfg", "value: selected\n") - recursive := true - patterns := []string{"*.cfg"} - engineOptions := toEngineOptions(Options[config]{Imports: true, ImportDefaults: ImportDefaults{Recursive: &recursive, Pattern: patterns}}) - recursive = false - patterns[0] = "[" - loadedEngine, err := engine.Load(File(recursiveMain).toEngine(), secretBinding(), engineOptions) - if err != nil || loadedEngine.Config.Value != "selected" { - t.Fatalf("load after mutating caller defaults = %#v, %v", loadedEngine, err) + writeFacadeImportFile(t, directory, "parts/nested/value.cfg", "imported: selected\n") + + recursive := true + patterns := []string{"*.cfg"} + definition := Definition[config]{Version: &VersionRule{ + Path: Keys("version"), + Check: func(version Version, _ *Validation) error { + if version != (Version{Present: true, Text: "v1"}) { + t.Errorf("version callback received %#v", version) + } + recursive = false + patterns[0] = "*.skip" + return nil + }, + }} + load := func() (*Result[config], error) { + defaults := ImportDefaults{Recursive: &recursive, Pattern: patterns} + if layered { + return LoadLayered(Input{Source: File(mainPath), Imports: true}, LayeredOptions[config]{ + Definition: definition, SourceVersionMode: SourceVersionModeIfPresent, ImportDefaults: defaults, + }) + } + return Load(File(mainPath), Options[config]{ + Definition: definition, SourceVersionMode: SourceVersionModeIfPresent, Imports: true, ImportDefaults: defaults, + }) + } + + first, err := load() + if err != nil || first.Config.Imported != "selected" { + t.Fatalf("load with pre-callback defaults = %#v, %v", first, err) + } + if recursive || patterns[0] != "*.skip" { + t.Fatalf("version callback did not mutate caller defaults: recursive=%v pattern=%q", recursive, patterns[0]) + } + + second, err := load() + if err != nil || second.Config.Imported != "" { + t.Fatalf("subsequent load reused earlier defaults: %#v, %v", second, err) + } + }) } } diff --git a/internal/engine/destination_accounting_test.go b/internal/engine/destination_accounting_test.go index b5a8d23..d41df5a 100644 --- a/internal/engine/destination_accounting_test.go +++ b/internal/engine/destination_accounting_test.go @@ -458,7 +458,7 @@ func testAssembleLayeredInput[T any](request loadRequest[T], input assembledInpu accounting = &importAccounting{} } policy := acquisitionPolicy{ - importDefaults: snapshotImportDefaults(request.importDefaults), + importDefaults: request.importDefaults, sequencePolicy: request.sequencePolicy, importLimits: defaultImportLimits(), } diff --git a/internal/engine/import_spec.go b/internal/engine/import_spec.go index 168eb2e..e93c210 100644 --- a/internal/engine/import_spec.go +++ b/internal/engine/import_spec.go @@ -104,7 +104,6 @@ func validateImportDefaults(value ImportDefaults) []Diagnostic { } func resolveImportPolicy(spec importDeclarationSpec, defaults ImportDefaults) (resolvedImportPolicy, []Diagnostic) { - defaults = snapshotImportDefaults(defaults) policy := resolvedImportPolicy{ required: boolOr(spec.required, defaults.Required, false), follow: true, diff --git a/internal/engine/import_spec_test.go b/internal/engine/import_spec_test.go index a05dab7..048ca41 100644 --- a/internal/engine/import_spec_test.go +++ b/internal/engine/import_spec_test.go @@ -189,10 +189,18 @@ func TestResolveImportPolicyAppliesDefaultsIndependently(t *testing.T) { snapshot := snapshotImportDefaults(defaults) *defaults.Required = false + *defaults.Follow = false + *defaults.Hidden = false + *defaults.Recursive = false defaults.Pattern[0] = "*.toml" - if snapshot.Required == nil || !*snapshot.Required || snapshot.Pattern[0] != "*.yaml" { + if snapshot.Required == nil || !*snapshot.Required || snapshot.Follow == nil || !*snapshot.Follow || + snapshot.Hidden == nil || !*snapshot.Hidden || snapshot.Recursive == nil || !*snapshot.Recursive || snapshot.Pattern[0] != "*.yaml" { t.Fatalf("snapshot shares mutable inputs: %#v", snapshot) } + omittedSnapshot := snapshotImportDefaults(ImportDefaults{}) + if omittedSnapshot.Required != nil || omittedSnapshot.Follow != nil || omittedSnapshot.Hidden != nil || omittedSnapshot.Recursive != nil { + t.Fatal("snapshot changed omitted booleans into explicit values") + } empty := snapshotImportDefaults(ImportDefaults{Pattern: []string{}}) if empty.Pattern == nil || len(empty.Pattern) != 0 { t.Fatalf("snapshot lost explicit empty pattern: %#v", empty) diff --git a/internal/engine/layered.go b/internal/engine/layered.go index f087b7a..6e1a7b9 100644 --- a/internal/engine/layered.go +++ b/internal/engine/layered.go @@ -41,7 +41,7 @@ func LoadLayered[T any](main Input, secrets SecretBinding, supplied ...LayeredOp destination: main.Destination.clone(), placementMode: main.placementMode, }, definition: options.Definition, sourceVersionMode: options.SourceVersionMode, - importDefaults: snapshotImportDefaults(options.ImportDefaults), + importDefaults: options.ImportDefaults, overlay: options.Overlay, overlayFilesystem: options.OverlayFilesystem, overrides: append([]Override(nil), options.Overrides...), diff --git a/internal/engine/load.go b/internal/engine/load.go index 27d3b46..80eb594 100644 --- a/internal/engine/load.go +++ b/internal/engine/load.go @@ -185,7 +185,7 @@ func executeLoadRequest[T any](request loadRequest[T], secrets SecretBinding, lo accumulator := diagnosticAccumulator{} accounting := &importAccounting{} policy := acquisitionPolicy{ - importDefaults: snapshotImportDefaults(request.importDefaults), + importDefaults: request.importDefaults, sequencePolicy: request.sequencePolicy, importLimits: defaultImportLimits(), } diff --git a/layered.go b/layered.go index 400bc6b..2165889 100644 --- a/layered.go +++ b/layered.go @@ -75,8 +75,8 @@ func toEngineLayeredOptions[T any](options LayeredOptions[T]) engine.LayeredOpti SourceVersionMode: engine.SourceVersionModeFromFacade(uint8(options.SourceVersionMode)), Overlay: options.Overlay, OverlayFilesystem: toEngineFilesystemContext(options.OverlayFilesystem), + ImportDefaults: toEngineImportDefaults(options.ImportDefaults), } - result = engine.LayeredOptionsWithImportDefaults(result, toEngineImportDefaults(options.ImportDefaults)) if options.Underlays != nil { result.Underlays = make([]engine.Underlay, len(options.Underlays)) for index, group := range options.Underlays { diff --git a/lifecycle.go b/lifecycle.go index fd4a3bd..bf52dc1 100644 --- a/lifecycle.go +++ b/lifecycle.go @@ -317,9 +317,9 @@ func toEngineOptions[T any](options Options[T]) engine.Options[T] { Imports: options.Imports, Overlay: options.Overlay, SourceVersionMode: engine.SourceVersionModeFromFacade(uint8(options.SourceVersionMode)), + ImportDefaults: toEngineImportDefaults(options.ImportDefaults), } result = engine.OptionsWithPlacement(result, toEnginePath(options.Destination.clone()), uint8(options.PlacementMode)) - result = engine.OptionsWithImportDefaults(result, toEngineImportDefaults(options.ImportDefaults)) if options.Filesystem != nil { result.Filesystem = toEngineFilesystemContext(options.Filesystem) }