string param in URL when it's required

[JackLeeHal]

Please do not post any internal, closed source snippets on this public issue tracker!

Description

When string param in URL is required, generated code will contain error because it will be parsed by: https://github.com/anz-bank/sysl-go/blob/master/convert/convert.go#L67

Automatically to a pointer, but it's a string type not *string

Steps to Reproduce

1. A yaml that contains a get API with required string param in URL eg. GET example.com?querystring=xxx
2. Make sure querystring is mandatory
3. Do the codegen

Expected behavior

no error in the generated code

Actual behavior

error in the generated code

Your Environment

$ arrai info
Version    : v0.101.0
Git commit : b839f99fa1008951af2838f6dc3f03112f065ebb
Date       : 2020-07-24T03:58:08Z
OS/arch    : darwin/amd64
Go version : go1.14.6 linux/amd64

$ sysl info
  Version      : v0.193.0
  Git Commit   : 11f08909f8d18195868d06958bd6493125519060
  Date         : 2020-08-25T00:08:21Z
  Go Version   : go1.13.15 linux/amd64
  OS           : darwin/amd64

Our solution

change arrai transform from: https://github.com/anz-bank/sysl-go/blob/master/codegen/arrai/auto/svc_handler.arrai#L159-L171

let type = cond go.type(.type) {
    "*bool": "BoolPtr",
    "*int64": "IntPtr",
    "*convert.JSONTime": "TimePtr",
    _: "StringPtr",
};
$`
req.${go.name(.name)}, convErr = convert.StringTo${type}(ctx, ${.var})
if convErr != nil {
    common.HandleError(ctx, w, common.BadRequestError, "Invalid request", convErr, s.genCallback.MapError)
    return
}
 `

To:

let type = cond go.type(.type) {
     "*bool": "BoolPtr",
    "*int64": "IntPtr",
    "*convert.JSONTime": "TimePtr",
    _: "StringPtr",
};
$`${cond {
'string' != go.type(.type): $`
req.${go.name(.name)}, convErr = convert.StringTo${type}(ctx, ${.var})
if convErr != nil {
    common.HandleError(ctx, w, common.BadRequestError, "Invalid request", convErr, s.genCallback.MapError)
    return
}`,
_: $`
if ${.var} == "" {
    common.HandleError(ctx, w, common.BadRequestError, "Invalid request", convErr, s.genCallback.MapError)
    return
}
req.${go.name(.name)} = ${.var}
`
}}`

The generated code will change from:

QueryStringParam = restlib.GetQueryParam(r, "queryString")
    req.QueryString, convErr = convert.StringToStringPtr(ctx, QueryStringParam)
    if convErr != nil {
        common.HandleError(ctx, w, common.BadRequestError, "Invalid request", convErr, s.genCallback.MapError)
        return
    }

To:

QueryStringParam = restlib.GetQueryParam(r, "queryString")
    if QueryStringParam == "" {
        common.HandleError(ctx, w, common.BadRequestError, "Invalid request", convErr, s.genCallback.MapError)
        return
    }
    req.QueryString = QueryStringPara

[andrewemeryanz]

I've not entirely sure what the issue is. Let's discuss with some examples.

The following OpenAPI spec (containing optional and required string parameters):

petstore.yaml

```yaml openapi: "3.0.0" info: version: 1.0.0 title: Petstore paths: /pets: get: summary: List all pets operationId: listPets parameters: - name: strReq in: query required: true schema: type: string - name: strOpt in: query required: false schema: type: string responses: default: description: unexpected error content: application/json: schema: $ref: "#/components/schemas/Error" components: schemas: Error: type: object required: - code - message properties: code: type: integer format: int32 message: type: string ```

Produces the following Sysl:

# sysl import --app-name PetStore -i petstore.yaml -o petstore.sysl

PetStore "Petstore":
    @version = "1.0.0"
    @description =:
        | No description.

    /pets:
        GET ?strReq=string&strOpt=string?:
            | No description.
            return error <: Error

    !type Error:
        code <: int32:
            @json_tag = "code"
        message <: string:
            @json_tag = "message"

Which has it's optional and required query parameters parsed by the following block of code:

req.StrReq = restlib.GetQueryParam(r, "strReq")

var StrOptParam string

var convErr error
StrOptParam = restlib.GetQueryParam(r, "strOpt")
req.StrOpt, convErr = convert.StringToStringPtr(ctx, StrOptParam)
if convErr != nil {
    common.HandleError(ctx, w, common.BadRequestError, "Invalid request", convErr, s.genCallback.MapError)
    return
}

Can you describe what the issue is using this example?

[AriehSchneier]

I had a look into this issue and discovered that the bug is only because we were using an old version of the transform (the latest version already has this issue fixed). However I did notice that you aren't checking for an empty string like you do for all the other kinds of parameters. I think you should change this code:

               ${reqQueryParams where .@item('type')('primitive')?.s:"" = "STRING" >> \{'name': (s: name), ...}
                    $`req.${go.name(name)} = restlib.GetQueryParam(r, "${name}")`
                ::\i:\n}

To:

                ${reqQueryParams where .@item('type')('primitive')?.s:"" = "STRING" >> \{'name': (s: name), ...}
                    $`req.${go.name(name)} = restlib.GetQueryParam(r, "${name}")
                    if req.${go.name(name)} == "" {
                        common.HandleError(ctx, w, common.BadRequestError, "${name} query length is zero", nil, s.genCallback.MapError)
                        return
                    }`
                ::\i:\n}

[andrewemeryanz]

Thanks. Your approach is heading in the right direction but doesn't differentiate between missing and empty parameters. I'll prioritise and get this assigned.

[AriehSchneier]

The code already does similar for all the required header params.

[JackLeeHal]

Right, it's caused by our transform out of date, so close this issue.