Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

-mapfields-as-maps and -json possible incompability #227

Closed
andreeelopes opened this issue Aug 5, 2023 · 2 comments
Closed

-mapfields-as-maps and -json possible incompability #227

andreeelopes opened this issue Aug 5, 2023 · 2 comments

Comments

@andreeelopes
Copy link

andreeelopes commented Aug 5, 2023

Hey, it's possible I missed something in the docs regarding this subject, but there seems to be an issue with the json decoding when -mapfields-as-maps is passed to protoc-erl.

syntax = "proto3";

message Person {
  string name = 1;
  map<string, string> map = 2;
}
../gpb/bin/protoc-erl -I. -mapfields-as-maps -json  x.proto

erlc -I../gpb/include x.erl
Erlang/OTP 24 [erts-12.3.2.2] [source] [64-bit] [smp:10:10] [ds:10:10:10] [async-threads:1]

Eshell V12.3.2.2  (abort with ^G)
1> rr("x.hrl").
['Person']
3>
3> Bin = x:encode_msg(#'Person'{name="abc def", map=#{"a"=>"b"}}).
<<10,7,97,98,99,32,100,101,102,18,6,10,1,97,18,1,98>>
4>
4> Record = x:decode_msg(Bin, 'Person').
#'Person'{name = "abc def", map = #{"a" => "b"}}
5>
5> Map = x:to_json(Record).
** exception error: no match of right hand side value #{key => "a",
                                                        value => "b"}
     in function  x:'-tj_mapfield_fold/3-fun-0-'/4 (x.erl, line 624)
     in call from lists:foldl/3 (lists.erl, line 1267)

Could we add an extra condition here when records is matched, to also check if mapfields-as-maps is on?

Thanks in advance!

@tomas-abrahamsson
Copy link
Owner

Hi, you're absolutely right, that option was not properly handled together with the json option. Thanks for making me aware. I believe it is now fixed in the 4.19.8 release, which is fresh from the press. I'll close this issue. If you should find there's anything more to it, feel free to just reopen, or open another issue.

@tomas-abrahamsson
Copy link
Owner

tomas-abrahamsson commented Aug 6, 2023

Closing. (oops, had some trouble actually closing the issue, sorry for this extra comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants