gogo / protobuf

[Deprecated] Protocol Buffers for Go with Gadgets
Other
5.66k stars 806 forks source link

Allow set jsontag and moretags for oneof #623

Open krhubert opened 5 years ago

krhubert commented 5 years ago

Hi,

What do you think about setting tags for oneof directly? Syntax might look like:

syntax = "proto3";

import "gogo/protobuf/gogoproto/gogo.proto";

message T {
  oneof v {
    // ...
  } [
    (gogoproto.jsontag) = "v",
    (gogoproto.moretags) = 'customtag:"v"'
  ];
}

Then you gain have full control of the generated message tags. I found this as a limitation in my use-case.

octobocto commented 4 years ago

I would like this as well! The protoc-gen-gotag package has this feature, and have solved it like this:

The option has a tag, which adds that tag to both types.

  oneof one_of {
    option (tagger.oneof_tags) = "graphql:\"withNewTags,optional\"";
    string a = 5 [ (gogoproto.moretags) = "json:\"A\"" ];
    int32 b_jk = 6 [ (gogoproto.moretags) = "json:\"b_Jk\"" ];
  }

The generated code looks like this:

OneOf isExample_OneOf `protobuf_oneof:"one_of" graphql:"withNewTags,optional"`

And the option is added like this..

extend google.protobuf.OneofOptions {
    string oneof_tags = 847939;
}
torkelrogstad commented 4 years ago

Would a PR implementing this be welcome?

bwplotka commented 4 years ago

+1 on this. Is this a valid feature for gogo protobuf? :hugs:

bwplotka commented 4 years ago

Our use case is that we map from JSON -> Go PB -> proto serialized (and back) and we want to store main API in proto. This means we would like to do it "properly" in proto so e.g:

message Rule {
  oneof result {
    RecordingRule recording = 1;
    Alert alert= 2;
  }
}

This is does not work with just json tags. ):

cc @s-urbaniak

Wonder if we can workaround with custom unmarshal/marshal :thinking:

bwplotka commented 4 years ago

I would say now that workaround is actually the long term solution as well. This is because for our case we depend on special type field that tells the type of oneof in JSON.

This is how it looks if anyone is interested:

func (m *Rule) UnmarshalJSON(entry []byte) error {
    decider := struct {
        Type string `json:"type"`
    }{}
    if err := json.Unmarshal(entry, &decider); err != nil {
        return errors.Wrapf(err, "rule: type field unmarshal: %v", string(entry))
    }

    switch strings.ToLower(decider.Type) {
    case "recording":
        r := &RecordingRule{}
        if err := json.Unmarshal(entry, r); err != nil {
            return errors.Wrapf(err, "rule: recording rule unmarshal: %v", string(entry))
        }

        m.Result = &Rule_Recording{Recording: r}
    case "alerting":
        r := &Alert{}
        if err := json.Unmarshal(entry, r); err != nil {
            return errors.Wrapf(err, "rule: alerting rule unmarshal: %v", string(entry))
        }

        m.Result = &Rule_Alert{Alert: r}
    case "":
        return errors.Errorf("rule: no type field provided: %v", string(entry))
    default:
        return errors.Errorf("rule: unknown type field provided %s; %v", decider.Type, string(entry))
    }
    return nil
}

func (m *Rule) MarshalJSON() ([]byte, error) {
    if r := m.GetRecording(); r != nil {
        return json.Marshal(struct {
            *RecordingRule
            Type string `json:"type"`
        }{
            RecordingRule: r,
            Type:          RuleRecordingType,
        })
    }
    a := m.GetAlert()
    return json.Marshal(struct {
        *Alert
        Type string `json:"type"`
    }{
        Alert: a,
        Type:  RuleAlertingType,
    })
}