Reset inherited YAML identity on actual replacement
This commit is contained in:
@@ -169,7 +169,14 @@ matches, and nested helpers. Check version-evidence projection where applicable.
|
|||||||
|
|
||||||
## 5. Honor Identity Reset On Actual Inheritance Replacement — P2
|
## 5. Honor Identity Reset On Actual Inheritance Replacement — P2
|
||||||
|
|
||||||
Status: [ ] Open.
|
Status: [x] Complete.
|
||||||
|
|
||||||
|
Completion: inheritance resolves identity against actual child-value presence,
|
||||||
|
discarding parent YAML identity for an explicit replacement while preserving
|
||||||
|
agreement for retained values and authoritative application constraints. Added
|
||||||
|
policy-state and public-load regressions for changed/unkeyed/non-sequence
|
||||||
|
replacements, application agreement/conflict, and mode-only/append/upsert cases.
|
||||||
|
Verified focused inheritance tests and `./scripts/check.sh`.
|
||||||
|
|
||||||
Owners: [`inheritanceCollectionResolution` and `mergeInheritanceCollectionPolicies`](../../internal/engine/inheritance_collection.go),
|
Owners: [`inheritanceCollectionResolution` and `mergeInheritanceCollectionPolicies`](../../internal/engine/inheritance_collection.go),
|
||||||
especially lines 79–80 and 121.
|
especially lines 79–80 and 121.
|
||||||
|
|||||||
@@ -59,6 +59,7 @@ func inheritanceCollectionResolution(
|
|||||||
path Path,
|
path Path,
|
||||||
lowerScope, higherScope collectionPolicyScope,
|
lowerScope, higherScope collectionPolicyScope,
|
||||||
applicationPolicy sequencePolicy,
|
applicationPolicy sequencePolicy,
|
||||||
|
incoming *document.Node,
|
||||||
) (collectionPolicyResolution, collectionPolicy, collectionPolicy, bool, []Diagnostic) {
|
) (collectionPolicyResolution, collectionPolicy, collectionPolicy, bool, []Diagnostic) {
|
||||||
lower, hasLower := lowerScope.at(path)
|
lower, hasLower := lowerScope.at(path)
|
||||||
higher, hasHigher := higherScope.at(path)
|
higher, hasHigher := higherScope.at(path)
|
||||||
@@ -72,6 +73,11 @@ func inheritanceCollectionResolution(
|
|||||||
lowerForResolution.explicitMode = 0
|
lowerForResolution.explicitMode = 0
|
||||||
lowerForResolution.explicitModePresent = false
|
lowerForResolution.explicitModePresent = false
|
||||||
lowerForResolution.modeLocation = nil
|
lowerForResolution.modeLocation = nil
|
||||||
|
if incoming != nil && higher.explicitModePresent && higher.explicitMode == sequenceModeReplace {
|
||||||
|
// Actual replacement discards the parent's YAML identity before
|
||||||
|
// agreement. A mode-only declaration still governs retained records.
|
||||||
|
lowerForResolution = collectionPolicy{}
|
||||||
|
}
|
||||||
var lowerPointer *collectionPolicy
|
var lowerPointer *collectionPolicy
|
||||||
if hasLower {
|
if hasLower {
|
||||||
lowerPointer = &lowerForResolution
|
lowerPointer = &lowerForResolution
|
||||||
@@ -85,7 +91,7 @@ func inheritanceCollectionResolution(
|
|||||||
func mergeInheritanceCollectionPolicies(
|
func mergeInheritanceCollectionPolicies(
|
||||||
parent, child collectionPolicyScope,
|
parent, child collectionPolicyScope,
|
||||||
parentRootPath Path,
|
parentRootPath Path,
|
||||||
parentValue *document.Node,
|
parentValue, childValue *document.Node,
|
||||||
applicationPolicy sequencePolicy,
|
applicationPolicy sequencePolicy,
|
||||||
) (collectionPolicyScope, []Diagnostic) {
|
) (collectionPolicyScope, []Diagnostic) {
|
||||||
result := cloneCollectionPolicyScopeForInheritance(parent)
|
result := cloneCollectionPolicyScopeForInheritance(parent)
|
||||||
@@ -98,7 +104,13 @@ func mergeInheritanceCollectionPolicies(
|
|||||||
}
|
}
|
||||||
var diagnostics []Diagnostic
|
var diagnostics []Diagnostic
|
||||||
for _, path := range paths {
|
for _, path := range paths {
|
||||||
resolution, lower, _, active, issues := inheritanceCollectionResolution(path, parent, child, applicationPolicy)
|
relative, inside := relativeHistoryPath(parentRootPath, path)
|
||||||
|
var retained, incoming *document.Node
|
||||||
|
if inside {
|
||||||
|
retained, _ = documentAtPath(parentValue, relative)
|
||||||
|
incoming, _ = documentAtPath(childValue, relative)
|
||||||
|
}
|
||||||
|
resolution, lower, _, active, issues := inheritanceCollectionResolution(path, parent, child, applicationPolicy, incoming)
|
||||||
if len(issues) != 0 {
|
if len(issues) != 0 {
|
||||||
diagnostics = append(diagnostics, issues...)
|
diagnostics = append(diagnostics, issues...)
|
||||||
continue
|
continue
|
||||||
@@ -113,16 +125,15 @@ func mergeInheritanceCollectionPolicies(
|
|||||||
lower.modeLocation = nil
|
lower.modeLocation = nil
|
||||||
lowerPointer = &lower
|
lowerPointer = &lower
|
||||||
}
|
}
|
||||||
relative, inside := relativeHistoryPath(parentRootPath, path)
|
policy := resolution.policy
|
||||||
var retained *document.Node
|
if incoming == nil || resolution.operation != sequenceModeReplace {
|
||||||
if inside {
|
var foundDiagnostics []Diagnostic
|
||||||
retained, _ = documentAtPath(parentValue, relative)
|
policy, foundDiagnostics = collectionPolicyWithoutValue(lowerPointer, resolution.policy, retained, path)
|
||||||
}
|
|
||||||
policy, foundDiagnostics := collectionPolicyWithoutValue(lowerPointer, resolution.policy, retained, path)
|
|
||||||
if len(foundDiagnostics) != 0 {
|
if len(foundDiagnostics) != 0 {
|
||||||
diagnostics = append(diagnostics, foundDiagnostics...)
|
diagnostics = append(diagnostics, foundDiagnostics...)
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
}
|
||||||
if higher, ok := child.at(path); ok {
|
if higher, ok := child.at(path); ok {
|
||||||
policy = inheritanceRecordPoliciesFromHigher(policy, higher)
|
policy = inheritanceRecordPoliciesFromHigher(policy, higher)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,11 +1,99 @@
|
|||||||
package engine
|
package engine
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"reflect"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"gitea.maximumdirect.net/eric/ConfigKit/internal/document"
|
"gitea.maximumdirect.net/eric/ConfigKit/internal/document"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
func TestInheritanceReplacementResetsOnlyReplacedYamlIdentity(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
for _, test := range []struct {
|
||||||
|
name, key, value, applicationKey string
|
||||||
|
mode sequenceMode
|
||||||
|
wantFields []string
|
||||||
|
conflict bool
|
||||||
|
}{
|
||||||
|
{name: "changed key", key: "name", value: "[{name: child}]", mode: sequenceModeReplace, wantFields: []string{"name"}},
|
||||||
|
{name: "unkeyed replacement", value: "[{name: child}]", mode: sequenceModeReplace},
|
||||||
|
{name: "non-sequence replacement", value: "42", mode: sequenceModeReplace},
|
||||||
|
{name: "agreeing application", key: "name", value: "[{name: child}]", applicationKey: "name", mode: sequenceModeReplace, wantFields: []string{"name"}},
|
||||||
|
{name: "conflicting application", key: "name", value: "[{name: child}]", applicationKey: "id", mode: sequenceModeReplace, conflict: true},
|
||||||
|
{name: "application still validates replacement", value: "[{name: child}]", applicationKey: "id", mode: sequenceModeReplace, conflict: true},
|
||||||
|
{name: "mode-only key change", key: "name", mode: sequenceModeReplace, conflict: true},
|
||||||
|
{name: "mode-only keeps identity", mode: sequenceModeReplace, wantFields: []string{"id"}},
|
||||||
|
{name: "append key change", key: "name", value: "[{name: child}]", mode: sequenceModeAppend, conflict: true},
|
||||||
|
{name: "upsert key change", key: "name", value: "[{name: child}]", mode: sequenceModeUpsert, conflict: true},
|
||||||
|
} {
|
||||||
|
t.Run(test.name, func(t *testing.T) {
|
||||||
|
input := "base: {items: [{id: parent}]}\nchild:\n extends: base\n"
|
||||||
|
if test.value != "" {
|
||||||
|
input += " items: " + test.value + "\n"
|
||||||
|
}
|
||||||
|
root := parseItemPolicyDocument(t, "replacement", input)
|
||||||
|
policies := collectionPolicyScope{}
|
||||||
|
policies.set(Keys("base", "items"), collectionPolicy{identityFields: []string{"id"}, identityPresent: true})
|
||||||
|
higher := collectionPolicy{explicitMode: test.mode, explicitModePresent: true}
|
||||||
|
if test.key != "" {
|
||||||
|
higher.identityFields, higher.identityPresent = []string{test.key}, true
|
||||||
|
}
|
||||||
|
policies.set(Keys("child", "items"), higher)
|
||||||
|
application := sequencePolicy{}
|
||||||
|
if test.applicationKey != "" {
|
||||||
|
application = mustCompileSequenceIdentityPolicy(t, []sequenceIdentityRegistration{{
|
||||||
|
selector: sequenceIdentitySelector{literalSequenceSelector("child"), literalSequenceSelector("items")}, identityFields: []string{test.applicationKey},
|
||||||
|
}})
|
||||||
|
}
|
||||||
|
expanded, diagnostics := expandInheritanceWithCollectionRules(root, nil, policies, application, defaultProjectionLimits())
|
||||||
|
if test.conflict {
|
||||||
|
wantCode := CodeSequenceConflict
|
||||||
|
if test.applicationKey != "" && test.key == "" {
|
||||||
|
wantCode = CodeSequenceInvalid
|
||||||
|
}
|
||||||
|
if len(diagnostics) == 0 || diagnostics[0].Code != wantCode || diagnostics[0].Path.String() != "/child/items" && diagnostics[0].Path.String() != "/child/items/0/id" {
|
||||||
|
t.Fatalf("missing identity/constraint failure: %#v", diagnostics)
|
||||||
|
}
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if len(diagnostics) != 0 {
|
||||||
|
t.Fatal(diagnostics)
|
||||||
|
}
|
||||||
|
policy, found := expanded.collectionPolicies.at(Keys("child", "items"))
|
||||||
|
if !found || !reflect.DeepEqual(policy.identityFields, test.wantFields) || policy.identityPresent != (test.wantFields != nil) {
|
||||||
|
t.Fatalf("effective YAML identity = %#v, want %v", policy, test.wantFields)
|
||||||
|
}
|
||||||
|
if test.value != "" {
|
||||||
|
actual, _ := documentAtPath(expanded.root, Keys("child", "items"))
|
||||||
|
want, _ := documentAtPath(root, Keys("child", "items"))
|
||||||
|
if !configurationValuesEqual(actual, want) {
|
||||||
|
t.Fatal("explicit replacement retained parent payload")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestLoadInheritanceReplacementUsesNewYamlKey(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
loaded, err := testLoad[map[string]any](Bytes("replacement", []byte(`configkit:
|
||||||
|
sequences:
|
||||||
|
- {destination: [base, items], key: id}
|
||||||
|
- {destination: [child, items], key: name, mode: replace}
|
||||||
|
base: {items: [{id: parent}]}
|
||||||
|
child:
|
||||||
|
extends: base
|
||||||
|
items: [{name: child}]
|
||||||
|
`)))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
value, found := loaded.Explain(Keys("child", "items"))
|
||||||
|
if !found || value.Effective.YAML != "- name: child\n" {
|
||||||
|
t.Fatalf("replacement payload = %#v", value)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestInheritanceCombinesIdentifiedSequencesAtDestination(t *testing.T) {
|
func TestInheritanceCombinesIdentifiedSequencesAtDestination(t *testing.T) {
|
||||||
root := parseItemPolicyDocument(t, "inheritance-values", `base:
|
root := parseItemPolicyDocument(t, "inheritance-values", `base:
|
||||||
items:
|
items:
|
||||||
|
|||||||
@@ -222,7 +222,7 @@ func applyInheritanceScopeTemplates(adapter inheritanceScopeAdapter, order []int
|
|||||||
childPolicyScope := projectCurrentInheritanceCollectionPolicies(templatePolicies, node.path)
|
childPolicyScope := projectCurrentInheritanceCollectionPolicies(templatePolicies, node.path)
|
||||||
merged := inheritanceParentPayload(parent, node.identityFields)
|
merged := inheritanceParentPayload(parent, node.identityFields)
|
||||||
mergedPolicyScope, policyDiagnostics := mergeInheritanceCollectionPolicies(
|
mergedPolicyScope, policyDiagnostics := mergeInheritanceCollectionPolicies(
|
||||||
parentPolicyScope, childPolicyScope, node.path, parent, adapter.applicationPolicy(),
|
parentPolicyScope, childPolicyScope, node.path, parent, payload, adapter.applicationPolicy(),
|
||||||
)
|
)
|
||||||
if len(policyDiagnostics) != 0 {
|
if len(policyDiagnostics) != 0 {
|
||||||
return nil, policyDiagnostics
|
return nil, policyDiagnostics
|
||||||
@@ -258,7 +258,7 @@ func applyInheritanceScopeTemplates(adapter inheritanceScopeAdapter, order []int
|
|||||||
},
|
},
|
||||||
merge: func(childPath Path, existing, incoming *document.Node) (*document.Node, bool) {
|
merge: func(childPath Path, existing, incoming *document.Node) (*document.Node, bool) {
|
||||||
resolution, _, _, active, issues := inheritanceCollectionResolution(
|
resolution, _, _, active, issues := inheritanceCollectionResolution(
|
||||||
childPath, parentPolicyScope, childPolicyScope, adapter.applicationPolicy(),
|
childPath, parentPolicyScope, childPolicyScope, adapter.applicationPolicy(), incoming,
|
||||||
)
|
)
|
||||||
if len(issues) != 0 {
|
if len(issues) != 0 {
|
||||||
combinationDiagnostics = append(combinationDiagnostics, issues...)
|
combinationDiagnostics = append(combinationDiagnostics, issues...)
|
||||||
|
|||||||
Reference in New Issue
Block a user