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.

Sign up for GitHub

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

Enums roundtrip unknown variants #139

Merged
merged 7 commits into from
Oct 5, 2020
Merged

Enums roundtrip unknown variants #139

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b83792e
Roundtrip enums
bmoylan Sep 13, 2020
5e6e722
Move pattern to global variable
bmoylan Sep 15, 2020
eb48097
try removing ignored testcase
bmoylan Sep 15, 2020
23d5e14
Add generated changelog entries
bmoylan Sep 15, 2020
8533fb4
Update goastwriter
bmoylan Oct 1, 2020
ee180c8
ignore bad test case
bmoylan Oct 5, 2020
1b80d1a
Merge branch 'develop' into bm/enum-roundtrip
bmoylan Oct 5, 2020
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: 6 additions & 0 deletions changelog/@unreleased/pr-139.v2.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type: improvement
improvement:
description: Enums now preserve unknown variant values. The `UNKNOWN` value has
been removed.
links:
- https://github.com/palantir/conjure-go/pull/139
39 changes: 26 additions & 13 deletions conjure-api/conjure/spec/enums.conjure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,16 @@
package spec

import (
"regexp"
"strings"

"github.com/palantir/conjure-go-runtime/v2/conjure-go-contract/errors"
werror "github.com/palantir/witchcraft-go-error"
wparams "github.com/palantir/witchcraft-go-params"
)

var enumValuePattern = regexp.MustCompile("^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$")

type ErrorCode string

const (
Expand All @@ -19,13 +26,15 @@ const (
ErrorCodeTimeout ErrorCode = "TIMEOUT"
ErrorCodeCustomClient ErrorCode = "CUSTOM_CLIENT"
ErrorCodeCustomServer ErrorCode = "CUSTOM_SERVER"
ErrorCodeUnknown ErrorCode = "UNKNOWN"
)

func (e *ErrorCode) UnmarshalText(data []byte) error {
switch strings.ToUpper(string(data)) {
switch v := strings.ToUpper(string(data)); v {
default:
*e = ErrorCodeUnknown
if !enumValuePattern.MatchString(v) {
return werror.Convert(errors.NewInvalidArgument(wparams.NewSafeAndUnsafeParamStorer(map[string]interface{}{"enumType": "ErrorCode", "message": "enum value must match pattern ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$"}, map[string]interface{}{"enumValue": string(data)})))
}
*e = ErrorCode(v)
case "PERMISSION_DENIED":
*e = ErrorCodePermissionDenied
case "INVALID_ARGUMENT":
Expand Down Expand Up @@ -53,17 +62,19 @@ func (e *ErrorCode) UnmarshalText(data []byte) error {
type HttpMethod string

const (
HttpMethodGet HttpMethod = "GET"
HttpMethodPost HttpMethod = "POST"
HttpMethodPut HttpMethod = "PUT"
HttpMethodDelete HttpMethod = "DELETE"
HttpMethodUnknown HttpMethod = "UNKNOWN"
HttpMethodGet HttpMethod = "GET"
HttpMethodPost HttpMethod = "POST"
HttpMethodPut HttpMethod = "PUT"
HttpMethodDelete HttpMethod = "DELETE"
)

func (e *HttpMethod) UnmarshalText(data []byte) error {
switch strings.ToUpper(string(data)) {
switch v := strings.ToUpper(string(data)); v {
default:
*e = HttpMethodUnknown
if !enumValuePattern.MatchString(v) {
return werror.Convert(errors.NewInvalidArgument(wparams.NewSafeAndUnsafeParamStorer(map[string]interface{}{"enumType": "HttpMethod", "message": "enum value must match pattern ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$"}, map[string]interface{}{"enumValue": string(data)})))
}
*e = HttpMethod(v)
case "GET":
*e = HttpMethodGet
case "POST":
Expand All @@ -90,13 +101,15 @@ const (
PrimitiveTypeUuid PrimitiveType = "UUID"
PrimitiveTypeRid PrimitiveType = "RID"
PrimitiveTypeBearertoken PrimitiveType = "BEARERTOKEN"
PrimitiveTypeUnknown PrimitiveType = "UNKNOWN"
)

func (e *PrimitiveType) UnmarshalText(data []byte) error {
switch strings.ToUpper(string(data)) {
switch v := strings.ToUpper(string(data)); v {
default:
*e = PrimitiveTypeUnknown
if !enumValuePattern.MatchString(v) {
return werror.Convert(errors.NewInvalidArgument(wparams.NewSafeAndUnsafeParamStorer(map[string]interface{}{"enumType": "PrimitiveType", "message": "enum value must match pattern ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$"}, map[string]interface{}{"enumValue": string(data)})))
}
*e = PrimitiveType(v)
case "STRING":
*e = PrimitiveTypeString
case "DATETIME":
Expand Down
31 changes: 21 additions & 10 deletions conjure-go-verifier/conjure/verification/types/enums.conjure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,30 @@
package types

import (
"regexp"
"strings"

"github.com/palantir/conjure-go-runtime/v2/conjure-go-contract/errors"
werror "github.com/palantir/witchcraft-go-error"
wparams "github.com/palantir/witchcraft-go-params"
)

var enumValuePattern = regexp.MustCompile("^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$")

type EnumExample string

const (
EnumExampleOne EnumExample = "ONE"
EnumExampleTwo EnumExample = "TWO"
EnumExampleUnknown EnumExample = "UNKNOWN"
EnumExampleOne EnumExample = "ONE"
EnumExampleTwo EnumExample = "TWO"
)

func (e *EnumExample) UnmarshalText(data []byte) error {
switch strings.ToUpper(string(data)) {
switch v := strings.ToUpper(string(data)); v {
default:
*e = EnumExampleUnknown
if !enumValuePattern.MatchString(v) {
return werror.Convert(errors.NewInvalidArgument(wparams.NewSafeAndUnsafeParamStorer(map[string]interface{}{"enumType": "EnumExample", "message": "enum value must match pattern ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$"}, map[string]interface{}{"enumValue": string(data)})))
}
*e = EnumExample(v)
case "ONE":
*e = EnumExampleOne
case "TWO":
Expand All @@ -29,15 +38,17 @@ func (e *EnumExample) UnmarshalText(data []byte) error {
type Enum string

const (
EnumOne Enum = "ONE"
EnumTwo Enum = "TWO"
EnumUnknown Enum = "UNKNOWN"
EnumOne Enum = "ONE"
EnumTwo Enum = "TWO"
)

func (e *Enum) UnmarshalText(data []byte) error {
switch strings.ToUpper(string(data)) {
switch v := strings.ToUpper(string(data)); v {
default:
*e = EnumUnknown
if !enumValuePattern.MatchString(v) {
return werror.Convert(errors.NewInvalidArgument(wparams.NewSafeAndUnsafeParamStorer(map[string]interface{}{"enumType": "Enum", "message": "enum value must match pattern ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$"}, map[string]interface{}{"enumValue": string(data)})))
}
*e = Enum(v)
case "ONE":
*e = EnumOne
case "TWO":
Expand Down
6 changes: 5 additions & 1 deletion conjure-go-verifier/ignored-test-cases.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ client:
- "{\"Infinity\": true}"
- "{\"-Infinity\": true}"
- "{\"10\": true, \"3e2\": true}"
receiveMapEnumExampleAlias:
# This test case is changed in a later verifier version to UNKNOWN_VARIANT which is actually valid.
# https://github.com/palantir/conjure-verification/pull/114
- "{\"ONE\": \"\", \"TWO\": \"\", \"BAD VARIANT\": \"\"}"
receiveOptionalAnyAliasExample:
- "null"
receiveOptionalBearerTokenAliasExample:
Expand Down Expand Up @@ -100,7 +104,7 @@ client:
receiveRawOptionalExample:
- "null"
receiveSetDoubleAliasExample:
- "[100, 10.0, \"NaN\", \"Infinity\", \"-Infinity\"]"
- "[100, 10.0, \"NaN\", \"Infinity\", \"-Infinity\"]"
singlePathParamService:
pathParamAliasString:
- '""'
Expand Down
4 changes: 4 additions & 0 deletions conjure/conjure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,10 @@ func (c *outputFileCollector) VisitAlias(aliasDefinition spec.AliasDefinition) e
}

func (c *outputFileCollector) VisitEnum(enumDefinition spec.EnumDefinition) error {
if len(c.enums.Decls) == 0 {
// if this is our first enum, add the regex pattern
c.enums.Decls = append(c.enums.Decls, astForEnumPattern(c.enums.Info))
}
decls := astForEnum(enumDefinition, c.enums.Info)
c.enums.Decls = append(c.enums.Decls, decls...)
return nil
Expand Down
42 changes: 31 additions & 11 deletions conjure/conjure_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,21 +329,30 @@ var testCases = []struct {
package api

import (
"regexp"
"strings"

"github.com/palantir/conjure-go-runtime/v2/conjure-go-contract/errors"
werror "github.com/palantir/witchcraft-go-error"
wparams "github.com/palantir/witchcraft-go-params"
)

var enumValuePattern = regexp.MustCompile("^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$")

type ExampleEnumeration string

const (
ExampleEnumerationA ExampleEnumeration = "A"
ExampleEnumerationB ExampleEnumeration = "B"
ExampleEnumerationUnknown ExampleEnumeration = "UNKNOWN"
ExampleEnumerationA ExampleEnumeration = "A"
ExampleEnumerationB ExampleEnumeration = "B"
)

func (e *ExampleEnumeration) UnmarshalText(data []byte) error {
switch strings.ToUpper(string(data)) {
switch v := strings.ToUpper(string(data)); v {
default:
*e = ExampleEnumerationUnknown
if !enumValuePattern.MatchString(v) {
return werror.Convert(errors.NewInvalidArgument(wparams.NewSafeAndUnsafeParamStorer(map[string]interface{}{"enumType": "ExampleEnumeration", "message": "enum value must match pattern ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$"}, map[string]interface{}{"enumValue": string(data)})))
}
*e = ExampleEnumeration(v)
case "A":
*e = ExampleEnumerationA
case "B":
Expand Down Expand Up @@ -579,21 +588,30 @@ func (a *AliasAlias) UnmarshalYAML(unmarshal func(interface{}) error) error {
package api

import (
"regexp"
"strings"

"github.com/palantir/conjure-go-runtime/v2/conjure-go-contract/errors"
werror "github.com/palantir/witchcraft-go-error"
wparams "github.com/palantir/witchcraft-go-params"
)

var enumValuePattern = regexp.MustCompile("^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$")

type Months string

const (
MonthsJanuary Months = "JANUARY"
MonthsMultiMonths Months = "MULTI_MONTHS"
MonthsUnknown Months = "UNKNOWN"
)

func (e *Months) UnmarshalText(data []byte) error {
switch strings.ToUpper(string(data)) {
switch v := strings.ToUpper(string(data)); v {
default:
*e = MonthsUnknown
if !enumValuePattern.MatchString(v) {
return werror.Convert(errors.NewInvalidArgument(wparams.NewSafeAndUnsafeParamStorer(map[string]interface{}{"enumType": "Months", "message": "enum value must match pattern ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$"}, map[string]interface{}{"enumValue": string(data)})))
}
*e = Months(v)
case "JANUARY":
*e = MonthsJanuary
case "MULTI_MONTHS":
Expand All @@ -607,13 +625,15 @@ type Days string
const (
DaysFriday Days = "FRIDAY"
DaysSaturday Days = "SATURDAY"
DaysUnknown Days = "UNKNOWN"
)

func (e *Days) UnmarshalText(data []byte) error {
switch strings.ToUpper(string(data)) {
switch v := strings.ToUpper(string(data)); v {
default:
*e = DaysUnknown
if !enumValuePattern.MatchString(v) {
return werror.Convert(errors.NewInvalidArgument(wparams.NewSafeAndUnsafeParamStorer(map[string]interface{}{"enumType": "Days", "message": "enum value must match pattern ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$"}, map[string]interface{}{"enumValue": string(data)})))
}
*e = Days(v)
case "FRIDAY":
*e = DaysFriday
case "SATURDAY":
Expand Down
64 changes: 51 additions & 13 deletions conjure/enum.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,10 @@ import (
)

const (
enumReceiverName = "e"
unknownEnumValue = "UNKNOWN"
enumReceiverName = "e"
enumUpperVarName = "v"
enumPatternVarName = "enumValuePattern"
enumValuePattern = "^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$"
)

func astForEnum(enumDefinition spec.EnumDefinition, info types.PkgInfo) []astgen.ASTDecl {
Expand All @@ -54,32 +56,68 @@ func astForEnum(enumDefinition spec.EnumDefinition, info types.PkgInfo) []astgen
Values: []astgen.ASTExpr{expression.StringVal(currVal.Value)},
})
}
vals = append(vals, &astspec.Value{
Names: []string{enumName + toCamelCase(unknownEnumValue)},
Type: expression.Type(enumName),
Values: []astgen.ASTExpr{expression.StringVal(unknownEnumValue)},
})
valsDecl := &decl.Const{Values: vals}

unmarshalDecl := enumUnmarshalTextAST(enumDefinition, info)

return []astgen.ASTDecl{typeDef, valsDecl, unmarshalDecl}
}

func astForEnumPattern(info types.PkgInfo) astgen.ASTDecl {
info.AddImports("regexp")
matchString := expression.NewCallFunction("regexp", "MustCompile", expression.StringVal(enumValuePattern))
return &decl.Var{
Name: enumPatternVarName,
Value: matchString,
}
}

func enumUnmarshalTextAST(e spec.EnumDefinition, info types.PkgInfo) astgen.ASTDecl {
mapStringInterface := expression.Type(types.NewMapType(types.String, types.Any).GoType(info))
toCamelCase := varcaser.Caser{From: varcaser.ScreamingSnakeCase, To: varcaser.UpperCamelCase}.String

info.AddImports("strings")
info.AddImports("github.com/palantir/conjure-go-runtime/v2/conjure-go-contract/errors")
info.AddImports("github.com/palantir/witchcraft-go-error")
info.AddImports("github.com/palantir/witchcraft-go-params")

switchStmt := &statement.Switch{
Expression: expression.NewCallFunction("strings", "ToUpper", expression.NewCallExpression(expression.StringType, expression.VariableVal(dataVarName))),
Init: statement.NewAssignment(expression.VariableVal(enumUpperVarName), token.DEFINE, expression.NewCallFunction("strings", "ToUpper", expression.NewCallExpression(expression.StringType, expression.VariableVal(dataVarName)))),
Expression: expression.VariableVal(enumUpperVarName),
Cases: []statement.CaseClause{
// default case
{
Body: []astgen.ASTStmt{statement.NewAssignment(
expression.NewUnary(token.MUL, expression.VariableVal(enumReceiverName)),
token.ASSIGN,
expression.VariableVal(e.TypeName.Name+toCamelCase(unknownEnumValue)),
),
Body: []astgen.ASTStmt{
&statement.If{
Cond: expression.NewUnary(token.NOT,
expression.NewCallExpression(
expression.NewSelector(expression.VariableVal(enumPatternVarName), "MatchString"),
expression.VariableVal(enumUpperVarName),
),
),
Body: []astgen.ASTStmt{
statement.NewReturn(
expression.NewCallFunction("werror", "Convert",
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the value in adding in the additional werror.Convert given the NewInvalidArgument will return a proper error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adam asked the same question on Slack. This might be exposing a deficiency with how we implemented the conjure error, but because it's not a werror it doesn't have a stacktrace. I added the werror.Convert as low as possible to preserve the stack frame(s) between here and the next werror.Wrap.

expression.NewCallFunction("errors", "NewInvalidArgument",
expression.NewCallFunction("wparams", "NewSafeAndUnsafeParamStorer",
expression.NewCompositeLit(mapStringInterface,
expression.NewKeyValue(`"enumType"`, expression.StringVal(e.TypeName.Name)),
expression.NewKeyValue(`"message"`, expression.StringVal("enum value must match pattern "+enumValuePattern)),
),
expression.NewCompositeLit(mapStringInterface,
expression.NewKeyValue(`"enumValue"`, expression.NewCallExpression(expression.StringType, expression.VariableVal(dataVarName))),
),
),
),
),
),
},
},
statement.NewAssignment(
expression.NewUnary(token.MUL, expression.VariableVal(enumReceiverName)),
token.ASSIGN,
expression.NewCallExpression(expression.Type(e.TypeName.Name), expression.VariableVal(enumUpperVarName)),
),
},
},
},
Expand Down
Loading