Skip to content

Commit

Permalink
Argo/query params for all methods (#625)
Browse files Browse the repository at this point in the history
Allow query params on ALL http methods

Nothing in the standards definitions preclude query params for methods other than GET
We have use-cases where an endpoint wanted to use DELETE with query params and that was used by the downstream service.

With this change, a few things:
 - Allowed other methods to process query params
 - Fixed the test-cases -- all query params need to have the http.ref = "query.xxx" annotation
   In the test cases, we were grandfathering non-annotated fields in the struct as query params since these were only for GET and it worked well
 - One test case added for delete with query params
  • Loading branch information
argouber committed Aug 21, 2019
1 parent d9eb102 commit 457102c
Show file tree
Hide file tree
Showing 11 changed files with 1,848 additions and 109 deletions.
35 changes: 20 additions & 15 deletions codegen/method.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ func NewMethod(

method.setValidStatusCodes()

if method.HTTPMethod == "GET" && method.RequestType != "" {
if method.RequestType != "" {
if method.IsEndpoint {
err := method.setParseQueryParamStatements(funcSpec, packageHelper)
if err != nil {
Expand Down Expand Up @@ -1132,14 +1132,20 @@ func (ms *MethodSpec) setWriteQueryParamStatements(
) error {
var statements LineBuilder
var hasQueryFields bool
var stack = []string{}
var stack []string
isVoidReturn := funcSpec.ResultSpec.ReturnType == nil

visitor := func(
goPrefix string, thriftPrefix string, field *compile.FieldSpec,
) bool {
realType := compile.RootTypeSpec(field.Type)
longFieldName := goPrefix + "." + PascalCase(field.Name)

httpRefAnnotation := field.Annotations[ms.annotations.HTTPRef]
if !strings.HasPrefix(httpRefAnnotation, "query") {
return false
}

if len(stack) > 0 {
if !strings.HasPrefix(longFieldName, stack[len(stack)-1]) {
stack = stack[:len(stack)-1]
Expand All @@ -1152,13 +1158,17 @@ func (ms *MethodSpec) setWriteQueryParamStatements(

if field.Required {
statements.appendf("if r%s == nil {", longFieldName)
// TODO: generate correct number of nils...
statements.append("\treturn nil, nil, errors.New(")
// Generate correct number of nils...
if isVoidReturn {
statements.append("\treturn nil, errors.New(")
} else {
statements.append("\treturn nil, nil, errors.New(")
}
statements.appendf("\t\t\"The field %s is required\",",
longFieldName,
)
statements.append("\t)")
statements.appendf("}")
statements.append("}")
} else {
stack = append(stack, longFieldName)

Expand All @@ -1168,11 +1178,6 @@ func (ms *MethodSpec) setWriteQueryParamStatements(
return false
}

httpRefAnnotation := field.Annotations[ms.annotations.HTTPRef]
if httpRefAnnotation != "" && !strings.HasPrefix(httpRefAnnotation, "query") {
return false
}

longQueryName := ms.getLongQueryName(field, thriftPrefix)
identifierName := CamelCase(longQueryName) + "Query"
_, isList := realType.(*compile.ListSpec)
Expand Down Expand Up @@ -1250,6 +1255,11 @@ func (ms *MethodSpec) setParseQueryParamStatements(
longFieldName := goPrefix + "." + PascalCase(field.Name)
longQueryName := ms.getLongQueryName(field, thriftPrefix)

httpRefAnnotation := field.Annotations[ms.annotations.HTTPRef]
if !strings.HasPrefix(httpRefAnnotation, "query") {
return false
}

if len(stack) > 0 {
if !strings.HasPrefix(longFieldName, stack[len(stack)-1]) {
stack = stack[:len(stack)-1]
Expand Down Expand Up @@ -1325,11 +1335,6 @@ func (ms *MethodSpec) setParseQueryParamStatements(
}
identifierName := CamelCase(longQueryName) + "Query"

httpRefAnnotation := field.Annotations[ms.annotations.HTTPRef]
if httpRefAnnotation != "" && !strings.HasPrefix(httpRefAnnotation, "query") {
return false
}

okIdentifierName := CamelCase(longQueryName) + "Ok"
if field.Required {
statements.appendf("%s := req.CheckQueryValue(%q)",
Expand Down
82 changes: 79 additions & 3 deletions examples/example-gateway/build/clients/bar/bar.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 457102c

Please sign in to comment.