Skip to content

Commit 538c85e

Browse files
committed
Fix Ref overwrite
1 parent 89551fe commit 538c85e

File tree

4 files changed

+24
-73
lines changed

4 files changed

+24
-73
lines changed

pkg/generators/models/generate.go

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ func (g generator) createNamedSchemaForInlineSchemas(ctx context.Context, name s
328328
return nil
329329
}
330330

331-
ref = resolveAllOf(ref, nil)
331+
ref = resolveAllOf(ref, shouldCreate, nil)
332332
if ref.Ref != "" {
333333
return nil
334334
}
@@ -340,6 +340,7 @@ func (g generator) createNamedSchemaForInlineSchemas(ctx context.Context, name s
340340
err = g.createNamedSchemaForInlineSchema(ctx, name, ref, schemas)
341341
}
342342
case "array":
343+
// Follow array items schema
343344
err = g.createNamedSchemaForInlineSchemas(ctx, name, ref.Value.Items, true, schemas)
344345
case "", "object":
345346
// Create a ref if the object has properties or additionalProperties
@@ -349,19 +350,6 @@ func (g generator) createNamedSchemaForInlineSchemas(ctx context.Context, name s
349350

350351
// Check all properties for inline schemas
351352
for propName, propRef := range ref.Value.Properties {
352-
// special case for singleton allOf generally these are used to add a new description or override the
353-
// nullable flag
354-
if len(propRef.Value.AllOf) == 1 {
355-
pointer := propRef.Value.AllOf[0]
356-
propRef.Ref = pointer.Ref
357-
358-
// a sub-special case when the referenced type is an array. Because we don't create top-level models
359-
// for arrays, we actually need to follow this reference.
360-
resolved := resolveAllOf(propRef, nil)
361-
if resolved.Value.Type == "array" {
362-
propRef = resolved
363-
}
364-
}
365353
err = g.createNamedSchemaForInlineSchemas(ctx, fmt.Sprintf("%s_%s", name, propName), propRef, true, schemas)
366354
if err != nil {
367355
return err

pkg/generators/models/models.go

Lines changed: 20 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,7 @@ func (m *Model) Render(ctx context.Context, writer io.Writer) error {
460460
// resolveAllOf resolves the list of `allOf` definitions in the schema to a complete type merging
461461
// all the mentioned types.
462462
// `passed` can be nil, it's used recursively in order to avoid infinite loops
463-
func resolveAllOf(ref *openapi3.SchemaRef, passed passedSchemas) (out *openapi3.SchemaRef) {
463+
func resolveAllOf(ref *openapi3.SchemaRef, overrideRef bool, passed passedSchemas) (out *openapi3.SchemaRef) {
464464
defer func() {
465465
// clean up the allOf field after resolving it
466466
ref.Value.AllOf = nil
@@ -482,17 +482,6 @@ func resolveAllOf(ref *openapi3.SchemaRef, passed passedSchemas) (out *openapi3.
482482
passed = make(passedSchemas)
483483
}
484484

485-
// very special case where the allOf references
486-
// another named schema but is not actually merging
487-
// several schemas, this is usually just overriding
488-
// a value like nullable or the description
489-
if len(ref.Value.AllOf) == 1 {
490-
pointer := ref.Value.AllOf[0]
491-
passed[pointer] = true
492-
out = deepMerge(out, pointer, passed)
493-
return out
494-
}
495-
496485
// this is used for the special edge cases like this
497486
// allOf:
498487
// - description: "this will only set the description value"
@@ -503,27 +492,23 @@ func resolveAllOf(ref *openapi3.SchemaRef, passed passedSchemas) (out *openapi3.
503492
// the special case of a single `ref`, which will used the named `ColumnTypeMetadata` type.
504493
ref.Value.AllOf = removeSemanticallyEmptyRefs(ref.Value.AllOf)
505494

506-
isSelfRef := false
495+
// very special case where the allOf references
496+
// another named schema but is not actually merging
497+
// several schemas, this is usually just overriding
498+
// a value like nullable or the description
499+
if len(ref.Value.Properties) == 0 && len(ref.Value.AllOf) == 1 {
500+
pointer := ref.Value.AllOf[0]
501+
passed[pointer] = true
502+
out = deepMerge(out, pointer, overrideRef, passed)
503+
return out
504+
}
505+
507506
for _, subSchema := range ref.Value.AllOf {
508507
if passed[subSchema] {
509-
isSelfRef = true
510508
continue
511509
}
512510
passed[subSchema] = true
513-
out = deepMerge(out, subSchema, passed)
514-
}
515-
516-
// remove the reference field on AllOf values, these should be
517-
// flattened into a single (potentially inlined) struct.
518-
// We make exceptions for
519-
// * types are contain a self-reference, these can not be flattened
520-
// * types are a single allOf, these usually indicate merging in nullable properties, etc
521-
// it is not a _real_ allOf, rather we are just overriding fields on a single type.
522-
// this case should already be handled above, so this is really checking if AllOf > 0,
523-
// but checking >1 is more semantically correct
524-
//
525-
if !isSelfRef && len(ref.Value.AllOf) > 1 {
526-
out.Ref = ""
511+
out = deepMerge(out, subSchema, false, passed)
527512
}
528513

529514
return out
@@ -618,7 +603,7 @@ func isNonEmptySchemaRef(ref *openapi3.SchemaRef) bool {
618603
// deepMerge merges `right` into `left` schema recursively resolving types (e.g. `allOf`).
619604
// `passed` map will be populated with all the visited types during the resolution process, so we can
620605
// avoid the infinite loop.
621-
func deepMerge(left *openapi3.SchemaRef, right *openapi3.SchemaRef, passed passedSchemas) (out *openapi3.SchemaRef) {
606+
func deepMerge(left *openapi3.SchemaRef, right *openapi3.SchemaRef, overrideRef bool, passed passedSchemas) (out *openapi3.SchemaRef) {
622607
out = left
623608
if out == nil {
624609
out = &openapi3.SchemaRef{}
@@ -634,11 +619,11 @@ func deepMerge(left *openapi3.SchemaRef, right *openapi3.SchemaRef, passed passe
634619
right.Value = &openapi3.Schema{}
635620
}
636621

637-
right = resolveAllOf(right, passed)
638-
if right.Ref != "" {
622+
right = resolveAllOf(right, false, passed)
623+
if overrideRef && out.Ref == "" && right.Ref != "" {
639624
out.Ref = right.Ref
640625
}
641-
if right.Value.Type != "" {
626+
if out.Value.Type == "" && right.Value.Type != "" {
642627
out.Value.Type = right.Value.Type
643628
}
644629

@@ -679,6 +664,7 @@ func deepMerge(left *openapi3.SchemaRef, right *openapi3.SchemaRef, passed passe
679664
out.Value.AdditionalProperties = deepMerge(
680665
out.Value.AdditionalProperties,
681666
right.Value.AdditionalProperties,
667+
overrideRef,
682668
nil, // here we merge all over, without `passed`
683669
)
684670
}
@@ -688,12 +674,12 @@ func deepMerge(left *openapi3.SchemaRef, right *openapi3.SchemaRef, passed passe
688674
out.Value.Properties = make(map[string]*openapi3.SchemaRef)
689675
}
690676
for k, v := range right.Value.Properties {
691-
out.Value.Properties[k] = resolveAllOf(v, passed)
677+
out.Value.Properties[k] = resolveAllOf(v, true, passed)
692678
}
693679
}
694680

695681
if right.Value.Type == "array" && right.Value.Items != nil {
696-
out.Value.Items = resolveAllOf(right.Value.Items, passed)
682+
out.Value.Items = resolveAllOf(right.Value.Items, true, passed)
697683
}
698684

699685
return out
@@ -718,29 +704,6 @@ func structPropsFromRef(ref *openapi3.SchemaRef) (specs []PropSpec, imports map[
718704
imports = make(map[string]string)
719705
for _, name := range sortedKeys(ref.Value.Properties) {
720706
prop := ref.Value.Properties[name]
721-
722-
// special case for singleton allOf
723-
// generally these are used to add
724-
// a new description or override the
725-
// nullable flag
726-
if len(prop.Value.AllOf) == 1 {
727-
pointer := prop.Value.AllOf[0]
728-
prop.Ref = pointer.Ref
729-
730-
// a sub-special case when the referenced type
731-
// is an array. Because we don't create top-level
732-
// models for arrays, we actually need to follow
733-
// this reference.
734-
resolved := resolveAllOf(prop, nil)
735-
if resolved.Value.Type == "array" {
736-
prop = resolved
737-
}
738-
}
739-
740-
if prop.Ref == "" {
741-
prop = resolveAllOf(prop, nil)
742-
}
743-
744707
isRequired := checkIfRequired(name, ref.Value.Required)
745708
var spec *PropSpec
746709
spec, err = newPropertySpecFromRef(name, prop, isRequired, imports)

pkg/generators/models/testdata/cases/allof_mixin_skipping/api.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ components:
3535
allOf:
3636
- $ref: '#/components/schemas/ColumnTypeMetadata'
3737
- description: Array item type if this type is array
38-
nullable: true
38+
nullable: true
3939
nullable:
4040
$ref: '#/components/schemas/Nullability'
4141
originalName:

pkg/generators/models/testdata/cases/allof_self_reference/api.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,4 @@ components:
1414
description: the next item
1515
allOf:
1616
- $ref: "#/components/schemas/ListItem"
17-
- nullable: true
17+
nullable: true

0 commit comments

Comments
 (0)