Inconsistent behaviour when encoding oneof

[nsweeting]

Hi there,

We're seeing (what feels like) inconsistent behaviour when encoding oneof values.

Given the following protobuf:

syntax = "proto3";

message Foo {
  message Bar {
    string value = 1;
  }

  message Baz {}

  oneof value {
    Bar bar = 1;
    Baz baz = 2;
  }
}

Which compiles to the following elixir code:

defmodule Foo.Bar do
  @moduledoc false

  use Protobuf, protoc_gen_elixir_version: "0.12.0", syntax: :proto3

  field(:value, 1, type: :string)
end

defmodule Foo.Baz do
  @moduledoc false

  use Protobuf, protoc_gen_elixir_version: "0.12.0", syntax: :proto3
end

defmodule Foo do
  @moduledoc false

  use Protobuf, protoc_gen_elixir_version: "0.12.0", syntax: :proto3

  oneof(:value, 0)

  field(:bar, 1, type: Foo.Bar, oneof: 0)
  field(:baz, 2, type: Foo.Baz, oneof: 0)
end

When encoding/decoding a message that contains a map for Foo.Bar (not the struct) and it contains a value - things work as expected:

foo = %Foo{value: {:bar, %{value: "bar"}}}
foo |> Foo.encode() |> Foo.decode()

%Foo{
  value: {:bar, %Foo.Bar{value: "bar", __unknown_fields__: []}},
  __unknown_fields__: []
}

But when encoding/decoding an empty map - it simply gets dropped entirely - even though we are passing the proper tuple for the oneof:

foo = %Foo{value: {:baz, %{}}}
foo |> Foo.encode() |> Foo.decode()

%Foo{value: nil, __unknown_fields__: []}

Since we've noted which oneof it is - it seems like it should be included.

Thanks

[ericmj]

What happens if you use a struct instead of an empty map?

%Foo{value: {:baz, %Foo.Baz{}}}

The library should probably not accept maps in place of structs but it is possible that it sees the empty map as a default value which are not encoded and since it's not encoded no oneof field was selected.

[whatyouhide]

There's two issues here:

1. This should raise, I think:

Foo.encode(%Foo{value: {:bar, %{value: "bar"}}})

because {:bar, _} should only work with a Foo.Bar struct.

2. The other issue is that %Foo{value: {:baz, %Foo.Baz{}}} should not empty the field because :baz is not the default value.

We're investigating this!

cc @savhappy

[whatyouhide]

Also, 1. above might be related to https://github.com/elixir-protobuf/protobuf/issues/365.