Skip to content

Commit

Permalink
fix: some bugs (#797)
Browse files Browse the repository at this point in the history
- "v" was not passed through to go.mod (again)
- fixed use of bytes in request encoding

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
alecthomas and github-actions[bot] authored Jan 17, 2024
1 parent bfda068 commit defcf49
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 3 deletions.
41 changes: 41 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,47 @@ hot-reloading ftl agent:
$ ftl devel ./examples/echo ./examples/time
```

## Best practices

### Sum types

We're using a tool called go-check-sumtype, which is basically a hacky way to
check that switch statements on certain types are exhaustive. An example of that
is schema.Type. The way it works is that if you have a type switch like this:

```go
switch t := t.(type) {
case *schema.Int:
}
```

It will detect that the switch isn't exhaustive. However, if you have this:

```go
switch t := t.(type) {
case *schema.Int:
default:
return fmt.Errorf("unsupported type")
}
```

Then it will assume you're intentionally handling the default case, and won't
detect it. Instead, if you panic in the default case rather than returning an error,
`go-check-sumtype` will be able to detect the missing case. A panic is usually
what we want anyway, because it isn't a recoverable error.

TL;DR Don't do the above, do this:

```go
switch t := t.(type) {
case *schema.Int:

// All other cases
default:
panic(fmt.Sprintf("unsupported type %T"))
}
```

## Communications

### Issues
Expand Down
8 changes: 7 additions & 1 deletion backend/controller/ingress/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,10 @@ func parseQueryParams(values url.Values, data *schema.Data) (map[string]any, err
}

switch field.Type.(type) {
// Explicitly enumerate known types here so go-check-sumtype will tell
// us when we're missing a case.
case *schema.Bytes, *schema.Map, *schema.Optional, *schema.Time, *schema.Unit:

case *schema.Int, *schema.Float, *schema.String, *schema.Bool:
if len(value) > 1 {
return nil, fmt.Errorf("multiple values for %q are not supported", key)
Expand All @@ -364,15 +368,17 @@ func parseQueryParams(values url.Values, data *schema.Data) (map[string]any, err
return nil, fmt.Errorf("complex value %q is not supported, use '@json=' instead", value[0])
}
queryMap[key] = value[0]

case *schema.Array:
for _, v := range value {
if hasInvalidQueryChars(v) {
return nil, fmt.Errorf("complex value %q is not supported, use '@json=' instead", v)
}
}
queryMap[key] = value

default:
return nil, fmt.Errorf("unsupported type %T for field %q", field.Type, key)
panic(fmt.Sprintf("unsupported type %T for query parameter field %q", field.Type, key))
}
}

Expand Down
3 changes: 3 additions & 0 deletions backend/schema/protobuf.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,9 @@ func generateProtoType(t reflect.Type) string {
case reflect.Float64:
return "double"
case reflect.Slice:
if t.Elem().Kind() == reflect.Uint8 {
return "bytes"
}
return fmt.Sprintf("repeated %s", generateProtoType(t.Elem()))
case reflect.Map:
return fmt.Sprintf("map<string, %s>", generateProtoType(t.Elem()))
Expand Down
2 changes: 1 addition & 1 deletion go-runtime/compile/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func updateGoModule(goModPath string) (string, error) {
return "", fmt.Errorf("failed to parse %s: %w", goModPath, err)
}
if ftl.IsRelease(ftl.Version) {
if err := goModfile.AddRequire("github.com/TBD54566975/ftl", ftl.Version); err != nil {
if err := goModfile.AddRequire("github.com/TBD54566975/ftl", "v"+ftl.Version); err != nil {
return "", fmt.Errorf("failed to add github.com/TBD54566975/ftl to %s: %w", goModPath, err)
}
goModBytes = modfile.Format(goModfile.Syntax)
Expand Down
14 changes: 13 additions & 1 deletion go-runtime/encoding/encoding.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package encoding

import (
"bytes"
"encoding/base64"
"fmt"
"reflect"

Expand All @@ -29,6 +30,9 @@ func encodeValue(v reflect.Value, w *bytes.Buffer) error {
return encodeValue(v.Elem(), w)

case reflect.Slice:
if v.Type().Elem().Kind() == reflect.Uint8 {
return encodeBytes(v, w)
}
return encodeSlice(v, w)

case reflect.Map:
Expand All @@ -47,7 +51,7 @@ func encodeValue(v reflect.Value, w *bytes.Buffer) error {
return encodeBool(v, w)

default:
return fmt.Errorf("unsupported type: %s", v.Type())
panic(fmt.Sprintf("unsupported typefoo: %s", v.Type()))
}
}

Expand All @@ -67,6 +71,14 @@ func encodeStruct(v reflect.Value, w *bytes.Buffer) error {
return nil
}

func encodeBytes(v reflect.Value, w *bytes.Buffer) error {
w.WriteRune('"')
data := base64.StdEncoding.EncodeToString(v.Bytes())
w.WriteString(data)
w.WriteRune('"')
return nil
}

func encodeSlice(v reflect.Value, w *bytes.Buffer) error {
w.WriteRune('[')
for i := 0; i < v.Len(); i++ {
Expand Down

0 comments on commit defcf49

Please sign in to comment.