Limit inheritance policy work to the relevant scope
This commit is contained in:
@@ -847,7 +847,16 @@ do not change override semantics or impose batch limits.
|
||||
|
||||
## 21. Localize Inheritance Policy Work To The Relevant Scope — P2
|
||||
|
||||
Status: [ ] Open.
|
||||
Status: [x] Complete.
|
||||
|
||||
Completion: inheritance now keeps a per-load semantic path index, including
|
||||
separate indexes for record-owned scopes. Walker handoff and descendant
|
||||
retirement touch the changed scope; mapping and identified-record preparation
|
||||
reuse indexes across sibling queries. Only relevant policies are cloned into
|
||||
independent templates. Added wide independent YAML groups, pending-policy
|
||||
replacement, literal-key/index distinctions, and ownership regressions. Existing
|
||||
nested item/inheritance, mode-lifetime, and application-constraint coverage,
|
||||
`./scripts/check.sh`, and `go test -race ./internal/engine` pass.
|
||||
|
||||
Owners: `inheritancePolicyScopeAtPath` in
|
||||
[`inheritance.go`](../../internal/engine/inheritance.go), graph preparation in
|
||||
|
||||
@@ -114,26 +114,7 @@ func cloneInheritanceOperations(operations []inheritanceOperation) []inheritance
|
||||
}
|
||||
|
||||
func inheritancePolicyScopeAtPath(scope collectionPolicyScope, prefix Path) collectionPolicyScope {
|
||||
localized := collectionPolicyScope{}
|
||||
for _, entry := range scope.collections {
|
||||
if relative, inside := relativeHistoryPath(prefix, entry.destination); inside {
|
||||
localized.set(relative, cloneCollectionPolicyForInheritance(entry.policy))
|
||||
continue
|
||||
}
|
||||
relative, inside := relativeHistoryPath(entry.destination, prefix)
|
||||
if !inside || len(relative.segments) == 0 || relative.segments[0].kind != pathSegmentIndex {
|
||||
continue
|
||||
}
|
||||
record, found := entry.policy.recordPolicyAt(relative.segments[0].index)
|
||||
if !found {
|
||||
continue
|
||||
}
|
||||
remainder := Path{segments: append([]pathSegment(nil), relative.segments[1:]...)}
|
||||
for _, nested := range inheritancePolicyScopeAtPath(record.scope, remainder).collections {
|
||||
localized.set(nested.destination, cloneCollectionPolicyForInheritance(nested.policy))
|
||||
}
|
||||
}
|
||||
return localized
|
||||
return newInheritancePolicyIndex(scope).localize(prefix)
|
||||
}
|
||||
|
||||
func projectInheritancePolicyTemplates(templates []collectionPolicyScope, prefix Path) []projectedCollectionPolicy {
|
||||
|
||||
123
internal/engine/inheritance_policy_index.go
Normal file
123
internal/engine/inheritance_policy_index.go
Normal file
@@ -0,0 +1,123 @@
|
||||
package engine
|
||||
|
||||
// inheritancePolicyIndex borrows immutable policy entries. Semantic path
|
||||
// segments keep mapping keys distinct from sequence indexes. Record-owned
|
||||
// scopes have their own indexes, just as they have their own policy tables.
|
||||
type inheritancePolicyIndex struct {
|
||||
entry *collectionPolicyEntry
|
||||
children map[pathSegment]*inheritancePolicyIndex
|
||||
records map[int]*inheritancePolicyIndex
|
||||
}
|
||||
|
||||
func newInheritancePolicyIndex(scope collectionPolicyScope) *inheritancePolicyIndex {
|
||||
index := &inheritancePolicyIndex{}
|
||||
for _, entry := range scope.collections {
|
||||
index.set(entry)
|
||||
}
|
||||
return index
|
||||
}
|
||||
|
||||
func (index *inheritancePolicyIndex) set(entry collectionPolicyEntry) {
|
||||
node := index
|
||||
for _, segment := range entry.destination.segments {
|
||||
if node.children == nil {
|
||||
node.children = make(map[pathSegment]*inheritancePolicyIndex)
|
||||
}
|
||||
if node.children[segment] == nil {
|
||||
node.children[segment] = &inheritancePolicyIndex{}
|
||||
}
|
||||
node = node.children[segment]
|
||||
}
|
||||
node.entry = &entry
|
||||
node.records = nil
|
||||
for position, owner := range entry.policy.recordOrder {
|
||||
scope := entry.policy.recordPolicies[owner]
|
||||
if len(scope.collections) == 0 {
|
||||
continue
|
||||
}
|
||||
if node.records == nil {
|
||||
node.records = make(map[int]*inheritancePolicyIndex)
|
||||
}
|
||||
node.records[position] = newInheritancePolicyIndex(scope)
|
||||
}
|
||||
}
|
||||
|
||||
// localize copies only policies in the requested scope. Earlier record-owned
|
||||
// policies are considered before direct entries added by destination expansion.
|
||||
func (index *inheritancePolicyIndex) localize(prefix Path) collectionPolicyScope {
|
||||
localized := collectionPolicyScope{}
|
||||
node := index
|
||||
for offset, segment := range prefix.segments {
|
||||
if node == nil {
|
||||
return localized
|
||||
}
|
||||
if segment.kind == pathSegmentIndex {
|
||||
if record := node.records[segment.index]; record != nil {
|
||||
remainder := Path{segments: prefix.segments[offset+1:]}
|
||||
for _, entry := range record.localize(remainder).collections {
|
||||
localized.set(entry.destination, entry.policy)
|
||||
}
|
||||
}
|
||||
}
|
||||
node = node.children[segment]
|
||||
}
|
||||
var collect func(*inheritancePolicyIndex)
|
||||
collect = func(node *inheritancePolicyIndex) {
|
||||
if node == nil {
|
||||
return
|
||||
}
|
||||
if node.entry != nil {
|
||||
relative, _ := relativeHistoryPath(prefix, node.entry.destination)
|
||||
localized.set(relative, cloneCollectionPolicyForInheritance(node.entry.policy))
|
||||
}
|
||||
for _, child := range node.children {
|
||||
collect(child)
|
||||
}
|
||||
}
|
||||
collect(node)
|
||||
return localized
|
||||
}
|
||||
|
||||
func (index *inheritancePolicyIndex) absoluteScope(prefix Path) collectionPolicyScope {
|
||||
scope := collectionPolicyScope{}
|
||||
for _, entry := range index.localize(prefix).collections {
|
||||
scope.set(appendDestination(prefix, entry.destination), entry.policy)
|
||||
}
|
||||
return scope
|
||||
}
|
||||
|
||||
// replace updates the walker's owned table and its borrowed index together.
|
||||
// Retirement visits only direct entries below the changed destination.
|
||||
func (index *inheritancePolicyIndex) replace(scope *collectionPolicyScope, prefix Path, replacement collectionPolicyScope) {
|
||||
node := index
|
||||
var parent *inheritancePolicyIndex
|
||||
var last pathSegment
|
||||
for _, segment := range prefix.segments {
|
||||
if node == nil {
|
||||
break
|
||||
}
|
||||
parent, last, node = node, segment, node.children[segment]
|
||||
}
|
||||
var remove func(*inheritancePolicyIndex)
|
||||
remove = func(node *inheritancePolicyIndex) {
|
||||
if node == nil {
|
||||
return
|
||||
}
|
||||
if node.entry != nil {
|
||||
delete(scope.collections, node.entry.destination.String())
|
||||
}
|
||||
for _, child := range node.children {
|
||||
remove(child)
|
||||
}
|
||||
}
|
||||
remove(node)
|
||||
if parent == nil {
|
||||
*index = inheritancePolicyIndex{}
|
||||
} else {
|
||||
delete(parent.children, last)
|
||||
}
|
||||
for _, entry := range replacement.collections {
|
||||
scope.set(entry.destination, entry.policy)
|
||||
index.set(scope.collections[entry.destination.String()])
|
||||
}
|
||||
}
|
||||
111
internal/engine/inheritance_policy_index_test.go
Normal file
111
internal/engine/inheritance_policy_index_test.go
Normal file
@@ -0,0 +1,111 @@
|
||||
package engine
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestInheritancePolicyIndexKeepsSemanticScopesAndOwnedTemplates(t *testing.T) {
|
||||
policy := collectionPolicy{identityFields: []string{"id"}, identityPresent: true}
|
||||
nested := collectionPolicyScope{}
|
||||
nested.set(Keys("group", "base", "items"), policy)
|
||||
outer := policy
|
||||
record := newCollectionRecordPolicy()
|
||||
record.scope = nested
|
||||
outer.setRecordPolicyAt(0, record)
|
||||
scope := collectionPolicyScope{}
|
||||
scope.set(Keys("records"), outer)
|
||||
scope.set(Keys("numeric", "0", "items"), policy)
|
||||
index := newInheritancePolicyIndex(scope)
|
||||
|
||||
first := index.localize(Keys("records").Index(0).Key("group"))
|
||||
second := index.localize(Keys("records").Index(0).Key("group"))
|
||||
if len(first.collections) != 1 || len(second.collections) != 1 {
|
||||
t.Fatal("record-owned scope was not localized")
|
||||
}
|
||||
entry := first.collections["/base/items"]
|
||||
entry.policy.identityFields[0] = "changed"
|
||||
if second.collections["/base/items"].policy.identityFields[0] != "id" || nested.collections["/group/base/items"].policy.identityFields[0] != "id" {
|
||||
t.Fatal("localized policy shares mutable template state")
|
||||
}
|
||||
if len(index.localize(Keys("records", "0")).collections) != 0 || len(index.localize(Keys("numeric").Index(0)).collections) != 0 {
|
||||
t.Fatal("mapping keys and sequence indexes were conflated")
|
||||
}
|
||||
if len(index.localize(Keys("numeric", "0")).collections) != 1 {
|
||||
t.Fatal("numeric mapping key did not retain its policy")
|
||||
}
|
||||
left := index.localize(Keys("records"))
|
||||
right := index.localize(Keys("records"))
|
||||
leftOwner := left.collections[""].policy.recordOrder[0]
|
||||
rightOwner := right.collections[""].policy.recordOrder[0]
|
||||
if leftOwner == rightOwner || leftOwner == record.owner || rightOwner == record.owner {
|
||||
t.Fatal("independent templates share a record owner")
|
||||
}
|
||||
}
|
||||
|
||||
func TestInheritancePolicyIndexReplacesOnlyTheChangedScope(t *testing.T) {
|
||||
scope := collectionPolicyScope{}
|
||||
policy := collectionPolicy{explicitMode: sequenceModeAppend, explicitModePresent: true}
|
||||
for index := range 128 {
|
||||
scope.set(Keys(fmt.Sprintf("group-%03d", index), "pending"), policy)
|
||||
}
|
||||
index := newInheritancePolicyIndex(scope)
|
||||
changed := Keys("group-064")
|
||||
if len(index.absoluteScope(changed).collections) != 1 {
|
||||
t.Fatal("scope includes unrelated groups")
|
||||
}
|
||||
replacement := collectionPolicyScope{}
|
||||
replacement.set(changed.Key("introduced"), policy)
|
||||
index.replace(&scope, changed, replacement)
|
||||
if len(scope.collections) != 128 {
|
||||
t.Fatal("replacement lost unrelated pending declarations")
|
||||
}
|
||||
local := index.localize(changed)
|
||||
if _, found := local.collections["/introduced"]; !found || len(local.collections) != 1 {
|
||||
t.Fatal("retired policy survived or introduced policy was not indexed")
|
||||
}
|
||||
index.replace(&scope, changed, collectionPolicyScope{})
|
||||
if len(index.localize(changed).collections) != 0 || len(scope.collections) != 127 {
|
||||
t.Fatal("descendant retirement did not update the table and index together")
|
||||
}
|
||||
if len(index.localize(Keys("group-065")).collections) != 1 {
|
||||
t.Fatal("retirement changed another scope")
|
||||
}
|
||||
}
|
||||
|
||||
func TestInheritanceWideIndependentYamlCollectionScopes(t *testing.T) {
|
||||
var body strings.Builder
|
||||
body.WriteString("configkit:\n sequences:\n")
|
||||
for index := range 64 {
|
||||
fmt.Fprintf(&body, " - {destination: [groups, group-%03d, base, items], key: id, mode: append}\n", index)
|
||||
}
|
||||
body.WriteString("groups:\n")
|
||||
for index := range 64 {
|
||||
fmt.Fprintf(&body, " group-%03d:\n base: {items: [{id: shared, value: parent}]}\n child: {extends: base, items: [{id: shared, value: child}]}\n", index)
|
||||
}
|
||||
type item struct {
|
||||
ID string `yaml:"id"`
|
||||
Value string `yaml:"value"`
|
||||
}
|
||||
type definition struct {
|
||||
Items []item `yaml:"items"`
|
||||
}
|
||||
type config struct {
|
||||
Groups map[string]map[string]definition `yaml:"groups"`
|
||||
}
|
||||
loaded, err := testLoad[config](Bytes("wide-inheritance", []byte(body.String())))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
metadata := loaded.Metadata()
|
||||
for name, group := range loaded.Config.Groups {
|
||||
if len(group["base"].Items) != 1 || group["base"].Items[0].Value != "parent" || len(group["child"].Items) != 1 || group["child"].Items[0].Value != "child" {
|
||||
t.Fatalf("independent scope %s composed incorrectly: %#v", name, group)
|
||||
}
|
||||
path := Keys("groups", name, "child", "items").Index(0).Key("value")
|
||||
if info, exists := metadata.Lookup(path); !exists || !info.Explicit || info.Origin.Layer != LayerBase {
|
||||
t.Fatalf("inherited destination lost source metadata at %s", path)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -47,9 +47,10 @@ func analyzeInheritanceGraphWithCollectionRules(scope *document.Node, scopePath
|
||||
}}
|
||||
}
|
||||
|
||||
policyIndex := newInheritancePolicyIndex(policies)
|
||||
graph := &inheritanceGraph{
|
||||
scope: scope, scopePath: scopePath,
|
||||
collectionPolicies: cloneCollectionPolicyScopeForInheritance(policies), applicationPolicy: applicationPolicy,
|
||||
collectionPolicies: policyIndex.absoluteScope(scopePath), applicationPolicy: applicationPolicy,
|
||||
byName: make(map[string]int, len(scope.Mapping)),
|
||||
}
|
||||
entries := make(map[string]document.Entry, len(scope.Mapping))
|
||||
@@ -69,7 +70,7 @@ func analyzeInheritanceGraphWithCollectionRules(scope *document.Node, scopePath
|
||||
graph.byName[name] = index
|
||||
graph.nodes = append(graph.nodes, inheritanceTemplate{
|
||||
name: name, entry: entries[name], path: scopePath.Key(name),
|
||||
collectionPolicies: inheritancePolicyScopeAtPath(policies, scopePath.Key(name)),
|
||||
collectionPolicies: policyIndex.localize(scopePath.Key(name)),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -355,6 +356,7 @@ func applyInheritanceScopeTemplates(adapter inheritanceScopeAdapter, order []int
|
||||
root := adapter.rebuild(resolved)
|
||||
result.collectionPolicies = collectionPolicyScope{}
|
||||
result.origins = directInheritanceOrigins(&document.Node{Kind: root.Kind, Location: root.Location}, adapter.scopePath(), inputOrigins)
|
||||
policyIndex := newInheritancePolicyIndex(adapter.collectionPolicies())
|
||||
for _, slot := range adapter.slots() {
|
||||
if slot.hasTemplate {
|
||||
index := slot.template
|
||||
@@ -364,7 +366,7 @@ func applyInheritanceScopeTemplates(adapter inheritanceScopeAdapter, order []int
|
||||
}
|
||||
continue
|
||||
}
|
||||
localPolicies := inheritancePolicyScopeAtPath(adapter.collectionPolicies(), slot.path)
|
||||
localPolicies := policyIndex.localize(slot.path)
|
||||
projectInheritancePolicyRoots(&result.collectionPolicies, localPolicies, slot.path)
|
||||
for pointer, origin := range directInheritanceOrigins(slot.value, slot.path, inputOrigins) {
|
||||
result.origins[pointer] = origin
|
||||
|
||||
@@ -109,10 +109,11 @@ func (scope identifiedSequenceInheritanceScope) applicationPolicy() sequencePoli
|
||||
func (scope identifiedSequenceInheritanceScope) slotOverhead() int { return 0 }
|
||||
func (scope identifiedSequenceInheritanceScope) templates() []inheritanceExecutionTemplate {
|
||||
result := make([]inheritanceExecutionTemplate, len(scope.graph.nodes))
|
||||
policyIndex := newInheritancePolicyIndex(scope.policies)
|
||||
for index, node := range scope.graph.nodes {
|
||||
result[index] = inheritanceExecutionTemplate{
|
||||
entry: document.Entry{Value: node.item}, path: node.path,
|
||||
collectionPolicies: inheritancePolicyScopeAtPath(scope.policies, node.path),
|
||||
collectionPolicies: policyIndex.localize(node.path),
|
||||
parent: node.parent, hasParent: node.hasParent,
|
||||
hasDirective: node.directive != nil, cancelled: node.cancelled,
|
||||
directive: document.Entry{Value: node.directive}, directivePath: node.directivePath,
|
||||
|
||||
@@ -11,6 +11,7 @@ type inheritanceWalker struct {
|
||||
nodes int // logical non-directive nodes in the entire working tree
|
||||
origins map[string]Origin
|
||||
collectionPolicies collectionPolicyScope
|
||||
policyIndex *inheritancePolicyIndex
|
||||
applicationPolicy sequencePolicy
|
||||
limits projectionLimits
|
||||
retained *retainedNodeCounter
|
||||
@@ -66,6 +67,7 @@ func expandInheritanceWithCollectionRuleBudgets(input *document.Node, inputOrigi
|
||||
applicationPolicy: applicationPolicy,
|
||||
retained: newRetainedNodeCounter(historyLimit),
|
||||
}
|
||||
walker.policyIndex = newInheritancePolicyIndex(walker.collectionPolicies)
|
||||
if problem := walker.visit(&root, Path{}, true); problem != nil {
|
||||
return nil, []Diagnostic{*problem}
|
||||
}
|
||||
@@ -113,20 +115,13 @@ func (w *inheritanceWalker) visit(node **document.Node, path Path, unscoped bool
|
||||
limits.nodes -= w.nodes - before
|
||||
}
|
||||
resolved, diagnostics := expandSiblingDefinitionsWithCollectionRules(
|
||||
*node, path, limits, w.origins, w.collectionPolicies, w.applicationPolicy, w.retained,
|
||||
*node, path, limits, w.origins, w.policyIndex.absoluteScope(path), w.applicationPolicy, w.retained,
|
||||
)
|
||||
if len(diagnostics) != 0 {
|
||||
return &diagnostics[0]
|
||||
}
|
||||
*node = resolved.root
|
||||
for pointer, entry := range w.collectionPolicies.collections {
|
||||
if pathHasPrefix(entry.destination, path) {
|
||||
delete(w.collectionPolicies.collections, pointer)
|
||||
}
|
||||
}
|
||||
for _, entry := range resolved.collectionPolicies.collections {
|
||||
w.collectionPolicies.set(entry.destination, entry.policy)
|
||||
}
|
||||
w.policyIndex.replace(&w.collectionPolicies, path, resolved.collectionPolicies)
|
||||
deleteOriginSubtree(w.origins, path, original)
|
||||
for pointer, origin := range resolved.origins {
|
||||
w.origins[pointer] = origin
|
||||
@@ -203,20 +198,13 @@ func (w *inheritanceWalker) expandIdentifiedSequence(node **document.Node, path
|
||||
limits.nodes -= w.nodes - before
|
||||
}
|
||||
resolved, diagnostics := applyIdentifiedInheritanceTemplates(
|
||||
graph, limits, w.origins, w.collectionPolicies, w.applicationPolicy, w.retained,
|
||||
graph, limits, w.origins, w.policyIndex.absoluteScope(path), w.applicationPolicy, w.retained,
|
||||
)
|
||||
if len(diagnostics) != 0 {
|
||||
return &diagnostics[0]
|
||||
}
|
||||
*node = resolved.root
|
||||
for pointer, entry := range w.collectionPolicies.collections {
|
||||
if pathHasPrefix(entry.destination, path) {
|
||||
delete(w.collectionPolicies.collections, pointer)
|
||||
}
|
||||
}
|
||||
for _, entry := range resolved.collectionPolicies.collections {
|
||||
w.collectionPolicies.set(entry.destination, entry.policy)
|
||||
}
|
||||
w.policyIndex.replace(&w.collectionPolicies, path, resolved.collectionPolicies)
|
||||
deleteOriginSubtree(w.origins, path, original)
|
||||
for pointer, origin := range resolved.origins {
|
||||
w.origins[pointer] = origin
|
||||
|
||||
Reference in New Issue
Block a user