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

jsonPB marshaller with Enum as map value marshals enum to int #394

Open
espang opened this issue Mar 15, 2018 · 8 comments
Open

jsonPB marshaller with Enum as map value marshals enum to int #394

espang opened this issue Mar 15, 2018 · 8 comments

Comments

@espang
Copy link

espang commented Mar 15, 2018

Hi,

I found a inconsistency when I use the jsonpb marshaller with EnumAsInt not set. I have following messages defined in my proto file:

enum State {
    state1 = 0;
    state2 = 1;
}

message StatesResponse {
    map<string, State> states = 1;
}

message StateResponse {
    State state = 1;
}

The first one gets marshalled to:

{"states":{"18777d46-48b0-4659-8b72-f8ca69ee6536":1}}

the second one to:

{"state":"STATE2"}
@espang espang changed the title jsonPB marshaller with Enum as map values jsonPB marshaller with Enum as map value marshals enum to int Mar 15, 2018
@awalterschulze
Copy link
Member

Interesting. Do you think golang/protobuf has the same problem, or is it gogo/protobuf specific?

@espang
Copy link
Author

espang commented Mar 15, 2018

I don't know. We use gogo/protobuf and I only tested that. Will do a test with golang/protobuf

@timonwong
Copy link

timonwong commented Mar 19, 2018

EDIT: golang/protobuf has exactly the same issue:

https://github.com/golang/protobuf/blob/master/jsonpb/jsonpb_test.go#L388-L389

// TODO: This is broken.
//{"map<string, enum>", marshaler, &pb.Mappy{Enumy: map[string]pb.Numeral{"XIV": pb.Numeral_ROMAN}}, `{"enumy":{"XIV":"ROMAN"}`},
{"map<string, enum as int>", Marshaler{EnumsAsInts: true}, &pb.Mappy{Enumy: map[string]pb.Numeral{"XIV": pb.Numeral_ROMAN}}, `{"enumy":{"XIV":2}}`},

See golang/protobuf#408

@awalterschulze
Copy link
Member

awalterschulze commented Mar 19, 2018

Thank you @timonwong
Lets wait for golang/protobuf to fix this.
Then I will merge in their fix into gogo/protobuf.

@timonwong
Copy link

timonwong commented Aug 21, 2018

Update: As of golang/protobuf#645, it should be fixed?

@briansorahan
Copy link

@awalterschulze This would be really nice to have. Any ETA? I pulled gogo master in my GOPATH and ran a test and am seeing this exact issue.
It looks like golang/protobuf#645 was merged July 7, do you cherry pick or merge their master to yours?

@awalterschulze
Copy link
Member

@jmarais and @donaldgraham are the new maintainers of this project. Maybe they can take a look.

@briansorahan
Copy link

Ok thx for the quick response @awalterschulze

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

No branches or pull requests

4 participants