The Wayback Machine - http://web.archive.org/web/20210908060159/https://github.com/cayleygraph/cayley/issues/769
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

schema: currently impossible to omit fields with json tag #769

Open

dncohen opened this issue Feb 16, 2019 · 2 comments
Open

schema: currently impossible to omit fields with json tag #769

dncohen opened this issue Feb 16, 2019 · 2 comments

Comments

@dncohen
Copy link

@dncohen dncohen commented Feb 16, 2019

Description

Golang encoders that I'm familiar with (gob, json) will not encode fields that are potentially ambiguous. That is when one struct embeds others, i.e....

type A struct {Data string `json:"data"`}
type B struct {Data string `json:"data"`}
type Thing struct {
    A
    B
}

Above, Thing.Data is ambiguous, and the json (and gob) encoder will omit it. Cayley's schema will not omit it. It considers Thing.A.Data and Thing.B.Data different. And it will insert them both with the same predicate "data".

In json and gob encoding, this feature can be used to omit items from the encoding. Something like:

type A struct {LotsOfJunk []string `json:"data"`}

type B struct {
    A
    Foo string
    omitJunk interface{} `json:"data,omitempty"`
}

Here, B embeds A. But when encoding, the LotsOfJunk field in A is explicitly omitted, because the omitJunk field is there specifically to cause that omission.

In Cayley schema, there is no similar feature. It's impossible to omit embedded fields.

If you agree that Cayley should work similar to other encoders, I believe that the schema package could be changed to do this. Currently, rulesForStructTo is recursively called with prefix pref appended for each embedded struct (f.Anonymous). I believe there is no need to append the prefix. Instead, that function should pay attention not to field names but to predicates. Those are calculated in fieldRule, but not returned consistently.

I believe fieldRule could have another return parameter (the predicate), and rulesForStructTo can index rules by predicate, not field name. If rulesForStructTo finds multiple rules for a single predicate, it should be omitted. There may be a need to introduce an explicit "omit" rule.

Any thoughts?

Output of cayley version or commit hash:

f0dd103fbc44c697e7a063279b5850ec4a7b403a
@dennwc dennwc added the bug label Feb 18, 2019
@dennwc
Copy link
Member

@dennwc dennwc commented Feb 18, 2019

Thanks for a very detailed report and I do agree that it should use predicates instead of struct tags. Would you mind implementing this change? Will be happy to review it.

Regarding field omission, quad:"-" should allow omitting fields even if JSON tag is set. If it fails to do so - it's a bug.

@dncohen
Copy link
Author

@dncohen dncohen commented Feb 19, 2019

I'll take a stab at it, if I can make the time... will post here if I make that happen. Thanks for the replies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants