How do I compile map fields to "map: true" instead of "repeated: true"? about protobuf HOT 13 CLOSED

@whatyouhide @rbino Good news, it was a false alarm probably having to do with deps caching on my side (I used Hex version before I pulled the main branch code).

It is working fine, and looks like this:

field :query_params, 9,
    repeated: true,
    type: Tongo.Protos.CreateLogRequest.QueryParamsEntry,
    json_name: "queryParams",
    map: true

from protobuf.

@bjuretic can you provide a complete minimal example that fails (i.e. a .proto file) so I can take a look?

Note that after the fix the field is marked both as repeated and as map, from what I understood reading Protobuf docs map fields are effectively repeated fields too to support languages without a map typ

from protobuf.

Hey @asciibeats, we'll need more information on this. Can you please share

1. The .proto schema you're trying to compile (not all of it, just relevant field definition)
2. What's the current generated code
3. What you're expecting the generated code to look like

Thanks!

from protobuf.

Thanks for your quick reply and of course your work!

What i have is:

syntax = "proto3";

message Message {
  map<uint32, uint32> stuff = 1;
}

I do compile without anything fancy:

protoc --proto_path=priv --elixir_out=lib/schnitzel --elixir_opt=package_prefix=schnitzel.messages messages.proto

What i get is:

defmodule Schnitzel.Messages.Message.StuffEntry do
  @moduledoc false
  use Protobuf, map: true, protoc_gen_elixir_version: "0.10.0", syntax: :proto3

  field :key, 1, type: :uint32
  field :value, 2, type: :uint32
end
defmodule Schnitzel.Messages.Message do
  @moduledoc false
  use Protobuf, protoc_gen_elixir_version: "0.10.0", syntax: :proto3

  field :stuff, 1, repeated: true, type: Schnitzel.Messages.Message.StuffEntry
end

Which is not unexpected but what i would like to get is:

...

defmodule Schnitzel.Messages.Message do
  @moduledoc false
  use Protobuf, protoc_gen_elixir_version: "0.10.0", syntax: :proto3

  field :stuff, 1, map: true, type: Schnitzel.Messages.Message.StuffEntry
end

My goal is to be able to use elixir maps to create my messages. Changing "repeated: true" to "map: true" by hand works perfectly fine, but of course i have to do it after each compilation. I did look at your source code but was a bit overwelmed, so i wanted to ask first. Maybe there is an easy solution.

Would getting "map: true" be a sensible default?

Again, thank you!

from protobuf.

Ah, okay I see what you mean.

map: true is used for each map key-value pair. As you see in

message Message {
  map<uint32, uint32> stuff = 1;
}

there's no StuffEntry message. That's generated by Protobuf itself by the name of the field (stuff) plus the Entry suffix, to represent a single key-value entry in the map.

Are you having any issues or unexpected behavior in the Elixir code? 🤔

from protobuf.

Are you having any issues or unexpected behavior in the Elixir code?

You mean using:

defmodule Schnitzel.Messages.Message do
  ...
  field :stuff, 1, map: true, type: Schnitzel.Messages.Message.StuffEntry
end

No.

Just to clarify a bit more for everybody, my change enables me to do:

%Schnitzel.Messages.Message{stuff: %{1 => 2, 3 => 4}}

Instead of:

%Schnitzel.Messages.Message{stuff: [%Schnitzel.Messages.Message.StuffEntry{key: 1, value: 2}, %Schnitzel.Messages.Message.StuffEntry{key: 3, value: 4}]}

Side question: Is there a simpler "normal" way?

from protobuf.

No, the former should be supported! And it was, I’m pretty sure. So this might be a bug/regression after all, thanks for helping us triage 🐛

Wanna take a shot at fixing this? I won't be able to for a bit 🙏

from protobuf.

If i find the time, i will give it a shot.

What should a fix look like for you?

The easiest would probably be to just output "map: true" instead of "repeated: true" but obviously that would break current behaviour. Maybe the best is to add a compiler option to output "map: true" and in a second step remove/abstract away the "StuffEntry" message?

from protobuf.

I think a while ago this library used to still output the StuffEntry with map: true, but then it would be smart enough to figure out that Stuff is the map itself.

I think you'd need to figure out what protoc is passing down to us as the schema. I think it might be protoc itself creating StuffEntry, but I’m not sure.

As it stands right now, I won't have time to look at this for the near future. 😞

from protobuf.

Hi @asciibeats and @whatyouhide, I opened #325 that should hopefully fix the problem

from protobuf.

It does fix it. Thank you @rbino!

from protobuf.

You sure this is fixed? I installed from the main branch (the package and the escript as well) and it is still generating repeated: true instead of map: true for the simplest scenarios as in the original request. e.g. in proto file:

map<string, string> query_params = 1;

Fixing it manually in the generated file fixes the issue.

from protobuf.

@bjuretic any chance you have time to try and debug this and try a fix? 🙃

from protobuf.