connectrpc / connect-go

The Go implementation of Connect: Protobuf RPC that works.
https://connectrpc.com
Apache License 2.0
2.94k stars 96 forks source link

Emitting empty fields in JSON response #684

Closed ktogo closed 7 months ago

ktogo commented 7 months ago

This issue both provides temporary workaround for those who have same issues, and suggests new feature that allows solving the issue officially on connect.

Feature Request

Add new func to create customized protoJSONCodec and allow configuring protojson.MarshalOptions through it.

// codec.go
type protoJSONCodec struct {
    name           string
    marshalOptions protojson.MarshalOptions
}

func (c *protoJSONCodec) Marshal(message any) ([]byte, error) {
    // ...
    return c.marshalOptions.Marshal(protoMessage)
}
// option.go
func WithProtoJSONCodecWithMarshalOption(opts protojson.MarshalOptions) HandlerOption {
    return WithHandlerOptions(
        &protoJSONCodec{codecNameJSON, opts},
        &protoJSONCodec{codecNameJSONCharsetUTF8, opts},
    )
}

The Problem

Is your feature request related to a problem? Please describe.

By Protobuf spec, the JSON fields with default values (e.g. zero values for primitives) are omitted from output.

When generating JSON-encoded output from a protocol buffer, if a protobuf field has the default value and if the field doesn’t support field presence, it will be omitted from the output by default.

Thus, given following protobuf message and input, Connect generates JSON with lack of fields with the default values.

message GetTestResponse {
    string field_a = 1;
    string field_b = 1;
}
// autogenerated
type GetDraftContractApplicationsResponse struct {
    FieldA string `protobuf:"bytes,1,opt,name=field_a,proto3" json:"field_a,omitempty"`
    FieldB string `protobuf:"bytes,1,opt,name=field_b,proto3" json:"field_b,omitempty"`
}
connect.NewResponse(&GetDraftContractApplicationsResponse{
    FieldA: "not empty",
    FieldB: "",
})
{"fieldA":"not empty"} // "fieldB" is omitted due to emptiness

This issue also happens with non-connect gRPC implementation, for example:

And in that issue, the workaround is suggested as to use protojson.MarshalOptions to override that behavior.

Temporary workaround

Describe alternatives you've considered

Meanwhile, I have confirmed that adding following code to my app and pass it as option to connect handler solved the issue.

import (
    "bytes"
    "encoding/json"
    "errors"
    "fmt"

    "connectrpc.com/connect"
    "google.golang.org/protobuf/encoding/protojson"
    "google.golang.org/protobuf/proto"
    "google.golang.org/protobuf/runtime/protoiface"
)

const (
    codecNameJSON            = "json"                            // ported from connect@v1.14.0
    codecNameJSONCharsetUTF8 = codecNameJSON + "; charset=utf-8" // ported from connect@v1.14.0
)

// customized part: allows passing options to protojson.MarshalOptions
func customProtoJSONMarshalOptions() protojson.MarshalOptions {
    return protojson.MarshalOptions{
        EmitDefaultValues: true, // disable "omitempty" behavior for default (zero) values
    }
}

// ported from connect@v1.14.0
type protoJSONCodec struct {
    name string
}

var _ connect.Codec = (*protoJSONCodec)(nil)

// ported from connect@v1.14.0
func (c *protoJSONCodec) Name() string { return c.name }

// ported from connect@v1.14.0, with modification
func (c *protoJSONCodec) Marshal(message any) ([]byte, error) {
    protoMessage, ok := message.(proto.Message)
    if !ok {
        return nil, errNotProto(message)
    }
    return customProtoJSONMarshalOptions().Marshal(protoMessage) // modified here
}

// ported from connect@v1.14.0, with modification
func (c *protoJSONCodec) MarshalAppend(dst []byte, message any) ([]byte, error) {
    protoMessage, ok := message.(proto.Message)
    if !ok {
        return nil, errNotProto(message)
    }
    return customProtoJSONMarshalOptions().MarshalAppend(dst, protoMessage) // modified here
}

// ported from connect@v1.14.0
func (c *protoJSONCodec) Unmarshal(binary []byte, message any) error {
    protoMessage, ok := message.(proto.Message)
    if !ok {
        return errNotProto(message)
    }
    if len(binary) == 0 {
        return errors.New("zero-length payload is not a valid JSON object")
    }
    // Discard unknown fields so clients and servers aren't forced to always use
    // exactly the same version of the schema.
    options := protojson.UnmarshalOptions{DiscardUnknown: true}
    err := options.Unmarshal(binary, protoMessage)
    if err != nil {
        return fmt.Errorf("unmarshal into %T: %w", message, err)
    }
    return nil
}

// ported from connect@v1.14.0
func (c *protoJSONCodec) MarshalStable(message any) ([]byte, error) {
    // protojson does not offer a "deterministic" field ordering, but fields
    // are still ordered consistently by their index. However, protojson can
    // output inconsistent whitespace for some reason, therefore it is
    // suggested to use a formatter to ensure consistent formatting.
    // https://github.com/golang/protobuf/issues/1373
    messageJSON, err := c.Marshal(message)
    if err != nil {
        return nil, err
    }
    compactedJSON := bytes.NewBuffer(messageJSON[:0])
    if err = json.Compact(compactedJSON, messageJSON); err != nil {
        return nil, err
    }
    return compactedJSON.Bytes(), nil
}

// ported from connect@v1.14.0
func (c *protoJSONCodec) IsBinary() bool {
    return false
}

// ported from connect@v1.14.0
func withProtoJSONCodecs() connect.HandlerOption {
    return connect.WithHandlerOptions(
        connect.WithCodec(&protoJSONCodec{codecNameJSON}),
        connect.WithCodec(&protoJSONCodec{codecNameJSONCharsetUTF8}),
    )
}

// ported from connect@v1.14.0
func errNotProto(message any) error {
    if _, ok := message.(protoiface.MessageV1); ok {
        return fmt.Errorf("%T uses github.com/golang/protobuf, but connect-go only supports google.golang.org/protobuf: see https://go.dev/blog/protobuf-apiv2", message)
    }
    return fmt.Errorf("%T doesn't implement proto.Message", message)
}

and retrieve handler option from withProtoJSONCodecs() and pass it to any handler:

handlerOption := withProtoJSONCodecs()

Additional context Add any other context or screenshots about the feature request here.

srikrsna-buf commented 7 months ago

Hey! There's a community package for this: https://github.com/akshayjshah/connectproto

ktogo commented 7 months ago

Cool! Okay I'll use that package then! Thanks 👍🏻