From 62f7481b1908d983bd2af2cee2803d8bd32a2b27 Mon Sep 17 00:00:00 2001
From: Yicheng Qin <qycqycqycqycqyc@gmail.com>
Date: Sat, 22 Aug 2015 16:39:29 -0700
Subject: [PATCH] storage: keyIndex.get returns err when key is tombstoned

Before this commit, it will return wrong create index, mod index.

It lets findGeneration return error when rev is at the gap of two
generations. This leads to the change of compact() code.
---
 storage/index_test.go     |  8 ++------
 storage/key_index.go      | 25 ++++++++++++++-----------
 storage/key_index_test.go | 10 +++++-----
 3 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/storage/index_test.go b/storage/index_test.go
index 88255b950..92f179d97 100644
--- a/storage/index_test.go
+++ b/storage/index_test.go
@@ -91,14 +91,10 @@ func TestIndexTombstone(t *testing.T) {
 	if err != nil {
 		t.Errorf("tombstone error = %v, want nil", err)
 	}
-	rev, _, _, err := index.Get([]byte("foo"), 7)
-	if err != nil {
+	_, _, _, err = index.Get([]byte("foo"), 7)
+	if err != ErrRevisionNotFound {
 		t.Errorf("get error = %v, want nil", err)
 	}
-	w := revision{main: 7}
-	if !reflect.DeepEqual(rev, w) {
-		t.Errorf("get revision = %+v, want %+v", rev, w)
-	}
 }
 
 func TestContinuousCompact(t *testing.T) {
diff --git a/storage/key_index.go b/storage/key_index.go
index 180b607c0..65f70269a 100644
--- a/storage/key_index.go
+++ b/storage/key_index.go
@@ -144,18 +144,14 @@ func (ki *keyIndex) compact(atRev int64, available map[revision]struct{}) {
 		return true
 	}
 
-	g := ki.findGeneration(atRev)
-	if g == nil {
-		return
-	}
-
-	i := 0
-	for i <= len(ki.generations)-1 {
-		wg := &ki.generations[i]
-		if wg == g {
+	i, g := 0, &ki.generations[0]
+	// find first generation includes atRev or created after atRev
+	for i < len(ki.generations)-1 {
+		if tomb := g.revs[len(g.revs)-1].main; tomb > atRev {
 			break
 		}
 		i++
+		g = &ki.generations[i]
 	}
 
 	if !g.isEmpty() {
@@ -180,9 +176,11 @@ func (ki *keyIndex) isEmpty() bool {
 }
 
 // findGeneartion finds out the generation of the keyIndex that the
-// given index belongs to.
+// given rev belongs to. If the given rev is at the gap of two generations,
+// which means that the key does not exist at the given rev, it returns nil.
 func (ki *keyIndex) findGeneration(rev int64) *generation {
-	cg := len(ki.generations) - 1
+	lastg := len(ki.generations) - 1
+	cg := lastg
 
 	for cg >= 0 {
 		if len(ki.generations[cg].revs) == 0 {
@@ -190,6 +188,11 @@ func (ki *keyIndex) findGeneration(rev int64) *generation {
 			continue
 		}
 		g := ki.generations[cg]
+		if cg != lastg {
+			if tomb := g.revs[len(g.revs)-1].main; tomb <= rev {
+				return nil
+			}
+		}
 		if g.revs[0].main <= rev {
 			return &ki.generations[cg]
 		}
diff --git a/storage/key_index_test.go b/storage/key_index_test.go
index d419d9328..0490865ce 100644
--- a/storage/key_index_test.go
+++ b/storage/key_index_test.go
@@ -21,19 +21,19 @@ func TestKeyIndexGet(t *testing.T) {
 		wrev int64
 		werr error
 	}{
-		{13, 12, nil},
-		{13, 12, nil},
+		{13, 0, ErrRevisionNotFound},
+		{13, 0, ErrRevisionNotFound},
 
 		// get on generation 2
-		{12, 12, nil},
+		{12, 0, ErrRevisionNotFound},
 		{11, 10, nil},
 		{10, 10, nil},
 		{9, 8, nil},
 		{8, 8, nil},
-		{7, 6, nil},
+		{7, 0, ErrRevisionNotFound},
 
 		// get on generation 1
-		{6, 6, nil},
+		{6, 0, ErrRevisionNotFound},
 		{5, 4, nil},
 		{4, 4, nil},
 	}