Skip to content

Commit

Permalink
Merge e774851 into 632be75
Browse files Browse the repository at this point in the history
  • Loading branch information
@argouber
argouber committed Sep 19, 2019
2 parents 632be75 + e774851 commit af28e8c
Show file tree
Hide file tree
Showing 18 changed files with 11,605 additions and 7,249 deletions.
117 changes: 62 additions & 55 deletions codegen/method.go
View file
Expand Up @@ -46,6 +46,9 @@ const (
AntHTTPReqDefBoxed = "%s.http.req.def"
)

const queryAnnotationPrefix = "query."
const headerAnnotationPrefix = "headers."

// PathSegment represents a part of the http path.
type PathSegment struct {
Type string
Expand Down Expand Up @@ -238,14 +241,15 @@ func NewMethod(

method.setValidStatusCodes()

if method.HTTPMethod == "GET" && method.RequestType != "" {
if method.RequestType != "" {
hasNoBody := method.HTTPMethod == "GET" || method.HTTPMethod == "DELETE"
if method.IsEndpoint {
err := method.setParseQueryParamStatements(funcSpec, packageHelper)
err := method.setParseQueryParamStatements(funcSpec, packageHelper, hasNoBody)
if err != nil {
return nil, err
}
} else {
err := method.setWriteQueryParamStatements(funcSpec, packageHelper)
err := method.setWriteQueryParamStatements(funcSpec, packageHelper, hasNoBody)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -278,30 +282,6 @@ func NewMethod(
return method, nil
}

func (ms *MethodSpec) scanForNonParams(funcSpec *compile.FunctionSpec) bool {
hasNonParams := false

visitor := func(
goPrefix string, thriftPrefix string, field *compile.FieldSpec,
) bool {
realType := compile.RootTypeSpec(field.Type)
// ignore nested structs
if _, ok := realType.(*compile.StructSpec); ok {
return false
}

param, ok := field.Annotations[ms.annotations.HTTPRef]
if !ok || strings.HasPrefix(param, "params") {
hasNonParams = true
return true
}

return false
}
walkFieldGroups(compile.FieldGroup(funcSpec.ArgsSpec), visitor)
return hasNonParams
}

// setRequestType sets the request type of the method specification. If the
// "zanzibar.http.req.def.boxed" is true, then the first parameter will be used as
// the request body; otherwise a new struct is generated to bundle the request
Expand Down Expand Up @@ -626,8 +606,8 @@ func (ms *MethodSpec) setEndpointRequestHeaderFields(
}

if param, ok := field.Annotations[ms.annotations.HTTPRef]; ok {
if strings.HasPrefix(param, "headers.") {
headerName := param[8:]
if strings.HasPrefix(param, headerAnnotationPrefix) {
headerName := strings.TrimPrefix(param, headerAnnotationPrefix)
camelHeaderName := CamelCase(headerName)

fieldThriftType, err := GoType(packageHelper, field.Type)
Expand Down Expand Up @@ -752,8 +732,8 @@ func (ms *MethodSpec) setResponseHeaderFields(
goPrefix string, thriftPrefix string, field *compile.FieldSpec,
) bool {
if param, ok := field.Annotations[ms.annotations.HTTPRef]; ok {
if strings.HasPrefix(param, "headers.") {
headerName := param[8:]
if strings.HasPrefix(param, headerAnnotationPrefix) {
headerName := strings.TrimPrefix(param, headerAnnotationPrefix)
ms.ResHeaderFields[headerName] = HeaderFieldInfo{
FieldIdentifier: goPrefix + "." + PascalCase(field.Name),
IsPointer: !field.Required,
Expand Down Expand Up @@ -794,8 +774,8 @@ func (ms *MethodSpec) setClientRequestHeaderFields(
}

if param, ok := field.Annotations[ms.annotations.HTTPRef]; ok {
if strings.HasPrefix(param, "headers.") {
headerName := param[8:]
if strings.HasPrefix(param, headerAnnotationPrefix) {
headerName := strings.TrimPrefix(param, headerAnnotationPrefix)
bodyIdentifier := goPrefix + "." + PascalCase(field.Name)
var headerNameValuePair string
if field.Required {
Expand Down Expand Up @@ -1104,6 +1084,7 @@ func getQueryEncodeExprPrimitive(typeSpec compile.TypeSpec) string {
case *compile.EnumSpec:
encodeExpression = "strconv.Itoa(int(%s))"
default:
// This is intentional -- lets evaluate why we would want other types here before opening the flood gates
panic(fmt.Sprintf(
"Unsupported type (%T) for %s as query string parameter",
typeSpec, typeSpec.ThriftName(),
Expand All @@ -1127,16 +1108,44 @@ func getQueryEncodeExpression(typeSpec compile.TypeSpec, valueName string) strin
return fmt.Sprintf(encodeExpression, valueName)
}

func (ms *MethodSpec) hasQueryParams(field *compile.FieldSpec, defaultIsQuery bool) bool {

httpRefAnnotation := field.Annotations[ms.annotations.HTTPRef]
if strings.HasPrefix(httpRefAnnotation, queryAnnotationPrefix) {
return true
}
// If it is a struct, recursively look to see if any of the fields are query params
if container, ok := compile.RootTypeSpec(field.Type).(*compile.StructSpec); ok {
visitor := func(goPrefix string, thriftPrefix string, field *compile.FieldSpec) bool {
annotation := field.Annotations[ms.annotations.HTTPRef]
if strings.HasPrefix(annotation, queryAnnotationPrefix) {
return true
}
return annotation == "" && defaultIsQuery
}
return walkFieldGroups(container.Fields, visitor)
}
return httpRefAnnotation == "" && defaultIsQuery
}

func (ms *MethodSpec) setWriteQueryParamStatements(
funcSpec *compile.FunctionSpec, packageHelper *PackageHelper,
funcSpec *compile.FunctionSpec, packageHelper *PackageHelper, hasNoBody bool,
) 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 {
// Skip if there are no query params in the field or its components
// However, if by definition there is no request body, untagged fields are mapped to query-params
// because "existing behavior" && cannot easily change that
if !ms.hasQueryParams(field, hasNoBody) {
return false
}

realType := compile.RootTypeSpec(field.Type)
longFieldName := goPrefix + "." + PascalCase(field.Name)

Expand All @@ -1146,19 +1155,21 @@ func (ms *MethodSpec) setWriteQueryParamStatements(
statements.append("}")
}
}

if _, ok := realType.(*compile.StructSpec); ok {
// If a field is a struct then skip

// If a field is a struct we need to look inside
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 +1179,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 @@ -1234,7 +1240,7 @@ func (ms *MethodSpec) setWriteQueryParamStatements(
}

func (ms *MethodSpec) setParseQueryParamStatements(
funcSpec *compile.FunctionSpec, packageHelper *PackageHelper,
funcSpec *compile.FunctionSpec, packageHelper *PackageHelper, hasNoBody bool,
) error {
// If a thrift field has a http.ref annotation then we
// should not read this field from query parameters.
Expand All @@ -1250,6 +1256,13 @@ func (ms *MethodSpec) setParseQueryParamStatements(
longFieldName := goPrefix + "." + PascalCase(field.Name)
longQueryName := ms.getLongQueryName(field, thriftPrefix)

// Skip if there are no query params in the field or its components
// However, if by definition there is no request body, untagged fields are mapped to query-params
// because "existing behavior" && cannot easily change that
if !ms.hasQueryParams(field, hasNoBody) {
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 +1338,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 Expand Up @@ -1427,9 +1435,8 @@ func (ms *MethodSpec) getLongQueryName(field *compile.FieldSpec, thriftPrefix st

queryName := field.Name
queryAnnotation := field.Annotations[ms.annotations.HTTPRef]
if strings.HasPrefix(queryAnnotation, "query.") {
// len("query.") == 6
queryName = queryAnnotation[6:]
if strings.HasPrefix(queryAnnotation, queryAnnotationPrefix) {
queryName = strings.TrimPrefix(queryAnnotation, queryAnnotationPrefix)
}

if thriftPrefix == "" {
Expand Down

0 comments on commit af28e8c

Please sign in to comment.