Delete CLI metadata through the replaced subtree
This commit is contained in:
@@ -818,7 +818,14 @@ collection-size limit.
|
|||||||
|
|
||||||
## 20. Remove Full Metadata Scans Per CLI Replacement Leaf — P2
|
## 20. Remove Full Metadata Scans Per CLI Replacement Leaf — P2
|
||||||
|
|
||||||
Status: [ ] Open.
|
Status: [x] Complete.
|
||||||
|
|
||||||
|
Completion: presence and origin removal now visits only the known replaced
|
||||||
|
pre-CLI subtree. Partial mappings still retain omitted fields and their metadata.
|
||||||
|
Added broad-mapping coverage with escaped keys, explicit zero/false values,
|
||||||
|
empty-list replacement, null pointers, retained siblings, and deleted descendant
|
||||||
|
metadata. Existing nested CLI and typed-ownership regressions and
|
||||||
|
`./scripts/check.sh` pass.
|
||||||
|
|
||||||
Owners: `updateOverridePresence`, `deletePresencePathSubtree`, and
|
Owners: `updateOverridePresence`, `deletePresencePathSubtree`, and
|
||||||
`updateOverrideOrigins` in
|
`updateOverrideOrigins` in
|
||||||
|
|||||||
@@ -2,7 +2,6 @@ package engine
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"reflect"
|
"reflect"
|
||||||
"strings"
|
|
||||||
|
|
||||||
"gitea.maximumdirect.net/eric/ConfigKit/internal/document"
|
"gitea.maximumdirect.net/eric/ConfigKit/internal/document"
|
||||||
)
|
)
|
||||||
@@ -171,17 +170,16 @@ func updateOverridePresence(presence map[string]inputPresence, existing, incomin
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
deletePresencePathSubtree(presence, path)
|
deletePresencePathSubtree(presence, existing, path)
|
||||||
collectPresence(incoming, path, presence)
|
collectPresence(incoming, path, presence)
|
||||||
}
|
}
|
||||||
|
|
||||||
func deletePresencePathSubtree(presence map[string]inputPresence, path Path) {
|
func deletePresencePathSubtree(presence map[string]inputPresence, existing *document.Node, path Path) {
|
||||||
pointer := path.String()
|
// The typed pre-CLI tree contains every effective descendant. Visit only
|
||||||
for candidate := range presence {
|
// the replaced subtree rather than searching unrelated metadata entries.
|
||||||
if candidate == pointer || strings.HasPrefix(candidate, pointer+"/") {
|
walkDocument(existing, path, func(path Path, _ *document.Node) {
|
||||||
delete(presence, candidate)
|
delete(presence, path.String())
|
||||||
}
|
})
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func updateOverrideOrigins(origins map[string]Origin, existing, incoming *document.Node, path Path, location document.Location) {
|
func updateOverrideOrigins(origins map[string]Origin, existing, incoming *document.Node, path Path, location document.Location) {
|
||||||
@@ -200,12 +198,9 @@ func updateOverrideOrigins(origins map[string]Origin, existing, incoming *docume
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
pointer := path.String()
|
walkDocument(existing, path, func(path Path, _ *document.Node) {
|
||||||
for candidate := range origins {
|
delete(origins, path.String())
|
||||||
if candidate == pointer || strings.HasPrefix(candidate, pointer+"/") {
|
})
|
||||||
delete(origins, candidate)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
collectOverlayOrigins(origins, incoming, path, LayerCLI)
|
collectOverlayOrigins(origins, incoming, path, LayerCLI)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
82
internal/engine/override_metadata_test.go
Normal file
82
internal/engine/override_metadata_test.go
Normal file
@@ -0,0 +1,82 @@
|
|||||||
|
package engine
|
||||||
|
|
||||||
|
import (
|
||||||
|
"fmt"
|
||||||
|
"reflect"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestBroadPartialCLIOverrideRetainsUnchangedMetadata(t *testing.T) {
|
||||||
|
type child struct {
|
||||||
|
Name string `yaml:"name"`
|
||||||
|
}
|
||||||
|
type record struct {
|
||||||
|
Count int `yaml:"count"`
|
||||||
|
Zero int `yaml:"zero"`
|
||||||
|
Enabled bool `yaml:"enabled"`
|
||||||
|
Items []child `yaml:"items"`
|
||||||
|
Maybe *child `yaml:"maybe"`
|
||||||
|
}
|
||||||
|
type config struct {
|
||||||
|
Records map[string]record `yaml:"records"`
|
||||||
|
}
|
||||||
|
var body strings.Builder
|
||||||
|
body.WriteString("records:\n")
|
||||||
|
updates := make(map[string]any)
|
||||||
|
for index := range 96 {
|
||||||
|
name := fmt.Sprintf("key/%d~", index)
|
||||||
|
fmt.Fprintf(&body, " %s: {count: 9, zero: 0, enabled: false, items: [{name: old}], maybe: {name: old}}\n", name)
|
||||||
|
if index%2 == 0 {
|
||||||
|
updates[name] = map[string]any{"count": 0, "items": []any{}, "maybe": nil}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
input := parseDocumentForDecode(t, body.String())
|
||||||
|
schema := mustSchemaFor[config](t)
|
||||||
|
decoded, issues := decodeConfiguration(schema, reflect.ValueOf(config{}), input)
|
||||||
|
if len(issues) != 0 {
|
||||||
|
t.Fatal(issues)
|
||||||
|
}
|
||||||
|
batch, diagnostics := preparePostStructuralOverrides(overridePreparationInput{root: input}, nil,
|
||||||
|
[]Override{{Path: Keys("records"), Value: updates, Flag: "--records"}}, nil, defaultImportLimits())
|
||||||
|
if len(diagnostics) != 0 {
|
||||||
|
t.Fatal(diagnostics)
|
||||||
|
}
|
||||||
|
result, issues := applyDecodedOverrides(schema, decoded, batch, decodeContext{})
|
||||||
|
if len(issues) != 0 {
|
||||||
|
t.Fatal(issues)
|
||||||
|
}
|
||||||
|
for index := range 96 {
|
||||||
|
name := fmt.Sprintf("key/%d~", index)
|
||||||
|
path := Keys("records", name)
|
||||||
|
value := result.value.Interface().(config).Records[name]
|
||||||
|
for _, key := range []string{"zero", "enabled"} {
|
||||||
|
pointer := path.Key(key).String()
|
||||||
|
if _, explicit := result.presence[pointer]; !explicit || result.origins[pointer].Layer != LayerBase {
|
||||||
|
t.Fatalf("retained explicit field lost metadata at %s", pointer)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if index%2 != 0 {
|
||||||
|
if value.Count != 9 || len(value.Items) != 1 || value.Maybe == nil || result.origins[path.Key("count").String()].Layer != LayerBase {
|
||||||
|
t.Fatalf("omitted record changed at %s", path)
|
||||||
|
}
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if value.Count != 0 || len(value.Items) != 0 || value.Maybe != nil {
|
||||||
|
t.Fatalf("CLI replacement failed at %s: %#v", path, value)
|
||||||
|
}
|
||||||
|
for _, removed := range []Path{path.Key("items").Index(0), path.Key("items").Index(0).Key("name"), path.Key("maybe").Key("name")} {
|
||||||
|
_, present := result.presence[removed.String()]
|
||||||
|
_, origin := result.origins[removed.String()]
|
||||||
|
if present || origin {
|
||||||
|
t.Fatalf("removed descendant kept metadata at %s", removed)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
for _, key := range []string{"count", "items", "maybe"} {
|
||||||
|
pointer := path.Key(key).String()
|
||||||
|
if presence, ok := result.presence[pointer]; !ok || presence.location.SourceID != batch.overrides[0].source.ID || result.origins[pointer].Layer != LayerCLI {
|
||||||
|
t.Fatalf("CLI field lost metadata at %s", pointer)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user