elixir-protobuf / google-protos

Elixir files generated from Google's protobuf files using protobuf-elixir
MIT License
16 stars 21 forks source link

wrappers always encoded as nil and decoded as default value #17

Closed sekiyama58 closed 2 years ago

sekiyama58 commented 2 years ago

Issue

The wrappers like Google.Protobuf.Int32Value are used to distinguish nil value and default value (such as 0). However, currently elixir-protobuf encodes both nil and default values as nil, and decodes both values as 0.

For example,

iex> Google.Protobuf.Int64Value.new!(value: nil) |> Protobuf.encode()
""
iex> Google.Protobuf.Int64Value.new!(value: 0) |> Protobuf.encode()
""
iex> Protobuf.decode("", Google.Protobuf.Int64Value)
%Google.Protobuf.Int64Value{__unknown_fields__: [], value: 0}

This is due to elixir protobuf library skips the encoding of fields with default values like 0, 0.0, "" and false: https://github.com/elixir-protobuf/protobuf/blob/cdf3acc53f619866b4921b8216d2531da52ceba7/lib/protobuf/encoder.ex#L85-L88

Possible Solution

One of the way to enable it to distinguish nil and default values is add the proto3_optional: true option to each field in wrappers.pb.ex, e.g.:

defmodule Google.Protobuf.UInt64Value do
  @moduledoc false
  use Protobuf, protoc_gen_elixir_version: "0.10.0", syntax: :proto3

  field :value, 1, type: :uint64, proto3_optional: true
end

This makes the library to suppress skipping the fields, so that nil and default values are distinguishable:

iex> Google.Protobuf.Int32Value.new!(value: nil) |> Protobuf.encode
""
iex> Google.Protobuf.Int32Value.new!(value: 0) |> Protobuf.encode
<<8, 0>>

iex> Protobuf.decode("", Google.Protobuf.Int32Value)
%Google.Protobuf.Int32Value{__unknown_fields__: [], value: nil}
iex> Protobuf.decode(<<8, 0>>, Google.Protobuf.Int32Value)
%Google.Protobuf.Int32Value{__unknown_fields__: [], value: 0}
ericmj commented 2 years ago

One of the way to enable it to distinguish nil and default values is add the proto3_optional: true option to each field in wrappers.pb.ex

That would be changing the schema which we cannot do.

The issue here seems to be setting the value field to nil, that is not valid value as it only accepts integers. I think we should instead fix Protobuf.encode/1 so that it raises when trying to encode nil for a type that does not accept it. Please open an issue on https://github.com/elixir-protobuf/protobuf for that.