Preserve retained input presence in nested CLI overrides
This commit is contained in:
@@ -1,7 +1,7 @@
|
|||||||
# Sequence Implementation Review Remediation
|
# Sequence Implementation Review Remediation
|
||||||
|
|
||||||
Status: eleven open findings. This is a review record, not an implementation
|
Status: remediation in progress. Completed findings are marked below; this
|
||||||
change or a new release gate.
|
record introduces no new release gate.
|
||||||
|
|
||||||
Reviewed commit: `0be7cdb00eff576fda6b30c0600b2134d4e2e086`.
|
Reviewed commit: `0be7cdb00eff576fda6b30c0600b2134d4e2e086`.
|
||||||
Implementation range: `bc6b977..0be7cdb`.
|
Implementation range: `bc6b977..0be7cdb`.
|
||||||
@@ -33,7 +33,13 @@ adding limits.
|
|||||||
|
|
||||||
## 1. Preserve Retained Explicit Values In Nested CLI Mapping Assignments — P1
|
## 1. Preserve Retained Explicit Values In Nested CLI Mapping Assignments — P1
|
||||||
|
|
||||||
Status: [ ] Open.
|
Status: [x] Complete.
|
||||||
|
|
||||||
|
Completion: override metadata now reads the actual target subtree through one
|
||||||
|
shared lookup. Ordinary/layered regressions preserve retained values, including
|
||||||
|
zero and false, their explicit origins, and absence-based resolver behavior.
|
||||||
|
Verified `go test ./internal/engine -run 'Override|CLI' -count=1` and
|
||||||
|
`./scripts/check.sh`.
|
||||||
|
|
||||||
Owner: [`applyDecodedOverrides`](../../internal/engine/override_decode.go),
|
Owner: [`applyDecodedOverrides`](../../internal/engine/override_decode.go),
|
||||||
especially lines 59–60 and the presence/origin update helpers.
|
especially lines 59–60 and the presence/origin update helpers.
|
||||||
|
|||||||
@@ -38,7 +38,10 @@ func applyDecodedOverrides(
|
|||||||
}
|
}
|
||||||
presence := cloneInputPresence(decoded.presence)
|
presence := cloneInputPresence(decoded.presence)
|
||||||
origins := cloneOrigins(decoded.origins)
|
origins := cloneOrigins(decoded.origins)
|
||||||
currentProjection := document.Clone(decoded.final)
|
var lookup *documentLookup
|
||||||
|
if len(batch.overrides) != 0 {
|
||||||
|
lookup = buildDocumentLookup(decoded.final)
|
||||||
|
}
|
||||||
|
|
||||||
for _, override := range batch.overrides {
|
for _, override := range batch.overrides {
|
||||||
if len(override.path.segments) == 0 {
|
if len(override.path.segments) == 0 {
|
||||||
@@ -56,8 +59,11 @@ func applyDecodedOverrides(
|
|||||||
|
|
||||||
location := document.Location{SourceID: override.source.ID}
|
location := document.Location{SourceID: override.source.ID}
|
||||||
recordOverrideAncestors(presence, origins, override.path, location)
|
recordOverrideAncestors(presence, origins, override.path, location)
|
||||||
updateOverridePresence(presence, currentProjection, override.value, override.path, location)
|
// Prepared paths cannot overlap, so other assignments cannot change
|
||||||
updateOverrideOrigins(origins, currentProjection, override.value, override.path, location)
|
// this target's pre-CLI subtree.
|
||||||
|
existing, _ := documentAtPathWithLookup(decoded.final, override.path, lookup)
|
||||||
|
updateOverridePresence(presence, existing, override.value, override.path, location)
|
||||||
|
updateOverrideOrigins(origins, existing, override.value, override.path, location)
|
||||||
}
|
}
|
||||||
final, projectionIssue := projectConfiguration(schema, working, defaultProjectionLimits())
|
final, projectionIssue := projectConfiguration(schema, working, defaultProjectionLimits())
|
||||||
if projectionIssue != nil {
|
if projectionIssue != nil {
|
||||||
|
|||||||
@@ -5,6 +5,67 @@ import (
|
|||||||
"testing"
|
"testing"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
func TestNestedCLIOverridePreservesRetainedInputPresence(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
type nested struct {
|
||||||
|
Changed int `yaml:"changed"`
|
||||||
|
Count int `yaml:"count"`
|
||||||
|
Zero int `yaml:"zero"`
|
||||||
|
Enabled bool `yaml:"enabled"`
|
||||||
|
}
|
||||||
|
type settings struct {
|
||||||
|
Nested nested `yaml:"nested"`
|
||||||
|
}
|
||||||
|
type config struct {
|
||||||
|
Settings settings `yaml:"settings"`
|
||||||
|
}
|
||||||
|
for _, layered := range []bool{false, true} {
|
||||||
|
name := "ordinary"
|
||||||
|
if layered {
|
||||||
|
name = "layered"
|
||||||
|
}
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
source := Bytes("input", []byte("settings:\n nested:\n changed: 1\n count: 2\n zero: 0\n enabled: false\n"))
|
||||||
|
definition := Definition[config]{Resolve: func(value *config, resolution *Resolution) error {
|
||||||
|
for _, key := range []string{"count", "zero", "enabled"} {
|
||||||
|
info, ok := resolution.InputInfo(Keys("settings", "nested", key))
|
||||||
|
if !ok || !info.Explicit || info.Origin.Layer != LayerBase {
|
||||||
|
t.Errorf("retained %s input info = %#v, %v", key, info, ok)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Default(resolution, Keys("settings", "nested", "count"), &value.Settings.Nested.Count, 99, "fill missing count")
|
||||||
|
Default(resolution, Keys("settings", "nested", "zero"), &value.Settings.Nested.Zero, 99, "fill missing zero")
|
||||||
|
Default(resolution, Keys("settings", "nested", "enabled"), &value.Settings.Nested.Enabled, true, "fill missing enabled")
|
||||||
|
return nil
|
||||||
|
}}
|
||||||
|
overrides := []Override{{Path: Keys("settings"), Value: map[string]any{"nested": map[string]any{"changed": 3}}, Flag: "--settings"}}
|
||||||
|
var result *testResult[config]
|
||||||
|
var err error
|
||||||
|
if layered {
|
||||||
|
result, err = testGroupedLoad(loadRequest[config]{main: loadInput{source: source}, definition: definition, overrides: overrides})
|
||||||
|
} else {
|
||||||
|
result, err = testLoad(source, Options[config]{Definition: definition, Overrides: overrides})
|
||||||
|
}
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if got, want := result.Config.Settings.Nested, (nested{Changed: 3, Count: 2}); got != want {
|
||||||
|
t.Fatalf("resolved nested values = %#v, want %#v", got, want)
|
||||||
|
}
|
||||||
|
for _, key := range []string{"count", "zero", "enabled"} {
|
||||||
|
info, ok := result.Metadata().Lookup(Keys("settings", "nested", key))
|
||||||
|
if !ok || !info.Explicit || info.Origin.Layer != LayerBase || len(info.Origin.Locations) != 1 || info.Origin.Locations[0].SourceID != "source-1" {
|
||||||
|
t.Errorf("retained %s metadata = %#v, %v", key, info, ok)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
info, ok := result.Metadata().Lookup(Keys("settings", "nested", "changed"))
|
||||||
|
if !ok || !info.Explicit || info.Origin.Layer != LayerCLI {
|
||||||
|
t.Errorf("changed metadata = %#v, %v", info, ok)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestPostDecodeOverrideLifecycleMatchesOrdinaryAndLayeredLoads(t *testing.T) {
|
func TestPostDecodeOverrideLifecycleMatchesOrdinaryAndLayeredLoads(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
type config struct {
|
type config struct {
|
||||||
|
|||||||
Reference in New Issue
Block a user