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

default enum stringer settings produce wrong json-marshalled output #10

Closed
awalterschulze opened this issue Dec 3, 2014 · 7 comments
Closed

Comments

@awalterschulze
Copy link
Member

From jtolds on March 27, 2014 23:00:30

What steps will reproduce the problem? 1. build protocol buffers with gogoprotobuf defaults.
2. observe that the String() method works correctly on enums.
3. marshal the protocol buffer with an enum field What is the expected output? What do you see instead? you'd expect to see the name of the enum field (what goprotobuf does) as opposed to the integer value, but you see the integer value in the marshalled json instead. Please provide any additional information below. because of how the generator code is working, you get to pick. if you want the enum functionality to work correctly, you can disable the enum stringer

option (gogoproto.goproto_enum_stringer_all) = false;

but then you don't get the enum String() method.

generator.go has this code:

if gogoproto.IsGoEnumStringer(g.file.FileDescriptorProto, enum.EnumDescriptorProto) {
    g.P("func (x ", ccTypeName, ") String() string {")
    g.In()
    g.P("return ", g.Pkg["proto"], ".EnumName(", ccTypeName, "_name, int32(x))")
    g.Out()
    g.P("}")
}

if !gogoproto.IsGoEnumStringer(g.file.FileDescriptorProto, enum.EnumDescriptorProto) {
    g.P("func (x ", ccTypeName, ") MarshalJSON() ([]byte, error) {")
    g.In()
    g.P("return ", g.Pkg["proto"], ".MarshalJSONEnum(", ccTypeName, "_name, int32(x))")
    g.Out()
    g.P("}")
}

seems to me like you shouldn't have to choose, and you should get MarshalJSON anyway.

Original issue: http://code.google.com/p/gogoprotobuf/issues/detail?id=10

@awalterschulze
Copy link
Member Author

From awalterschulze on March 28, 2014 03:26:17

Does adding

option (gogoproto.enum_stringer_all) = true;

fix your issue?

@awalterschulze
Copy link
Member Author

From jtolds on March 28, 2014 08:59:47

Yeah, that fixes it, but it seemed like the default behavior is supposed to match goprotobuf. I didn't expect to have to change any settings if I wanted to match the goprotobuf behavior.

@awalterschulze
Copy link
Member Author

From awalterschulze on March 29, 2014 00:58:14

Ok but the default setting is

option (gogoproto.goproto_enum_stringer_all) = true;

and that matches the behaviour of goprotobuf.

Status: Done

@awalterschulze
Copy link
Member Author

From jtolds on March 29, 2014 02:50:31

Given the following protocol buffer definition without any extensions,

message MyMessage {
enum MyEnum {
VAL1 = 1;
VAL2 = 2;
}
optional MyEnum setting = 1;
}

the following tests pass with goprotobuf but do not pass with gogoprotobuf.

package test

import (
"encoding/json"
"testing"
)

func TestString(t *testing.T) {
msg := &MyMessage{Setting: MyMessage_VAL1.Enum()}
if msg.Setting.String() != "VAL1" {
t.Fatal("string value expected")
}
}

func TestMarshalJSON(t *testing.T) {
data, err := json.Marshal(&MyMessage{Setting: MyMessage_VAL1.Enum()})
if err != nil {
t.Fatal(err)
}
var val interface{}
err = json.Unmarshal(data, &val)
if err != nil {
t.Fatal(err)
}
if (val.(map[string]interface{}))["setting"].(string) != "VAL1" {
t.Fatal("string value expected")
}
}

shouldn't they pass?

@awalterschulze
Copy link
Member Author

From jtolds on March 29, 2014 02:51:42

Hopefully the provided test case is concrete enough, but my original ticket is poorly trying to explain that gogoprotobuf for some reason disables MarshalJSON on enums by default, and the only way to enable MarshalJSON for enums is to disable the default Stringer.

@awalterschulze
Copy link
Member Author

From awalterschulze on March 29, 2014 03:25:40

I suspect you have not updated your goprotobuf, since October 2013 https://code.google.com/p/goprotobuf/source/detail?r=61664b8425f3e0e4371d4b56016b224b9d69cbbb This test fails on my February 2014 goprotobuf

@awalterschulze
Copy link
Member Author

From jtolds on March 29, 2014 11:58:05

oh my goodness i'm so sorry for wasting your time. that's totally what happened

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

1 participant