From 51767d243827647d9de2947fb797e7db58f89b83 Mon Sep 17 00:00:00 2001 From: Alec Thomas Date: Mon, 8 Apr 2024 14:28:39 +1000 Subject: [PATCH] fix: ensure zero fields are included in ingress responses There should ideally be no relationship between the ingress encoder and the encoding package, but for now this is the simplest solution. Fixes #1196 --- backend/controller/ingress/alias.go | 15 ++++++--------- backend/controller/ingress/request.go | 3 +-- backend/controller/ingress/response.go | 3 +-- backend/schema/field.go | 10 ++++++---- go-runtime/encoding/encoding.go | 24 ++++++++++++++---------- go-runtime/encoding/encoding_test.go | 2 +- 6 files changed, 29 insertions(+), 28 deletions(-) diff --git a/backend/controller/ingress/alias.go b/backend/controller/ingress/alias.go index 9e87d5b538..c316998a43 100644 --- a/backend/controller/ingress/alias.go +++ b/backend/controller/ingress/alias.go @@ -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,10 +63,11 @@ 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 }) @@ -77,8 +75,7 @@ func transformFromAliasedFields(ref *schema.Ref, sch *schema.Schema, request map 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 diff --git a/backend/controller/ingress/request.go b/backend/controller/ingress/request.go index 287a16127d..369723fd5b 100644 --- a/backend/controller/ingress/request.go +++ b/backend/controller/ingress/request.go @@ -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 { diff --git a/backend/controller/ingress/response.go b/backend/controller/ingress/response.go index b70e72e839..7efe402b86 100644 --- a/backend/controller/ingress/response.go +++ b/backend/controller/ingress/response.go @@ -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 diff --git a/backend/schema/field.go b/backend/schema/field.go index dc3c2e56a2..3fdc71d600 100644 --- a/backend/schema/field.go +++ b/backend/schema/field.go @@ -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 { diff --git a/go-runtime/encoding/encoding.go b/go-runtime/encoding/encoding.go index ef5ce85468..f7e3c6eb4f 100644 --- a/go-runtime/encoding/encoding.go +++ b/go-runtime/encoding/encoding.go @@ -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(',') } diff --git a/go-runtime/encoding/encoding_test.go b/go-runtime/encoding/encoding_test.go index f943754c3b..4fc6105fec 100644 --- a/go-runtime/encoding/encoding_test.go +++ b/go-runtime/encoding/encoding_test.go @@ -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 {