Skip to content

Commit

Permalink
fix: ensure zero fields are included in ingress responses (#1198)
Browse files Browse the repository at this point in the history
There should ideally be no relationship between the ingress encoder and
the encoding package, but for now this is the simplest solution.

Fixes #1196
alecthomas authored Apr 8, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent 349e935 commit bc2191c
Showing 6 changed files with 29 additions and 28 deletions.
15 changes: 6 additions & 9 deletions backend/controller/ingress/alias.go
Original file line number Diff line number Diff line change
@@ -53,9 +53,6 @@ func transformAliasedFields(sch *schema.Schema, t schema.Type, obj any, aliaser
}

case *schema.Optional:
if obj == nil {
return nil
}
return transformAliasedFields(sch, t.Type, obj, aliaser)

case *schema.Any, *schema.Bool, *schema.Bytes, *schema.Float, *schema.Int,
@@ -66,19 +63,19 @@ func transformAliasedFields(sch *schema.Schema, t schema.Type, obj any, aliaser

func transformFromAliasedFields(ref *schema.Ref, sch *schema.Schema, request map[string]any) (map[string]any, error) {
return request, transformAliasedFields(sch, ref, request, func(obj map[string]any, field *schema.Field) string {
jsonAlias := field.Alias(schema.AliasKindJSON)
if _, ok := obj[field.Name]; !ok && jsonAlias != "" && obj[jsonAlias] != nil {
obj[field.Name] = obj[jsonAlias]
delete(obj, jsonAlias)
if jsonAlias, ok := field.Alias(schema.AliasKindJSON).Get(); ok {
if _, ok := obj[field.Name]; !ok && obj[jsonAlias] != nil {
obj[field.Name] = obj[jsonAlias]
delete(obj, jsonAlias)
}
}
return field.Name
})
}

func transformToAliasedFields(ref *schema.Ref, sch *schema.Schema, request map[string]any) (map[string]any, error) {
return request, transformAliasedFields(sch, ref, request, func(obj map[string]any, field *schema.Field) string {
jsonAlias := field.Alias(schema.AliasKindJSON)
if jsonAlias != "" && field.Name != jsonAlias {
if jsonAlias, ok := field.Alias(schema.AliasKindJSON).Get(); ok && field.Name != jsonAlias {
obj[jsonAlias] = obj[field.Name]
delete(obj, field.Name)
return jsonAlias
3 changes: 1 addition & 2 deletions backend/controller/ingress/request.go
Original file line number Diff line number Diff line change
@@ -291,8 +291,7 @@ func parseQueryParams(values url.Values, data *schema.Data) (map[string]any, err

var field *schema.Field
for _, f := range data.Fields {
jsonAlias := f.Alias(schema.AliasKindJSON)
if (jsonAlias != "" && jsonAlias == key) || f.Name == key {
if jsonAlias, ok := f.Alias(schema.AliasKindJSON).Get(); (ok && jsonAlias == key) || f.Name == key {
field = f
}
for _, typeParam := range data.TypeParameters {
3 changes: 1 addition & 2 deletions backend/controller/ingress/response.go
Original file line number Diff line number Diff line change
@@ -80,8 +80,7 @@ func bodyForType(typ schema.Type, sch *schema.Schema, data []byte) ([]byte, erro
}

err = transformAliasedFields(sch, t, response, func(obj map[string]any, field *schema.Field) string {
jsonAlias := field.Alias(schema.AliasKindJSON)
if jsonAlias != "" && field.Name != jsonAlias {
if jsonAlias, ok := field.Alias(schema.AliasKindJSON).Get(); ok && field.Name != jsonAlias {
obj[jsonAlias] = obj[field.Name]
delete(obj, field.Name)
return jsonAlias
10 changes: 6 additions & 4 deletions backend/schema/field.go
Original file line number Diff line number Diff line change
@@ -6,6 +6,8 @@ import (

"google.golang.org/protobuf/proto"

"github.com/alecthomas/types/optional"

schemapb "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/schema"
)

@@ -49,14 +51,14 @@ func (f *Field) ToProto() proto.Message {
}
}

// Alias returns the alias for the given kind, or "" if not found.
func (f *Field) Alias(kind AliasKind) string {
// Alias returns the alias for the given kind.
func (f *Field) Alias(kind AliasKind) optional.Option[string] {
for _, md := range f.Metadata {
if a, ok := md.(*MetadataAlias); ok && a.Kind == kind {
return a.Alias
return optional.Some(a.Alias)
}
}
return ""
return optional.None[string]()
}

func fieldListToSchema(s []*schemapb.Field) []*Field {
24 changes: 14 additions & 10 deletions go-runtime/encoding/encoding.go
Original file line number Diff line number Diff line change
@@ -9,7 +9,6 @@ import (
"encoding/json"
"fmt"
"reflect"
"strings"

"github.com/TBD54566975/ftl/backend/schema/strcase"
)
@@ -113,16 +112,21 @@ func encodeStruct(v reflect.Value, w *bytes.Buffer) error {
afterFirst := false
for i := range v.NumField() {
ft := v.Type().Field(i)
t := ft.Type
fv := v.Field(i)
// Types that can be skipped if they're zero.
if (t.Kind() == reflect.Slice && fv.Len() == 0) ||
(t.Kind() == reflect.Map && fv.Len() == 0) ||
(t.String() == "ftl.Unit" && fv.IsZero()) ||
(strings.HasPrefix(t.String(), "ftl.Option[") && fv.IsZero()) ||
(t == reflect.TypeOf((*any)(nil)).Elem() && fv.IsZero()) {
continue
}
// TODO: If these fields are skipped, the ingress encoder will not include
// them in the output. There should ideally be no relationship between
// the ingress encoder and the encoding package, but for now this is the
// simplest solution.

// t := ft.Type
// // Types that can be skipped if they're zero.
// if (t.Kind() == reflect.Slice && fv.Len() == 0) ||
// (t.Kind() == reflect.Map && fv.Len() == 0) ||
// (t.String() == "ftl.Unit" && fv.IsZero()) ||
// (strings.HasPrefix(t.String(), "ftl.Option[") && fv.IsZero()) ||
// (t == reflect.TypeOf((*any)(nil)).Elem() && fv.IsZero()) {
// continue
// }
if afterFirst {
w.WriteRune(',')
}
2 changes: 1 addition & 1 deletion go-runtime/encoding/encoding_test.go
Original file line number Diff line number Diff line change
@@ -37,7 +37,7 @@ func TestMarshal(t *testing.T) {
{name: "UnitField", input: struct {
String string
Unit ftl.Unit
}{String: "something", Unit: ftl.Unit{}}, expected: `{"string":"something"}`},
}{String: "something", Unit: ftl.Unit{}}, expected: `{"string":"something","unit":{}}`},
}

for _, tt := range tests {

0 comments on commit bc2191c

Please sign in to comment.