Fix error message from KeyForObj when passing an invalid struct.

service/datastore/datastore_test.go

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // adapted from github.com/golang/appengine/datastore
 
 package datastore
 
 import (
         "fmt"
@@ skipping 43 matching lines @@
                         break
                 }
         }
         return nil
 }
 
 func (f *fakeDatastore) PutMulti(keys []*Key, vals []PropertyMap, cb PutMultiCB) error {
         if keys[0].Kind() == "FailAll" {
                 return errors.New("PutMulti fail all")
         }
-        assertExtra := false
-        if _, err := vals[0].GetMeta("assertExtra"); err == nil {
-                assertExtra = true
-        }
+        _, assertExtra := vals[0].GetMeta("assertExtra")
         for i, k := range keys {
                 err := error(nil)
                 if k.Kind() == "Fail" {
                         err = errors.New("PutMulti fail")
                 } else {
                         So(vals[i]["Value"], ShouldResemble, []Property{MkProperty(i)})
                         if assertExtra {
                                 So(vals[i]["Extra"], ShouldResemble, []Property{MkProperty("whoa")})
                         }
                         if k.Incomplete() {
@@ skipping 84 matching lines @@
 func (f *FakePLS) Save(withMeta bool) (PropertyMap, error) {
         if f.failSave {
                 return nil, errors.New("FakePLS.Save")
         }
         ret := PropertyMap{
                 "Value": {MkProperty(f.Value)},
                 "Extra": {MkProperty("whoa")},
         }
         if withMeta {
                 id, _ := f.GetMeta("id")
-                So(ret.SetMeta("id", id), ShouldBeNil)
+                So(ret.SetMeta("id", id), ShouldBeTrue)
                 if f.Kind == "" {
-                        So(ret.SetMeta("kind", "FakePLS"), ShouldBeNil)
+                        So(ret.SetMeta("kind", "FakePLS"), ShouldBeTrue)
                 } else {
-                        So(ret.SetMeta("kind", f.Kind), ShouldBeNil)
+                        So(ret.SetMeta("kind", f.Kind), ShouldBeTrue)
                 }
-                So(ret.SetMeta("assertExtra", true), ShouldBeNil)
+                So(ret.SetMeta("assertExtra", true), ShouldBeTrue)
         }
         return ret, nil
 }
 
-func (f *FakePLS) GetMetaDefault(key string, dflt interface{}) interface{} {
-        return GetMetaDefaultImpl(f.GetMeta, key, dflt)
-}
-
-func (f *FakePLS) GetMeta(key string) (interface{}, error) {
+func (f *FakePLS) GetMeta(key string) (interface{}, bool) {
         if f.failGetMeta {
-                return nil, errors.New("FakePLS.GetMeta")
+                return nil, false
         }
         switch key {
         case "id":
                 if f.StringID != "" {
-                        return f.StringID, nil
+                        return f.StringID, true
                 }
-                return f.IntID, nil
+                return f.IntID, true
         case "kind":
                 if f.Kind == "" {
-                        return "FakePLS", nil
+                        return "FakePLS", true
                 }
-                return f.Kind, nil
+                return f.Kind, true
         }
-        return nil, ErrMetaFieldUnset
+        return nil, false
 }
 
 func (f *FakePLS) GetAllMeta() PropertyMap {
         ret := PropertyMap{}
-        if id, err := f.GetMeta("id"); err != nil {
-                So(ret.SetMeta("id", id), ShouldBeNil)
+        if id, ok := f.GetMeta("id"); !ok {
+                ret.SetMeta("id", id)
         }
-        if kind, err := f.GetMeta("kind"); err != nil {
-                So(ret.SetMeta("kind", kind), ShouldBeNil)
+        if kind, ok := f.GetMeta("kind"); !ok {
+                ret.SetMeta("kind", kind)
         }
         return ret
 }
 
-func (f *FakePLS) SetMeta(key string, val interface{}) error {
+func (f *FakePLS) SetMeta(key string, val interface{}) bool {
         if f.failSetMeta {
-                return errors.New("FakePL.SetMeta")
+                return false
         }
         if key == "id" {
                 switch x := val.(type) {
                 case int64:
                         f.IntID = x
                 case string:
                         f.StringID = x
                 }
-                return nil
+                return true
         }
         if key == "kind" {
                 f.Kind = val.(string)
-                return nil
+                return true
         }
-        return ErrMetaFieldUnset
+        return false
 }
 
 func (f *FakePLS) Problem() error {
         if f.failProblem {
                 return errors.New("FakePLS.Problem")
         }
         return nil
 }
 
 type MGSWithNoKind struct {
         S string
 }
 
-func (s *MGSWithNoKind) GetMetaDefault(key string, dflt interface{}) interface{} {
-        return GetMetaDefaultImpl(s.GetMeta, key, dflt)
-}
-
-func (s *MGSWithNoKind) GetMeta(key string) (interface{}, error) {
-        return nil, ErrMetaFieldUnset
+func (s *MGSWithNoKind) GetMeta(key string) (interface{}, bool) {
+        return nil, false
 }
 
 func (s *MGSWithNoKind) GetAllMeta() PropertyMap {
         return PropertyMap{}
 }
 
-func (s *MGSWithNoKind) SetMeta(key string, val interface{}) error {
-        return ErrMetaFieldUnset
+func (s *MGSWithNoKind) SetMeta(key string, val interface{}) bool {
+        return false
 }
 
 var _ MetaGetterSetter = (*MGSWithNoKind)(nil)
 
 func TestKeyForObj(t *testing.T) {
         t.Parallel()
 
         Convey("Test interface.KeyForObj", t, func() {
                 c := info.Set(context.Background(), fakeInfo{})
                 c = SetRawFactory(c, fakeDatastoreFactory)
@@ skipping 21 matching lines @@
                         })
 
                         Convey("struct containing $id and $parent", func() {
                                 So(ds.KeyForObj(&CommonStruct{ID: 4}).String(), ShouldEqual, `s~aid:ns:/CommonStruct,4`)
 
                                 So(ds.KeyForObj(&CommonStruct{ID: 4, Parent: k}).String(), ShouldEqual, `s~aid:ns:/Hello,"world"/CommonStruct,4`)
                         })
 
                         Convey("a propmap with $key", func() {
                                 pm := PropertyMap{}
-                                So(pm.SetMeta("key", k), ShouldBeNil)
+                                So(pm.SetMeta("key", k), ShouldBeTrue)
                                 So(ds.KeyForObj(pm).String(), ShouldEqual, `s~aid:ns:/Hello,"world"`)
                         })
 
                         Convey("a propmap with $id, $kind, $parent", func() {
                                 pm := PropertyMap{}
-                                So(pm.SetMeta("id", 100), ShouldBeNil)
-                                So(pm.SetMeta("kind", "Sup"), ShouldBeNil)
+                                So(pm.SetMeta("id", 100), ShouldBeTrue)
+                                So(pm.SetMeta("kind", "Sup"), ShouldBeTrue)
                                 So(ds.KeyForObj(pm).String(), ShouldEqual, `s~aid:ns:/Sup,100`)
 
-                                So(pm.SetMeta("parent", k), ShouldBeNil)
+                                So(pm.SetMeta("parent", k), ShouldBeTrue)
                                 So(ds.KeyForObj(pm).String(), ShouldEqual, `s~aid:ns:/Hello,"world"/Sup,100`)
                         })
 
                         Convey("a pls with $id, $parent", func() {
                                 pls := GetPLS(&CommonStruct{ID: 1})
                                 So(ds.KeyForObj(pls).String(), ShouldEqual, `s~aid:ns:/CommonStruct,1`)
 
-                                So(pls.SetMeta("parent", k), ShouldBeNil)
+                                So(pls.SetMeta("parent", k), ShouldBeTrue)
                                 So(ds.KeyForObj(pls).String(), ShouldEqual, `s~aid:ns:/Hello,"world"/CommonStruct,1`)
                         })
 
                         Convey("can see if things exist", func() {
                                 e, err := ds.Exists(k)
                                 So(err, ShouldBeNil)
                                 So(e, ShouldBeTrue)
 
                                 e, err = ds.Exists(ds.MakeKey("DNE", "nope"))
                                 So(err, ShouldBeNil)
                                 So(e, ShouldBeFalse)
 
                                 _, err = ds.Exists(ds.MakeKey("Fail", "boom"))
                                 So(err, ShouldErrLike, "GetMulti fail")
                         })
 
                 })
 
                 Convey("bad", func() {
                         Convey("a propmap without $kind", func() {
                                 pm := PropertyMap{}
-                                So(pm.SetMeta("id", 100), ShouldBeNil)
+                                So(pm.SetMeta("id", 100), ShouldBeTrue)
                                 So(func() { ds.KeyForObj(pm) }, ShouldPanic)
                         })
+
+                        Convey("a bad object", func() {
+                                type BadObj struct {
+                                        ID int64 `gae:"$id"`
+
+                                        NonSerializableField complex64
+                                }
+
+                                So(func() { ds.KeyForObjErr(&BadObj{ID: 1}) }, ShouldPanicLike,
+                                        `field "NonSerializableField" has invalid type: complex64`)
+                        })
                 })
         })
 }
 
 func TestPut(t *testing.T) {
         t.Parallel()
 
         Convey("Test Put/PutMulti", t, func() {
                 c := info.Set(context.Background(), fakeInfo{})
                 c = SetRawFactory(c, fakeDatastoreFactory)
                 ds := Get(c)
 
                 Convey("bad", func() {
                         Convey("static can't serialize", func() {
                                 bss := []badStruct{{}, {}}
-                                So(ds.PutMulti(bss).Error(), ShouldContainSubstring, "invalid PutMulti input")
+                                So(func() { ds.PutMulti(bss) }, ShouldPanicLike,
+                                        `field "Compy" has invalid type`)
                         })
 
                         Convey("static ptr can't serialize", func() {
                                 bss := []*badStruct{{}, {}}
-                                So(ds.PutMulti(bss).Error(), ShouldContainSubstring, "invalid PutMulti input")
+                                So(func() { ds.PutMulti(bss) }, ShouldPanicLike,
+                                        `field "Compy" has invalid type: complex64`)
                         })
 
                         Convey("static bad type (non-slice)", func() {
-                                So(ds.PutMulti(100).Error(), ShouldContainSubstring, "invalid PutMulti input")
+                                So(func() { ds.PutMulti(100) }, ShouldPanicLike,
+                                        "invalid argument type: expected slice, got int")
                         })
 
                         Convey("static bad type (slice of bad type)", func() {
-                                So(ds.PutMulti([]int{}).Error(), ShouldContainSubstring, "invalid PutMulti input")
+                                So(func() { ds.PutMulti([]int{}) }, ShouldPanicLike,
+                                        "invalid argument type: []int")
                         })
 
                         Convey("dynamic can't serialize", func() {
                                 fplss := []FakePLS{{failSave: true}, {}}
-                                So(ds.PutMulti(fplss).Error(), ShouldContainSubstring, "FakePLS.Save")
+                                So(ds.PutMulti(fplss), ShouldErrLike, "FakePLS.Save")
                         })
 
                         Convey("can't get keys", func() {
                                 fplss := []FakePLS{{failGetMeta: true}, {}}
-                                So(ds.PutMulti(fplss).Error(), ShouldContainSubstring, "unable to extract $kind")
+                                So(ds.PutMulti(fplss), ShouldErrLike, "unable to extract $kind")
                         })
 
                         Convey("get single error for RPC failure", func() {
                                 fplss := []FakePLS{{Kind: "FailAll"}, {}}
-                                So(ds.PutMulti(fplss).Error(), ShouldEqual, "PutMulti fail all")
+                                So(ds.PutMulti(fplss), ShouldErrLike, "PutMulti fail all")
                         })
 
                         Convey("get multi error for individual failures", func() {
                                 fplss := []FakePLS{{}, {Kind: "Fail"}}
                                 So(ds.PutMulti(fplss), ShouldResemble, errors.MultiError{nil, errors.New("PutMulti fail")})
                         })
 
                         Convey("put with non-modifyable type is an error", func() {
                                 cs := CommonStruct{}
-                                So(ds.Put(cs).Error(), ShouldContainSubstring, "invalid Put input type")
+                                So(ds.Put(cs), ShouldErrLike, "invalid Put input type")
                         })
 
                         Convey("struct with no $kind is an error", func() {
                                 s := MGSWithNoKind{}
-                                So(ds.Put(&s).Error(), ShouldContainSubstring, "unable to extract $kind")
+                                So(ds.Put(&s), ShouldErrLike, "unable to extract $kind")
                         })
                 })
 
                 Convey("ok", func() {
                         Convey("[]S", func() {
                                 css := make([]CommonStruct, 7)
                                 for i := range css {
                                         if i == 4 {
                                                 css[i].ID = 200
                                         }
@@ skipping 54 matching lines @@
                         })
 
                         Convey("[]P (map)", func() {
                                 pms := make([]PropertyMap, 7)
                                 for i := range pms {
                                         pms[i] = PropertyMap{
                                                 "$kind": {MkProperty("Pmap")},
                                                 "Value": {MkProperty(i)},
                                         }
                                         if i == 4 {
-                                                So(pms[i].SetMeta("id", int64(200)), ShouldBeNil)
+                                                So(pms[i].SetMeta("id", int64(200)), ShouldBeTrue)
                                         }
                                 }
                                 So(ds.PutMulti(pms), ShouldBeNil)
                                 for i, pm := range pms {
                                         expect := int64(i + 1)
                                         if i == 4 {
                                                 expect = 200
                                         }
                                         So(ds.KeyForObj(pm).String(), ShouldEqual, fmt.Sprintf("s~aid:ns:/Pmap,%d", expect))
                                 }
@@ skipping 18 matching lines @@
                         })
 
                         Convey("[]*P (map)", func() {
                                 pms := make([]*PropertyMap, 7)
                                 for i := range pms {
                                         pms[i] = &PropertyMap{
                                                 "$kind": {MkProperty("Pmap")},
                                                 "Value": {MkProperty(i)},
                                         }
                                         if i == 4 {
-                                                So(pms[i].SetMeta("id", int64(200)), ShouldBeNil)
+                                                So(pms[i].SetMeta("id", int64(200)), ShouldBeTrue)
                                         }
                                 }
                                 So(ds.PutMulti(pms), ShouldBeNil)
                                 for i, pm := range pms {
                                         expect := int64(i + 1)
                                         if i == 4 {
                                                 expect = 200
                                         }
                                         So(ds.KeyForObj(*pm).String(), ShouldEqual, fmt.Sprintf("s~aid:ns:/Pmap,%d", expect))
                                 }
@@ skipping 67 matching lines @@
 
         Convey("Test Get/GetMulti", t, func() {
                 c := info.Set(context.Background(), fakeInfo{})
                 c = SetRawFactory(c, fakeDatastoreFactory)
                 ds := Get(c)
                 So(ds, ShouldNotBeNil)
 
                 Convey("bad", func() {
                         Convey("static can't serialize", func() {
                                 toGet := []badStruct{{}, {}}
-                                So(ds.GetMulti(toGet).Error(), ShouldContainSubstring, "invalid GetMulti input")
+                                So(func() { ds.GetMulti(toGet) }, ShouldPanicLike,
+                                        `field "Compy" has invalid type: complex64`)
                         })
 
                         Convey("can't get keys", func() {
                                 fplss := []FakePLS{{failGetMeta: true}, {}}
-                                So(ds.GetMulti(fplss).Error(), ShouldContainSubstring, "unable to extract $kind")
+                                So(ds.GetMulti(fplss), ShouldErrLike, "unable to extract $kind")
                         })
 
                         Convey("get single error for RPC failure", func() {
                                 fplss := []FakePLS{
                                         {IntID: 1, Kind: "FailAll"},
                                         {IntID: 2},
                                 }
                                 So(ds.GetMulti(fplss).Error(), ShouldEqual, "GetMulti fail all")
                         })
 
                         Convey("get multi error for individual failures", func() {
                                 fplss := []FakePLS{{IntID: 1}, {IntID: 2, Kind: "Fail"}}
                                 So(ds.GetMulti(fplss), ShouldResemble, errors.MultiError{nil, errors.New("GetMulti fail")})
                         })
 
                         Convey("get with non-modifiable type is an error", func() {
                                 cs := CommonStruct{}
-                                So(ds.Get(cs).Error(), ShouldContainSubstring, "invalid Get input type")
+                                So(ds.Get(cs), ShouldErrLike, "invalid Get input type")
                         })
 
-                        Convey("failure to save metadata is an issue too", func() {
-                                cs := &FakePLS{failGetMeta: true}
-                                So(ds.Get(cs).Error(), ShouldContainSubstring, "unable to extract $kind")
+                        Convey("failure to save metadata is no problem though", func() {
+                                // It just won't save the key
+                                cs := &FakePLS{IntID: 10, failSetMeta: true}
+                                So(ds.Get(cs), ShouldBeNil)
                         })
                 })
 
                 Convey("ok", func() {
                         Convey("Get", func() {
                                 cs := &CommonStruct{ID: 1}
                                 So(ds.Get(cs), ShouldBeNil)
                                 So(cs.Value, ShouldEqual, 1)
                         })
 
@@ skipping 21 matching lines @@
         Convey("Test GetAll", t, func() {
                 c := info.Set(context.Background(), fakeInfo{})
                 c = SetRawFactory(c, fakeDatastoreFactory)
                 ds := Get(c)
                 So(ds, ShouldNotBeNil)
 
                 q := NewQuery("").Limit(5)
 
                 Convey("bad", func() {
                         Convey("nil target", func() {
-                                So(ds.GetAll(q, (*[]PropertyMap)(nil)).Error(), ShouldContainSubstring, "dst: <nil>")
+                                So(ds.GetAll(q, (*[]PropertyMap)(nil)), ShouldErrLike, "dst: <nil>")
                         })
 
                         Convey("bad type", func() {
                                 output := 100
-                                So(ds.GetAll(q, &output).Error(), ShouldContainSubstring, "invalid GetAll input type")
+                                So(func() { ds.GetAll(q, &output) }, ShouldPanicLike,
+                                        "invalid argument type: expected slice, got int")
                         })
 
                         Convey("bad type (non pointer)", func() {
-                                So(ds.GetAll(q, "moo").Error(), ShouldContainSubstring, "must have a ptr-to-slice")
+                                So(ds.GetAll(q, "moo"), ShouldErrLike, "must have a ptr-to-slice")
                         })
 
                         Convey("bad type (underspecified)", func() {
                                 output := []PropertyLoadSaver(nil)
-                                So(ds.GetAll(q, &output).Error(), ShouldContainSubstring, "invalid GetAll input type")
+                                So(ds.GetAll(q, &output), ShouldErrLike, "invalid GetAll input type")
                         })
                 })
 
                 Convey("ok", func() {
                         Convey("*[]S", func() {
                                 output := []CommonStruct(nil)
                                 So(ds.GetAll(q, &output), ShouldBeNil)
                                 So(len(output), ShouldEqual, 5)
                                 for i, o := range output {
                                         So(o.ID, ShouldEqual, i+1)
@@ skipping 20 matching lines @@
                                         So(o.IntID, ShouldEqual, i+1)
                                         So(o.Value, ShouldEqual, i)
                                 }
                         })
 
                         Convey("*[]P (map)", func() {
                                 output := []PropertyMap(nil)
                                 So(ds.GetAll(q, &output), ShouldBeNil)
                                 So(len(output), ShouldEqual, 5)
                                 for i, o := range output {
-                                        k, err := o.GetMeta("key")
-                                        So(err, ShouldBeNil)
+                                        k, ok := o.GetMeta("key")
+                                        So(ok, ShouldBeTrue)
                                         So(k.(*Key).IntID(), ShouldEqual, i+1)
                                         So(o["Value"][0].Value().(int64), ShouldEqual, i)
                                 }
                         })
 
                         Convey("*[]*P", func() {
                                 output := []*FakePLS(nil)
                                 So(ds.GetAll(q, &output), ShouldBeNil)
                                 So(len(output), ShouldEqual, 5)
                                 for i, o := range output {
                                         So(o.gotLoaded, ShouldBeTrue)
                                         So(o.IntID, ShouldEqual, i+1)
                                         So(o.Value, ShouldEqual, i)
                                 }
                         })
 
                         Convey("*[]*P (map)", func() {
                                 output := []*PropertyMap(nil)
                                 So(ds.GetAll(q, &output), ShouldBeNil)
                                 So(len(output), ShouldEqual, 5)
                                 for i, op := range output {
                                         o := *op
-                                        k, err := o.GetMeta("key")
-                                        So(err, ShouldBeNil)
+                                        k, ok := o.GetMeta("key")
+                                        So(ok, ShouldBeTrue)
                                         So(k.(*Key).IntID(), ShouldEqual, i+1)
                                         So(o["Value"][0].Value().(int64), ShouldEqual, i)
                                 }
                         })
 
                         Convey("*[]*Key", func() {
                                 output := []*Key(nil)
                                 So(ds.GetAll(q, &output), ShouldBeNil)
                                 So(len(output), ShouldEqual, 5)
                                 for i, k := range output {
@@ skipping 11 matching lines @@
         Convey("Test Run", t, func() {
                 c := info.Set(context.Background(), fakeInfo{})
                 c = SetRawFactory(c, fakeDatastoreFactory)
                 ds := Get(c)
                 So(ds, ShouldNotBeNil)
 
                 q := NewQuery("kind").Limit(5)
 
                 Convey("bad", func() {
                         assertBadTypePanics := func(cb interface{}) {
-                                defer func() {
-                                        err, _ := recover().(error)
-                                        So(err, ShouldNotBeNil)
-                                        So(err.Error(), ShouldContainSubstring,
-                                                "cb does not match the required callback signature")
-                                }()
-                                So(ds.Run(q, cb), ShouldBeNil)
+                                So(func() { ds.Run(q, cb) }, ShouldPanicLike,
+                                        "cb does not match the required callback signature")
                         }
 
                         Convey("not a function", func() {
                                 assertBadTypePanics("I am a potato")
                         })
 
                         Convey("bad proto type", func() {
-                                assertBadTypePanics(func(v int, _ CursorCB) bool {
+                                cb := func(v int, _ CursorCB) bool {
                                         panic("never here!")
-                                })
+                                }
+                                So(func() { ds.Run(q, cb) }, ShouldPanicLike,
+                                        "invalid argument type: int")
                         })
 
                         Convey("wrong # args", func() {
                                 assertBadTypePanics(func(v CommonStruct, _ CursorCB) {
                                         panic("never here!")
                                 })
                         })
 
                         Convey("wrong ret type", func() {
                                 assertBadTypePanics(func(v CommonStruct, _ CursorCB) error {
@@ skipping 46 matching lines @@
                                         }
                                         So(fpls.Value, ShouldEqual, i)
                                         i++
                                         return true
                                 }), ShouldBeNil)
                         })
 
                         Convey("*P (map)", func() {
                                 i := 0
                                 So(ds.Run(q, func(pm *PropertyMap, _ CursorCB) bool {
-                                        k, err := pm.GetMeta("key")
-                                        So(err, ShouldBeNil)
+                                        k, ok := pm.GetMeta("key")
+                                        So(ok, ShouldBeTrue)
                                         So(k.(*Key).IntID(), ShouldEqual, i+1)
                                         So((*pm)["Value"][0].Value(), ShouldEqual, i)
                                         i++
                                         return true
                                 }), ShouldBeNil)
                         })
 
                         Convey("S", func() {
                                 i := 0
                                 So(ds.Run(q, func(cs CommonStruct, _ CursorCB) bool {
@@ skipping 11 matching lines @@
                                         So(fpls.IntID, ShouldEqual, i+1)
                                         So(fpls.Value, ShouldEqual, i)
                                         i++
                                         return true
                                 }), ShouldBeNil)
                         })
 
                         Convey("P (map)", func() {
                                 i := 0
                                 So(ds.Run(q, func(pm PropertyMap, _ CursorCB) bool {
-                                        k, err := pm.GetMeta("key")
-                                        So(err, ShouldBeNil)
+                                        k, ok := pm.GetMeta("key")
+                                        So(ok, ShouldBeTrue)
                                         So(k.(*Key).IntID(), ShouldEqual, i+1)
                                         So(pm["Value"][0].Value(), ShouldEqual, i)
                                         i++
                                         return true
                                 }), ShouldBeNil)
                         })
 
                         Convey("Key", func() {
                                 i := 0
                                 So(ds.Run(q, func(k *Key, _ CursorCB) bool {
                                         So(k.IntID(), ShouldEqual, i+1)
                                         i++
                                         return true
                                 }), ShouldBeNil)
                         })
 
                 })
         })
 }