Skip to content

Commit

Permalink
datastore: implement structCodec using internal/fields
Browse files Browse the repository at this point in the history
The changes to behavior can be seen in the modification of test
cases in datastore_test.go. These changes are:
- allow recursive structs
- allow 'slice of slices' (this should have happened with entity
  values change) unless 'flatten' option is turned on
- fall back to case-insensitivity for field names

Additionally, this change introduces errors when an invalid struct
field tag option is found.

Change-Id: Id374b3a1ed8bb13d608af9fddd4537672669ca54
Reviewed-on: https://code-review.googlesource.com/9770
Reviewed-by: Jonathan Amsterdam <jba@google.com>
  • Loading branch information
Sarah Adams committed Dec 9, 2016
1 parent ee72afc commit 1e032f3
Show file tree
Hide file tree
Showing 6 changed files with 156 additions and 188 deletions.
48 changes: 30 additions & 18 deletions datastore/datastore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ type Tagged struct {
J int `datastore:",noindex" json:"j"`

Y0 `datastore:"-"`
Z chan int `datastore:"-,"`
Z chan int `datastore:"-"`
}

type InvalidTagged1 struct {
Expand All @@ -280,6 +280,14 @@ type InvalidTagged2 struct {
J int `datastore:"I"`
}

type InvalidTagged3 struct {
X string `datastore:"-,noindex"`
}

type InvalidTagged4 struct {
X string `datastore:",garbage"`
}

type Inner1 struct {
W int32
X string
Expand Down Expand Up @@ -341,7 +349,7 @@ type SliceOfSlices struct {
S []struct {
J int
F []float64
}
} `datastore:",flatten"`
}

type Recursive struct {
Expand Down Expand Up @@ -949,16 +957,6 @@ var testCases = []testCase{
"",
"",
},
{
"save props load tagged",
&PropertyList{
Property{Name: "A", Value: int64(11), NoIndex: true},
Property{Name: "a", Value: int64(12), NoIndex: true},
},
&Tagged{A: 12},
"",
`cannot load field "A"`,
},
{
"invalid tagged1",
&InvalidTagged1{I: 1},
Expand All @@ -969,8 +967,22 @@ var testCases = []testCase{
{
"invalid tagged2",
&InvalidTagged2{I: 1, J: 2},
&InvalidTagged2{},
"struct tag has repeated property name",
&InvalidTagged2{J: 2},
"",
"",
},
{
"invalid tagged3",
&InvalidTagged3{X: "hello"},
&InvalidTagged3{},
"struct tag has invalid property name: \"-\"",
"",
},
{
"invalid tagged4",
&InvalidTagged4{X: "hello"},
&InvalidTagged4{},
"struct tag has invalid option: \"garbage\"",
"",
},
{
Expand Down Expand Up @@ -1661,15 +1673,15 @@ var testCases = []testCase{
{
"recursive struct",
&Recursive{},
nil,
"recursive struct",
&Recursive{},
"",
"",
},
{
"mutually recursive struct",
&MutuallyRecursive0{},
nil,
"recursive struct",
&MutuallyRecursive0{},
"",
"",
},
{
Expand Down
5 changes: 1 addition & 4 deletions datastore/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,7 @@ Valid value types are:
- structs whose fields are all valid value types,
- slices of any of the above.
Slices of structs are valid, as are structs that contain slices. However, if
one struct contains another, then at most one of those can be repeated. This
disqualifies recursively defined struct types: any struct T that (directly or
indirectly) contains a []T.
Slices of structs are valid, as are structs that contain slices.
The Get and Put functions load and save an entity's contents. An entity's
contents are typically represented by a struct pointer.
Expand Down
36 changes: 24 additions & 12 deletions datastore/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"strings"
"time"

"cloud.google.com/go/internal/fields"
pb "google.golang.org/genproto/googleapis/datastore/v1"
)

Expand Down Expand Up @@ -63,7 +64,7 @@ type propertyLoader struct {
m map[string]int
}

func (l *propertyLoader) load(codec *structCodec, structValue reflect.Value, p Property, prev map[string]struct{}) string {
func (l *propertyLoader) load(codec fields.List, structValue reflect.Value, p Property, prev map[string]struct{}) string {
sl, ok := p.Value.([]interface{})
if !ok {
return l.loadOneElement(codec, structValue, p, prev)
Expand All @@ -80,17 +81,17 @@ func (l *propertyLoader) load(codec *structCodec, structValue reflect.Value, p P
// loadOneElement loads the value of Property p into structValue based on the provided
// codec. codec is used to find the field in structValue into which p should be loaded.
// prev is the set of property names already seen for structValue.
func (l *propertyLoader) loadOneElement(codec *structCodec, structValue reflect.Value, p Property, prev map[string]struct{}) string {
func (l *propertyLoader) loadOneElement(codec fields.List, structValue reflect.Value, p Property, prev map[string]struct{}) string {
var sliceOk bool
var sliceIndex int
var v reflect.Value

name := p.Name
for name != "" {
// First we try to find a field with name matching
// the value of 'name' exactly.
decoder, ok := codec.fields[name]
if ok {
// the value of 'name' exactly (though case-insensitively).
field := codec.Match(name)
if field != nil {
name = ""
} else {
// Now try for legacy flattened nested field (named eg. "A.B.C.D").
Expand All @@ -103,7 +104,7 @@ func (l *propertyLoader) loadOneElement(codec *structCodec, structValue reflect.
// eg. for name "A.B.C.D", split off "A.B.C" and try to
// find a field in the codec with this name.
// Loop again with "A.B", etc.
for !ok {
for field == nil {
i := strings.LastIndex(parent, ".")
if i < 0 {
return "no such struct field"
Expand All @@ -112,22 +113,26 @@ func (l *propertyLoader) loadOneElement(codec *structCodec, structValue reflect.
return "field name cannot end with '.'"
}
parent, child = name[:i], name[i+1:]
decoder, ok = codec.fields[parent]
field = codec.Match(parent)
}

name = child
}

v = initField(structValue, decoder.path)
v = initField(structValue, field.Index)
if !v.IsValid() {
return "no such struct field"
}
if !v.CanSet() {
return "cannot set struct field"
}

if decoder.structCodec != nil {
codec = decoder.structCodec
var err error
if field.Type.Kind() == reflect.Struct {
codec, err = structCache.Fields(field.Type)
if err != nil {
return err.Error()
}
structValue = v
}

Expand All @@ -142,6 +147,12 @@ func (l *propertyLoader) loadOneElement(codec *structCodec, structValue reflect.
v.Set(reflect.Append(v, reflect.New(v.Type().Elem()).Elem()))
}
structValue = v.Index(sliceIndex)
if structValue.Type().Kind() == reflect.Struct {
codec, err = structCache.Fields(structValue.Type())
if err != nil {
return err.Error()
}
}
sliceOk = true
}
}
Expand Down Expand Up @@ -245,8 +256,9 @@ func setVal(v reflect.Value, p Property) string {

// if ent has a Key value and our struct has a Key field,
// load the Entity's Key value into the Key field on the struct.
if ent.Key != nil && pls.codec.keyField != -1 {
pls.v.Field(pls.codec.keyField).Set(reflect.ValueOf(ent.Key))
keyField := pls.codec.Match(keyFieldName)
if keyField != nil && ent.Key != nil {
pls.v.FieldByIndex(keyField.Index).Set(reflect.ValueOf(ent.Key))
}

err = pls.Load(ent.Properties)
Expand Down
Loading

0 comments on commit 1e032f3

Please sign in to comment.