Refactor keys and queries in datastore service and implementation.

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"
         "testing"
 
         "github.com/luci/gae/service/info"
         "github.com/luci/luci-go/common/errors"
+        . "github.com/luci/luci-go/common/testing/assertions"
         . "github.com/smartystreets/goconvey/convey"
         "golang.org/x/net/context"
 )
 
 func fakeDatastoreFactory(c context.Context) RawInterface {
         i := info.Get(c)
         return &fakeDatastore{
                 aid: i.FullyQualifiedAppID(),
                 ns:  i.GetNamespace(),
         }
 }
 
-type fakeQuery struct {
-        Query
-
-        limit int
-
-        err          error
-        errSingle    error
-        errSingleIdx int
-}
-
-func (q *fakeQuery) KeysOnly() Query {
-        return q
-}
-
-func (q *fakeQuery) Limit(i int) Query {
-        q.limit = i
-        return q
-}
-
-func (q *fakeQuery) FailAll() Query {
-        q.err = errors.New("Query fail all")
-        return q
-}
-
-func (q *fakeQuery) Fail(i int) Query {
-        q.errSingleIdx = i
-        q.errSingle = errors.New("Query fail")
-        return q
-}
-
 type fakeDatastore struct {
         RawInterface
         aid string
         ns  string
 }
 
-func (f *fakeDatastore) NewKey(kind, stringID string, intID int64, parent Key) Key {
-        id := interface{}(stringID)
-        if stringID == "" {
-                id = intID
-        }
-        return mkKey(f.aid, f.ns, kind, id, parent)
+func (f *fakeDatastore) mkKey(elems ...interface{}) *Key {
+        return MakeKey(f.aid, f.ns, elems...)
 }
 
-func (f *fakeDatastore) NewQuery(string) Query {
-        return &fakeQuery{}
-}
+func (f *fakeDatastore) Run(fq *FinalizedQuery, cb RawRunCB) error {
+        lim, _ := fq.Limit()
 
-func (f *fakeDatastore) Run(q Query, cb RawRunCB) error {
-        rq := q.(*fakeQuery)
-        if rq.err != nil {
-                return rq.err
-        }
-        for i := 0; i < rq.limit; i++ {
-                if rq.errSingle != nil && i == rq.errSingleIdx {
-                        return rq.errSingle
-                } else {
-                        k := f.NewKey("Kind", "", int64(i+1), nil)
-                        if i == 10 {
-                                k = f.NewKey("Kind", "eleven", 0, nil)
+        for i := int32(0); i < lim; i++ {
+                if v, ok := fq.eqFilts["$err_single"]; ok {
+                        idx := fq.eqFilts["$err_single_idx"][0].Value().(int64)
+                        if idx == int64(i) {
+                                return errors.New(v[0].Value().(string))
                         }
-                        pm := PropertyMap{"Value": {MkProperty(i)}}
-                        if !cb(k, pm, nil) {
-                                break
-                        }
+                }
+                k := f.mkKey("Kind", i+1)
+                if i == 10 {
+                        k = f.mkKey("Kind", "eleven")
+                }
+                pm := PropertyMap{"Value": {MkProperty(i)}}
+                if !cb(k, pm, nil) {
+                        break
                 }
         }
         return nil
 }
 
-func (f *fakeDatastore) PutMulti(keys []Key, vals []PropertyMap, cb PutMultiCB) error {
-        if keys[0].Kind() == "FailAll" {
+func (f *fakeDatastore) PutMulti(keys []*Key, vals []PropertyMap, cb PutMultiCB) error {
+        if keys[0].Last().Kind == "FailAll" {
                 return errors.New("PutMulti fail all")
         }
         assertExtra := false
         if _, err := vals[0].GetMeta("assertExtra"); err == nil {
                 assertExtra = true
         }
         for i, k := range keys {
                 err := error(nil)
-                if k.Kind() == "Fail" {
+                if k.Last().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() {
-                                k = mkKey(k.AppID(), k.Namespace(), k.Kind(), int64(i+1), k.Parent())
+                                k = NewKey(k.AppID(), k.Namespace(), k.Last().Kind, "", int64(i+1), k.Parent())
                         }
                 }
                 cb(k, err)
         }
         return nil
 }
 
-func (f *fakeDatastore) GetMulti(keys []Key, _meta MultiMetaGetter, cb GetMultiCB) error {
-        if keys[0].Kind() == "FailAll" {
+func (f *fakeDatastore) GetMulti(keys []*Key, _meta MultiMetaGetter, cb GetMultiCB) error {
+        if keys[0].Last().Kind == "FailAll" {
                 return errors.New("GetMulti fail all")
         }
         for i, k := range keys {
-                if k.Kind() == "Fail" {
+                if k.Last().Kind == "Fail" {
                         cb(nil, errors.New("GetMulti fail"))
                 } else {
                         cb(PropertyMap{"Value": {MkProperty(i + 1)}}, nil)
                 }
         }
         return nil
 }
 
-func (f *fakeDatastore) DeleteMulti(keys []Key, cb DeleteMultiCB) error {
-        if keys[0].Kind() == "FailAll" {
+func (f *fakeDatastore) DeleteMulti(keys []*Key, cb DeleteMultiCB) error {
+        if keys[0].Last().Kind == "FailAll" {
                 return errors.New("DeleteMulti fail all")
         }
         for _, k := range keys {
-                if k.Kind() == "Fail" {
+                if k.Last().Kind == "Fail" {
                         cb(errors.New("DeleteMulti fail"))
                 } else {
                         cb(nil)
                 }
         }
         return nil
 }
 
 type badStruct struct {
         ID    int64     `gae:"$id"`
         Compy complex64 // bad type
 }
 
 type CommonStruct struct {
         ID     int64 `gae:"$id"`
-        Parent Key   `gae:"$parent"`
+        Parent *Key  `gae:"$parent"`
 
         Value int64
 }
 
 type permaBad struct {
         PropertyLoadSaver
 }
 
 func (f *permaBad) Load(pm PropertyMap) error {
         return errors.New("permaBad")
@@ skipping 97 matching lines @@
 }
 
 func TestKeyForObj(t *testing.T) {
         t.Parallel()
 
         Convey("Test interface.KeyForObj", t, func() {
                 c := info.Set(context.Background(), fakeInfo{})
                 c = SetRawFactory(c, fakeDatastoreFactory)
                 ds := Get(c)
 
-                k := ds.NewKey("Hello", "world", 0, nil)
+                k := ds.MakeKey("Hello", "world")
 
                 Convey("good", func() {
                         Convey("struct containing $key", func() {
                                 type keyStruct struct {
-                                        Key Key `gae:"$key"`
+                                        Key *Key `gae:"$key"`
                                 }
 
                                 ks := &keyStruct{k}
                                 So(ds.KeyForObj(ks), ShouldEqual, k)
                         })
 
                         Convey("struct containing default $id and $kind", func() {
                                 type idStruct struct {
                                         id  string `gae:"$id,wut"`
                                         knd string `gae:"$kind,SuperKind"`
                                 }
 
-                                So(ds.KeyForObj(&idStruct{}).String(), ShouldEqual, `/SuperKind,wut`)
+                                So(ds.KeyForObj(&idStruct{}).String(), ShouldEqual, `s~aid:ns:/SuperKind,"wut"`)

[iannucci, 2015/09/18 04:31:53]

oh, and I disambiguate between StringID and IntID too.

                         })
 
                         Convey("struct containing $id and $parent", func() {
-                                So(ds.KeyForObj(&CommonStruct{ID: 4}).String(), ShouldEqual, `/CommonStruct,4`)
+                                So(ds.KeyForObj(&CommonStruct{ID: 4}).String(), ShouldEqual, `s~aid:ns:/CommonStruct,4`)
 
-                                So(ds.KeyForObj(&CommonStruct{ID: 4, Parent: k}).String(), ShouldEqual, `/Hello,world/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{}
                                 pm.SetMeta("key", k)
-                                So(ds.KeyForObj(pm).String(), ShouldEqual, `/Hello,world`)
+                                So(ds.KeyForObj(pm).String(), ShouldEqual, `s~aid:ns:/Hello,"world"`)
                         })
 
                         Convey("a propmap with $id, $kind, $parent", func() {
                                 pm := PropertyMap{}
                                 pm.SetMeta("id", 100)
                                 pm.SetMeta("kind", "Sup")
-                                So(ds.KeyForObj(pm).String(), ShouldEqual, `/Sup,100`)
+                                So(ds.KeyForObj(pm).String(), ShouldEqual, `s~aid:ns:/Sup,100`)
 
                                 pm.SetMeta("parent", k)
-                                So(ds.KeyForObj(pm).String(), ShouldEqual, `/Hello,world/Sup,100`)
+                                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, `/CommonStruct,1`)
+                                So(ds.KeyForObj(pls).String(), ShouldEqual, `s~aid:ns:/CommonStruct,1`)
 
                                 pls.SetMeta("parent", k)
-                                So(ds.KeyForObj(pls).String(), ShouldEqual, `/Hello,world/CommonStruct,1`)
+                                So(ds.KeyForObj(pls).String(), ShouldEqual, `s~aid:ns:/Hello,"world"/CommonStruct,1`)
                         })
 
                 })
 
                 Convey("bad", func() {
                         Convey("a propmap without $kind", func() {
                                 pm := PropertyMap{}
                                 pm.SetMeta("id", 100)
                                 So(func() { ds.KeyForObj(pm) }, ShouldPanic)
                         })
@@ skipping 107 matching lines @@
                                 for i, fpls := range fplss {
                                         expect := int64(i + 1)
                                         if i == 4 {
                                                 expect = 200
                                         }
                                         So(fpls.IntID, ShouldEqual, expect)
                                 }
 
                                 pm := PropertyMap{"Value": {MkProperty(0)}, "$kind": {MkPropertyNI("Pmap")}}
                                 So(ds.Put(pm), ShouldBeNil)
-                                So(ds.KeyForObj(pm).IntID(), ShouldEqual, 1)
+                                So(ds.KeyForObj(pm).Last().IntID, ShouldEqual, 1)
                         })
 
                         Convey("[]P (map)", func() {
                                 pms := make([]PropertyMap, 7)
                                 for i := range pms {
                                         pms[i] = PropertyMap{
                                                 "$kind": {MkProperty("Pmap")},
                                                 "Value": {MkProperty(i)},
                                         }
                                         if i == 4 {
                                                 pms[i].SetMeta("id", int64(200))
                                         }
                                 }
                                 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("/Pmap,%d", expect))
+                                        So(ds.KeyForObj(pm).String(), ShouldEqual, fmt.Sprintf("s~aid:ns:/Pmap,%d", expect))
                                 }
                         })
 
                         Convey("[]*P", func() {
                                 fplss := make([]*FakePLS, 7)
                                 for i := range fplss {
                                         fplss[i] = &FakePLS{Value: int64(i)}
                                         if i == 4 {
                                                 fplss[i].IntID = int64(200)
                                         }
@@ skipping 18 matching lines @@
                                         if i == 4 {
                                                 pms[i].SetMeta("id", int64(200))
                                         }
                                 }
                                 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("/Pmap,%d", expect))
+                                        So(ds.KeyForObj(*pm).String(), ShouldEqual, fmt.Sprintf("s~aid:ns:/Pmap,%d", expect))
                                 }
                         })
 
                         Convey("[]I", func() {
                                 ifs := []interface{}{
                                         &CommonStruct{Value: 0},
                                         &FakePLS{Value: 1},
                                         PropertyMap{"Value": {MkProperty(2)}, "$kind": {MkPropertyNI("Pmap")}},
                                         &PropertyMap{"Value": {MkProperty(3)}, "$kind": {MkPropertyNI("Pmap")}},
                                 }
                                 So(ds.PutMulti(ifs), ShouldBeNil)
                                 for i := range ifs {
                                         switch i {
                                         case 0:
                                                 So(ifs[i].(*CommonStruct).ID, ShouldEqual, 1)
                                         case 1:
                                                 fpls := ifs[i].(*FakePLS)
                                                 So(fpls.IntID, ShouldEqual, 2)
                                         case 2:
-                                                So(ds.KeyForObj(ifs[i].(PropertyMap)).String(), ShouldEqual, "/Pmap,3")
+                                                So(ds.KeyForObj(ifs[i].(PropertyMap)).String(), ShouldEqual, "s~aid:ns:/Pmap,3")
                                         case 3:
-                                                So(ds.KeyForObj(*ifs[i].(*PropertyMap)).String(), ShouldEqual, "/Pmap,4")
+                                                So(ds.KeyForObj(*ifs[i].(*PropertyMap)).String(), ShouldEqual, "s~aid:ns:/Pmap,4")
                                         }
                                 }
                         })
 
                 })
 
         })
 }
 
 func TestDelete(t *testing.T) {
         t.Parallel()
 
         Convey("Test Delete/DeleteMulti", t, func() {
                 c := info.Set(context.Background(), fakeInfo{})
                 c = SetRawFactory(c, fakeDatastoreFactory)
                 ds := Get(c)
                 So(ds, ShouldNotBeNil)
 
                 Convey("bad", func() {
                         Convey("get single error for RPC failure", func() {
-                                keys := []Key{
-                                        mkKey("s~aid", "ns", "FailAll", 1, nil),
-                                        mkKey("s~aid", "ns", "Ok", 1, nil),
+                                keys := []*Key{
+                                        MakeKey("s~aid", "ns", "FailAll", 1),
+                                        MakeKey("s~aid", "ns", "Ok", 1),
                                 }
                                 So(ds.DeleteMulti(keys).Error(), ShouldEqual, "DeleteMulti fail all")
                         })
 
                         Convey("get multi error for individual failure", func() {
-                                keys := []Key{
-                                        ds.NewKey("Ok", "", 1, nil),
-                                        ds.NewKey("Fail", "", 2, nil),
+                                keys := []*Key{
+                                        ds.MakeKey("Ok", 1),
+                                        ds.MakeKey("Fail", 2),
                                 }
                                 So(ds.DeleteMulti(keys).Error(), ShouldEqual, "DeleteMulti fail")
                         })
 
                         Convey("get single error when deleting a single", func() {
-                                k := ds.NewKey("Fail", "", 1, nil)
+                                k := ds.MakeKey("Fail", 1)
                                 So(ds.Delete(k).Error(), ShouldEqual, "DeleteMulti fail")
                         })
                 })
 
         })
 }
 
 func TestGet(t *testing.T) {
         t.Parallel()
 
@@ skipping 40 matching lines @@
 
                 Convey("ok", func() {
                         Convey("Get", func() {
                                 cs := &CommonStruct{ID: 1}
                                 So(ds.Get(cs), ShouldBeNil)
                                 So(cs.Value, ShouldEqual, 1)
                         })
 
                         Convey("Raw access too", func() {
                                 rds := ds.Raw()
-                                keys := []Key{rds.NewKey("Kind", "", 1, nil)}
+                                keys := []*Key{ds.MakeKey("Kind", 1)}
                                 So(rds.GetMulti(keys, nil, func(pm PropertyMap, err error) {
                                         So(err, ShouldBeNil)
                                         So(pm["Value"][0].Value(), ShouldEqual, 1)
                                 }), ShouldBeNil)
                         })
                 })
 
         })
 }
 
 func TestGetAll(t *testing.T) {
         t.Parallel()
 
         Convey("Test GetAll", t, func() {
                 c := info.Set(context.Background(), fakeInfo{})
                 c = SetRawFactory(c, fakeDatastoreFactory)
                 ds := Get(c)
                 So(ds, ShouldNotBeNil)
 
-                q := ds.NewQuery("").Limit(5)
+                q := NewQuery("").Limit(5)
 
                 Convey("bad", func() {
                         Convey("nil target", func() {
                                 So(ds.GetAll(q, (*[]PropertyMap)(nil)).Error(), ShouldContainSubstring, "dst: <nil>")
                         })
 
                         Convey("bad type", func() {
                                 output := 100
                                 So(ds.GetAll(q, &output).Error(), ShouldContainSubstring, "invalid GetAll input type")
                         })
@@ skipping 40 matching lines @@
                                 }
                         })
 
                         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)
-                                        So(k.(Key).IntID(), ShouldEqual, i+1)
+                                        So(k.(*Key).Last().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)
-                                        So(k.(Key).IntID(), ShouldEqual, i+1)
+                                        So(k.(*Key).Last().IntID, ShouldEqual, i+1)
                                         So(o["Value"][0].Value().(int64), ShouldEqual, i)
                                 }
                         })
 
-                        Convey("*[]Key", func() {
-                                output := []Key(nil)
+                        Convey("*[]*Key", func() {
+                                output := []*Key(nil)
                                 So(ds.GetAll(q, &output), ShouldBeNil)
                                 So(len(output), ShouldEqual, 5)
                                 for i, k := range output {
-                                        So(k.IntID(), ShouldEqual, i+1)
+                                        So(k.Last().IntID, ShouldEqual, i+1)
                                 }
                         })
 
                 })
         })
 }
 
 func TestRun(t *testing.T) {
         t.Parallel()
 
         Convey("Test Run", t, func() {
                 c := info.Set(context.Background(), fakeInfo{})
                 c = SetRawFactory(c, fakeDatastoreFactory)
                 ds := Get(c)
                 So(ds, ShouldNotBeNil)
 
-                q := ds.NewQuery("").Limit(5)
+                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")
                                 }()
                                 ds.Run(q, cb)
@@ skipping 21 matching lines @@
                                 })
                         })
 
                         Convey("bad 2nd arg", func() {
                                 assertBadTypePanics(func(v CommonStruct, _ Cursor) bool {
                                         panic("never here!")
                                 })
                         })
 
                         Convey("early abort on error", func() {
-                                rq := q.(*fakeQuery).Fail(3)
+                                q = q.Eq("$err_single", "Query fail").Eq("$err_single_idx", 3)

[iannucci, 2015/09/18 04:31:53]

because the RawInterface no longer mints Query objects, I use this hack to
communicate failure modes to my crappy fake RawInterface implementation above.

                                 i := 0
-                                So(ds.Run(rq, func(c CommonStruct, _ CursorCB) bool {
+                                So(ds.Run(q, func(c CommonStruct, _ CursorCB) bool {
                                         i++
                                         return true
-                                }).Error(), ShouldEqual, "Query fail")
+                                }), ShouldErrLike, "Query fail")
                                 So(i, ShouldEqual, 3)
                         })
 
                         Convey("return error on serialization failure", func() {
                                 So(ds.Run(q, func(_ permaBad, _ CursorCB) bool {
                                         panic("never here")
                                 }).Error(), ShouldEqual, "permaBad")
                         })
                 })
 
@@ skipping 21 matching lines @@
                                         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)
-                                        So(k.(Key).IntID(), ShouldEqual, i+1)
+                                        So(k.(*Key).Last().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 {
                                         So(cs.ID, ShouldEqual, i+1)
@@ skipping 12 matching lines @@
                                         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)
-                                        So(k.(Key).IntID(), ShouldEqual, i+1)
+                                        So(k.(*Key).Last().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)
+                                So(ds.Run(q, func(k *Key, _ CursorCB) bool {
+                                        So(k.Last().IntID, ShouldEqual, i+1)
                                         i++
                                         return true
                                 }), ShouldBeNil)
                         })
 
                 })
         })
 }