Add ability to override just the metadata in a struct.
This is useful when the '$id' is actually multiple fields packed into a string,
but it's generally available for any metadata field.
R=dnj@chromium.org, estaab@chromium.org, maruel@chromium.org, tandrii@chromium.org, vadimsh@chromium.org
BUG=https://github.com/luci/gae/issues/15
Review URL: https://codereview.chromium.org/1401533007
diff --git a/service/datastore/pls.go b/service/datastore/pls.go
index 8661372..e3bb319 100644
--- a/service/datastore/pls.go
+++ b/service/datastore/pls.go
@@ -86,6 +86,13 @@
// // 'Lines' will serialized to the datastore in the field 'Lines'
// Lines []string
// }
+//
+// A pointer-to-struct may also implement MetaGetterSetter to provide more
+// sophistocated metadata values. Explicitly defined fields (as shown above)
+// always take precedence over fields manipulated by the MetaGetterSetter
+// methods. So if your GetMeta handles "kind", but you explicitly have a
+// $kind field, the $kind field will take precedence and your GetMeta
+// implementation will not be called for "kind".
func GetPLS(obj interface{}) PropertyLoadSaver {
v := reflect.ValueOf(obj)
if v.Kind() != reflect.Ptr || v.Elem().Kind() != reflect.Struct {
diff --git a/service/datastore/pls_impl.go b/service/datastore/pls_impl.go
index 067ed06..535b8d5 100644
--- a/service/datastore/pls_impl.go
+++ b/service/datastore/pls_impl.go
@@ -35,6 +35,7 @@
byName map[string]int
byIndex []structTag
hasSlice bool
+ mgs bool
problem error
}
@@ -43,6 +44,13 @@
c *structCodec
}
+func (p *structPLS) getMGS() MetaGetterSetter {
+ if !p.c.mgs {
+ return nil
+ }
+ return p.o.Addr().Interface().(MetaGetterSetter)
+}
+
var _ PropertyLoadSaver = (*structPLS)(nil)
// typeMismatchReason returns a string explaining why the property p could not
@@ -289,14 +297,23 @@
if err := p.Problem(); err != nil {
return nil, err
}
- idx, ok := p.c.byMeta[key]
- if !ok {
- if key == "kind" {
- return p.getDefaultKind(), nil
- }
- return nil, ErrMetaFieldUnset
+
+ if idx, ok := p.c.byMeta[key]; ok {
+ return p.getMetaFor(idx), nil
}
- return p.getMetaFor(idx), nil
+
+ if p.c.mgs {
+ ret, err := p.getMGS().GetMeta(key)
+ if err == nil {
+ return ret, err
+ } else if err != ErrMetaFieldUnset {
+ return nil, err
+ }
+ }
+ if key == "kind" {
+ return p.getDefaultKind(), nil
+ }
+ return nil, ErrMetaFieldUnset
}
func (p *structPLS) getMetaFor(idx int) interface{} {
@@ -315,7 +332,15 @@
}
func (p *structPLS) GetAllMeta() PropertyMap {
- ret := make(PropertyMap, len(p.c.byMeta)+1)
+ ret := PropertyMap(nil)
+ needKind := true
+ if p.c.mgs {
+ ret = p.getMGS().GetAllMeta()
+ _, haveKind := ret["$kind"]
+ needKind = !haveKind
+ } else {
+ ret = make(PropertyMap, len(p.c.byMeta)+1)
+ }
for k, idx := range p.c.byMeta {
val := p.getMetaFor(idx)
p := Property{}
@@ -324,8 +349,10 @@
}
ret["$"+k] = []Property{p}
}
- if _, ok := p.c.byMeta["kind"]; !ok {
- ret["$kind"] = []Property{MkPropertyNI(p.getDefaultKind())}
+ if needKind {
+ if _, ok := p.c.byMeta["kind"]; !ok {
+ ret["$kind"] = []Property{MkPropertyNI(p.getDefaultKind())}
+ }
}
return ret
}
@@ -340,6 +367,9 @@
}
idx, ok := p.c.byMeta[key]
if !ok {
+ if p.c.mgs {
+ return p.getMGS().SetMeta(key, val)
+ }
return ErrMetaFieldUnset
}
if !p.c.byIndex[idx].canSet {
@@ -418,6 +448,7 @@
byName: make(map[string]int, t.NumField()),
byMeta: make(map[string]int, t.NumField()),
problem: errRecursiveStruct, // we'll clear this later if it's not recursive
+ mgs: reflect.PtrTo(t).Implements(typeOfMGS),
}
defer func() {
// If the codec has a problem, free up the indexes
diff --git a/service/datastore/pls_test.go b/service/datastore/pls_test.go
index 3ec9867..7cb5a2b 100644
--- a/service/datastore/pls_test.go
+++ b/service/datastore/pls_test.go
@@ -537,6 +537,88 @@
K *Key
}
+type IDParser struct {
+ _kind string `gae:"$kind,CoolKind"`
+
+ // real $id is myParentName|myID
+ parent string `gae:"-"`
+ id int64 `gae:"-"`
+}
+
+var _ MetaGetterSetter = (*IDParser)(nil)
+
+func (i *IDParser) getFullID() string {
+ return fmt.Sprintf("%s|%d", i.parent, i.id)
+}
+
+func (i *IDParser) GetAllMeta() PropertyMap {
+ pm := PropertyMap{}
+ pm.SetMeta("id", i.getFullID())
+ return pm
+}
+
+func (i *IDParser) GetMeta(key string) (interface{}, error) {
+ if key == "id" {
+ return i.getFullID(), nil
+ }
+ return nil, ErrMetaFieldUnset
+}
+
+func (i *IDParser) GetMetaDefault(key string, dflt interface{}) interface{} {
+ return GetMetaDefaultImpl(i.GetMeta, key, dflt)
+}
+
+func (i *IDParser) SetMeta(key string, value interface{}) (err error) {
+ if key == "id" {
+ // let the panics flooowwww
+ vS := strings.SplitN(value.(string), "|", 2)
+ i.parent = vS[0]
+ i.id, err = strconv.ParseInt(vS[1], 10, 64)
+ return
+ }
+ return ErrMetaFieldUnset
+}
+
+type KindOverride struct {
+ ID int64 `gae:"$id"`
+
+ customKind string `gae:"-"`
+}
+
+var _ MetaGetterSetter = (*KindOverride)(nil)
+
+func (i *KindOverride) GetAllMeta() PropertyMap {
+ pm := PropertyMap{}
+ if i.customKind != "" {
+ pm.SetMeta("kind", i.customKind)
+ }
+ return pm
+}
+
+func (i *KindOverride) GetMeta(key string) (interface{}, error) {
+ if key == "kind" && i.customKind != "" {
+ return i.customKind, nil
+ }
+ return nil, ErrMetaFieldUnset
+}
+
+func (i *KindOverride) GetMetaDefault(key string, dflt interface{}) interface{} {
+ return GetMetaDefaultImpl(i.GetMeta, key, dflt)
+}
+
+func (i *KindOverride) SetMeta(key string, value interface{}) error {
+ if key == "kind" {
+ kind := value.(string)
+ if kind != "KindOverride" {
+ i.customKind = kind
+ } else {
+ i.customKind = ""
+ }
+ return nil
+ }
+ return ErrMetaFieldUnset
+}
+
type testCase struct {
desc string
src interface{}
@@ -1718,5 +1800,48 @@
pls := GetPLS(&BadDefault{})
So(pls.Problem().Error(), ShouldContainSubstring, "bad type")
})
+
+ Convey("MetaGetterSetter implementation (IDParser)", func() {
+ idp := &IDParser{parent: "moo", id: 100}
+ pls := GetPLS(idp)
+ So(pls.GetMetaDefault("id", ""), ShouldEqual, "moo|100")
+ So(pls.GetMetaDefault("kind", ""), ShouldEqual, "CoolKind")
+
+ So(pls.SetMeta("kind", "Something"), ShouldErrLike, "unexported field")
+ So(pls.SetMeta("id", "happy|27"), ShouldBeNil)
+
+ So(idp.parent, ShouldEqual, "happy")
+ So(idp.id, ShouldEqual, 27)
+
+ So(pls.GetAllMeta(), ShouldResemble, PropertyMap{
+ "$id": {MkPropertyNI("happy|27")},
+ "$kind": {MkPropertyNI("CoolKind")},
+ })
+ })
+
+ Convey("MetaGetterSetter implementation (KindOverride)", func() {
+ ko := &KindOverride{ID: 20}
+ pls := GetPLS(ko)
+ So(pls.GetMetaDefault("kind", ""), ShouldEqual, "KindOverride")
+
+ ko.customKind = "something"
+ So(pls.GetMetaDefault("kind", ""), ShouldEqual, "something")
+
+ So(pls.SetMeta("kind", "Nerp"), ShouldBeNil)
+ So(ko.customKind, ShouldEqual, "Nerp")
+
+ So(pls.SetMeta("kind", "KindOverride"), ShouldBeNil)
+ So(ko.customKind, ShouldEqual, "")
+
+ So(pls.GetAllMeta(), ShouldResemble, PropertyMap{
+ "$id": {MkPropertyNI(20)},
+ "$kind": {MkPropertyNI("KindOverride")},
+ })
+ ko.customKind = "wut"
+ So(pls.GetAllMeta(), ShouldResemble, PropertyMap{
+ "$id": {MkPropertyNI(20)},
+ "$kind": {MkPropertyNI("wut")},
+ })
+ })
})
}
diff --git a/service/datastore/properties.go b/service/datastore/properties.go
index 60d8b42..9746c67 100644
--- a/service/datastore/properties.go
+++ b/service/datastore/properties.go
@@ -680,15 +680,7 @@
// which was held by this PropertyLoadSaver.
Save(withMeta bool) (PropertyMap, error)
- MetaGetter
-
- // GetAllMeta returns a PropertyMap with all of the metadata in this
- // PropertyLoadSaver. If a metadata field has an error during serialization,
- // it is skipped.
- GetAllMeta() PropertyMap
-
- // SetMeta allows you to set the current value of the meta-keyed field.
- SetMeta(key string, val interface{}) error
+ MetaGetterSetter
// Problem indicates that this PLS has a fatal problem. Usually this is
// set when the underlying struct has recursion, invalid field types, nested
@@ -696,6 +688,30 @@
Problem() error
}
+// MetaGetterSetter is the subset of PropertyLoadSaver which pertains to
+// getting and saving metadata.
+//
+// A *struct may implement this interface to provide metadata which is
+// supplimental to the variety described by GetPLS. For example, this could be
+// used to implement a parsed-out $kind or $id.
+type MetaGetterSetter interface {
+ MetaGetter
+
+ // GetAllMeta returns a PropertyMap with all of the metadata in this
+ // MetaGetterSetter. If a metadata field has an error during serialization,
+ // it is skipped.
+ //
+ // If a *struct is implementing this, then it only needs to return the
+ // metadata fields which would be returned by its GetMeta implementation, and
+ // the `GetPLS` implementation will add any statically-defined metadata
+ // fields. So if GetMeta provides $id, but there's a simple tagged field for
+ // $kind, this method is only expected to return a PropertyMap with "$id".
+ GetAllMeta() PropertyMap
+
+ // SetMeta allows you to set the current value of the meta-keyed field.
+ SetMeta(key string, val interface{}) error
+}
+
// PropertyMap represents the contents of a datastore entity in a generic way.
// It maps from property name to a list of property values which correspond to
// that property name. It is the spiritual successor to PropertyList from the
diff --git a/service/datastore/reflect.go b/service/datastore/reflect.go
index d4b5b78..0ff5e83 100644
--- a/service/datastore/reflect.go
+++ b/service/datastore/reflect.go
@@ -23,4 +23,5 @@
typeOfString = reflect.TypeOf("")
typeOfTime = reflect.TypeOf(time.Time{})
typeOfToggle = reflect.TypeOf(Auto)
+ typeOfMGS = reflect.TypeOf((*MetaGetterSetter)(nil)).Elem()
)
