store: clean up event.go, node.go and add tests

Updates IsCreated logic on event.go. Cleans up node.go
and adds tests to it.

store/event.go

@@ -49,12 +49,7 @@ func (e *Event) IsCreated() bool {
 	if e.Action == Create {
 		return true
 	}
-
-	if e.Action == Set && e.PrevNode == nil {
-		return true
-	}
-
-	return false
+	return e.Action == Set && e.PrevNode == nil
 }
 
 func (e *Event) Index() uint64 {

store/node.go

@@ -25,10 +25,10 @@ import (
 
 // explanations of Compare function result
 const (
-	CompareMatch         = 0
-	CompareIndexNotMatch = 1
-	CompareValueNotMatch = 2
-	CompareNotMatch      = 3
+	CompareMatch = iota
+	CompareIndexNotMatch
+	CompareValueNotMatch
+	CompareNotMatch
 )
 
 var Permanent time.Time
@@ -101,7 +101,7 @@ func (n *node) IsPermanent() bool {
 // If the node is a directory, the function will return true.
 // Otherwise the function will return false.
 func (n *node) IsDir() bool {
-	return !(n.Children == nil)
+	return n.Children != nil
 }
 
 // Read function gets the value of the node.
@@ -192,9 +192,7 @@ func (n *node) Add(child *node) *etcdErr.Error {
 
 	_, name := path.Split(child.Path)
 
-	_, ok := n.Children[name]
-
-	if ok {
+	if _, ok := n.Children[name]; ok {
 		return etcdErr.NewError(etcdErr.EcodeNodeExist, "", n.store.CurrentIndex)
 	}
 
@@ -205,20 +203,6 @@ func (n *node) Add(child *node) *etcdErr.Error {
 
 // Remove function remove the node.
 func (n *node) Remove(dir, recursive bool, callback func(path string)) *etcdErr.Error {
-
-	if n.IsDir() {
-		if !dir {
-			// cannot delete a directory without recursive set to true
-			return etcdErr.NewError(etcdErr.EcodeNotFile, n.Path, n.store.CurrentIndex)
-		}
-
-		if len(n.Children) != 0 && !recursive {
-			// cannot delete a directory if it is not empty and the operation
-			// is not recursive
-			return etcdErr.NewError(etcdErr.EcodeDirNotEmpty, n.Path, n.store.CurrentIndex)
-		}
-	}
-
 	if !n.IsDir() { // key-value pair
 		_, name := path.Split(n.Path)
 
@@ -238,6 +222,17 @@ func (n *node) Remove(dir, recursive bool, callback func(path string)) *etcdErr.
 		return nil
 	}
 
+	if !dir {
+		// cannot delete a directory without dir set to true
+		return etcdErr.NewError(etcdErr.EcodeNotFile, n.Path, n.store.CurrentIndex)
+	}
+
+	if len(n.Children) != 0 && !recursive {
+		// cannot delete a directory if it is not empty and the operation
+		// is not recursive
+		return etcdErr.NewError(etcdErr.EcodeDirNotEmpty, n.Path, n.store.CurrentIndex)
+	}
+
 	for _, child := range n.Children { // delete all children
 		child.Remove(true, true, callback)
 	}
@@ -254,7 +249,6 @@ func (n *node) Remove(dir, recursive bool, callback func(path string)) *etcdErr.
 		if !n.IsPermanent() {
 			n.store.ttlKeyHeap.remove(n)
 		}
-
 	}
 
 	return nil
@@ -314,28 +308,31 @@ func (n *node) Repr(recursive, sorted bool, clock clockwork.Clock) *NodeExtern {
 }
 
 func (n *node) UpdateTTL(expireTime time.Time) {
-
 	if !n.IsPermanent() {
 		if expireTime.IsZero() {
 			// from ttl to permanent
 			n.ExpireTime = expireTime
 			// remove from ttl heap
 			n.store.ttlKeyHeap.remove(n)
-		} else {
-			// update ttl
-			n.ExpireTime = expireTime
-			// update ttl heap
-			n.store.ttlKeyHeap.update(n)
+			return
 		}
 
-	} else {
-		if !expireTime.IsZero() {
-			// from permanent to ttl
-			n.ExpireTime = expireTime
-			// push into ttl heap
-			n.store.ttlKeyHeap.push(n)
-		}
+		// update ttl
+		n.ExpireTime = expireTime
+		// update ttl heap
+		n.store.ttlKeyHeap.update(n)
+		return
 	}
+
+	if expireTime.IsZero() {
+		return
+	}
+
+	// from permanent to ttl
+	n.ExpireTime = expireTime
+	// push into ttl heap
+	n.store.ttlKeyHeap.push(n)
+	return
 }
 
 // Compare function compares node index and value with provided ones.
@@ -379,7 +376,7 @@ func (n *node) Clone() *node {
 
 // recoverAndclean function help to do recovery.
 // Two things need to be done: 1. recovery structure; 2. delete expired nodes
-
+//
 // If the node is a directory, it will help recover children's parent pointer and recursively
 // call this function on its children.
 // We check the expire last since we need to recover the whole structure first and add all the
@@ -396,5 +393,4 @@ func (n *node) recoverAndclean() {
 	if !n.ExpireTime.IsZero() {
 		n.store.ttlKeyHeap.push(n)
 	}
-
 }

store/node_test.go

@@ -0,0 +1,247 @@
+// Copyright 2015 CoreOS, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package store
+
+import (
+	"testing"
+	"time"
+
+	"github.com/coreos/etcd/Godeps/_workspace/src/github.com/jonboulle/clockwork"
+)
+
+var (
+	key, val   = "foo", "bar"
+	val1, val2 = "bar1", "bar2"
+	expiration = time.Minute
+)
+
+func TestNewKVIs(t *testing.T) {
+	nd := newTestKV()
+
+	if nd.IsHidden() {
+		t.Errorf("nd.Hidden() = %v, want = false", nd.IsHidden())
+	}
+
+	if nd.IsPermanent() {
+		t.Errorf("nd.IsPermanent() = %v, want = false", nd.IsPermanent())
+	}
+
+	if nd.IsDir() {
+		t.Errorf("nd.IsDir() = %v, want = false", nd.IsDir())
+	}
+}
+
+func TestNewKVReadWriteCompare(t *testing.T) {
+	nd := newTestKV()
+
+	if v, err := nd.Read(); v != val || err != nil {
+		t.Errorf("value = %s and err = %v, want value = %s and err = nil", v, err, val)
+	}
+
+	if err := nd.Write(val1, nd.CreatedIndex+1); err != nil {
+		t.Errorf("nd.Write error = %v, want = nil", err)
+	} else {
+		if v, err := nd.Read(); v != val1 || err != nil {
+			t.Errorf("value = %s and err = %v, want value = %s and err = nil", v, err, val1)
+		}
+	}
+	if err := nd.Write(val2, nd.CreatedIndex+2); err != nil {
+		t.Errorf("nd.Write error = %v, want = nil", err)
+	} else {
+		if v, err := nd.Read(); v != val2 || err != nil {
+			t.Errorf("value = %s and err = %v, want value = %s and err = nil", v, err, val2)
+		}
+	}
+
+	if ok, which := nd.Compare(val2, 2); !ok || which != 0 {
+		t.Errorf("ok = %v and which = %d, want ok = true and which = 0", ok, which)
+	}
+}
+
+func TestNewKVExpiration(t *testing.T) {
+	nd := newTestKV()
+
+	if _, ttl := nd.expirationAndTTL(clockwork.NewFakeClock()); ttl > expiration.Nanoseconds() {
+		t.Errorf("ttl = %d, want %d < %d", ttl, ttl, expiration.Nanoseconds())
+	}
+
+	newExpiration := time.Hour
+	nd.UpdateTTL(time.Now().Add(newExpiration))
+	if _, ttl := nd.expirationAndTTL(clockwork.NewFakeClock()); ttl > newExpiration.Nanoseconds() {
+		t.Errorf("ttl = %d, want %d < %d", ttl, ttl, newExpiration.Nanoseconds())
+	}
+	if ns, err := nd.List(); ns != nil || err == nil {
+		t.Errorf("nodes = %v and err = %v, want nodes = nil and err != nil", ns, err)
+	}
+
+	en := nd.Repr(false, false, clockwork.NewFakeClock())
+	if en.Key != nd.Path {
+		t.Errorf("en.Key = %s, want = %s", en.Key, nd.Path)
+	}
+	if *(en.Value) != nd.Value {
+		t.Errorf("*(en.Key) = %s, want = %s", *(en.Value), nd.Value)
+	}
+}
+
+func TestNewKVListReprCompareClone(t *testing.T) {
+	nd := newTestKV()
+
+	if ns, err := nd.List(); ns != nil || err == nil {
+		t.Errorf("nodes = %v and err = %v, want nodes = nil and err != nil", ns, err)
+	}
+
+	en := nd.Repr(false, false, clockwork.NewFakeClock())
+	if en.Key != nd.Path {
+		t.Errorf("en.Key = %s, want = %s", en.Key, nd.Path)
+	}
+	if *(en.Value) != nd.Value {
+		t.Errorf("*(en.Key) = %s, want = %s", *(en.Value), nd.Value)
+	}
+
+	cn := nd.Clone()
+	if cn.Path != nd.Path {
+		t.Errorf("cn.Path = %s, want = %s", cn.Path, nd.Path)
+	}
+	if cn.Value != nd.Value {
+		t.Errorf("cn.Value = %s, want = %s", cn.Value, nd.Value)
+	}
+}
+
+func TestNewKVRemove(t *testing.T) {
+	nd := newTestKV()
+
+	if v, err := nd.Read(); v != val || err != nil {
+		t.Errorf("value = %s and err = %v, want value = %s and err = nil", v, err, val)
+	}
+
+	if err := nd.Write(val1, nd.CreatedIndex+1); err != nil {
+		t.Errorf("nd.Write error = %v, want = nil", err)
+	} else {
+		if v, err := nd.Read(); v != val1 || err != nil {
+			t.Errorf("value = %s and err = %v, want value = %s and err = nil", v, err, val1)
+		}
+	}
+	if err := nd.Write(val2, nd.CreatedIndex+2); err != nil {
+		t.Errorf("nd.Write error = %v, want = nil", err)
+	} else {
+		if v, err := nd.Read(); v != val2 || err != nil {
+			t.Errorf("value = %s and err = %v, want value = %s and err = nil", v, err, val2)
+		}
+	}
+
+	if err := nd.Remove(false, false, nil); err != nil {
+		t.Errorf("nd.Remove err = %v, want = nil", err)
+	} else {
+		// still readable
+		if v, err := nd.Read(); v != val2 || err != nil {
+			t.Errorf("value = %s and err = %v, want value = %s and err = nil", v, err, val2)
+		}
+		if len(nd.store.ttlKeyHeap.array) != 0 {
+			t.Errorf("len(nd.store.ttlKeyHeap.array) = %d, want = 0", len(nd.store.ttlKeyHeap.array))
+		}
+		if len(nd.store.ttlKeyHeap.keyMap) != 0 {
+			t.Errorf("len(nd.store.ttlKeyHeap.keyMap) = %d, want = 0", len(nd.store.ttlKeyHeap.keyMap))
+		}
+	}
+}
+
+func TestNewDirIs(t *testing.T) {
+	nd, _ := newTestDir()
+	if nd.IsHidden() {
+		t.Errorf("nd.Hidden() = %v, want = false", nd.IsHidden())
+	}
+
+	if nd.IsPermanent() {
+		t.Errorf("nd.IsPermanent() = %v, want = false", nd.IsPermanent())
+	}
+
+	if !nd.IsDir() {
+		t.Errorf("nd.IsDir() = %v, want = true", nd.IsDir())
+	}
+}
+
+func TestNewDirReadWriteListReprClone(t *testing.T) {
+	nd, _ := newTestDir()
+
+	if _, err := nd.Read(); err == nil {
+		t.Errorf("err = %v, want err != nil", err)
+	}
+
+	if err := nd.Write(val, nd.CreatedIndex+1); err == nil {
+		t.Errorf("err = %v, want err != nil", err)
+	}
+
+	if ns, err := nd.List(); ns == nil && err != nil {
+		t.Errorf("nodes = %v and err = %v, want nodes = nil and err == nil", ns, err)
+	}
+
+	en := nd.Repr(false, false, clockwork.NewFakeClock())
+	if en.Key != nd.Path {
+		t.Errorf("en.Key = %s, want = %s", en.Key, nd.Path)
+	}
+
+	cn := nd.Clone()
+	if cn.Path != nd.Path {
+		t.Errorf("cn.Path = %s, want = %s", cn.Path, nd.Path)
+	}
+}
+
+func TestNewDirExpirationTTL(t *testing.T) {
+	nd, _ := newTestDir()
+
+	if _, ttl := nd.expirationAndTTL(clockwork.NewFakeClock()); ttl > expiration.Nanoseconds() {
+		t.Errorf("ttl = %d, want %d < %d", ttl, ttl, expiration.Nanoseconds())
+	}
+
+	newExpiration := time.Hour
+	nd.UpdateTTL(time.Now().Add(newExpiration))
+	if _, ttl := nd.expirationAndTTL(clockwork.NewFakeClock()); ttl > newExpiration.Nanoseconds() {
+		t.Errorf("ttl = %d, want %d < %d", ttl, ttl, newExpiration.Nanoseconds())
+	}
+}
+
+func TestNewDirChild(t *testing.T) {
+	nd, child := newTestDir()
+
+	if err := nd.Add(child); err != nil {
+		t.Errorf("nd.Add(child) err = %v, want = nil", err)
+	} else {
+		if len(nd.Children) == 0 {
+			t.Errorf("len(nd.Children) = %d, want = 1", len(nd.Children))
+		}
+	}
+
+	if err := child.Remove(true, true, nil); err != nil {
+		t.Errorf("child.Remove err = %v, want = nil", err)
+	} else {
+		if len(nd.Children) != 0 {
+			t.Errorf("len(nd.Children) = %d, want = 0", len(nd.Children))
+		}
+	}
+}
+
+func newTestKV() *node {
+	expiration := time.Minute
+	nd := newKV(newStore(), key, val, 0, nil, time.Now().Add(expiration))
+	return nd
+}
+
+func newTestDir() (*node, *node) {
+	s := newStore()
+	nd := newDir(s, key, 0, nil, time.Now().Add(expiration))
+	cKey, cVal := "hello", "world"
+	child := newKV(s, cKey, cVal, 0, nd, time.Now().Add(expiration))
+	return nd, child
+}