Skip to content
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

clientgen: fix handling of optionals #726

Merged
merged 2 commits into from
May 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions e2e-tests/echo_app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,10 @@ type NonBasicResponse struct {
RawStruct json.RawMessage

// Query
QueryString string
QueryNumber int
QueryString string
QueryNumber int
OptQueryString string
OptQueryNumber int

// Path
PathString string
Expand Down
6 changes: 4 additions & 2 deletions e2e-tests/testdata/echo/echo/echo.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,10 @@ type NonBasicData struct {
RawStruct json.RawMessage

// Query
QueryString string `query:"string"`
QueryNumber int `query:"no"`
QueryString string `query:"string"`
QueryNumber int `query:"no"`
OptQueryNumber int `query:"optnum" encore:"optional"`
OptQueryString string `query:"optstr" encore:"optional"`

// Path Parameters
PathString string
Expand Down
44 changes: 26 additions & 18 deletions e2e-tests/testdata/echo_client/golang/client/goclient.go

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

2 changes: 2 additions & 0 deletions e2e-tests/testdata/echo_client/js/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,8 @@ class EchoServiceClient {

const query = makeRecord({
no: String(params.QueryNumber),
optnum: params.OptQueryNumber === undefined ? undefined : String(params.OptQueryNumber),
optstr: params.OptQueryString,
string: params.QueryString,
})

Expand Down
4 changes: 4 additions & 0 deletions e2e-tests/testdata/echo_client/ts/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,8 @@ export namespace echo {
QueryString: string

QueryNumber: number
OptQueryNumber?: number
OptQueryString?: string
/**
* Path Parameters
*/
Expand Down Expand Up @@ -364,6 +366,8 @@ export namespace echo {

const query = makeRecord<string, string | string[]>({
no: String(params.QueryNumber),
optnum: params.OptQueryNumber === undefined ? undefined : String(params.OptQueryNumber),
optstr: params.OptQueryString,
string: params.QueryString,
})

Expand Down
6 changes: 3 additions & 3 deletions internal/clientgen/client_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
//go:build go1.18
// +build go1.18

package clientgen

import (
Expand Down Expand Up @@ -67,6 +64,9 @@ func TestClientCodeGeneration(t *testing.T) {
if testName != file.Name() && !strings.Contains(testName, "_") {
c.Run(testName, func(c *qt.C) {
language, ok := Detect(file.Name())
if strings.Contains(file.Name(), "openapi") {
language, ok = LangOpenAPI, true
}
c.Assert(ok, qt.IsTrue, qt.Commentf("Unable to detect language type for %s", file.Name()))

generatedClient, err := Client(language, "app", res.Meta, nil)
Expand Down
23 changes: 15 additions & 8 deletions internal/clientgen/javascript.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ func (js *javascript) rpcCallSite(w *indentWriter, rpc *meta.RPC, rpcPath string
dict := make(map[string]string)
for _, field := range reqEnc.HeaderParameters {
ref := js.Dot("params", field.SrcName)
dict[field.WireFormat] = js.convertBuiltinToString(field.Type.GetBuiltin(), ref)
dict[field.WireFormat] = js.convertBuiltinToString(field.Type.GetBuiltin(), ref, field.Optional)
}

w.WriteString("const headers = makeRecord(")
Expand All @@ -275,11 +275,12 @@ func (js *javascript) rpcCallSite(w *indentWriter, rpc *meta.RPC, rpcPath string
for _, field := range reqEnc.QueryParameters {
if list := field.Type.GetList(); list != nil {
dict[field.WireFormat] = js.Dot("params", field.SrcName) +
".map((v) => " + js.convertBuiltinToString(list.Elem.GetBuiltin(), "v") + ")"
".map((v) => " + js.convertBuiltinToString(list.Elem.GetBuiltin(), "v", field.Optional) + ")"
} else {
dict[field.WireFormat] = js.convertBuiltinToString(
field.Type.GetBuiltin(),
js.Dot("params", field.SrcName),
field.Optional,
)
}
}
Expand Down Expand Up @@ -552,10 +553,10 @@ if (authData) {
if list := field.Type.GetList(); list != nil {
w.WriteString(
js.Dot("authData", field.SrcName) +
".map((v) => " + js.convertBuiltinToString(list.Elem.GetBuiltin(), "v") + ")",
".map((v) => " + js.convertBuiltinToString(list.Elem.GetBuiltin(), "v", field.Optional) + ")",
)
} else {
w.WriteString(js.convertBuiltinToString(field.Type.GetBuiltin(), js.Dot("authData", field.SrcName)))
w.WriteString(js.convertBuiltinToString(field.Type.GetBuiltin(), js.Dot("authData", field.SrcName), field.Optional))
}
w.WriteString("\n")
}
Expand All @@ -565,7 +566,7 @@ if (authData) {
w.WriteString("init.headers[\"")
w.WriteString(field.WireFormat)
w.WriteString("\"] = ")
w.WriteString(js.convertBuiltinToString(field.Type.GetBuiltin(), js.Dot("authData", field.SrcName)))
w.WriteString(js.convertBuiltinToString(field.Type.GetBuiltin(), js.Dot("authData", field.SrcName), field.Optional))
w.WriteString("\n")
}
} else {
Expand Down Expand Up @@ -658,15 +659,21 @@ function mustBeSet(field, value) {
}
}

func (js *javascript) convertBuiltinToString(typ schema.Builtin, val string) string {
func (js *javascript) convertBuiltinToString(typ schema.Builtin, val string, isOptional bool) string {
var code string
switch typ {
case schema.Builtin_STRING:
return val
case schema.Builtin_JSON:
return fmt.Sprintf("JSON.stringify(%s)", val)
code = fmt.Sprintf("JSON.stringify(%s)", val)
default:
return fmt.Sprintf("String(%s)", val)
code = fmt.Sprintf("String(%s)", val)
}

if isOptional {
code = fmt.Sprintf("%s === undefined ? undefined : %s", val, code)
}
return code
}

func (js *javascript) convertStringToBuiltin(typ schema.Builtin, val string) string {
Expand Down
11 changes: 6 additions & 5 deletions internal/clientgen/openapi/openapi.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package openapi

import (
"encoding/json"
"fmt"
"go/doc/comment"
"strings"
Expand Down Expand Up @@ -60,9 +61,9 @@ func (g *Generator) Generate(p clientgentypes.GenerateParams) error {
if err != nil {
return errors.Wrap(err, "marshal openapi spec")
}
p.Buf.Write(out)

return nil
// Pretty-print the JSON output
return json.Indent(p.Buf, out, "", " ")
}

func (g *Generator) addService(svc *meta.Service) error {
Expand Down Expand Up @@ -152,7 +153,7 @@ func (g *Generator) newOperationForEncoding(rpc *meta.RPC, method string, reqEnc
AllowEmptyValue: true,
AllowReserved: false,
Deprecated: false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh nice, in the future we could even detect this and include it in the meta

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's a good idea :)

Required: false,
Required: !param.Optional,
Schema: g.schemaType(param.Type),
Example: nil,
Examples: nil,
Expand All @@ -173,7 +174,7 @@ func (g *Generator) newOperationForEncoding(rpc *meta.RPC, method string, reqEnc
AllowEmptyValue: true,
AllowReserved: false,
Deprecated: false,
Required: false,
Required: !param.Optional,
Schema: g.schemaType(param.Type),
Example: nil,
Examples: nil,
Expand Down Expand Up @@ -211,7 +212,7 @@ func (g *Generator) newOperationForEncoding(rpc *meta.RPC, method string, reqEnc
AllowEmptyValue: true,
AllowReserved: false,
Deprecated: false,
Required: false,
Required: !param.Optional,
Schema: g.schemaType(param.Type),
Example: nil,
Examples: nil,
Expand Down
113 changes: 113 additions & 0 deletions internal/clientgen/testdata/expected_baseauth_openapi.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
{
"components": {
"responses": {
"APIError": {
"content": {
"application/json": {
"schema": {
"externalDocs": {
"url": "https://pkg.go.dev/encore.dev/beta/errs#Error"
},
"properties": {
"code": {
"description": "Error code",
"example": "not_found",
"externalDocs": {
"url": "https://pkg.go.dev/encore.dev/beta/errs#ErrCode"
},
"type": "string"
},
"details": {
"description": "Error details",
"type": "object"
},
"message": {
"description": "Error message",
"type": "string"
}
},
"title": "APIError",
"type": "object"
}
}
},
"description": "Error response"
}
}
},
"info": {
"description": "Generated by encore",
"title": "API for app",
"version": "1",
"x-logo": {
"altText": "Encore logo",
"backgroundColor": "#EEEEE1",
"url": "https://encore.dev/assets/branding/logo/logo-black.png"
}
},
"openapi": "3.0.0",
"paths": {
"/svc.DummyAPI": {
"post": {
"operationId": "POST:svc.DummyAPI",
"requestBody": {
"content": {
"application/json": {
"schema": {
"properties": {
"Message": {
"type": "string"
}
},
"type": "object"
}
}
}
},
"responses": {
"200": {
"description": "Success response"
},
"default": {
"$ref": "#/components/responses/APIError"
}
},
"summary": "DummyAPI is a dummy endpoint.\n"
}
},
"/svc.Private": {
"post": {
"operationId": "POST:svc.Private",
"requestBody": {
"content": {
"application/json": {
"schema": {
"properties": {
"Message": {
"type": "string"
}
},
"type": "object"
}
}
}
},
"responses": {
"200": {
"description": "Success response"
},
"default": {
"$ref": "#/components/responses/APIError"
}
},
"summary": "Private is a basic auth endpoint.\n"
}
}
},
"servers": [
{
"description": "Encore local dev environment",
"url": "http://localhost:4000"
}
]
}
Loading