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

Created protoDocument from proto.Message preserves generated XXX_* fields. #1367

Closed
hosaka opened this issue Mar 16, 2019 · 4 comments
Closed
Assignees
Labels
api: firestore Issues related to the Firestore API. type: question Request for information or clarification. Not an issue.

Comments

@hosaka
Copy link

hosaka commented Mar 16, 2019

Cross posting from firebase/firebase-admin-go#228 as suggested by @hiranya911

Hello,

  • Operating System version: Ubuntu 18.04.2 LTS
  • Firebase SDK version: 3.6.0
  • Library version: _____
  • Firebase Product: (auth, storage)

Steps to reproduce:

I am looking for a quick protobuf to firestore push, without fiddling with json or string maps in go. Reading the code I saw that data interface{} can be a struct or a pointer to a struct, passing the pb.Message works perfectly fine, however it also serializes the generated XXX_ fields in my pb.go file. Please see the code for a quick example.

Screenshot from 2019-03-11 20-00-49

Is this left as is on purpose or perhaps it's an oversight?
Thank you in advance!

Relevant Code:

message Note {
    int32 Id = 1;
    int32 GroupId = 2;
    int32 LocationId = 3;
    int32 OwnerId = 4;
}
func (s *storeService) RegisterNote(ctx context.Context, in *Note) (*StoreResponse, error) {
	client, err := app.Firestore(ctx)

	_, _, err = client.Collection("notes").Add(ctx, in)

	return &StoreResponse{Success: true}, nil
}
@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Mar 16, 2019
@jeanbza jeanbza added type: question Request for information or clarification. Not an issue. api: firestore Issues related to the Firestore API. and removed triage me I really want to be triaged. labels Mar 18, 2019
@jeanbza
Copy link
Contributor

jeanbza commented Mar 18, 2019

cc @mikelehen @schmidt-sebastian

@jeanbza
Copy link
Contributor

jeanbza commented Mar 21, 2019

Is this left as is on purpose or perhaps it's an oversight?

This seems to be intentional, AFAICT. structToProtoValue grabs all fields from a struct and creates a map out of it. I don't think it's a good idea for us to be building protobuf-aware code, so I lean towards keeping it this way. If you'd like to omit these fields, I'd consider serializing it to whatever form that you prefer.

Note that this problem should disappear after golang/protobuf#276.

@jeanbza jeanbza closed this as completed Mar 21, 2019
@hosaka
Copy link
Author

hosaka commented Mar 22, 2019

@jadekler Thanks for the heads up. I do understand that protobuf-aware skips will just increase the amount of work in the future. It's not a huge issue, as I don't mind just leaving those fields in, they introduce little overhead in the grand scheme of things. I will keep an eye on that PR.

@jeanbza
Copy link
Contributor

jeanbza commented Mar 22, 2019

Sounds good! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: firestore Issues related to the Firestore API. type: question Request for information or clarification. Not an issue.
Projects
None yet
Development

No branches or pull requests

3 participants