diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 2a7edf2..2ac80de 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -73,7 +73,13 @@ and an absence-based resolver default that must not overwrite retained input. ## 2. Read Upsert Origins At Input Paths And Write Them At Output Paths — P2 -Status: [ ] Open. +Status: [x] Complete. + +Completion: sequence origin copying now separates incoming and effective paths +for precedence and inheritance. A regression covers reordered matches, appended +records, composite origins, and actual resource reads from distinct declaring +directories. Verified focused origin/inheritance/overlay/provenance tests, +`go test -race ./internal/engine`, and `./scripts/check.sh`. Owners: [`applyOverlaySequenceEvidence`](../../internal/engine/overlay.go), particularly lines 369 and 381, and the analogous sequence-origin copying in diff --git a/internal/engine/inheritance_collection.go b/internal/engine/inheritance_collection.go index b04d2af..3896fc4 100644 --- a/internal/engine/inheritance_collection.go +++ b/internal/engine/inheritance_collection.go @@ -234,7 +234,7 @@ func applyInheritanceSequenceOrigins( } for _, projection := range combined.higherItems { if higher != nil && higher.Kind == document.Sequence && projection.inputIndex >= 0 && projection.inputIndex < len(higher.Sequence) { - copyInheritanceSubtree(origins, incomingOrigins, higher.Sequence[projection.inputIndex], path.Index(projection.outputIndex)) + copyInheritanceSubtreeAt(origins, incomingOrigins, higher.Sequence[projection.inputIndex], path.Index(projection.inputIndex), path.Index(projection.outputIndex)) } } return @@ -251,7 +251,7 @@ func applyInheritanceSequenceOrigins( outputPath := path.Index(replacement.outputIndex) previous := displacedOrigins(lower.Sequence[replacement.lowerIndex], origins, lowerPath) deleteOriginSubtree(origins, lowerPath, lower.Sequence[replacement.lowerIndex]) - copyInheritanceSubtree(origins, incomingOrigins, higher.Sequence[replacement.higherIndex], outputPath) + copyInheritanceSubtreeAt(origins, incomingOrigins, higher.Sequence[replacement.higherIndex], higherPath, outputPath) result.replacements = append(result.replacements, inheritanceReplacement{ path: outputPath, before: lower.Sequence[replacement.lowerIndex], after: higher.Sequence[replacement.higherIndex], previous: previous, effective: cloneOrigin(incomingOrigins[higherPath.String()]), @@ -259,7 +259,7 @@ func applyInheritanceSequenceOrigins( } for _, projection := range combined.higherItems { if higher != nil && higher.Kind == document.Sequence && projection.inputIndex >= 0 && projection.inputIndex < len(higher.Sequence) { - copyInheritanceSubtree(origins, incomingOrigins, higher.Sequence[projection.inputIndex], path.Index(projection.outputIndex)) + copyInheritanceSubtreeAt(origins, incomingOrigins, higher.Sequence[projection.inputIndex], path.Index(projection.inputIndex), path.Index(projection.outputIndex)) } } } diff --git a/internal/engine/inheritance_origin.go b/internal/engine/inheritance_origin.go index 617fad9..0234674 100644 --- a/internal/engine/inheritance_origin.go +++ b/internal/engine/inheritance_origin.go @@ -70,8 +70,14 @@ func mergeInheritanceOrigin(origins map[string]Origin, path Path, incoming Origi } func copyInheritanceSubtree(target, incoming map[string]Origin, node *document.Node, root Path) { - walkDocument(node, root, func(path Path, _ *document.Node) { - target[path.String()] = cloneOrigin(incoming[path.String()]) + copyInheritanceSubtreeAt(target, incoming, node, root, root) +} + +func copyInheritanceSubtreeAt(target, incoming map[string]Origin, node *document.Node, from, to Path) { + walkDocument(node, Path{}, func(relative Path, _ *document.Node) { + source := appendDestination(from, relative) + destination := appendDestination(to, relative) + target[destination.String()] = cloneOrigin(incoming[source.String()]) }) } diff --git a/internal/engine/overlay.go b/internal/engine/overlay.go index 3a6d42e..f848cbd 100644 --- a/internal/engine/overlay.go +++ b/internal/engine/overlay.go @@ -340,7 +340,7 @@ func applyOverlaySequenceEvidence( for _, projection := range combined.higherItems { if higher != nil && higher.Kind == document.Sequence && projection.inputIndex >= 0 && projection.inputIndex < len(higher.Sequence) { source := higher.Sequence[projection.inputIndex] - collectOverlayOriginsFrom(result.origins, incomingOrigins, source, path.Index(projection.outputIndex), layer) + collectOverlayOriginsAt(result.origins, incomingOrigins, source, path.Index(projection.inputIndex), path.Index(projection.outputIndex), layer) } } if lower != nil { @@ -366,7 +366,7 @@ func applyOverlaySequenceEvidence( displaced := displacedOrigins(lowerItem, result.origins, lowerPath) previous := cloneOrigin(result.origins[lowerPath.String()]) deleteOriginSubtree(result.origins, lowerPath, lowerItem) - collectOverlayOriginsFrom(result.origins, incomingOrigins, higherItem, outputPath, layer) + collectOverlayOriginsAt(result.origins, incomingOrigins, higherItem, higherPath, outputPath, layer) result.overlays = append(result.overlays, overlayRecord{ change: Change{ Path: outputPath.clone(), Origin: incomingOrigin(incomingOrigins, higherPath, higherItem, layer), @@ -378,7 +378,7 @@ func applyOverlaySequenceEvidence( for _, projection := range combined.higherItems { if higher != nil && higher.Kind == document.Sequence && projection.inputIndex >= 0 && projection.inputIndex < len(higher.Sequence) { higherItem := higher.Sequence[projection.inputIndex] - collectOverlayOriginsFrom(result.origins, incomingOrigins, higherItem, path.Index(projection.outputIndex), layer) + collectOverlayOriginsAt(result.origins, incomingOrigins, higherItem, path.Index(projection.inputIndex), path.Index(projection.outputIndex), layer) } } } @@ -416,17 +416,15 @@ func collectOverlayOrigins(origins map[string]Origin, node *document.Node, path } func collectOverlayOriginsFrom(origins, incomingOrigins map[string]Origin, node *document.Node, path Path, layer Layer) { - origins[path.String()] = incomingOrigin(incomingOrigins, path, node, layer) - switch node.Kind { - case document.Mapping: - for _, entry := range node.Mapping { - collectOverlayOriginsFrom(origins, incomingOrigins, entry.Value, path.Key(entry.Key), layer) - } - case document.Sequence: - for index, child := range node.Sequence { - collectOverlayOriginsFrom(origins, incomingOrigins, child, path.Index(index), layer) - } - } + collectOverlayOriginsAt(origins, incomingOrigins, node, path, path, layer) +} + +func collectOverlayOriginsAt(origins, incomingOrigins map[string]Origin, node *document.Node, from, to Path, layer Layer) { + walkDocument(node, Path{}, func(relative Path, value *document.Node) { + source := appendDestination(from, relative) + destination := appendDestination(to, relative) + origins[destination.String()] = incomingOrigin(incomingOrigins, source, value, layer) + }) } func mergeOverlayOrigin(origins map[string]Origin, path Path, location document.Location, layer Layer) { diff --git a/internal/engine/sequence_origin_test.go b/internal/engine/sequence_origin_test.go new file mode 100644 index 0000000..fcc5517 --- /dev/null +++ b/internal/engine/sequence_origin_test.go @@ -0,0 +1,100 @@ +package engine + +import ( + "io/fs" + "path" + "reflect" + "testing" + "testing/fstest" +) + +func TestSequenceUpsertProjectsIncomingOrigins(t *testing.T) { + t.Parallel() + type details struct { + Label string `yaml:"label"` + Resource string `yaml:"resource"` + } + type item struct { + ID string `yaml:"id"` + Details details `yaml:"details"` + } + type catalog struct { + Items []item `yaml:"items"` + } + type family struct { + Base catalog `yaml:"base"` + Child catalog `yaml:"child"` + } + for _, inheritance := range []bool{false, true} { + name := "layered" + destination := "[items]" + if inheritance { + name, destination = "inheritance", "[child, items]" + } + t.Run(name, func(t *testing.T) { + files := fstest.MapFS{} + for _, id := range []string{"a", "b", "c"} { + files[id+"/record.yml"] = &fstest.MapFile{Data: []byte("configkit: {imports: [{path: helper.yml}]}\nid: " + id + "\ndetails: {label: " + id + "}\n")} + files[id+"/helper.yml"] = &fstest.MapFile{Data: []byte("details: {resource: asset.txt}\n")} + files[id+"/asset.txt"] = &fstest.MapFile{Data: []byte(id + " asset")} + } + main := "configkit:\n sequences:\n - destination: " + destination + "\n key: id\n imports:\n - glob: '*/record.yml'\n destination: " + destination + "\n placement: sequence-item\n" + if inheritance { + main += "base:\n items: [{id: b}, {id: a}, {id: keep}]\nchild:\n extends: base\n" + } + files["main.yml"] = &fstest.MapFile{Data: []byte(main)} + source := NewFilesystem("catalog", files).File("main.yml") + var inspection *Inspection + var items []item + collection := Keys("items") + if inheritance { + loaded, err := testLoad(source, Options[family]{Imports: true}) + if err != nil { + t.Fatal(err) + } + inspection, items, collection = loaded.Inspection, loaded.Config.Child.Items, Keys("child", "items") + } else { + loaded, err := testGroupedLoad(loadRequest[catalog]{ + main: loadInput{source: source, imports: true}, + underlays: []loadGroup{{inputs: []loadInput{{source: Bytes("lower", []byte("items: [{id: b}, {id: a}, {id: keep}]\n"))}}}}, + }) + if err != nil { + t.Fatal(err) + } + inspection, items = loaded.Inspection, loaded.Config.Items + } + ids := make([]string, len(items)) + for index, item := range items { + ids[index] = item.ID + } + if !reflect.DeepEqual(ids, []string{"b", "a", "keep", "c"}) { + t.Fatalf("upsert order = %v", ids) + } + for _, index := range []int{0, 1, 3} { + item := items[index] + info, ok := inspection.Metadata().Lookup(collection.Index(index).Key("details")) + if !ok || len(info.Origin.Locations) != 2 { + t.Fatalf("%s composite origin = %#v, %v", item.ID, info, ok) + } + for _, location := range info.Origin.Locations { + context, ok := inspection.SourceContext(location.SourceID) + if !ok || context.BaseDir != item.ID { + t.Errorf("%s origin context = %#v, %v", item.ID, context, ok) + } + } + resource, ok := inspection.Metadata().Lookup(collection.Index(index).Key("details").Key("resource")) + if !ok || len(resource.Origin.Locations) != 1 { + t.Fatalf("%s resource origin = %#v", item.ID, resource) + } + context, ok := inspection.SourceContext(resource.Origin.Locations[0].SourceID) + if !ok { + t.Fatal("resource source context missing") + } + data, err := fs.ReadFile(context.FS, path.Join(context.BaseDir, item.Details.Resource)) + if err != nil || string(data) != item.ID+" asset" { + t.Errorf("%s declaring resource = %q, %v", item.ID, data, err) + } + } + }) + } +}