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

Fallback to JSON Struct Tags #88

Closed
steve-gray opened this issue Jan 28, 2018 · 4 comments
Closed

Fallback to JSON Struct Tags #88

steve-gray opened this issue Jan 28, 2018 · 4 comments
Assignees

Comments

@steve-gray
Copy link

steve-gray commented Jan 28, 2018

One small bugbear I've had for a long time with MGO is that unlike Mapstructure you can't set the TagName to use on structures. The general use-case here is letting structures be decorated with json annotations, and then having both mapstructure, mgo and pretty much anything else all work off the same tags - so I don't have:

type SomeStruct struct {
   SnakedField int `json:"snaked_field" mapstructure:"snaked_field" bson:"snaked_field"`
}

I've put an option in a fork of the code that enables this by adding a new global flag to the BSON parser, which can be set per-application. This allows for opting-in to using the JSON annotation if, and only if both the new option is enabled and there are no BSON annotations on the structure.

steve-gray@6f2a776

To 'truly' fix this at a BSON encoder level requires a major contract change, because it relies heavily on stateless global functions - so the notion of an encoder/decoder context would need to be added - and it'd break too much compatibility - unless I've missed something.

If folks are happy with it coming in, I can tweak it and submit the PR (or happy for someone to cherry-pick it) - it seems harmless enough and will aid mightily with struct-pollution in codebases. My main use case for MGO is as a provider that's one-of-many/pluggable - and not requiring every user to add 50 struct tags to a model to support all possible storage engines.

@domodwyer
Copy link

domodwyer commented Jan 28, 2018

Hi @steve-gray

I fully understand the use case here - a lot of applications using mgo/MongoDB also expose data via JSON, and I bet there's been more than a few lost hours to typos between the bson and json tags! Totally happy to accept this as a PR.

My only comments would be to defer performing the JSON tag search till after determining there's no BSON tag (avoids searching for an unused JSON tag) and to include the docs for useJSONTagFallback in SetJSONFallback() as thats all that will show in godoc - minor things!

Having this behind a feature flag was a pleasant surprise, you clearly take backwards compatibility to heart - thanks for taking the time to contribute.

Dom

@domodwyer
Copy link

Closing as #91 will fix imminently.

@steve-gray
Copy link
Author

@domodwyer - Any timeline on merge to mainline? I'm looking to leverage this functionality and I'm struggling with having type incompatibility between code that references my custom fork and 3rd party code that references the globalsign instance on a project.

@domodwyer
Copy link

Hi @steve-gray

Really sorry about the delay - the infrastructure I use to test mgo was in use for other things - I'll try and arrange to cut a release tomorrow.

Dom

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

2 participants