From eef9700d10605f8b37fdd4762658d3726682eb9a Mon Sep 17 00:00:00 2001 From: sdghchj Date: Mon, 21 Nov 2022 15:37:07 +0800 Subject: [PATCH 1/5] fix #1390 --- generics_test.go | 2 + parser.go | 18 +++- schema.go | 5 ++ testdata/generics_nested/expected.json | 110 ++++++++++++++++++++----- 4 files changed, 110 insertions(+), 25 deletions(-) diff --git a/generics_test.go b/generics_test.go index d62f3898c..d4d292e6e 100644 --- a/generics_test.go +++ b/generics_test.go @@ -7,6 +7,7 @@ import ( "encoding/json" "fmt" "go/ast" + "io/fs" "os" "path/filepath" "testing" @@ -70,6 +71,7 @@ func TestParseGenericsNested(t *testing.T) { assert.NoError(t, err) b, err := json.MarshalIndent(p.swagger, "", " ") assert.NoError(t, err) + os.WriteFile("testdata/generics_nested/expected.json", b, fs.ModePerm) assert.Equal(t, string(expected), string(b)) } diff --git a/parser.go b/parser.go index e9d44e00e..aee432cfd 100644 --- a/parser.go +++ b/parser.go @@ -1281,9 +1281,21 @@ func (parser *Parser) parseStructField(file *ast.File, field *ast.Field) (map[st } } - err = ps.ComplementSchema(schema) - if err != nil { - return nil, nil, err + if IsRefSchema(schema) { + additionalSchema := *schema + err = ps.ComplementSchema(&additionalSchema) + if err != nil { + return nil, nil, err + } + if !reflect.DeepEqual(schema, &additionalSchema) { + additionalSchema.Ref = spec.Ref{} + schema = spec.ComposedSchema(*schema, additionalSchema) + } + } else { + err = ps.ComplementSchema(schema) + if err != nil { + return nil, nil, err + } } var tagRequired []string diff --git a/schema.go b/schema.go index 5949113fb..21cb2e2ba 100644 --- a/schema.go +++ b/schema.go @@ -135,6 +135,11 @@ func ignoreNameOverride(name string) bool { return len(name) != 0 && name[0] == IgnoreNameOverridePrefix } +// IsRefSchema whether a schema is a reference schema. +func IsRefSchema(schema *spec.Schema) bool { + return schema.Ref.Ref.GetURL() != nil +} + // RefSchema build a reference schema. func RefSchema(refType string) *spec.Schema { return spec.RefSchema("#/definitions/" + refType) diff --git a/testdata/generics_nested/expected.json b/testdata/generics_nested/expected.json index a2d005505..2d58d1f85 100644 --- a/testdata/generics_nested/expected.json +++ b/testdata/generics_nested/expected.json @@ -187,8 +187,14 @@ "type": "object", "properties": { "itemOne": { - "description": "ItemsOne is the first thing", - "$ref": "#/definitions/types.Post" + "allOf": [ + { + "$ref": "#/definitions/types.Post" + }, + { + "description": "ItemsOne is the first thing" + } + ] }, "itemsTwo": { "description": "ItemsTwo is the second thing", @@ -206,8 +212,14 @@ "type": "object", "properties": { "itemOne": { - "description": "ItemsOne is the first thing", - "$ref": "#/definitions/types.Post" + "allOf": [ + { + "$ref": "#/definitions/types.Post" + }, + { + "description": "ItemsOne is the first thing" + } + ] }, "itemsTwo": { "description": "ItemsTwo is the second thing", @@ -222,8 +234,14 @@ "type": "object", "properties": { "itemOne": { - "description": "ItemsOne is the first thing", - "$ref": "#/definitions/types.Post" + "allOf": [ + { + "$ref": "#/definitions/types.Post" + }, + { + "description": "ItemsOne is the first thing" + } + ] }, "itemsTwo": { "description": "ItemsTwo is the second thing", @@ -265,8 +283,14 @@ "type": "object", "properties": { "items": { - "description": "Items from the list response", - "$ref": "#/definitions/types.Post" + "allOf": [ + { + "$ref": "#/definitions/types.Post" + }, + { + "description": "Items from the list response" + } + ] } } }, @@ -274,8 +298,14 @@ "type": "object", "properties": { "items": { - "description": "Items from the list response", - "$ref": "#/definitions/web.GenericInnerType-array_types_Post" + "allOf": [ + { + "$ref": "#/definitions/web.GenericInnerType-array_types_Post" + }, + { + "description": "Items from the list response" + } + ] }, "status": { "description": "Status of some other stuff", @@ -287,8 +317,14 @@ "type": "object", "properties": { "items": { - "description": "Items from the list response", - "$ref": "#/definitions/web.GenericInnerType-types_Post" + "allOf": [ + { + "$ref": "#/definitions/web.GenericInnerType-types_Post" + }, + { + "description": "Items from the list response" + } + ] }, "status": { "description": "Status of some other stuff", @@ -438,8 +474,14 @@ "type": "object", "properties": { "itemOne": { - "description": "ItemsOne is the first thing", - "$ref": "#/definitions/types.Post" + "allOf": [ + { + "$ref": "#/definitions/types.Post" + }, + { + "description": "ItemsOne is the first thing" + } + ] }, "itemsTwo": { "description": "ItemsTwo is the second thing", @@ -458,8 +500,14 @@ "type": "object", "properties": { "itemOne": { - "description": "ItemsOne is the first thing", - "$ref": "#/definitions/types.Post" + "allOf": [ + { + "$ref": "#/definitions/types.Post" + }, + { + "description": "ItemsOne is the first thing" + } + ] }, "itemsTwo": { "description": "ItemsTwo is the second thing", @@ -478,8 +526,14 @@ "type": "object", "properties": { "itemOne": { - "description": "ItemsOne is the first thing", - "$ref": "#/definitions/types.Post" + "allOf": [ + { + "$ref": "#/definitions/types.Post" + }, + { + "description": "ItemsOne is the first thing" + } + ] }, "itemsTwo": { "description": "ItemsTwo is the second thing", @@ -498,8 +552,14 @@ "type": "object", "properties": { "itemOne": { - "description": "ItemsOne is the first thing", - "$ref": "#/definitions/web.GenericInnerType-array_types_Post" + "allOf": [ + { + "$ref": "#/definitions/web.GenericInnerType-array_types_Post" + }, + { + "description": "ItemsOne is the first thing" + } + ] }, "itemsTwo": { "description": "ItemsTwo is the second thing", @@ -521,8 +581,14 @@ "type": "object", "properties": { "itemOne": { - "description": "ItemsOne is the first thing", - "$ref": "#/definitions/web.GenericInnerType-types_Post" + "allOf": [ + { + "$ref": "#/definitions/web.GenericInnerType-types_Post" + }, + { + "description": "ItemsOne is the first thing" + } + ] }, "itemsTwo": { "description": "ItemsTwo is the second thing", From d66b66c7734c8d3a93bf30dd2442e2ee74370b6a Mon Sep 17 00:00:00 2001 From: sdghchj Date: Mon, 21 Nov 2022 19:38:38 +0800 Subject: [PATCH 2/5] fix #1390 --- field_parser.go | 24 ++++++-------- generics_test.go | 2 -- parser.go | 18 ++--------- testdata/generics_nested/expected.json | 44 +++++++------------------- 4 files changed, 24 insertions(+), 64 deletions(-) diff --git a/field_parser.go b/field_parser.go index 66f85427f..48a4f8e27 100644 --- a/field_parser.go +++ b/field_parser.go @@ -10,7 +10,6 @@ import ( "sync" "unicode" - "github.com/go-openapi/jsonreference" "github.com/go-openapi/spec" ) @@ -213,6 +212,16 @@ func (ps *tagBaseFieldParser) ComplementSchema(schema *spec.Schema) error { return fmt.Errorf("invalid type for field: %s", ps.field.Names[0]) } + if IsRefSchema(schema) { + oldSchema := schema + schema = &spec.Schema{} + defer func() { + if !reflect.ValueOf(*schema).IsZero() { + *oldSchema = *(schema.WithAllOf(*oldSchema)) + } + }() + } + if ps.field.Tag == nil { if ps.field.Doc != nil { schema.Description = strings.TrimSpace(ps.field.Doc.Text()) @@ -353,19 +362,6 @@ func (ps *tagBaseFieldParser) ComplementSchema(schema *spec.Schema) error { schema.ReadOnly = ps.tag.Get(readOnlyTag) == "true" - if !reflect.ValueOf(schema.Ref).IsZero() && schema.ReadOnly { - schema.AllOf = []spec.Schema{*spec.RefSchema(schema.Ref.String())} - schema.Ref = spec.Ref{ - Ref: jsonreference.Ref{ - HasFullURL: false, - HasURLPathOnly: false, - HasFragmentOnly: false, - HasFileScheme: false, - HasFullFilePath: false, - }, - } // clear out existing ref - } - defaultTagValue := ps.tag.Get(defaultTag) if defaultTagValue != "" { value, err := defineType(field.schemaType, defaultTagValue) diff --git a/generics_test.go b/generics_test.go index d4d292e6e..d62f3898c 100644 --- a/generics_test.go +++ b/generics_test.go @@ -7,7 +7,6 @@ import ( "encoding/json" "fmt" "go/ast" - "io/fs" "os" "path/filepath" "testing" @@ -71,7 +70,6 @@ func TestParseGenericsNested(t *testing.T) { assert.NoError(t, err) b, err := json.MarshalIndent(p.swagger, "", " ") assert.NoError(t, err) - os.WriteFile("testdata/generics_nested/expected.json", b, fs.ModePerm) assert.Equal(t, string(expected), string(b)) } diff --git a/parser.go b/parser.go index aee432cfd..e9d44e00e 100644 --- a/parser.go +++ b/parser.go @@ -1281,21 +1281,9 @@ func (parser *Parser) parseStructField(file *ast.File, field *ast.Field) (map[st } } - if IsRefSchema(schema) { - additionalSchema := *schema - err = ps.ComplementSchema(&additionalSchema) - if err != nil { - return nil, nil, err - } - if !reflect.DeepEqual(schema, &additionalSchema) { - additionalSchema.Ref = spec.Ref{} - schema = spec.ComposedSchema(*schema, additionalSchema) - } - } else { - err = ps.ComplementSchema(schema) - if err != nil { - return nil, nil, err - } + err = ps.ComplementSchema(schema) + if err != nil { + return nil, nil, err } var tagRequired []string diff --git a/testdata/generics_nested/expected.json b/testdata/generics_nested/expected.json index 2d58d1f85..850fc5ae4 100644 --- a/testdata/generics_nested/expected.json +++ b/testdata/generics_nested/expected.json @@ -187,12 +187,10 @@ "type": "object", "properties": { "itemOne": { + "description": "ItemsOne is the first thing", "allOf": [ { "$ref": "#/definitions/types.Post" - }, - { - "description": "ItemsOne is the first thing" } ] }, @@ -212,12 +210,10 @@ "type": "object", "properties": { "itemOne": { + "description": "ItemsOne is the first thing", "allOf": [ { "$ref": "#/definitions/types.Post" - }, - { - "description": "ItemsOne is the first thing" } ] }, @@ -234,12 +230,10 @@ "type": "object", "properties": { "itemOne": { + "description": "ItemsOne is the first thing", "allOf": [ { "$ref": "#/definitions/types.Post" - }, - { - "description": "ItemsOne is the first thing" } ] }, @@ -283,12 +277,10 @@ "type": "object", "properties": { "items": { + "description": "Items from the list response", "allOf": [ { "$ref": "#/definitions/types.Post" - }, - { - "description": "Items from the list response" } ] } @@ -298,12 +290,10 @@ "type": "object", "properties": { "items": { + "description": "Items from the list response", "allOf": [ { "$ref": "#/definitions/web.GenericInnerType-array_types_Post" - }, - { - "description": "Items from the list response" } ] }, @@ -317,12 +307,10 @@ "type": "object", "properties": { "items": { + "description": "Items from the list response", "allOf": [ { "$ref": "#/definitions/web.GenericInnerType-types_Post" - }, - { - "description": "Items from the list response" } ] }, @@ -474,12 +462,10 @@ "type": "object", "properties": { "itemOne": { + "description": "ItemsOne is the first thing", "allOf": [ { "$ref": "#/definitions/types.Post" - }, - { - "description": "ItemsOne is the first thing" } ] }, @@ -500,12 +486,10 @@ "type": "object", "properties": { "itemOne": { + "description": "ItemsOne is the first thing", "allOf": [ { "$ref": "#/definitions/types.Post" - }, - { - "description": "ItemsOne is the first thing" } ] }, @@ -526,12 +510,10 @@ "type": "object", "properties": { "itemOne": { + "description": "ItemsOne is the first thing", "allOf": [ { "$ref": "#/definitions/types.Post" - }, - { - "description": "ItemsOne is the first thing" } ] }, @@ -552,12 +534,10 @@ "type": "object", "properties": { "itemOne": { + "description": "ItemsOne is the first thing", "allOf": [ { "$ref": "#/definitions/web.GenericInnerType-array_types_Post" - }, - { - "description": "ItemsOne is the first thing" } ] }, @@ -581,12 +561,10 @@ "type": "object", "properties": { "itemOne": { + "description": "ItemsOne is the first thing", "allOf": [ { "$ref": "#/definitions/web.GenericInnerType-types_Post" - }, - { - "description": "ItemsOne is the first thing" } ] }, From 733dbbb5874cba0380020ed213a665fafe4eb204 Mon Sep 17 00:00:00 2001 From: sdghchj Date: Mon, 21 Nov 2022 20:28:50 +0800 Subject: [PATCH 3/5] fix tests --- parser_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/parser_test.go b/parser_test.go index 644227f6b..2f217f3b6 100644 --- a/parser_test.go +++ b/parser_test.go @@ -2610,7 +2610,11 @@ func Test(){ }, "test6": { "description": "test6", - "$ref": "#/definitions/api.MyMapType" + "allOf": [ + { + "$ref": "#/definitions/api.MyMapType" + } + ] }, "test7": { "description": "test7", From d0beb72ef2eed55920840864d3e5588cdcbf9044 Mon Sep 17 00:00:00 2001 From: sdghchj Date: Mon, 21 Nov 2022 20:42:59 +0800 Subject: [PATCH 4/5] fix tests --- parser_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/parser_test.go b/parser_test.go index 2f217f3b6..5bd7d8fdd 100644 --- a/parser_test.go +++ b/parser_test.go @@ -2506,7 +2506,11 @@ func Test(){ }, "test2": { "description": "test2", - "$ref": "#/definitions/api.Child" + "allOf": [ + { + "$ref": "#/definitions/api.Child" + } + ] } } } From 4370d4527b8e233d11b1febbb600889f4d3e1396 Mon Sep 17 00:00:00 2001 From: sdghchj Date: Tue, 22 Nov 2022 23:37:28 +0800 Subject: [PATCH 5/5] replace defer mode --- field_parser.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/field_parser.go b/field_parser.go index 48a4f8e27..1a5cc6eaf 100644 --- a/field_parser.go +++ b/field_parser.go @@ -206,6 +206,7 @@ func splitNotWrapped(s string, sep rune) []string { return result } +// ComplementSchema complement schema with field properties func (ps *tagBaseFieldParser) ComplementSchema(schema *spec.Schema) error { types := ps.p.GetSchemaTypePath(schema, 2) if len(types) == 0 { @@ -213,15 +214,22 @@ func (ps *tagBaseFieldParser) ComplementSchema(schema *spec.Schema) error { } if IsRefSchema(schema) { - oldSchema := schema - schema = &spec.Schema{} - defer func() { - if !reflect.ValueOf(*schema).IsZero() { - *oldSchema = *(schema.WithAllOf(*oldSchema)) - } - }() + var newSchema = spec.Schema{} + err := ps.complementSchema(&newSchema, types) + if err != nil { + return err + } + if !reflect.ValueOf(newSchema).IsZero() { + *schema = *(newSchema.WithAllOf(*schema)) + } + return nil } + return ps.complementSchema(schema, types) +} + +// complementSchema complement schema with field properties +func (ps *tagBaseFieldParser) complementSchema(schema *spec.Schema, types []string) error { if ps.field.Tag == nil { if ps.field.Doc != nil { schema.Description = strings.TrimSpace(ps.field.Doc.Text())