Skip to content

Commit 702cc91

Browse files
fix(output-options): correctly reference x-go-type-skip-optional-pointer in allOf (#2042)
If using the global `prefer-skip-optional-pointer: true` alongside a field-level `x-go-type-skip-optional-pointer: false` on a type that's referenced by an `allOf`, we don't correctly follow the field-level type reference and override the value. This correctly generates the type. Co-authored-by: Jamie Tanna <[email protected]>
1 parent 62fec2f commit 702cc91

File tree

4 files changed

+96
-15
lines changed

4 files changed

+96
-15
lines changed

examples/output-options/preferskipoptionalpointer/api.yaml

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,31 @@ components:
3737
client:
3838
description: This field is optional, but the `prefer-skip-optional-pointer` Output Option ensures that this should not have an optional pointer.
3939
$ref: '#/components/schemas/Client'
40+
ReferencesATypeWithAnExtensionInsideAllOf:
41+
type: object
42+
description: This type has a field which references - via an `allOf` - a type which should have an optional pointer, as the field-level definition of `x-go-type-skip-optional-pointer` overrides the `prefer-skip-optional-pointer` Output Option, as well as a field without that property.
43+
additionalProperties: false
44+
required:
45+
- description
46+
properties:
47+
noExtension:
48+
# NOTE this will not generate pointer type as global preference `prefer-skip-optional-pointer: true`
49+
allOf:
50+
- $ref: '#/components/schemas/ReferencedWithoutExtension'
51+
withExtensionPointer:
52+
# NOTE this will generate pointer type (despite global preference `prefer-skip-optional-pointer: true`) as we have field-level `x-go-type-skip-optional-pointer: false`
53+
allOf:
54+
- $ref: '#/components/schemas/ReferencedWithExtension'
55+
ReferencedWithoutExtension:
56+
type: object
57+
additionalProperties: false
58+
properties:
59+
foo:
60+
type: string
61+
ReferencedWithExtension:
62+
type: object
63+
additionalProperties: false
64+
properties:
65+
foo:
66+
type: string
67+
x-go-type-skip-optional-pointer: false

examples/output-options/preferskipoptionalpointer/gen.go

Lines changed: 16 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

examples/output-options/preferskipoptionalpointer/gen_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,42 @@ func TestNestedType(t *testing.T) {
8080
})
8181
}
8282

83+
func TestReferencesATypeWithAnExtension(t *testing.T) {
84+
t.Run("zero value", func(t *testing.T) {
85+
typeWithExt := ReferencesATypeWithAnExtensionInsideAllOf{}
86+
87+
assert.Zero(t, typeWithExt)
88+
assert.Zero(t, typeWithExt.NoExtension)
89+
assert.Nil(t, typeWithExt.WithExtensionPointer)
90+
})
91+
92+
t.Run("value on noExtension", func(t *testing.T) {
93+
typeWithExt := ReferencesATypeWithAnExtensionInsideAllOf{
94+
NoExtension: ReferencedWithoutExtension{"value"},
95+
WithExtensionPointer: nil,
96+
}
97+
98+
b, err := json.Marshal(typeWithExt)
99+
require.NoError(t, err)
100+
101+
assert.True(t, jsonContainsKey(b, "noExtension"))
102+
assert.False(t, jsonContainsKey(b, "withExtensionPointer"))
103+
})
104+
105+
t.Run("value on noExtension and withExtensionPointer", func(t *testing.T) {
106+
typeWithExt := ReferencesATypeWithAnExtensionInsideAllOf{
107+
NoExtension: ReferencedWithoutExtension{"value"},
108+
WithExtensionPointer: &ReferencedWithExtension{"ptrValue"},
109+
}
110+
111+
b, err := json.Marshal(typeWithExt)
112+
require.NoError(t, err)
113+
114+
assert.True(t, jsonContainsKey(b, "noExtension"))
115+
assert.True(t, jsonContainsKey(b, "withExtensionPointer"))
116+
})
117+
}
118+
83119
// jsonContainsKey checks if the given JSON object contains the specified key at the top level.
84120
func jsonContainsKey(b []byte, key string) bool {
85121
var m map[string]any

pkg/codegen/schema.go

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,18 @@ func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) {
260260

261261
schema := sref.Value
262262

263+
// Check x-go-type-skip-optional-pointer, which will override if the type
264+
// should be a pointer or not when the field is optional.
265+
// NOTE skipOptionalPointer will be defaulted to the global value, but can be overridden on a per-type/-field basis
266+
skipOptionalPointer := globalState.options.OutputOptions.PreferSkipOptionalPointer
267+
if extension, ok := schema.Extensions[extPropGoTypeSkipOptionalPointer]; ok {
268+
var err error
269+
skipOptionalPointer, err = extParsePropGoTypeSkipOptionalPointer(extension)
270+
if err != nil {
271+
return Schema{}, fmt.Errorf("invalid value for %q: %w", extPropGoTypeSkipOptionalPointer, err)
272+
}
273+
}
274+
263275
// If Ref is set on the SchemaRef, it means that this type is actually a reference to
264276
// another type. We're not de-referencing, so simply use the referenced type.
265277
if IsGoTypeReference(sref.Ref) {
@@ -274,15 +286,14 @@ func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) {
274286
Description: schema.Description,
275287
DefineViaAlias: true,
276288
OAPISchema: schema,
277-
SkipOptionalPointer: globalState.options.OutputOptions.PreferSkipOptionalPointer,
289+
SkipOptionalPointer: skipOptionalPointer,
278290
}, nil
279291
}
280292

281293
outSchema := Schema{
282-
Description: schema.Description,
283-
OAPISchema: schema,
284-
// NOTE that SkipOptionalPointer will be defaulted to the global value, but can be overridden on a per-type/-field basis
285-
SkipOptionalPointer: globalState.options.OutputOptions.PreferSkipOptionalPointer,
294+
Description: schema.Description,
295+
OAPISchema: schema,
296+
SkipOptionalPointer: skipOptionalPointer,
286297
}
287298

288299
// AllOf is interesting, and useful. It's the union of a number of other
@@ -298,16 +309,6 @@ func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) {
298309
return mergedSchema, nil
299310
}
300311

301-
// Check x-go-type-skip-optional-pointer, which will override if the type
302-
// should be a pointer or not when the field is optional.
303-
if extension, ok := schema.Extensions[extPropGoTypeSkipOptionalPointer]; ok {
304-
skipOptionalPointer, err := extParsePropGoTypeSkipOptionalPointer(extension)
305-
if err != nil {
306-
return outSchema, fmt.Errorf("invalid value for %q: %w", extPropGoTypeSkipOptionalPointer, err)
307-
}
308-
outSchema.SkipOptionalPointer = skipOptionalPointer
309-
}
310-
311312
// Check x-go-type, which will completely override the definition of this
312313
// schema with the provided type.
313314
if extension, ok := schema.Extensions[extPropGoType]; ok {

0 commit comments

Comments
 (0)