cayleygraph / cayley

An open-source graph database
https://cayley.io
Apache License 2.0
14.84k stars 1.25k forks source link

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

Open dncohen opened 5 years ago

dncohen commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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.