Huangyan9188 / gogoprotobuf

Automatically exported from code.google.com/p/gogoprotobuf
Other
0 stars 0 forks source link

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

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
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 reported on code.google.com by jtolds on 27 Mar 2014 at 9:00

GoogleCodeExporter commented 9 years ago
Does adding 

option (gogoproto.enum_stringer_all) = true;

fix your issue?

Original comment by awalterschulze on 28 Mar 2014 at 10:26

GoogleCodeExporter commented 9 years ago
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.

Original comment by jtolds on 28 Mar 2014 at 3:59

GoogleCodeExporter commented 9 years ago
Ok but the default setting is 

option (gogoproto.goproto_enum_stringer_all) = true;

and that matches the behaviour of goprotobuf.

Original comment by awalterschulze on 29 Mar 2014 at 7:58

GoogleCodeExporter commented 9 years ago
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?

Original comment by jtolds on 29 Mar 2014 at 9:50

GoogleCodeExporter commented 9 years ago
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.

Original comment by jtolds on 29 Mar 2014 at 9:51

GoogleCodeExporter commented 9 years ago
I suspect you have not updated your goprotobuf, since October 2013

http://code.google.com/p/goprotobuf/source/detail?r=61664b8425f3e0e4371d4b56016b
224b9d69cbbb

This test fails on my February 2014 goprotobuf

Original comment by awalterschulze on 29 Mar 2014 at 10:25

GoogleCodeExporter commented 9 years ago
oh my goodness i'm so sorry for wasting your time. that's totally what happened

Original comment by jtolds on 29 Mar 2014 at 6:58