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

DRV-368: Added support for omitempty field tag #124

Closed
wants to merge 2 commits into from
Closed

DRV-368: Added support for omitempty field tag #124

wants to merge 2 commits into from

Conversation

mscno
Copy link

@mscno mscno commented Jan 5, 2021

This commit fixes #123 and enables discarding zero valued fields when saving structs that are tagged with omitempty

Test is included, existing functionality with tagging with a name and ignoring with - is maintained

func TestSerializeStructWithOmitEmptyTags(t *testing.T) {
	type user struct {
		Name     string  `fauna:"name,omitempty"`
		LastName string  `fauna:"last_name,omitempty"`
		Age      int     `fauna:"age,omitempty"`
		Siblings int     `fauna:"siblings,omitempty"`
		Height   float64 `fauna:"height,omitempty"`
		Weight   float64 `fauna:"weight,omitempty"`
	}

	assertJSON(t,
		Obj{"data": user{"Jhon", "", 18, 0, 123.123, 0.0}},
		`{"object":{"data":{"object":{"age":18, "height":123.123, "name":"Jhon"}}}}`,
	)
}

This enables discarding zero valued fields
@trevorsibanda trevorsibanda changed the title Added support for omitempty field tag DRV-368: Added support for omitempty field tag Jan 6, 2021
@mscno
Copy link
Author

mscno commented Jan 14, 2021

tests and fixes for deserialization still needed, will get that one of the next days

@mscno
Copy link
Author

mscno commented Jan 14, 2021

I added a WIP of sucessfully decoding omitempty structs.

This however caused another test to fail:

TestRunClientTests/TestEvalCountMeanSumExpression

Probably need someone with more intricate knowledge of the FQL language and serialization/deserialization to finish and fix this

@marrony
Copy link
Contributor

marrony commented Feb 9, 2021

@mscno sorry but what's the purpose of the tag? I could not identify by the tests. Is it to omit the field from json serialization?

@mscno
Copy link
Author

mscno commented Feb 9, 2021

hi @marrony, yes the purpose is to avoid serialization of the json structs to allow for partial update of a document. Currently when doing partial updates using a struct, the zero values will overwrite all the existing values in the document. This requires users that want to do partial updates to a document, to do the update manually using FQL object constructors.

I gave a more thorough description in the issue: #123

Other databases (Google Cloud Datastore/Firestore) and Pg-Go (Postgres) drivers has support for ignoring zero values, which enables you do to a partial update on a document/row without sending all existing values over the wire.

Height float64 `fauna:"height,omitempty"`
Weight float64 `fauna:"weight,omitempty"`
HeightPointer *float64 `fauna:"height_pointer,omitempty"`
WeightPointer *float64 `fauna:"weight_pointer,omitempty"`
Copy link
Contributor

@marrony marrony Feb 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you enhance this struct to make it clear that the empty values will not be serialized (btw, it was really hard to see that weight_pointer field was meant to omitted ), you don't need to a "real" user struct, I'm more interested to see the all the combinations of data types you can omit, something along in the lines

	type omitStruct struct {
		String string `fauna:str,omitempty`,
		Int *int `fauna:int,omitempty`
		....
	}

	assertJSON(t,
 		Obj{"data": omitStruct{nil, nil, ....}},
 		`{"object":{"data":{"object":{}}}}`,
 	)

	assertJSON(t,
 		Obj{"data": omitStruct{"String", nil, ....}},
 		`{"object":{"data":{"object":{"str":"String"}}}}`,
 	)

	x := 10
	assertJSON(t,
 		Obj{"data": omitStruct{nil, &x, ....}},
 		`{"object":{"data":{"object":{"int":10}}}}`,
 	)

 	//more assertions

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

certainly, ill update the tests to make it more clear how it handles different empty values.

@parkhomenko
Copy link
Contributor

hi @mscno,
sorry for bothering you, but do you have plans on wrapping up this PR?

@vadimLF
Copy link
Contributor

vadimLF commented Mar 19, 2021

This was done with #134. This PR can be closed.

@vadimLF vadimLF closed this Mar 19, 2021
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

Successfully merging this pull request may close these issues.

Fauna-go should ignore zero values when serializing/saving
4 participants