Skip to content

Commit 27c7acb

Browse files
committed
Preserve typed property when allOf branches overlap
1 parent 6121bcb commit 27c7acb

3 files changed

Lines changed: 83 additions & 2 deletions

File tree

pkg/codegen/schema_merge.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -713,18 +713,36 @@ func mergeOpenapiSchemas(s1, s2 *base.Schema) (*base.Schema, error) {
713713
// Required. We merge these.
714714
result.Required = append(s1.Required, s2.Required...)
715715

716-
// We merge all properties
716+
// We merge all properties. When both schemas declare a property with
717+
// the same name, the typed declaration wins over an untyped one
718+
// (which usually only carries annotations like example or
719+
// description). Without this, the later schema silently overwrote
720+
// the earlier one and any type/enum/constraint it carried was lost.
721+
//
722+
// We deliberately pick one of the original SchemaProxies rather
723+
// than fabricating a merged proxy via CreateSchemaProxy: downstream
724+
// code reads GoLow().GetReference() to attribute properties back to
725+
// the spec, and a synthetic proxy has no low-level backing.
717726
for k, v := range s1.Properties.FromOldest() {
718727
if result.Properties == nil {
719728
result.Properties = orderedmap.New[string, *base.SchemaProxy]()
720729
}
721730
result.Properties.Set(k, v)
722731
}
723732
for k, v := range s2.Properties.FromOldest() {
724-
// TODO: detect conflicts
725733
if result.Properties == nil {
726734
result.Properties = orderedmap.New[string, *base.SchemaProxy]()
727735
}
736+
737+
if existing, exists := result.Properties.Get(k); exists {
738+
if len(getSchemaType(existing.Schema())) == 0 && len(getSchemaType(v.Schema())) > 0 {
739+
result.Properties.Set(k, v)
740+
}
741+
// Otherwise the existing (s1's) property keeps its place;
742+
// either it already has a type and we don't want to drop it,
743+
// or neither side has a type and the choice is moot.
744+
continue
745+
}
728746
result.Properties.Set(k, v)
729747
}
730748

pkg/codegen/schema_merge_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,35 @@ func TestMergeOpenapiSchemas(t *testing.T) {
119119
assert.NotEmpty(t, code)
120120
})
121121

122+
t.Run("overlapping property keeps typed declaration", func(t *testing.T) {
123+
// When two allOf branches declare the same property and only
124+
// one of them gives it a type (the other only attaches an
125+
// annotation such as `example`), the typed declaration must
126+
// survive the merge. Previously the second branch silently
127+
// overwrote the first, turning a `status: string` into an
128+
// empty struct.
129+
contents, err := os.ReadFile("testdata/merge-overlapping-property-typed-wins.yml")
130+
require.NoError(t, err)
131+
132+
opts := Configuration{
133+
PackageName: "testpkg",
134+
Output: &Output{
135+
UseSingleFile: true,
136+
},
137+
}
138+
139+
code, err := Generate(contents, opts)
140+
require.NoError(t, err)
141+
combined := code.GetCombined()
142+
143+
assert.Contains(t, combined, "type InvoiceCancellation struct",
144+
"InvoiceCancellation should be a struct")
145+
assert.Regexp(t, `(?m)Status\s+\*?string`, combined,
146+
"Status should keep the string type from the referenced Invoice schema, not collapse to an empty struct from the annotation-only override")
147+
assert.NotRegexp(t, `(?m)Status\s+\*?struct\{\}`, combined,
148+
"Status must not be generated as struct{}; that would mean the annotation-only override dropped the typed declaration")
149+
})
150+
122151
t.Run("allOf with sibling properties preserves all fields", func(t *testing.T) {
123152
contents, err := os.ReadFile("testdata/allof-with-sibling-properties.yml")
124153
require.NoError(t, err)
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
openapi: 3.0.0
2+
info:
3+
title: Overlapping property in allOf preserves the typed declaration
4+
version: 1.0.0
5+
paths:
6+
/test:
7+
get:
8+
responses:
9+
'200':
10+
description: OK
11+
content:
12+
application/json:
13+
schema:
14+
$ref: '#/components/schemas/InvoiceCancellation'
15+
components:
16+
schemas:
17+
Invoice:
18+
type: object
19+
properties:
20+
id:
21+
type: string
22+
status:
23+
type: string
24+
enum:
25+
- open
26+
- paid
27+
- cancelled
28+
InvoiceCancellation:
29+
allOf:
30+
- $ref: '#/components/schemas/Invoice'
31+
- type: object
32+
properties:
33+
status:
34+
example: cancelled

0 commit comments

Comments
 (0)