-
Notifications
You must be signed in to change notification settings - Fork 921
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support struct response with @Failture #831
Conversation
generate/swaggergen/g_docs.go
Outdated
ss := strings.TrimSpace(t[len("@Failure"):]) | ||
rs := swagger.Response{} | ||
st := strings.TrimSpace(t[len("@Failure"):]) | ||
var cd []rune | ||
var start bool | ||
for i, s := range st { | ||
if unicode.IsSpace(s) { | ||
if start { | ||
rs.Description = strings.TrimSpace(st[i+1:]) | ||
break | ||
} else { | ||
continue | ||
respCode, pos := peekNextSplitString(ss) | ||
ss = strings.TrimSpace(ss[pos:]) | ||
respType, pos := peekNextSplitString(ss) | ||
if respType == "{object}" || respType == "{array}" { | ||
isArray := respType == "{array}" | ||
ss = strings.TrimSpace(ss[pos:]) | ||
schemaName, pos := peekNextSplitString(ss) | ||
if schemaName == "" { | ||
beeLogger.Log.Fatalf("[%s.%s] Schema must follow {object} or {array}", controllerName, funcName) | ||
} | ||
if strings.HasPrefix(schemaName, "[]") { | ||
schemaName = schemaName[2:] | ||
isArray = true | ||
} | ||
schema := swagger.Schema{} | ||
if sType, ok := basicTypes[schemaName]; ok { | ||
typeFormat := strings.Split(sType, ":") | ||
schema.Type = typeFormat[0] | ||
schema.Format = typeFormat[1] | ||
} else { | ||
m, mod, realTypes := getModel(schemaName) | ||
schema.Ref = "#/definitions/" + m | ||
if _, ok := modelsList[pkgpath+controllerName]; !ok { | ||
modelsList[pkgpath+controllerName] = make(map[string]swagger.Schema) | ||
} | ||
modelsList[pkgpath+controllerName][schemaName] = mod | ||
appendModels(pkgpath, controllerName, realTypes) | ||
} | ||
start = true | ||
cd = append(cd, s) | ||
if isArray { | ||
rs.Schema = &swagger.Schema{ | ||
Type: astTypeArray, | ||
Items: &schema, | ||
} | ||
} else { | ||
rs.Schema = &schema | ||
} | ||
rs.Description = strings.TrimSpace(ss[pos:]) | ||
} else { | ||
rs.Description = strings.TrimSpace(ss) | ||
} | ||
opts.Responses[string(cd)] = rs | ||
opts.Responses[respCode] = rs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you help to extra this part to be a new function which returns the swagger.Response
:
parseFailureResponse(annotation string) swagger.Response
And then you can write unit tests for this function.
I know this repository has no unit tests, but we can improve it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turning this duplication into a standalone function would work, but with the current one-liner and shared memory model I don't think it's possible to write good unit tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not complicate.
You only need to reset the package variable when step in and step out the function, like this:
func xxx() {
reset()
defer reset()
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@flycash You can take a look at my new commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I check your commit and it looks good. But I didn't see any tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A reset()
function is hard for me as I am not clear about the overall structure of the program. So I can't write this unit test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need to reset all variable, you can only reset those variables you use
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have never learn about package "go/ast", so I have no ieda to reset any variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You only need to test parseResponse
function. Not reset the AST tree... I mean reset the package variables in swaggergen pkg if you use them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you think it is easy, you can finish this unit test.
* fix: some minor bugs and make Dockerfile more productive. * fix: make GitHub CI configuration support build image with STANDARD target. * fix: Naming the base stage in multi-stage builds with lowercase letters to support various operating systems. * fix: copy swagger to the image as well.
#830
Just migrate the code from the @success section into the @failure section.