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

Allow set jsontag and moretags for oneof #623

Open
krhubert opened this issue Sep 9, 2019 · 5 comments
Open

Allow set jsontag and moretags for oneof #623

krhubert opened this issue Sep 9, 2019 · 5 comments

Comments

@krhubert
Copy link
Contributor

krhubert commented Sep 9, 2019

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
Copy link

octobocto commented Jan 13, 2020

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
Copy link

Would a PR implementing this be welcome?

@bwplotka
Copy link

bwplotka commented Mar 9, 2020

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

@bwplotka
Copy link

bwplotka commented Mar 9, 2020

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 🤔

@bwplotka
Copy link

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,
	})
}

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

4 participants