Merge pull request #67 from ipfs/fix/mount-wrap

Fix subquery closing in mount, improve namespace.Query

.gx/lastpubver

-1.2.1: QmSiN66ybp5udnQnvhb6euiWiiQWdGvwMhAWa95cC1DTCV
+1.2.2: QmVSase1JP7cq9QkPT46oNwdp9pT6kBkG3oqS14y3QcZjG

mount/mount.go

@@ -126,9 +126,7 @@ func (d *Datastore) Query(q query.Query) (query.Results, error) {
 
 	// current itorator state
 	var res query.Results
-	var ds datastore.Datastore
 	var mount datastore.Key
-	var rest datastore.Key
 	i := 0
 
 	return query.ResultsFromIterator(q, query.Iterator{
@@ -137,19 +135,19 @@ func (d *Datastore) Query(q query.Query) (query.Results, error) {
 			var more bool
 
 			for try := true; try; try = len(dses) > i {
-				if ds == nil {
+				if res == nil {
 					if len(dses) <= i {
 						//This should not happen normally
 						return query.Result{}, false
 					}
 
-					ds = dses[i]
+					dst := dses[i]
 					mount = mounts[i]
-					rest = rests[i]
+					rest := rests[i]
 
 					q2 := q
 					q2.Prefix = rest.String()
-					r, err := ds.Query(q2)
+					r, err := dst.Query(q2)
 					if err != nil {
 						return query.Result{Error: err}, false
 					}
@@ -158,7 +156,12 @@ func (d *Datastore) Query(q query.Query) (query.Results, error) {
 
 				r, more = res.NextSync()
 				if !more {
-					ds = nil
+					err := res.Close()
+					if err != nil {
+						return query.Result{Error: err}, false
+					}
+					res = nil
+
 					i++
 					more = len(dses) > i
 				} else {
@@ -170,7 +173,7 @@ func (d *Datastore) Query(q query.Query) (query.Results, error) {
 			return r, more
 		},
 		Close: func() error {
-			if len(mounts) > i {
+			if len(mounts) > i && res != nil {
 				return res.Close()
 			}
 			return nil

mount/mount_test.go

 package mount_test
 
 import (
+	"errors"
 	"testing"
 
 	"github.com/ipfs/go-datastore"
@@ -238,6 +239,11 @@ func TestQuerySimple(t *testing.T) {
 	if !seen {
 		t.Errorf("did not see wanted key %q in %+v", myKey, entries)
 	}
+
+	err = res.Close()
+	if err != nil {
+		t.Errorf("result.Close failed %d", err)
+	}
 }
 
 func TestQueryCross(t *testing.T) {
@@ -292,6 +298,11 @@ func TestQueryCross(t *testing.T) {
 	if seen != 4 {
 		t.Errorf("expected to see 3 values, saw %d", seen)
 	}
+
+	err = res.Close()
+	if err != nil {
+		t.Errorf("result.Close failed %d", err)
+	}
 }
 
 func TestLookupPrio(t *testing.T) {
@@ -330,3 +341,33 @@ func TestLookupPrio(t *testing.T) {
 		t.Fatalf("wrong value: %v != %v", g, e)
 	}
 }
+
+type errQueryDS struct {
+	datastore.NullDatastore
+}
+
+func (d *errQueryDS) Query(q query.Query) (query.Results, error) {
+	return nil, errors.New("test error")
+}
+
+func TestErrQueryClose(t *testing.T) {
+	eqds := &errQueryDS{}
+	mds := datastore.NewMapDatastore()
+
+	m := mount.New([]mount.Mount{
+		{Prefix: datastore.NewKey("/"), Datastore: mds},
+		{Prefix: datastore.NewKey("/foo"), Datastore: eqds},
+	})
+
+	m.Put(datastore.NewKey("/baz"), "123")
+
+	qr, err := m.Query(query.Query{})
+	if err != nil {
+		t.Fatalf("Query error: %v", err)
+	}
+
+	e, ok := qr.NextSync()
+	if ok != false || e.Error == nil {
+		t.Errorf("Query was ok or q.Error was nil")
+	}
+}

namespace/namespace.go

@@ -55,7 +55,9 @@ type datastore struct {
 }
 
 // Query implements Query, inverting keys on the way back out.
+// This function assumes that child datastore.Query returns ordered results
 func (d *datastore) Query(q dsq.Query) (dsq.Results, error) {
+	q.Prefix = d.prefix.Child(ds.NewKey(q.Prefix)).String()
 	qr, err := d.raw.Query(q)
 	if err != nil {
 		return nil, err
@@ -73,7 +75,7 @@ func (d *datastore) Query(q dsq.Query) (dsq.Results, error) {
 				}
 				k := ds.RawKey(r.Entry.Key)
 				if !d.prefix.IsAncestorOf(k) {
-					continue
+					return dsq.Result{}, false
 				}
 
 				r.Entry.Key = d.Datastore.InvertKey(k).String()

namespace/namespace_test.go

@@ -78,6 +78,67 @@ func (ks *DSSuite) testBasic(c *C, prefix string) {
 	}
 }
 
+func (ks *DSSuite) TestQuery(c *C) {
+	mpds := ds.NewMapDatastore()
+	nsds := ns.Wrap(mpds, ds.NewKey("/foo"))
+
+	keys := strsToKeys([]string{
+		"abc/foo",
+		"bar/foo",
+		"foo/bar",
+		"foo/bar/baz",
+		"foo/baz/abc",
+		"xyz/foo",
+	})
+
+	for _, k := range keys {
+		err := mpds.Put(k, []byte(k.String()))
+		c.Check(err, Equals, nil)
+	}
+
+	qres, err := nsds.Query(dsq.Query{})
+	c.Check(err, Equals, nil)
+
+	expect := []dsq.Entry{
+		{Key: "/bar", Value: []byte("/foo/bar")},
+		{Key: "/bar/baz", Value: []byte("/foo/bar/baz")},
+		{Key: "/baz/abc", Value: []byte("/foo/baz/abc")},
+	}
+
+	results, err := qres.Rest()
+	c.Check(err, Equals, nil)
+	sort.Slice(results, func(i, j int) bool { return results[i].Key < results[j].Key })
+
+	for i, ent := range results {
+		c.Check(ent.Key, Equals, expect[i].Key)
+		entval, _ := ent.Value.([]byte)
+		expval, _ := expect[i].Value.([]byte)
+		c.Check(string(entval), Equals, string(expval))
+	}
+
+	err = qres.Close()
+	c.Check(err, Equals, nil)
+
+	qres, err = nsds.Query(dsq.Query{Prefix: "bar"})
+	c.Check(err, Equals, nil)
+
+	expect = []dsq.Entry{
+		{Key: "/bar", Value: []byte("/foo/bar")},
+		{Key: "/bar/baz", Value: []byte("/foo/bar/baz")},
+	}
+
+	results, err = qres.Rest()
+	c.Check(err, Equals, nil)
+	sort.Slice(results, func(i, j int) bool { return results[i].Key < results[j].Key })
+
+	for i, ent := range results {
+		c.Check(ent.Key, Equals, expect[i].Key)
+		entval, _ := ent.Value.([]byte)
+		expval, _ := expect[i].Value.([]byte)
+		c.Check(string(entval), Equals, string(expval))
+	}
+}
+
 func strsToKeys(strs []string) []ds.Key {
 	keys := make([]ds.Key, len(strs))
 	for i, s := range strs {

package.json

@@ -25,6 +25,6 @@
   "license": "MIT",
   "name": "go-datastore",
   "releaseCmd": "git commit -a -m \"gx publish $VERSION\"",
-  "version": "1.2.1"
+  "version": "1.2.2"
 }
 

syncmount/mount.go

@@ -133,9 +133,7 @@ func (d *Datastore) Query(q query.Query) (query.Results, error) {
 
 	// current itorator state
 	var res query.Results
-	var dst ds.Datastore
 	var mount ds.Key
-	var rest ds.Key
 	i := 0
 
 	return query.ResultsFromIterator(q, query.Iterator{
@@ -144,15 +142,15 @@ func (d *Datastore) Query(q query.Query) (query.Results, error) {
 			var more bool
 
 			for try := true; try; try = len(dses) > i {
-				if dst == nil {
+				if res == nil {
 					if len(dses) <= i {
 						//This should not happen normally
 						return query.Result{}, false
 					}
 
-					dst = dses[i]
+					dst := dses[i]
 					mount = mounts[i]
-					rest = rests[i]
+					rest := rests[i]
 
 					q2 := q
 					q2.Prefix = rest.String()
@@ -165,7 +163,12 @@ func (d *Datastore) Query(q query.Query) (query.Results, error) {
 
 				r, more = res.NextSync()
 				if !more {
-					dst = nil
+					err := res.Close()
+					if err != nil {
+						return query.Result{Error: err}, false
+					}
+					res = nil
+
 					i++
 					more = len(dses) > i
 				} else {
@@ -177,7 +180,7 @@ func (d *Datastore) Query(q query.Query) (query.Results, error) {
 			return r, more
 		},
 		Close: func() error {
-			if len(mounts) > i {
+			if len(mounts) > i && res != nil {
 				return res.Close()
 			}
 			return nil

syncmount/mount_test.go

 package syncmount_test
 
 import (
+	"errors"
 	"testing"
 
 	"github.com/ipfs/go-datastore"
@@ -238,6 +239,11 @@ func TestQuerySimple(t *testing.T) {
 	if !seen {
 		t.Errorf("did not see wanted key %q in %+v", myKey, entries)
 	}
+
+	err = res.Close()
+	if err != nil {
+		t.Errorf("result.Close failed %d", err)
+	}
 }
 
 func TestQueryCross(t *testing.T) {
@@ -292,6 +298,11 @@ func TestQueryCross(t *testing.T) {
 	if seen != 4 {
 		t.Errorf("expected to see 3 values, saw %d", seen)
 	}
+
+	err = res.Close()
+	if err != nil {
+		t.Errorf("result.Close failed %d", err)
+	}
 }
 
 func TestLookupPrio(t *testing.T) {
@@ -330,3 +341,33 @@ func TestLookupPrio(t *testing.T) {
 		t.Fatalf("wrong value: %v != %v", g, e)
 	}
 }
+
+type errQueryDS struct {
+	datastore.NullDatastore
+}
+
+func (d *errQueryDS) Query(q query.Query) (query.Results, error) {
+	return nil, errors.New("test error")
+}
+
+func TestErrQueryClose(t *testing.T) {
+	eqds := &errQueryDS{}
+	mds := datastore.NewMapDatastore()
+
+	m := mount.New([]mount.Mount{
+		{Prefix: datastore.NewKey("/"), Datastore: mds},
+		{Prefix: datastore.NewKey("/foo"), Datastore: eqds},
+	})
+
+	m.Put(datastore.NewKey("/baz"), "123")
+
+	qr, err := m.Query(query.Query{})
+	if err != nil {
+		t.Fatalf("Query error: %v", err)
+	}
+
+	e, ok := qr.NextSync()
+	if ok != false || e.Error == nil {
+		t.Errorf("Query was ok or q.Error was nil")
+	}
+}