Skip to content

Commit 6746289

Browse files
committed
feat(osgen)!: decode unions by the spec's discriminator, and name them after their schema
osgen never read the OpenAPI `discriminator`. Eight schemas in the spec declare one, and every branch of all eight resolves to a distinct value, so 146 subtypes that the payload names outright were instead decoded by guesswork: a JSON token-class heuristic, or for the ambiguous ones no decoding at all until a caller picked a branch by hand. The discriminator is how OpenAPI expresses polymorphism, and the spec encodes the hierarchy through allOf: a subtype is `allOf[Base, {type: enum[keyword]}]`, so resolving a discriminant means walking allOf transitively -- the constant sits on the narrowing member while the inherited fields come from the base. discriminatorValues does that, and refuses the whole union unless every branch resolves distinctly, so a partially-understood schema falls back rather than emitting a decoder with an unreachable case. Six emitted unions gain a real Type(), discriminant consts, and an UnmarshalJSON that reads one property and decodes exactly that branch: CommonMappingProperty (on `type`, with the spec's x-default naming the implicit object mapping), CommonAnalysis{Analyzer,CharFilterDefinition,TokenFilterDefinition, TokenizerDefinition,Normalizer}, plus ClusterRemoteInfoCluster on `mode`. A value naming no branch is an error rather than a silent mis-decode. MovingAverageAggregation also declares one but is request-body-only and never emitted. A $ref'd union was keyed by the caller's field path, so a shared schema was re-emitted once per referencing field and its identity -- along with its discriminator -- was discarded: _common.analysis___Analyzer became IndicesIndexSettingsAnalysisAnalyzerValue, named after the single field that happened to reach it. A $ref'd schema now keeps its own name and is emitted once. That collapses 212 unions to 122, and the removals are consolidations rather than losses: 138 redundant copies of 87 distinct shapes become 35 copies of 91 shapes, with more distinct shapes surviving than before. Spec $ref counts show the fan-in -- Script 33, StringOrStringArray 19, FieldValue 17, Sort 17. ClusterRemoteInfoResp drops out of the json.RawMessage allowlist: it was an untyped map and is now the typed, discriminated union. Accessors on a request-selected union no longer lie. AsSum() against histogram bytes returned a zero Sum with a nil error, indistinguishable from a real zero, because encoding/json ignores unknown keys and a decode-error check cannot see the mismatch; a required-property probe now rejects it. Constructors for a discriminated branch also left the discriminant field empty, so a marshal-unmarshal round trip failed on an empty discriminator. Branch names come from the schema key's local segment rather than the qualified Go type, so a const does not restate the group prefix its union name already carries. Where a `<Union><Branch>Type` const would collide with a `<Union>Type` enum type -- three analysis unions with a `Definition` branch beside a `<Thing>Definition` union -- the branch is renamed, scoped to $ref branches so inline branches keep their construction. Vocabulary follows the spec instead of the decode mechanism. IsLazy and LazyAccessors named a strategy while conflating two unrelated situations, "the spec provides no discriminator" with "we never read the discriminator", and become RequestSelected: the aggregation and suggest unions carry x-supports-typed-keys and genuinely cannot be discriminated, since the branch is chosen by the request and echoed only in the response map key. TypeLazyUnion becomes TypeAmbiguousWire, unionNeedsTryEach becomes branchesCollideOnTokenClass, resolveParentScopedUnion becomes resolveRefUnion. TokenClass stays, documented as the fallback it is rather than the primary strategy. BREAKING CHANGE: 135 union types are removed and 45 added as $ref'd schemas consolidate onto their spec names. MGetRespBodyDocsItem becomes MGetRespItem, MSearchMultiSearchResultResponsesItem becomes MSearchRespItem, SortResultsItem and CommonAggregationsCompositeAggregateKeyValue become FieldValue, ErrorCauseHeaderValue becomes StringOrStringArray, SearchResultAggregationsValue becomes CommonAggregationsAggregate, and CatRecoveryRecord{Start,Stop}TimeMillis with Replication{,Index}FollowerStatusTotalWriteTimeMillis become StringifiedEpochTimeUnitMillis. SearchHitsMetadataTotal.SearchTotalHits becomes TotalHits. Branch accessors lose the group prefix the union name already carries, so MGetMultiGetError becomes MultiGetError. The six discriminated unions gain a Type() and consts they did not have. UnionBranchError gains Err and Unwrap, and Got may be "incompatible payload" when the payload cannot be the requested branch. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
1 parent 9edf259 commit 6746289

88 files changed

Lines changed: 14866 additions & 28998 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

cmd/osgen/api_render_union.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,21 @@ package main
88

99
import "github.com/opensearch-project/opensearch-go/v5/cmd/osgen/ir"
1010

11-
// unionNeedsTryEach returns true if any two branches share the same token class,
12-
// meaning byte-prefix discrimination is insufficient for at least one pair.
13-
func unionNeedsTryEach(branches []unionBranch) bool {
11+
// branchesCollideOnTokenClass reports whether any two branches decode from the
12+
// same JSON token, which makes the first byte insufficient to pick one.
13+
//
14+
// This is the FALLBACK test, consulted only for unions the spec leaves
15+
// undiscriminated. A union that declares an OpenAPI `discriminator` reads its
16+
// branch from a named property (see discriminatorValues) and never consults the
17+
// token class at all -- most of the OpenSearch DSL unions are all-object and so
18+
// collide here trivially, which is exactly why the spec bothers to declare a
19+
// discriminator for them.
20+
//
21+
// When branches DO collide and no discriminator resolves, the union is
22+
// ir.TypeAmbiguousWire and classifyUnions picks the payload-inspection strategy:
23+
// a key-presence merge, request selection, or the try-each decoder of last
24+
// resort.
25+
func branchesCollideOnTokenClass(branches []unionBranch) bool {
1426
if len(branches) < 2 {
1527
return false
1628
}

cmd/osgen/emit/build.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,8 @@ func Build(spec *ir.Spec, cfg BuildConfig) []Target {
202202
}
203203

204204
// splitUnionsFromSiblings partitions a sibling-types list into struct-shape
205-
// types and discriminated unions. Unions need the UnionFragment template
206-
// (which renders Type/branch accessors and try-each unmarshal); structs go
205+
// types and unions. Unions need the UnionFragment template
206+
// (which renders Type/branch accessors and the decode strategy); structs go
207207
// through SiblingTypesFragment. Without this split, a union sibling
208208
// rendered as a struct emits as `type Foo struct {}` because its Branches
209209
// aren't Fields.
@@ -212,7 +212,7 @@ func Build(spec *ir.Spec, cfg BuildConfig) []Target {
212212
func splitUnionsFromSiblings(types []*ir.Type) ([]*ir.Type, []*ir.Type) {
213213
var structs, unions []*ir.Type
214214
for _, st := range types {
215-
if st.Kind == ir.TypeUnion || st.Kind == ir.TypeLazyUnion {
215+
if st.Kind == ir.TypeUnion || st.Kind == ir.TypeAmbiguousWire {
216216
unions = append(unions, st)
217217
} else {
218218
structs = append(structs, st)

cmd/osgen/emit/build_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,15 @@ func TestSplitUnionsFromSiblings(t *testing.T) {
5454
},
5555
{
5656
name: "all unions (lazy)",
57-
input: []*ir.Type{{Name: "A", Kind: ir.TypeLazyUnion}, {Name: "B", Kind: ir.TypeLazyUnion}},
57+
input: []*ir.Type{{Name: "A", Kind: ir.TypeAmbiguousWire}, {Name: "B", Kind: ir.TypeAmbiguousWire}},
5858
wantStructs: 0,
5959
wantUnions: 2,
6060
},
6161
{
6262
name: "mixed -- the ReindexSourceSort case",
6363
input: []*ir.Type{
6464
{Name: "ReindexSource", Kind: ir.TypeStruct},
65-
{Name: "ReindexSourceSort", Kind: ir.TypeLazyUnion},
65+
{Name: "ReindexSourceSort", Kind: ir.TypeAmbiguousWire},
6666
{Name: "ReindexRemoteSource", Kind: ir.TypeStruct},
6767
{Name: "ReindexSourceSlice", Kind: ir.TypeStruct},
6868
},
@@ -72,7 +72,7 @@ func TestSplitUnionsFromSiblings(t *testing.T) {
7272
{
7373
name: "mixed strict + lazy unions",
7474
input: []*ir.Type{
75-
{Name: "Lazy", Kind: ir.TypeLazyUnion},
75+
{Name: "RequestSel", Kind: ir.TypeAmbiguousWire},
7676
{Name: "Struct", Kind: ir.TypeStruct},
7777
{Name: "Strict", Kind: ir.TypeUnion},
7878
},
@@ -95,10 +95,10 @@ func TestSplitUnionsFromSiblings(t *testing.T) {
9595
// Verify no struct ended up in unions and vice versa.
9696
for _, s := range structs {
9797
require.NotEqual(t, ir.TypeUnion, s.Kind, "struct slice contains union %q", s.Name)
98-
require.NotEqual(t, ir.TypeLazyUnion, s.Kind, "struct slice contains lazy union %q", s.Name)
98+
require.NotEqual(t, ir.TypeAmbiguousWire, s.Kind, "struct slice contains lazy union %q", s.Name)
9999
}
100100
for _, u := range unions {
101-
require.Contains(t, []ir.TypeKind{ir.TypeUnion, ir.TypeLazyUnion}, u.Kind, "union slice contains non-union %q", u.Name)
101+
require.Contains(t, []ir.TypeKind{ir.TypeUnion, ir.TypeAmbiguousWire}, u.Kind, "union slice contains non-union %q", u.Name)
102102
}
103103
})
104104
}

cmd/osgen/emit/export_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func UnionFromResponses(resp *ir.Type, reg *ir.TypeRegistry) (string, string, st
4444
}
4545

4646
// ResolveUnionShape mirrors UnionFromResponses for direct calls
47-
// against a TypeUnion / TypeLazyUnion (skipping the Responses-field
47+
// against a TypeUnion / TypeAmbiguousWire (skipping the Responses-field
4848
// indirection). Returns (unionName, success, errorBranch).
4949
func ResolveUnionShape(t *ir.Type, reg *ir.TypeRegistry) (string, string, string) {
5050
u := resolveUnionShape(t, reg)

cmd/osgen/emit/frag_dispatch.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ func elementTypeHasShards(goType string, reg *ir.TypeRegistry) bool {
233233
}
234234
// Element is a discriminated union -- delegate to union resolution
235235
// to see whether any branch carries Shards.
236-
if resolved.Kind == ir.TypeLazyUnion || resolved.Kind == ir.TypeUnion {
236+
if resolved.Kind == ir.TypeAmbiguousWire || resolved.Kind == ir.TypeUnion {
237237
return resolveUnionShape(resolved, reg).success != ""
238238
}
239239
return false
@@ -288,13 +288,13 @@ type unionShape struct {
288288
errorBranch string // error branch accessor Name (Status + Error), or ""
289289
}
290290

291-
// resolveUnionShape walks a TypeLazyUnion/TypeUnion's branches and
291+
// resolveUnionShape walks a TypeAmbiguousWire/TypeUnion's branches and
292292
// classifies them. Branches whose resolved type has a Shards field
293293
// become the success branch; branches with both Status and Error
294294
// fields become the error branch. Returns zero-value if the type isn't
295295
// a union or no branches matched.
296296
func resolveUnionShape(t *ir.Type, reg *ir.TypeRegistry) unionShape {
297-
if t == nil || (t.Kind != ir.TypeLazyUnion && t.Kind != ir.TypeUnion) {
297+
if t == nil || (t.Kind != ir.TypeAmbiguousWire && t.Kind != ir.TypeUnion) {
298298
return unionShape{}
299299
}
300300
out := unionShape{unionName: t.Name}
@@ -654,7 +654,7 @@ func unionFromResponses(resp *ir.Type, reg *ir.TypeRegistry) unionShape {
654654
if !ok {
655655
return unionShape{}
656656
}
657-
if resolved.Kind != ir.TypeLazyUnion && resolved.Kind != ir.TypeUnion {
657+
if resolved.Kind != ir.TypeAmbiguousWire && resolved.Kind != ir.TypeUnion {
658658
return unionShape{}
659659
}
660660
return resolveUnionShape(resolved, reg)

cmd/osgen/emit/frag_dispatch_helpers_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ func TestElementTypeHasShards(t *testing.T) {
275275
regType(reg, &ir.Type{
276276
Name: "ItemUnion",
277277
Scope: ir.ScopeLocal,
278-
Kind: ir.TypeLazyUnion,
278+
Kind: ir.TypeAmbiguousWire,
279279
Branches: []ir.UnionBranch{
280280
{Name: "ShardBranch", GoType: "ShardBranch"},
281281
{Name: "ErrBranch", GoType: "ErrBranch"},
@@ -446,7 +446,7 @@ func TestResolveUnionShape(t *testing.T) {
446446
name: "union with Shards + Status/Error branches",
447447
input: &ir.Type{
448448
Name: "ItemUnion",
449-
Kind: ir.TypeLazyUnion,
449+
Kind: ir.TypeAmbiguousWire,
450450
Branches: []ir.UnionBranch{
451451
{Name: "ShardBranch", GoType: "ShardBranch"},
452452
{Name: "ErrBranch", GoType: "ErrBranch"},
@@ -460,7 +460,7 @@ func TestResolveUnionShape(t *testing.T) {
460460
name: "union with only success branch",
461461
input: &ir.Type{
462462
Name: "OnlyShard",
463-
Kind: ir.TypeLazyUnion,
463+
Kind: ir.TypeAmbiguousWire,
464464
Branches: []ir.UnionBranch{
465465
{Name: "ShardBranch", GoType: "ShardBranch"},
466466
{Name: "PlainBranch", GoType: "PlainBranch"},
@@ -473,7 +473,7 @@ func TestResolveUnionShape(t *testing.T) {
473473
name: "branch type not in registry is skipped",
474474
input: &ir.Type{
475475
Name: "Mixed",
476-
Kind: ir.TypeLazyUnion,
476+
Kind: ir.TypeAmbiguousWire,
477477
Branches: []ir.UnionBranch{
478478
{Name: "Missing", GoType: "Missing"},
479479
{Name: "ShardBranch", GoType: "ShardBranch"},
@@ -507,7 +507,7 @@ func TestUnionFromResponses(t *testing.T) {
507507
regType(reg, &ir.Type{
508508
Name: "MsearchItemUnion",
509509
Scope: ir.ScopeLocal,
510-
Kind: ir.TypeLazyUnion,
510+
Kind: ir.TypeAmbiguousWire,
511511
Branches: []ir.UnionBranch{
512512
{Name: "Item", GoType: "Item"},
513513
},

cmd/osgen/emit/frag_shared_types.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ func NewSharedTypesFile(outDir, pkg string, types []*ir.Type) Target {
100100
func NewUnionTypesFile(outDir, pkg string, types []*ir.Type) Target {
101101
var unionTypes []*ir.Type
102102
for _, t := range types {
103-
if (t.Kind == ir.TypeUnion || t.Kind == ir.TypeLazyUnion) && t.Scope == ir.ScopeShared {
103+
if (t.Kind == ir.TypeUnion || t.Kind == ir.TypeAmbiguousWire) && t.Scope == ir.ScopeShared {
104104
unionTypes = append(unionTypes, t)
105105
}
106106
}
@@ -127,7 +127,7 @@ func NewEnumTypesFile(outDir, pkg string, types []*ir.Type) Target {
127127
enumTypes = append(enumTypes, t)
128128
case ir.TypeStringEnum:
129129
stringEnumTypes = append(stringEnumTypes, t)
130-
case ir.TypeStruct, ir.TypeUnion, ir.TypeLazyUnion:
130+
case ir.TypeStruct, ir.TypeUnion, ir.TypeAmbiguousWire:
131131
// Emitted by other fragments (SharedTypesFragment / UnionFragment).
132132
}
133133
}

0 commit comments

Comments
 (0)