From 13a8e948950460a26de6274ce037b3b0b7ed2d38 Mon Sep 17 00:00:00 2001
From: Euan Kemp <euank@euank.com>
Date: Wed, 25 Jul 2018 00:53:27 -0700
Subject: [PATCH] pkg/client: return error for unindexed attr search

Calling 'GetPermanodesWithAttr', as currently written, will return an
empty list for any unindexed parameters.

Rather than always returning an empty list if the user asks for a
unindexed attribute, error out.

Also add documentation of this potential foot-gun for users of the
client.

Change-Id: Iadc1b5a00aa709584affb89a1397e82aaffb692f
---
 pkg/client/client.go         |  4 ++++
 pkg/index/index.go           |  3 +++
 pkg/index/indextest/tests.go | 45 ++++++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+)

diff --git a/pkg/client/client.go b/pkg/client/client.go
index 679b1fda3..d2a8647da 100644
--- a/pkg/client/client.go
+++ b/pkg/client/client.go
@@ -716,6 +716,10 @@ func (c *Client) GetRecentPermanodes(ctx context.Context, req *search.RecentRequ
 	return res, nil
 }
 
+// GetPermanodesWithAttr searches for permanodes that match the given search request.
+// The Fuzzy option in the request must not be set, and the Attribute option
+// must be set.
+// Only indexed attributes may be queried.
 func (c *Client) GetPermanodesWithAttr(ctx context.Context, req *search.WithAttrRequest) (*search.WithAttrResponse, error) {
 	sr, err := c.SearchRoot()
 	if err != nil {
diff --git a/pkg/index/index.go b/pkg/index/index.go
index f160bb63c..e088c0cd4 100644
--- a/pkg/index/index.go
+++ b/pkg/index/index.go
@@ -1031,6 +1031,9 @@ func (x *Index) SearchPermanodesWithAttr(ctx context.Context, dest chan<- blob.R
 	if request.Attribute == "" {
 		return errors.New("index: missing Attribute in SearchPermanodesWithAttr")
 	}
+	if !IsIndexedAttribute(request.Attribute) {
+		return fmt.Errorf("SearchPermanodesWithAttr: called with a non-indexed attribute %q", request.Attribute)
+	}
 
 	keyId, err := x.KeyId(ctx, request.Signer)
 	if err == sorted.ErrNotFound {
diff --git a/pkg/index/indextest/tests.go b/pkg/index/indextest/tests.go
index 73876971a..2c048f7f8 100644
--- a/pkg/index/indextest/tests.go
+++ b/pkg/index/indextest/tests.go
@@ -317,6 +317,7 @@ func Index(t *testing.T, initIdx func() *index.Index) {
 	t.Logf("set attribute %q", rootClaim)
 
 	pnChild := id.NewPermanode()
+	id.SetAttribute(pnChild, "unindexed", "lost in time and space")
 	br3 := id.SetAttribute(pnChild, "tag", "bar")
 	br3Time := id.LastTime()
 	t.Logf("set attribute %q", br3)
@@ -536,6 +537,50 @@ func Index(t *testing.T, initIdx func() *index.Index) {
 		}
 	}
 
+	// SearchPermanodesWithAttr - match none with attr type "tag=nosuchtag"
+	{
+		ch := make(chan blob.Ref, 10)
+		req := &camtypes.PermanodeByAttrRequest{
+			Signer:    id.SignerBlobRef,
+			Attribute: "tag",
+			Query:     "nosuchtag",
+		}
+		err := id.Index.SearchPermanodesWithAttr(ctx, ch, req)
+		if err != nil {
+			t.Fatalf("SearchPermanodesWithAttr = %v", err)
+		}
+		var got []blob.Ref
+		for r := range ch {
+			got = append(got, r)
+		}
+		want := []blob.Ref{}
+		if len(got) != len(want) {
+			t.Errorf("SearchPermanodesWithAttr results differ.\n got: %q\nwant: %q",
+				got, want)
+		}
+	}
+	// SearchPermanodesWithAttr - error for unindexed attr
+	{
+		ch := make(chan blob.Ref, 10)
+		req := &camtypes.PermanodeByAttrRequest{
+			Signer:    id.SignerBlobRef,
+			Attribute: "unindexed",
+		}
+		err := id.Index.SearchPermanodesWithAttr(ctx, ch, req)
+		if err != nil {
+			t.Fatalf("SearchPermanodesWithAttr = %v", err)
+		}
+		var got []blob.Ref
+		for r := range ch {
+			got = append(got, r)
+		}
+		want := []blob.Ref{}
+		if len(got) != len(want) {
+			t.Errorf("SearchPermanodesWithAttr results differ.\n got: %q\nwant: %q",
+				got, want)
+		}
+	}
+
 	// Delete value "pony" of type "title" (which does not actually exist) for pn
 	br4 := id.DelAttribute(pn, "title", "pony")
 	br4Time := id.LastTime()