Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions compilers/openapi/conformance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ func conformanceCases() []conformanceCase {
{"allof-ref-branch-siblings", assertAllOfRefBranchSiblings, []string{"intersection", "untagged-unions"}},
{"allof-boolean-branch", assertAllOfBooleanBranch, []string{"intersection"}},
{"allof-conflicting-type", assertAllOfConflictingType, nil},
{"allof-position-constraints", assertAllOfPositionConstraints, []string{"intersection"}},
{"oneof-discriminated", assertOneOfDiscriminated, []string{"tagged-unions"}},
{"discriminator-inheritance", assertDiscriminatorInheritance, []string{"tagged-unions", "inheritance"}},
{"discriminator-default-mapping", assertDiscriminatorDefaultMapping, []string{"tagged-unions"}},
Expand Down Expand Up @@ -820,6 +821,9 @@ func assertAllOfOneOfCooccurrence(t *testing.T, doc *ir.Document, _ []ir.Diagnos
assert.Equal(t, namedID("Base"), v.Base.Target)
require.Len(t, v.Mixins, 1)
assert.Equal(t, namedID(branch), v.Mixins[0].Target)
require.NotNil(t, v.Constraints, "the enclosing schema's own minProperties rides on every variant too (GitHub #407)")
require.NotNil(t, v.Constraints.MinProps)
assert.Equal(t, int64(2), *v.Constraints.MinProps)
}

mixed, ok := doc.Types[namedID("MixedKinds")].(*ir.Model)
Expand Down Expand Up @@ -884,6 +888,31 @@ func assertInlineBranchHint(t *testing.T, doc *ir.Document) {
"the outside reference resolves to that same node rather than hoisting a second")
}

// assertAllOfPositionConstraints pins GitHub #407: a value constraint written
// beside an allOf composing position reaches the same ir.Model.Constraints
// field a plain object's does, rather than falling to Unmodeled under a
// diagnostic that claims the node has no home for it — the node demonstrably
// does, since ViaModel's sibling proves the field exists and is read.
func assertAllOfPositionConstraints(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) {
viaModel, ok := doc.Types[namedID("ViaModel")].(*ir.Model)
require.True(t, ok)
require.NotNil(t, viaModel.Constraints)
require.NotNil(t, viaModel.Constraints.MinProps)
assert.Equal(t, int64(2), *viaModel.Constraints.MinProps)

viaAllOf, ok := doc.Types[namedID("ViaAllOf")].(*ir.Model)
require.True(t, ok)
require.NotNil(t, viaAllOf.Base, "the sole allOf $ref still becomes Base")
assert.Equal(t, namedID("Base"), viaAllOf.Base.Target)
require.NotNil(t, viaAllOf.Constraints, "the composing position's own minProperties reaches the model it built, exactly as a plain object's does")
require.NotNil(t, viaAllOf.Constraints.MinProps)
assert.Equal(t, int64(2), *viaAllOf.Constraints.MinProps)
assert.NotContains(t, viaAllOf.Unmodeled, "openapi:minProperties",
"minProperties has a home now, so it is not also kept verbatim")
assert.Empty(t, diagsAt(diags, diag.DegradedConstruct, "/components/schemas/ViaAllOf"),
"and no diagnostic claims the node has no home for it")
}

func assertOneOfDiscriminated(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
pet, ok := doc.Types[namedID("Pet")].(*ir.Union)
require.True(t, ok, "oneOf survives as a Union node, never collapsed")
Expand Down
23 changes: 20 additions & 3 deletions compilers/openapi/internal/schema/compose.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,19 @@ import (
// hierarchy) becomes Base; other $refs become Mixins in source order; inline
// branches contribute their properties, each carrying provenance into the
// allOf branch it came from.
//
// The composing position's own value constraints (minProperties, maxProperties,
// ...) are read the same way lowerModel reads a plain object's: a bound written
// beside the allOf constrains the composed model exactly as one written beside
// `type: object` constrains it, so it belongs on the same field rather than
// falling to Unmodeled for want of a fill this lowering forgot to do (GitHub
// #407).
func lowerAllOf(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth int, s *oas3.Schema, pointer, hint string) (ir.TypeID, []ir.Diagnostic) {
var diags []ir.Diagnostic
id := internNode(c, ts, pointer, hint, func(common ir.TypeCommon) ir.TypeDef {
m := &ir.Model{TypeCommon: common}
cons, consDiags := schemaConstraints(c, &common.Unmodeled, s, pointer)
diags = append(diags, consDiags...)
m := &ir.Model{TypeCommon: common, Constraints: cons}
diags = append(diags, fillAllOf(c, ts, anchors, depth, m, s, pointer)...)
diags = append(diags, fillModelProperties(c, ts, anchors, depth, m, s, pointer)...)
diags = append(diags, applyCompositionRequired(c, m, s, pointer)...)
Expand Down Expand Up @@ -976,9 +985,17 @@ func composedVariantNullable(body *oas3.Schema, branch ir.TypeRef) bool {
// and any shared additionalProperties node are the single set the source
// declared, named after the enclosing schema rather than after whichever branch
// happened to build them first.
//
// The enclosing schema's own value constraints are read the same way, for the
// same reason lowerAllOf reads them (GitHub #407): `S ∧ (X | Y)` distributes to
// `(S ∧ X) | (S ∧ Y)`, and a bound S declares constrains both sides of that
// disjunction, so each variant carries its own copy rather than the bound
// reaching no field at all — this path never runs the pointer's own census, so
// nothing else stamps it.
func buildComposedVariant(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth int, body composedBody, branch ir.TypeID, common ir.TypeCommon) (ir.TypeDef, []ir.Diagnostic) {
m := &ir.Model{TypeCommon: common}
diags := fillAllOf(c, ts, anchors, depth, m, body.schema, body.pointer)
cons, diags := schemaConstraints(c, &common.Unmodeled, body.schema, body.pointer)
m := &ir.Model{TypeCommon: common, Constraints: cons}
diags = append(diags, fillAllOf(c, ts, anchors, depth, m, body.schema, body.pointer)...)
diags = append(diags, fillModelProperties(c, ts, anchors, depth, m, body.schema, body.pointer)...)
diags = append(diags, applyCompositionRequired(c, m, body.schema, body.pointer)...)
diags = append(diags, fillAdditional(c, ts, anchors, depth, m, body.schema, body.pointer, body.hint)...)
Expand Down
12 changes: 7 additions & 5 deletions compilers/openapi/internal/schema/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -837,11 +837,13 @@ func typeShapedBy(td ir.TypeDef, st oas3.SchemaType) bool {
// anything that missed the field here reaches no field anywhere.
//
// Having the field is not reading it, so the two node kinds that have one ask
// whether it was filled: a Model lowerAllOf composed carries no constraints at
// all, where one lowerModel built carries whatever schemaConstraints read. A
// List is absent for the stronger reason — lowerArray fills its Constraints from
// listConstraints, whose collection bounds valueConstraintKeywords deliberately
// excludes, so no value constraint ever reaches it however full the field looks.
// whether it was filled: a Model carries whatever schemaConstraints read,
// whether lowerModel or lowerAllOf built it — both fill Constraints the same
// way (GitHub #407), so the composing position's own bound has the same home a
// plain object's does. A List is absent for the stronger reason — lowerArray
// fills its Constraints from listConstraints, whose collection bounds
// valueConstraintKeywords deliberately excludes, so no value constraint ever
// reaches it however full the field looks.
func constraintsHome(td ir.TypeDef) bool {
switch n := td.(type) {
case *ir.Scalar:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@
"nullable": false
}
],
"constraints": {
"uniqueItems": false,
"minProps": 2
},
"abstract": false,
"positional": false,
"inputOnly": false
Expand Down Expand Up @@ -106,6 +110,10 @@
"nullable": false
}
],
"constraints": {
"uniqueItems": false,
"minProps": 2
},
"abstract": false,
"positional": false,
"inputOnly": false
Expand Down Expand Up @@ -597,7 +605,7 @@
{
"format": "openapi@3.1",
"path": "allof-oneof-cooccurrence.yaml",
"hash": "9deaf845e7a310c701113bd61e60a7e0d170cbf96860921816623667813e458f"
"hash": "a7ad825638ecc39459061faaec4308e1f47ab61c99d30cad7b57373bbbab329a"
}
]
}
5 changes: 4 additions & 1 deletion testdata/conformance/openapi/allof-oneof-cooccurrence.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,16 @@ components:
properties:
b: {type: string}
# allOf and oneOf at one level conjoin: the value satisfies Base AND exactly
# one of A/B. Both sides must survive (reference-learnings B11).
# one of A/B. Both sides must survive (reference-learnings B11). minProperties
# is the enclosing schema's own value constraint, distributed across both
# variants the same as Base is (GitHub #407).
Combo:
allOf:
- {$ref: '#/components/schemas/Base'}
oneOf:
- {$ref: '#/components/schemas/A'}
- {$ref: '#/components/schemas/B'}
minProperties: 2
# An inline branch names no referent to conjoin Base with, so this union is
# kept verbatim instead of half-distributed (ir-design 4.8).
MixedKinds:
Expand Down
175 changes: 175 additions & 0 deletions testdata/conformance/openapi/allof-position-constraints.golden.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
{
"irVersion": "0.4.0",
"name": "AllOfPositionConstraints",
"version": "1.0.0",
"docs": {},
"services": [
{
"id": "s/openapi/0",
"name": {
"source": "AllOfPositionConstraints",
"canonical": "all_of_position_constraints"
},
"docs": {},
"auth": null,
"provenance": {
"source": 0
}
}
],
"types": {
"t/openapi/components/schemas/Base": {
"kind": "model",
"id": "t/openapi/components/schemas/Base",
"name": {
"source": "Base",
"canonical": "base"
},
"anonymous": false,
"docs": {},
"sensitive": false,
"provenance": {
"source": 0,
"pointer": "/components/schemas/Base"
},
"properties": [
{
"id": "p/openapi/components/schemas/Base/properties/id",
"name": {
"source": "id",
"canonical": "id"
},
"wireName": "id",
"type": {
"target": "t/prim/string",
"nullable": false
},
"required": false,
"clientOptional": false,
"defaultAdded": false,
"visibility": {
"none": false
},
"flatten": false,
"eventHeader": false,
"eventPayload": false,
"secret": false,
"docs": {},
"provenance": {
"source": 0,
"pointer": "/components/schemas/Base/properties/id"
}
}
],
"abstract": false,
"positional": false,
"inputOnly": false
},
"t/openapi/components/schemas/ViaAllOf": {
"kind": "model",
"id": "t/openapi/components/schemas/ViaAllOf",
"name": {
"source": "ViaAllOf",
"canonical": "via_all_of"
},
"anonymous": false,
"docs": {},
"sensitive": false,
"provenance": {
"source": 0,
"pointer": "/components/schemas/ViaAllOf"
},
"base": {
"target": "t/openapi/components/schemas/Base",
"nullable": false
},
"constraints": {
"uniqueItems": false,
"minProps": 2
},
"abstract": false,
"positional": false,
"inputOnly": false
},
"t/openapi/components/schemas/ViaModel": {
"kind": "model",
"id": "t/openapi/components/schemas/ViaModel",
"name": {
"source": "ViaModel",
"canonical": "via_model"
},
"anonymous": false,
"docs": {},
"sensitive": false,
"provenance": {
"source": 0,
"pointer": "/components/schemas/ViaModel"
},
"properties": [
{
"id": "p/openapi/components/schemas/ViaModel/properties/a",
"name": {
"source": "a",
"canonical": "a"
},
"wireName": "a",
"type": {
"target": "t/prim/string",
"nullable": false
},
"required": false,
"clientOptional": false,
"defaultAdded": false,
"visibility": {
"none": false
},
"flatten": false,
"eventHeader": false,
"eventPayload": false,
"secret": false,
"docs": {},
"provenance": {
"source": 0,
"pointer": "/components/schemas/ViaModel/properties/a"
}
}
],
"constraints": {
"uniqueItems": false,
"minProps": 2
},
"abstract": false,
"positional": false,
"inputOnly": false
},
"t/prim/string": {
"kind": "primitive",
"id": "t/prim/string",
"name": {},
"anonymous": false,
"docs": {},
"sensitive": false,
"provenance": {
"source": 0
},
"prim": "string"
}
},
"servers": [
{
"name": {
"hint": "server"
},
"urlTemplate": "/",
"description": {},
"auth": null
}
],
"sources": [
{
"format": "openapi@3.1",
"path": "allof-position-constraints.yaml",
"hash": "8f0d33d038c59706de71c4c801a8989898caa9309509475fab447cdd84e79496"
}
]
}
8 changes: 8 additions & 0 deletions testdata/conformance/openapi/allof-position-constraints.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
openapi: 3.1.0
info: {title: AllOfPositionConstraints, version: "1.0.0"}
paths: {}
components:
schemas:
Base: {type: object, properties: {id: {type: string}}}
ViaModel: {type: object, properties: {a: {type: string}}, minProperties: 2}
ViaAllOf: {allOf: [{$ref: '#/components/schemas/Base'}], minProperties: 2}
Loading