diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index dccef0d..5acc494 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -347,6 +347,13 @@ Findings: **D04, F05**. Owners: `internal/engine/sequence_combine.go`, `inheritance*.go`, `compose.go`, `history.go`, `secret.go`, `destination.go`, and associated tests. +Status: **complete** (D04, F05). Prior completed commits: `2736ddf`, +`deb9aa5`, `ae3408a`, `ea27f17`, `08a8c25`, `877c497`, `8aba43d`, +`7bf2f0c`, `0f9ecf1`, `119b42c`. Validation passed: +`env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'Sequence|Collection|Inheritance|History|Secret|Destination' -count=1`, +`env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -count=1`, +and `git diff --check`. + Remove unread `discardedLower` and `inheritanceReplacement.path`, `previous`, and `effective`, plus calculations solely producing them. Keep replacement `before` and `after` trees for accounting, projections, active origins/history, and append's diff --git a/internal/engine/compose.go b/internal/engine/compose.go index efb26ea..968d685 100644 --- a/internal/engine/compose.go +++ b/internal/engine/compose.go @@ -483,21 +483,6 @@ func allMappingClaims(claims []compositionClaim) bool { return true } -func mergeDisjointMappings(target, incoming *document.Node, index *mappingIndex) { - for _, incomingEntry := range incoming.Mapping { - position := index.find(target, incomingEntry.Key) - if position < 0 { - cloned := incomingEntry - cloned.Value = document.Clone(incomingEntry.Value) - index.append(target, cloned) - continue - } - // Claim validation guarantees that the only remaining overlap is a - // mapping pair. Empty mappings naturally contribute no child entries. - mergeDisjointMappings(target.Mapping[position].Value, incomingEntry.Value, index) - } -} - func documentEntryIndex(entries []document.Entry, key string) int { for index := range entries { if entries[index].Key == key { diff --git a/internal/engine/destination.go b/internal/engine/destination.go index 7bdceb2..efdd4c6 100644 --- a/internal/engine/destination.go +++ b/internal/engine/destination.go @@ -18,13 +18,6 @@ const ( placementModeSequenceItem ) -// PlacementModeFromFacade preserves root-owned numeric values until preflight -// can reject unsupported modes. -func PlacementModeFromFacade(value uint8) placementMode { return placementMode(value) } - -// PlacementModeForFacade converts private source evidence to the root-owned mode. -func PlacementModeForFacade(value placementMode) uint8 { return uint8(value) } - // InputWithPlacement copies the root-owned mode into a private engine input. func InputWithPlacement(input Input, value uint8) Input { input.placementMode = placementMode(value) diff --git a/internal/engine/history.go b/internal/engine/history.go index a0fc903..758410b 100644 --- a/internal/engine/history.go +++ b/internal/engine/history.go @@ -237,10 +237,6 @@ func applyHistoryApplication(root **document.Node, application historyApplicatio mergeHistoryAtPath(root, application.path, application.value, index, application.sequencesByPath) } -func mergeHistoryDocument(target **document.Node, incoming *document.Node, index *mappingIndex) { - mergeHistoryDocumentAtPath(target, incoming, Path{}, index, nil) -} - func mergeHistoryDocumentAtPath(target **document.Node, incoming *document.Node, path Path, index *mappingIndex, sequences map[string]sequenceHistoryContributionIndex) { if incoming == nil { return diff --git a/internal/engine/inheritance.go b/internal/engine/inheritance.go index 8436193..eb32d8f 100644 --- a/internal/engine/inheritance.go +++ b/internal/engine/inheritance.go @@ -129,15 +129,11 @@ func projectInheritancePolicyTemplates(templates []collectionPolicyScope, prefix return projected } -// A replacement retains the displaced source/derivation record while the -// effective origin belongs to the child. Value trees are borrowed immutable -// inputs or detached templates and are budgeted with retained history later. +// A replacement retains the before and after values for history accounting. +// The trees are borrowed immutable inputs or detached templates. type inheritanceReplacement struct { - path Path - before *document.Node - after *document.Node - previous []displacedDefinition - effective Origin + before *document.Node + after *document.Node } // A link represents one shared relationship edge. Each contributor retains @@ -173,11 +169,6 @@ func inheritanceDirective(node *document.Node) (document.Entry, bool) { return document.Entry{}, false } -func inheritanceDirectiveLocation(node *document.Node) Location { - entry, _ := inheritanceDirective(node) - return publicLocation(entry.Value.Location) -} - func withoutInheritanceDirective(node *document.Node) *document.Node { payload := *node payload.Mapping = make([]document.Entry, 0, len(node.Mapping)) diff --git a/internal/engine/inheritance_collection.go b/internal/engine/inheritance_collection.go index 6af19bf..d229aad 100644 --- a/internal/engine/inheritance_collection.go +++ b/internal/engine/inheritance_collection.go @@ -237,10 +237,8 @@ func applyInheritanceSequenceOrigins( var replacements []inheritanceReplacement if operation == sequenceModeReplace { if lower != nil { - displaced := displacedOrigins(lower, origins, path) replacements = append(replacements, inheritanceReplacement{ - path: path.clone(), before: lower, after: higher, - previous: displaced, effective: cloneOrigin(incomingOrigins[path.String()]), + before: lower, after: higher, }) deleteOriginSubtree(origins, path, lower) } @@ -265,12 +263,10 @@ func applyInheritanceSequenceOrigins( lowerPath := path.Index(replacement.lowerIndex) higherPath := path.Index(replacement.higherIndex) outputPath := path.Index(replacement.outputIndex) - previous := displacedOrigins(lower.Sequence[replacement.lowerIndex], origins, lowerPath) deleteOriginSubtree(origins, lowerPath, lower.Sequence[replacement.lowerIndex]) copyInheritanceSubtreeAt(origins, incomingOrigins, higher.Sequence[replacement.higherIndex], higherPath, outputPath) replacements = append(replacements, inheritanceReplacement{ - path: outputPath, before: lower.Sequence[replacement.lowerIndex], after: higher.Sequence[replacement.higherIndex], - previous: previous, effective: cloneOrigin(incomingOrigins[higherPath.String()]), + before: lower.Sequence[replacement.lowerIndex], after: higher.Sequence[replacement.higherIndex], }) } for _, projection := range combined.higherItems { diff --git a/internal/engine/inheritance_combine.go b/internal/engine/inheritance_combine.go index 33271b1..ce2d33d 100644 --- a/internal/engine/inheritance_combine.go +++ b/internal/engine/inheritance_combine.go @@ -55,10 +55,8 @@ func combineInheritanceTemplate( mergeInheritanceOrigin(mergedOrigins, childPath, child.origins[childPath.String()]) }, replaced: func(childPath Path, existing, incoming *document.Node) { - previous := displacedOrigins(existing, mergedOrigins, childPath) result.replacements = append(result.replacements, inheritanceReplacement{ - path: childPath.clone(), before: existing, after: incoming, - previous: previous, effective: cloneOrigin(child.origins[childPath.String()]), + before: existing, after: incoming, }) deleteOriginSubtree(mergedOrigins, childPath, existing) copyInheritanceSubtree(mergedOrigins, child.origins, incoming, childPath) diff --git a/internal/engine/inheritance_identified_test.go b/internal/engine/inheritance_identified_test.go index c62b6d6..3e6a3e4 100644 --- a/internal/engine/inheritance_identified_test.go +++ b/internal/engine/inheritance_identified_test.go @@ -198,11 +198,6 @@ func TestIdentifiedInheritanceResolvesNestedCollectionsAtEachDestination(t *test if _, found := expanded.origins["/jobs/0/steps/1/value"]; !found { t.Fatal("nested inherited value did not retain its destination origin") } - for _, replacement := range expanded.replacements { - if replacement.path.String() == "/jobs/0/id" { - t.Fatal("parent identity was treated as payload replaced by the child's identity") - } - } } func TestIdentifiedInheritanceRespectsOutputNodeBudget(t *testing.T) { diff --git a/internal/engine/inheritance_origin_test.go b/internal/engine/inheritance_origin_test.go index bec8a98..6d3c534 100644 --- a/internal/engine/inheritance_origin_test.go +++ b/internal/engine/inheritance_origin_test.go @@ -44,11 +44,8 @@ func TestInheritedOriginsKeepSourcePathsPresenceAndIndependentReplacement(t *tes if len(count.Locations) != 1 || count.Locations[0].SourceID != "source-2" || len(projectInheritanceSteps(count)) != 0 { t.Fatalf("direct replacement origin = %#v", count) } - if len(result.replacements) != 1 || result.replacements[0].path.String() != "/child/count" || - result.replacements[0].before.Text != "0" || result.replacements[0].after.Text != "3" || - len(result.replacements[0].previous) != 1 || - len(projectInheritanceSteps(result.replacements[0].previous[0].origin)) != 1 { - t.Fatalf("displaced inheritance = %#v", result.replacements) + if len(result.replacements) != 1 || result.replacements[0].before.Text != "0" || result.replacements[0].after.Text != "3" { + t.Fatalf("replacement accounting values = %#v", result.replacements) } settings := result.origins["/child/settings"] if len(settings.Locations) != 2 || settings.Locations[0].SourceID != "source-1" || diff --git a/internal/engine/secret.go b/internal/engine/secret.go index c38735b..0f4bcc1 100644 --- a/internal/engine/secret.go +++ b/internal/engine/secret.go @@ -134,12 +134,6 @@ func classifySecretPathsWithEvidence(schema *schemaDescriptor, predicate func(Pa return closeSecretDependencies(paths, secret, dependencies, inherited) } -func addSecretEvidencePath(paths map[string]Path, destination, local Path) Path { - path := appendDestination(destination, local) - addSecretPath(paths, path.String()) - return path -} - func addSecretPath(paths map[string]Path, pointer string) { if existing, found := paths[pointer]; found && existing.valid() { // Accumulator keys already match their paths' canonical spelling. History diff --git a/internal/engine/sequence_combine.go b/internal/engine/sequence_combine.go index a63f66d..41ac3c1 100644 --- a/internal/engine/sequence_combine.go +++ b/internal/engine/sequence_combine.go @@ -23,7 +23,6 @@ type sequenceCombinationResult struct { higherItems []sequenceItemProjection replacements []sequenceItemReplacement appendedHigher []int - discardedLower []int } func combineSequence( @@ -36,7 +35,7 @@ func combineSequence( } switch policy.operation { case sequenceModeReplace: - return replaceSequence(lower, higher, policy.effectiveIdentityFields, context) + return replaceSequence(higher, policy.effectiveIdentityFields, context) case sequenceModeAppend, sequenceModeUpsert: return combineIdentifiedSequences(lower, higher, policy.operation, policy.effectiveIdentityFields, context) default: @@ -55,7 +54,7 @@ func unchangedSequenceValue(lower *document.Node) sequenceCombinationResult { } func replaceSequence( - lower, higher *document.Node, + higher *document.Node, identityFields []string, context sequenceCombinationContext, ) (sequenceCombinationResult, []Diagnostic) { @@ -68,9 +67,6 @@ func replaceSequence( if higher.Kind == document.Sequence { result.higherItems = sequenceItemProjections(len(higher.Sequence), 0) } - if lower != nil && lower.Kind == document.Sequence { - result.discardedLower = sequenceIndexes(len(lower.Sequence)) - } return result, nil } diff --git a/internal/engine/sequence_combine_test.go b/internal/engine/sequence_combine_test.go index eaea064..ed5140c 100644 --- a/internal/engine/sequence_combine_test.go +++ b/internal/engine/sequence_combine_test.go @@ -19,13 +19,12 @@ func TestCombineSequenceOperationsAndItemEvidence(t *testing.T) { wantHigher []sequenceItemProjection wantReplacements []sequenceItemReplacement wantAppended []int - wantDiscarded []int }{ { name: "replace uses only higher records", lower: "items: [{id: a}, {id: b}]\n", higher: "items: [{id: b, text: revised}]\n", operation: sequenceModeReplace, want: []string{"b"}, - wantHigher: []sequenceItemProjection{{inputIndex: 0, outputIndex: 0}}, wantDiscarded: []int{0, 1}, + wantHigher: []sequenceItemProjection{{inputIndex: 0, outputIndex: 0}}, }, { name: "append retains then appends", @@ -57,7 +56,7 @@ func TestCombineSequenceOperationsAndItemEvidence(t *testing.T) { if got := sequenceIdentityValues(result.value, "id"); !reflect.DeepEqual(got, test.want) { t.Fatalf("combined IDs = %#v, want %#v", got, test.want) } - if !reflect.DeepEqual(result.lowerItems, test.wantLower) || !reflect.DeepEqual(result.higherItems, test.wantHigher) || !reflect.DeepEqual(result.replacements, test.wantReplacements) || !reflect.DeepEqual(result.appendedHigher, test.wantAppended) || !reflect.DeepEqual(result.discardedLower, test.wantDiscarded) { + if !reflect.DeepEqual(result.lowerItems, test.wantLower) || !reflect.DeepEqual(result.higherItems, test.wantHigher) || !reflect.DeepEqual(result.replacements, test.wantReplacements) || !reflect.DeepEqual(result.appendedHigher, test.wantAppended) { t.Fatalf("item evidence = %#v", result) } }) @@ -112,7 +111,7 @@ func TestReplaceSequenceAllowsNullAndNonSequenceValues(t *testing.T) { lower := sequenceFromYAML(t, "items: [{id: lower}]\n") higher := sequenceFromYAML(t, test.higher) result, diagnostics := combineSequence(lower, higher, sequenceCombinationPolicy(sequenceModeReplace), sequenceCombinationContext{destination: Keys("items")}) - if len(diagnostics) != 0 || result.value == nil || result.value.Kind != test.wantKind || !reflect.DeepEqual(result.discardedLower, []int{0}) { + if len(diagnostics) != 0 || result.value == nil || result.value.Kind != test.wantKind { t.Fatalf("replacement result = %#v, %#v", result, diagnostics) } }) @@ -172,7 +171,7 @@ func TestCombineSequenceReplacementResetsLowerIdentity(t *testing.T) { higher := sequenceFromYAML(t, "items: [{name: new}]\n") policy := sequenceCombinationPolicy(sequenceModeReplace, "name") result, diagnostics := combineSequence(lower, higher, policy, sequenceCombinationContext{destination: Keys("items")}) - if len(diagnostics) != 0 || !reflect.DeepEqual(sequenceIdentityValues(result.value, "name"), []string{"new"}) || !reflect.DeepEqual(result.discardedLower, []int{0}) { + if len(diagnostics) != 0 || !reflect.DeepEqual(sequenceIdentityValues(result.value, "name"), []string{"new"}) { t.Fatalf("replacement applied obsolete identity: %#v, %#v", result, diagnostics) } }