Skip to content

Commit

Permalink
fix: struct order bug (#270)
Browse files Browse the repository at this point in the history
* sort the typeNames so that parsing definitions is deterministic.

* fix bug with required properties on anonymous field that occurs based on order of parsing structs.
  • Loading branch information
rubensayshi authored and easonlin404 committed Dec 22, 2018
1 parent 0e12fd5 commit 78c4dff
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 27 deletions.
56 changes: 36 additions & 20 deletions parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,15 @@ func (parser *Parser) isInStructStack(refTypeName string) bool {

// ParseDefinitions parses Swagger Api definitions.
func (parser *Parser) ParseDefinitions() {
for refTypeName, typeSpec := range parser.registerTypes {
// sort the typeNames so that parsing definitions is deterministic
typeNames := make([]string, 0, len(parser.registerTypes))
for refTypeName := range parser.registerTypes {
typeNames = append(typeNames, refTypeName)
}
sort.Strings(typeNames)

for _, refTypeName := range typeNames {
typeSpec := parser.registerTypes[refTypeName]
ss := strings.Split(refTypeName, ".")
pkgName := ss[0]
parser.structStack = nil
Expand All @@ -432,10 +440,10 @@ func (parser *Parser) ParseDefinition(pkgName, typeName string, typeSpec *ast.Ty
parser.structStack = append(parser.structStack, refTypeName)

log.Println("Generating " + refTypeName)
parser.swagger.Definitions[refTypeName] = parser.parseTypeExpr(pkgName, typeName, typeSpec.Type, true)
parser.swagger.Definitions[refTypeName] = parser.parseTypeExpr(pkgName, typeName, typeSpec.Type)
}

func (parser *Parser) collectRequiredFields(pkgName string, properties map[string]spec.Schema) (requiredFields []string) {
func (parser *Parser) collectRequiredFields(pkgName string, properties map[string]spec.Schema, extraRequired []string) (requiredFields []string) {
// created sorted list of properties keys so when we iterate over them it's deterministic
ks := make([]string, 0, len(properties))
for k := range properties {
Expand All @@ -461,6 +469,12 @@ func (parser *Parser) collectRequiredFields(pkgName string, properties map[strin
properties[k] = prop
}

if extraRequired != nil {
requiredFields = append(requiredFields, extraRequired...)
}

sort.Strings(requiredFields)

return
}

Expand All @@ -473,7 +487,7 @@ func fullTypeName(pkgName, typeName string) string {

// parseTypeExpr parses given type expression that corresponds to the type under
// given name and package, and returns swagger schema for it.
func (parser *Parser) parseTypeExpr(pkgName, typeName string, typeExpr ast.Expr, flattenRequired bool) spec.Schema {
func (parser *Parser) parseTypeExpr(pkgName, typeName string, typeExpr ast.Expr) spec.Schema {
switch expr := typeExpr.(type) {
// type Foo struct {...}
case *ast.StructType:
Expand All @@ -482,11 +496,14 @@ func (parser *Parser) parseTypeExpr(pkgName, typeName string, typeExpr ast.Expr,
return schema
}

extraRequired := make([]string, 0)
properties := make(map[string]spec.Schema)
for _, field := range expr.Fields.List {
var fieldProps map[string]spec.Schema
var requiredFromAnon []string
if field.Names == nil {
fieldProps = parser.parseAnonymousField(pkgName, field)
fieldProps, requiredFromAnon = parser.parseAnonymousField(pkgName, field)
extraRequired = append(extraRequired, requiredFromAnon...)
} else {
fieldProps = parser.parseStruct(pkgName, field)
}
Expand All @@ -496,17 +513,16 @@ func (parser *Parser) parseTypeExpr(pkgName, typeName string, typeExpr ast.Expr,
}
}

required := parser.collectRequiredFields(pkgName, properties)
// collect requireds from our properties and anonymous fields
required := parser.collectRequiredFields(pkgName, properties, extraRequired)

// unset required from properties because we've aggregated them
if flattenRequired {
for k, prop := range properties {
tname := prop.SchemaProps.Type[0]
if tname != "object" {
prop.SchemaProps.Required = make([]string, 0)
}
properties[k] = prop
// unset required from properties because we've collected them
for k, prop := range properties {
tname := prop.SchemaProps.Type[0]
if tname != "object" {
prop.SchemaProps.Required = make([]string, 0)
}
properties[k] = prop
}

return spec.Schema{
Expand All @@ -528,11 +544,11 @@ func (parser *Parser) parseTypeExpr(pkgName, typeName string, typeExpr ast.Expr,

// type Foo *Baz
case *ast.StarExpr:
return parser.parseTypeExpr(pkgName, typeName, expr.X, true)
return parser.parseTypeExpr(pkgName, typeName, expr.X)

// type Foo []Baz
case *ast.ArrayType:
itemSchema := parser.parseTypeExpr(pkgName, "", expr.Elt, true)
itemSchema := parser.parseTypeExpr(pkgName, "", expr.Elt)
return spec.Schema{
SchemaProps: spec.SchemaProps{
Type: []string{"array"},
Expand Down Expand Up @@ -723,7 +739,7 @@ func (parser *Parser) parseStruct(pkgName string, field *ast.Field) (properties
return
}

func (parser *Parser) parseAnonymousField(pkgName string, field *ast.Field) map[string]spec.Schema {
func (parser *Parser) parseAnonymousField(pkgName string, field *ast.Field) (map[string]spec.Schema, []string) {
properties := make(map[string]spec.Schema)

fullTypeName := ""
Expand All @@ -736,7 +752,7 @@ func (parser *Parser) parseAnonymousField(pkgName string, field *ast.Field) map[
}
default:
log.Printf("Field type of '%T' is unsupported. Skipping", ftype)
return properties
return properties, []string{}
}

typeName := fullTypeName
Expand All @@ -746,7 +762,7 @@ func (parser *Parser) parseAnonymousField(pkgName string, field *ast.Field) map[
}

typeSpec := parser.TypeDefinitions[pkgName][typeName]
schema := parser.parseTypeExpr(pkgName, typeName, typeSpec.Type, false)
schema := parser.parseTypeExpr(pkgName, typeName, typeSpec.Type)

schemaType := "unknown"
if len(schema.SchemaProps.Type) > 0 {
Expand All @@ -764,7 +780,7 @@ func (parser *Parser) parseAnonymousField(pkgName string, field *ast.Field) map[
log.Printf("Can't extract properties from a schema of type '%s'", schemaType)
}

return properties
return properties, schema.SchemaProps.Required
}

func (parser *Parser) parseField(field *ast.Field) *structField {
Expand Down
30 changes: 28 additions & 2 deletions parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func TestGetSchemes(t *testing.T) {

}

func TestParseSimpleApi(t *testing.T) {
func TestParseSimpleApi1(t *testing.T) {
expected := `{
"swagger": "2.0",
"info": {
Expand Down Expand Up @@ -681,7 +681,33 @@ func TestParseSimpleApi(t *testing.T) {
}
}
},
"web.Pet5": {
"web.Pet5a": {
"type": "object",
"required": [
"name",
"odd"
],
"properties": {
"name": {
"type": "string"
},
"odd": {
"type": "boolean"
}
}
},
"web.Pet5b": {
"type": "object",
"required": [
"name"
],
"properties": {
"name": {
"type": "string"
}
}
},
"web.Pet5c": {
"type": "object",
"required": [
"name",
Expand Down
14 changes: 12 additions & 2 deletions testdata/simple/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,17 @@ type Pet3 struct {
ID int `json:"id"`
}

// @Success 200 {object} web.Pet5 "ok"
func GetPet5() {
// @Success 200 {object} web.Pet5a "ok"
func GetPet5a() {

}

// @Success 200 {object} web.Pet5b "ok"
func GetPet5b() {

}

// @Success 200 {object} web.Pet5c "ok"
func GetPet5c() {

}
14 changes: 11 additions & 3 deletions testdata/simple/web/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,19 @@ type RevValue struct {
Crosses []cross.Cross `json:"crosses"`
}

type Pet4 struct {
// Below we have Pet5b as base type and Pet5a and Pet5c both have Pet5b as anonymous field, inheriting it's properties
// By using these names we ensure that our test will fill if the order of parsing matters at all

type Pet5a struct {
*Pet5b
Odd bool `json:"odd" binding:"required"`
}

type Pet5b struct {
Name string `json:"name" binding:"required"`
}

type Pet5 struct {
*Pet4
type Pet5c struct {
*Pet5b
Odd bool `json:"odd" binding:"required"`
}

0 comments on commit 78c4dff

Please sign in to comment.