Prepare sequence registration preflight
This commit is contained in:
73
docs/adr/0014-sequence-placement-and-composition.md
Normal file
73
docs/adr/0014-sequence-placement-and-composition.md
Normal file
@@ -0,0 +1,73 @@
|
||||
# ADR 0014: Place and Compose Sequence Items Explicitly
|
||||
|
||||
## Status
|
||||
|
||||
Accepted. Implementation status and sequencing are tracked in the
|
||||
[sequence composition implementation plan](../roadmap/implementation.md).
|
||||
|
||||
## Date
|
||||
|
||||
2026-10-16
|
||||
|
||||
## Context
|
||||
|
||||
ConfigKit strictly composes independent documents by recursively combining
|
||||
mappings and rejecting other overlapping values. This makes two independently
|
||||
authored sequence fragments conflict, even when their records are distinct.
|
||||
Precedence assembly has a different purpose: a higher-priority present sequence
|
||||
replaces the lower-priority sequence as one value. Those existing rules are
|
||||
predictable, but they do not express the separate operation of collecting
|
||||
independent records while rejecting duplicate identities.
|
||||
|
||||
Likewise, mapping destinations place a source beneath literal mapping keys, but
|
||||
there is no explicit way to state that a source mapping represents one item in a
|
||||
sequence. Inferring that intent from document shape or fields would make source
|
||||
meaning implicit and change existing placement behavior.
|
||||
|
||||
## Decision
|
||||
|
||||
Keep mapping placement as the zero-value behavior and add an explicit item
|
||||
placement choice for a source whose mapping should become one element at a
|
||||
non-root sequence destination. Item placement provides structure only; it does
|
||||
not infer or synthesize identity values.
|
||||
|
||||
Allow a definition to register literal mapping paths and one literal identity
|
||||
field for sequences at those paths. Within each strict composition tier, a
|
||||
registered sequence accepts mapping records with a present string identity,
|
||||
appends distinct records in contribution order, and rejects repeated exact
|
||||
identity strings. Identical registrations coalesce; conflicting identity fields
|
||||
for the same path are invalid. Registration does not recursively select paths
|
||||
inside sequence records.
|
||||
|
||||
Keep precedence assembly unchanged. A present higher-priority sequence replaces
|
||||
the lower-priority value wholesale, including at a registered path. Registration
|
||||
changes strict composition only; it does not turn precedence into append or
|
||||
identity-based merging.
|
||||
|
||||
This decision preserves ConfigKit's distinction between independent
|
||||
same-priority contributions and intentional precedence. Explicit source
|
||||
placement and definition-owned composition policy make the two required choices
|
||||
visible while keeping the existing zero-value load behavior.
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
- **Infer item placement from a source mapping or identity field.** Shape-based
|
||||
inference would make ordinary mapping placement depend on document contents
|
||||
and would leave the intended destination ambiguous.
|
||||
- **Append all overlapping sequences during strict composition.** This would
|
||||
change existing conflict semantics at unregistered paths and would not detect
|
||||
duplicate records.
|
||||
- **Merge matching records recursively by identity.** This would add a second
|
||||
recursive merge rule with different conflict and precedence behavior from
|
||||
ConfigKit's existing mapping composition and overlay operations.
|
||||
- **Use identity to merge sequences across precedence tiers.** This would
|
||||
change established whole-value replacement semantics and conflate strict
|
||||
composition with precedence.
|
||||
|
||||
## Consequences
|
||||
|
||||
Consumers can explicitly place mappings as sequence items and opt selected
|
||||
sequences into duplicate-rejecting strict composition. Unregistered sequences
|
||||
retain their current strict-conflict behavior, and higher-priority sequences
|
||||
continue to replace lower-priority ones. The implementation plan owns the exact
|
||||
API, YAML syntax, diagnostics, and implementation status.
|
||||
@@ -213,6 +213,24 @@ Scope: one ADR and engine option normalization; no new public fields yet.
|
||||
Validation: focused option tests and `./scripts/check.sh`. Completion: existing
|
||||
loads remain unchanged, with only private registered-policy input newly accepted.
|
||||
|
||||
Completion evidence: added Accepted ADR 0014 and private engine registration
|
||||
normalization. Ordinary and layered entry points compile/coalesce policy and
|
||||
reject invalid registrations before source reads, then carry the compiled policy
|
||||
through the load request while clearing raw registration slices from internal
|
||||
handoffs. No public API or composition behavior was added. Focused coverage
|
||||
checks duplicate/conflicting definitions, empty identity names, numeric and
|
||||
escaped literal keys, invalid explicit indexes, unused/root registrations,
|
||||
zero-option behavior, and reader preservation on preflight failure. These checks
|
||||
passed with Go `go1.26.9 linux/amd64`:
|
||||
|
||||
- `GOTOOLCHAIN=local GOPROXY=off go test ./internal/engine -run 'SequenceRegistration' -count=1`
|
||||
- `GOTOOLCHAIN=local ./scripts/check.sh`
|
||||
- `git diff --check`
|
||||
|
||||
The compiled registration policy is intentionally not consumed by composition
|
||||
until the next implementation unit. No environmental limitation affected
|
||||
validation; checks ran on Linux/amd64.
|
||||
|
||||
## Stage 2 — Implement registered strict sequence validation and assembly
|
||||
|
||||
Scope: `internal/engine/compose.go` and focused composition tests.
|
||||
|
||||
@@ -16,6 +16,8 @@ type Definition[T any] struct {
|
||||
Validate func(*T, *Validation)
|
||||
Version *VersionRule
|
||||
Secret func(Path) bool
|
||||
|
||||
sequenceRegistrations []sequenceRegistration
|
||||
}
|
||||
|
||||
// Options contains the application definition and per-load input selections.
|
||||
@@ -137,12 +139,16 @@ func Load[T any](source Source, secrets SecretBinding, supplied ...Options[T]) (
|
||||
|
||||
func loadWithSecretLookup[T any](source Source, secrets SecretBinding, lookup secretLookup, supplied ...Options[T]) (*Loaded[T], error) {
|
||||
options, diagnostics := selectOptions(source, supplied)
|
||||
policy, policyDiagnostics := compileSequencePolicy(options.Definition.sequenceRegistrations)
|
||||
diagnostics = append(diagnostics, policyDiagnostics...)
|
||||
if len(diagnostics) != 0 {
|
||||
return nil, (&diagnosticAccumulator{diagnostics: diagnostics}).loadError()
|
||||
}
|
||||
definition := options.Definition
|
||||
definition.sequenceRegistrations = nil
|
||||
request := loadRequest[T]{
|
||||
main: loadInput{source: source, imports: options.Imports, filesystem: options.Filesystem},
|
||||
definition: options.Definition, overlay: options.Overlay,
|
||||
definition: definition, sequencePolicy: policy, overlay: options.Overlay,
|
||||
overrides: options.Overrides, ordinary: true,
|
||||
sourceVersionMode: options.SourceVersionMode,
|
||||
effectiveVersion: effectiveSourceVersionRule(options.Definition.Version, Path{}, options.SourceVersionMode),
|
||||
|
||||
@@ -10,6 +10,7 @@ type loadRequest[T any] struct {
|
||||
main loadInput
|
||||
underlays []loadGroup
|
||||
definition Definition[T]
|
||||
sequencePolicy sequencePolicy
|
||||
sourceVersionMode sourceVersionMode
|
||||
effectiveVersion *VersionRule
|
||||
overlay string
|
||||
@@ -37,6 +38,10 @@ type loadGroup struct{ inputs []loadInput }
|
||||
func loadGroupedWithSecretLookup[T any](request loadRequest[T], secrets SecretBinding, lookup secretLookup) (*Loaded[T], error) {
|
||||
request.ordinary = false
|
||||
var diagnostics []Diagnostic
|
||||
policy, policyDiagnostics := compileSequencePolicy(request.definition.sequenceRegistrations)
|
||||
request.sequencePolicy = policy
|
||||
diagnostics = append(diagnostics, policyDiagnostics...)
|
||||
request.definition.sequenceRegistrations = nil
|
||||
if normalized, err := normalizeDestination(request.main.destination); err != nil {
|
||||
diagnostics = append(diagnostics, invalidDestinationDiagnostic(request.main.destination))
|
||||
} else {
|
||||
|
||||
38
internal/engine/sequence_registration.go
Normal file
38
internal/engine/sequence_registration.go
Normal file
@@ -0,0 +1,38 @@
|
||||
package engine
|
||||
|
||||
type sequenceRegistration struct {
|
||||
path Path
|
||||
identityField string
|
||||
}
|
||||
|
||||
type sequencePolicy struct {
|
||||
identityFields map[string]string
|
||||
}
|
||||
|
||||
func compileSequencePolicy(registrations []sequenceRegistration) (sequencePolicy, []Diagnostic) {
|
||||
policy := sequencePolicy{identityFields: make(map[string]string, len(registrations))}
|
||||
var diagnostics []Diagnostic
|
||||
for _, registration := range registrations {
|
||||
path, err := normalizeDestination(registration.path)
|
||||
if err != nil {
|
||||
diagnostics = append(diagnostics, Diagnostic{
|
||||
Code: CodeOptionsInvalid, Severity: SeverityError,
|
||||
Path: registration.path.clone(), Message: "sequence registration path must contain only non-reserved mapping keys",
|
||||
})
|
||||
continue
|
||||
}
|
||||
|
||||
key := path.String()
|
||||
if current, ok := policy.identityFields[key]; ok {
|
||||
if current != registration.identityField {
|
||||
diagnostics = append(diagnostics, Diagnostic{
|
||||
Code: CodeOptionsInvalid, Severity: SeverityError,
|
||||
Path: path, Message: "sequence registrations at one path must use the same identity field",
|
||||
})
|
||||
}
|
||||
continue
|
||||
}
|
||||
policy.identityFields[key] = registration.identityField
|
||||
}
|
||||
return policy, canonicalDiagnostics(diagnostics)
|
||||
}
|
||||
127
internal/engine/sequence_registration_test.go
Normal file
127
internal/engine/sequence_registration_test.go
Normal file
@@ -0,0 +1,127 @@
|
||||
package engine
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestCompileSequencePolicyNormalizesRegistrations(t *testing.T) {
|
||||
parsed, err := ParsePath("/items/0/a~1b")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
registrations []sequenceRegistration
|
||||
want map[string]string
|
||||
wantError bool
|
||||
}{
|
||||
{
|
||||
name: "identical registrations coalesce",
|
||||
registrations: []sequenceRegistration{
|
||||
{path: Keys("items"), identityField: "id"},
|
||||
{path: Keys("items"), identityField: "id"},
|
||||
},
|
||||
want: map[string]string{"/items": "id"},
|
||||
},
|
||||
{
|
||||
name: "different identity fields conflict",
|
||||
registrations: []sequenceRegistration{
|
||||
{path: Keys("items"), identityField: "id"},
|
||||
{path: Keys("items"), identityField: "name"},
|
||||
},
|
||||
wantError: true,
|
||||
},
|
||||
{
|
||||
name: "empty identity field is literal",
|
||||
registrations: []sequenceRegistration{{path: Keys("items"), identityField: ""}},
|
||||
want: map[string]string{"/items": ""},
|
||||
},
|
||||
{
|
||||
name: "numeric and escaped tokens remain literal keys",
|
||||
registrations: []sequenceRegistration{{path: parsed, identityField: "id"}},
|
||||
want: map[string]string{"/items/0/a~1b": "id"},
|
||||
},
|
||||
{
|
||||
name: "explicit index is invalid",
|
||||
registrations: []sequenceRegistration{{path: Keys("items").Index(0), identityField: "id"}},
|
||||
wantError: true,
|
||||
},
|
||||
{
|
||||
name: "unused and root registrations are accepted",
|
||||
registrations: []sequenceRegistration{
|
||||
{path: Path{}, identityField: "id"},
|
||||
{path: Keys("unused"), identityField: "name"},
|
||||
},
|
||||
want: map[string]string{"": "id", "/unused": "name"},
|
||||
},
|
||||
{name: "zero option policy", want: map[string]string{}},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
policy, diagnostics := compileSequencePolicy(test.registrations)
|
||||
if (len(diagnostics) != 0) != test.wantError {
|
||||
t.Fatalf("diagnostics = %#v, wantError = %t", diagnostics, test.wantError)
|
||||
}
|
||||
if test.wantError {
|
||||
if diagnostics[0].Code != CodeOptionsInvalid {
|
||||
t.Fatalf("diagnostic code = %q", diagnostics[0].Code)
|
||||
}
|
||||
return
|
||||
}
|
||||
if len(policy.identityFields) != len(test.want) {
|
||||
t.Fatalf("compiled fields = %#v, want %#v", policy.identityFields, test.want)
|
||||
}
|
||||
for path, field := range test.want {
|
||||
if got := policy.identityFields[path]; got != field {
|
||||
t.Errorf("identity field at %q = %q, want %q", path, got, field)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSequenceRegistrationPreflightDoesNotReadSources(t *testing.T) {
|
||||
t.Parallel()
|
||||
type config struct {
|
||||
Value string `yaml:"value"`
|
||||
}
|
||||
|
||||
ordinaryReader := strings.NewReader("value: ordinary\n")
|
||||
_, err := Load[config](Reader("ordinary.yml", ordinaryReader), SecretBinding{}, Options[config]{
|
||||
Definition: Definition[config]{sequenceRegistrations: []sequenceRegistration{{path: Keys("items").Index(0), identityField: "id"}}},
|
||||
})
|
||||
requireSequenceRegistrationOptionError(t, err)
|
||||
if ordinaryReader.Len() == 0 {
|
||||
t.Fatal("ordinary reader was consumed before invalid registration was rejected")
|
||||
}
|
||||
|
||||
mainReader := strings.NewReader("value: main\n")
|
||||
underlayReader := strings.NewReader("value: underlay\n")
|
||||
_, err = LoadLayered[config](Input{Source: Reader("main.yml", mainReader)}, SecretBinding{}, LayeredOptions[config]{
|
||||
Definition: Definition[config]{sequenceRegistrations: []sequenceRegistration{{path: Keys("items").Index(0), identityField: "id"}}},
|
||||
Underlays: []Underlay{{Inputs: []Input{{Source: Reader("underlay.yml", underlayReader)}}}},
|
||||
})
|
||||
requireSequenceRegistrationOptionError(t, err)
|
||||
if mainReader.Len() == 0 || underlayReader.Len() == 0 {
|
||||
t.Fatal("layered source was consumed before invalid registration was rejected")
|
||||
}
|
||||
}
|
||||
|
||||
func requireSequenceRegistrationOptionError(t *testing.T, err error) {
|
||||
t.Helper()
|
||||
if err == nil {
|
||||
t.Fatal("expected invalid sequence registration error")
|
||||
}
|
||||
var loadErr *LoadError
|
||||
if !errors.As(err, &loadErr) {
|
||||
t.Fatalf("error type = %T, want *LoadError", err)
|
||||
}
|
||||
diagnostics := loadErr.Diagnostics()
|
||||
if len(diagnostics) != 1 || diagnostics[0].Code != CodeOptionsInvalid {
|
||||
t.Fatalf("diagnostics = %#v, want one options diagnostic", diagnostics)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user