Merge pull request #433 from xiangli-cmu/prevNode

feat(node_extern.go) add prevNode field

server/v2/tests/delete_handler_test.go

@@ -26,7 +26,7 @@ func TestV2DeleteKey(t *testing.T) {
 		assert.Equal(t, resp.StatusCode, http.StatusOK)
 		body := tests.ReadBody(resp)
 		assert.Nil(t, err, "")
-		assert.Equal(t, string(body), `{"action":"delete","node":{"key":"/foo/bar","modifiedIndex":3,"createdIndex":2}}`, "")
+		assert.Equal(t, string(body), `{"action":"delete","node":{"key":"/foo/bar","modifiedIndex":3,"createdIndex":2},"prevNode":{"key":"/foo/bar","value":"XXX","modifiedIndex":2,"createdIndex":2}}`, "")
 	})
 }
 
@@ -48,7 +48,7 @@ func TestV2DeleteEmptyDirectory(t *testing.T) {
 		assert.Equal(t, resp.StatusCode, http.StatusOK)
 		body := tests.ReadBody(resp)
 		assert.Nil(t, err, "")
-		assert.Equal(t, string(body), `{"action":"delete","node":{"key":"/foo","dir":true,"modifiedIndex":3,"createdIndex":2}}`, "")
+		assert.Equal(t, string(body), `{"action":"delete","node":{"key":"/foo","dir":true,"modifiedIndex":3,"createdIndex":2},"prevNode":{"key":"/foo","dir":true,"modifiedIndex":2,"createdIndex":2}}`, "")
 	})
 }
 
@@ -70,7 +70,7 @@ func TestV2DeleteNonEmptyDirectory(t *testing.T) {
 		assert.Equal(t, resp.StatusCode, http.StatusOK)
 		body := tests.ReadBody(resp)
 		assert.Nil(t, err, "")
-		assert.Equal(t, string(body), `{"action":"delete","node":{"key":"/foo","dir":true,"modifiedIndex":3,"createdIndex":2}}`, "")
+		assert.Equal(t, string(body), `{"action":"delete","node":{"key":"/foo","dir":true,"modifiedIndex":3,"createdIndex":2},"prevNode":{"key":"/foo","dir":true,"modifiedIndex":2,"createdIndex":2}}`, "")
 	})
 }
 
@@ -87,7 +87,7 @@ func TestV2DeleteDirectoryRecursiveImpliesDir(t *testing.T) {
 		assert.Equal(t, resp.StatusCode, http.StatusOK)
 		body := tests.ReadBody(resp)
 		assert.Nil(t, err, "")
-		assert.Equal(t, string(body), `{"action":"delete","node":{"key":"/foo","dir":true,"modifiedIndex":3,"createdIndex":2}}`, "")
+		assert.Equal(t, string(body), `{"action":"delete","node":{"key":"/foo","dir":true,"modifiedIndex":3,"createdIndex":2},"prevNode":{"key":"/foo","dir":true,"modifiedIndex":2,"createdIndex":2}}`, "")
 	})
 }
 

store/event.go

@@ -12,8 +12,9 @@ const (
 )
 
 type Event struct {
-	Action string      `json:"action"`
-	Node   *NodeExtern `json:"node,omitempty"`
+	Action   string      `json:"action"`
+	Node     *NodeExtern `json:"node,omitempty"`
+	PrevNode *NodeExtern `json:"prevNode,omitempty"`
 }
 
 func newEvent(action string, key string, modifiedIndex, createdIndex uint64) *Event {
@@ -34,7 +35,7 @@ func (e *Event) IsCreated() bool {
 		return true
 	}
 
-	if e.Action == Set && e.Node.PrevValue == "" {
+	if e.Action == Set && e.PrevNode == nil {
 		return true
 	}
 
@@ -52,7 +53,7 @@ func (event *Event) Response(currentIndex uint64) interface{} {
 			Action:     event.Action,
 			Key:        event.Node.Key,
 			Value:      event.Node.Value,
-			PrevValue:  event.Node.PrevValue,
+			PrevValue:  event.PrevNode.Value,
 			Index:      event.Node.ModifiedIndex,
 			TTL:        event.Node.TTL,
 			Expiration: event.Node.Expiration,

store/node.go

@@ -231,9 +231,9 @@ func (n *node) Remove(dir, recursive bool, callback func(path string)) *etcdErr.
 	return nil
 }
 
-func (n *node) Repr(recurisive, sorted bool) NodeExtern {
+func (n *node) Repr(recurisive, sorted bool) *NodeExtern {
 	if n.IsDir() {
-		node := NodeExtern{
+		node := &NodeExtern{
 			Key:           n.Path,
 			Dir:           true,
 			ModifiedIndex: n.ModifiedIndex,
@@ -272,7 +272,7 @@ func (n *node) Repr(recurisive, sorted bool) NodeExtern {
 		return node
 	}
 
-	node := NodeExtern{
+	node := &NodeExtern{
 		Key:           n.Path,
 		Value:         n.Value,
 		ModifiedIndex: n.ModifiedIndex,

store/node_extern.go

@@ -10,7 +10,6 @@ import (
 // TTL is time to live in second
 type NodeExtern struct {
 	Key           string      `json:"key, omitempty"`
-	PrevValue     string      `json:"-"`
 	Value         string      `json:"value,omitempty"`
 	Dir           bool        `json:"dir,omitempty"`
 	Expiration    *time.Time  `json:"expiration,omitempty"`
@@ -20,7 +19,7 @@ type NodeExtern struct {
 	CreatedIndex  uint64      `json:"createdIndex,omitempty"`
 }
 
-type NodeExterns []NodeExtern
+type NodeExterns []*NodeExtern
 
 // interfaces for sorting
 func (ns NodeExterns) Len() int {

store/store.go

@@ -225,10 +225,9 @@ func (s *store) CompareAndSwap(nodePath string, prevValue string, prevIndex uint
 	s.CurrentIndex++
 
 	e := newEvent(CompareAndSwap, nodePath, s.CurrentIndex, n.CreatedIndex)
+	e.PrevNode = n.Repr(false, false)
 	eNode := e.Node
 
-	eNode.PrevValue = n.Value
-
 	// if test succeed, write the value
 	n.Write(value, s.CurrentIndex)
 	n.UpdateTTL(expireTime)
@@ -267,12 +266,11 @@ func (s *store) Delete(nodePath string, dir, recursive bool) (*Event, error) {
 
 	nextIndex := s.CurrentIndex + 1
 	e := newEvent(Delete, nodePath, nextIndex, n.CreatedIndex)
+	e.PrevNode = n.Repr(false, false)
 	eNode := e.Node
 
 	if n.IsDir() {
 		eNode.Dir = true
-	} else {
-		eNode.PrevValue = n.Value
 	}
 
 	callback := func(path string) { // notify function
@@ -326,6 +324,7 @@ func (s *store) CompareAndDelete(nodePath string, prevValue string, prevIndex ui
 	s.CurrentIndex++
 
 	e := newEvent(CompareAndDelete, nodePath, s.CurrentIndex, n.CreatedIndex)
+	e.PrevNode = n.Repr(false, false)
 
 	callback := func(path string) { // notify function
 		// notify the watchers with deleted set true
@@ -412,6 +411,7 @@ func (s *store) Update(nodePath string, newValue string, expireTime time.Time) (
 	}
 
 	e := newEvent(Update, nodePath, nextIndex, n.CreatedIndex)
+	e.PrevNode = n.Repr(false, false)
 	eNode := e.Node
 
 	if n.IsDir() && len(newValue) != 0 {
@@ -420,7 +420,6 @@ func (s *store) Update(nodePath string, newValue string, expireTime time.Time) (
 		return nil, etcdErr.NewError(etcdErr.EcodeNotFile, nodePath, currIndex)
 	}
 
-	eNode.PrevValue = n.Value
 	n.Write(newValue, nextIndex)
 	eNode.Value = newValue
 
@@ -482,7 +481,7 @@ func (s *store) internalCreate(nodePath string, dir bool, value string, unique,
 			if n.IsDir() {
 				return nil, etcdErr.NewError(etcdErr.EcodeNotFile, nodePath, currIndex)
 			}
-			eNode.PrevValue, _ = n.Read()
+			e.PrevNode = n.Repr(false, false)
 
 			n.Remove(false, false, nil)
 		} else {
@@ -557,6 +556,7 @@ func (s *store) DeleteExpiredKeys(cutoff time.Time) {
 
 		s.CurrentIndex++
 		e := newEvent(Expire, node.Path, s.CurrentIndex, node.CreatedIndex)
+		e.PrevNode = node.Repr(false, false)
 
 		callback := func(path string) { // notify function
 			// notify the watchers with deleted set true

store/store_test.go

@@ -93,7 +93,6 @@ func TestSet(t *testing.T) {
 	assert.Equal(t, e.Action, "set", "")
 	assert.Equal(t, e.Node.Key, "/foo", "")
 	assert.False(t, e.Node.Dir, "")
-	assert.Equal(t, e.Node.PrevValue, "", "")
 	assert.Equal(t, e.Node.Value, "", "")
 	assert.Nil(t, e.Node.Nodes, "")
 	assert.Nil(t, e.Node.Expiration, "")
@@ -106,12 +105,16 @@ func TestSet(t *testing.T) {
 	assert.Equal(t, e.Action, "set", "")
 	assert.Equal(t, e.Node.Key, "/foo", "")
 	assert.False(t, e.Node.Dir, "")
-	assert.Equal(t, e.Node.PrevValue, "", "")
 	assert.Equal(t, e.Node.Value, "bar", "")
 	assert.Nil(t, e.Node.Nodes, "")
 	assert.Nil(t, e.Node.Expiration, "")
 	assert.Equal(t, e.Node.TTL, 0, "")
 	assert.Equal(t, e.Node.ModifiedIndex, uint64(2), "")
+	// check prevNode
+	assert.NotNil(t, e.PrevNode, "")
+	assert.Equal(t, e.PrevNode.Key, "/foo", "")
+	assert.Equal(t, e.PrevNode.Value, "", "")
+	assert.Equal(t, e.PrevNode.ModifiedIndex, uint64(1), "")
 
 	// Set /dir as a directory
 	e, err = s.Set("/dir", true, "", Permanent)
@@ -119,7 +122,6 @@ func TestSet(t *testing.T) {
 	assert.Equal(t, e.Action, "set", "")
 	assert.Equal(t, e.Node.Key, "/dir", "")
 	assert.True(t, e.Node.Dir, "")
-	assert.Equal(t, e.Node.PrevValue, "", "")
 	assert.Equal(t, e.Node.Value, "", "")
 	assert.Nil(t, e.Node.Nodes, "")
 	assert.Nil(t, e.Node.Expiration, "")
@@ -136,7 +138,6 @@ func TestStoreCreateValue(t *testing.T) {
 	assert.Equal(t, e.Action, "create", "")
 	assert.Equal(t, e.Node.Key, "/foo", "")
 	assert.False(t, e.Node.Dir, "")
-	assert.Equal(t, e.Node.PrevValue, "", "")
 	assert.Equal(t, e.Node.Value, "bar", "")
 	assert.Nil(t, e.Node.Nodes, "")
 	assert.Nil(t, e.Node.Expiration, "")
@@ -149,7 +150,6 @@ func TestStoreCreateValue(t *testing.T) {
 	assert.Equal(t, e.Action, "create", "")
 	assert.Equal(t, e.Node.Key, "/empty", "")
 	assert.False(t, e.Node.Dir, "")
-	assert.Equal(t, e.Node.PrevValue, "", "")
 	assert.Equal(t, e.Node.Value, "", "")
 	assert.Nil(t, e.Node.Nodes, "")
 	assert.Nil(t, e.Node.Expiration, "")
@@ -195,10 +195,15 @@ func TestStoreUpdateValue(t *testing.T) {
 	assert.Equal(t, e.Action, "update", "")
 	assert.Equal(t, e.Node.Key, "/foo", "")
 	assert.False(t, e.Node.Dir, "")
-	assert.Equal(t, e.Node.PrevValue, "bar", "")
 	assert.Equal(t, e.Node.Value, "baz", "")
 	assert.Equal(t, e.Node.TTL, 0, "")
 	assert.Equal(t, e.Node.ModifiedIndex, uint64(2), "")
+	// check prevNode
+	assert.Equal(t, e.PrevNode.Key, "/foo", "")
+	assert.Equal(t, e.PrevNode.Value, "bar", "")
+	assert.Equal(t, e.PrevNode.TTL, 0, "")
+	assert.Equal(t, e.PrevNode.ModifiedIndex, uint64(1), "")
+
 	e, _ = s.Get("/foo", false, false)
 	assert.Equal(t, e.Node.Value, "baz", "")
 
@@ -208,10 +213,15 @@ func TestStoreUpdateValue(t *testing.T) {
 	assert.Equal(t, e.Action, "update", "")
 	assert.Equal(t, e.Node.Key, "/foo", "")
 	assert.False(t, e.Node.Dir, "")
-	assert.Equal(t, e.Node.PrevValue, "baz", "")
 	assert.Equal(t, e.Node.Value, "", "")
 	assert.Equal(t, e.Node.TTL, 0, "")
 	assert.Equal(t, e.Node.ModifiedIndex, uint64(3), "")
+	// check prevNode
+	assert.Equal(t, e.PrevNode.Key, "/foo", "")
+	assert.Equal(t, e.PrevNode.Value, "baz", "")
+	assert.Equal(t, e.PrevNode.TTL, 0, "")
+	assert.Equal(t, e.PrevNode.ModifiedIndex, uint64(2), "")
+
 	e, _ = s.Get("/foo", false, false)
 	assert.Equal(t, e.Node.Value, "", "")
 }
@@ -278,6 +288,10 @@ func TestStoreDeleteValue(t *testing.T) {
 	e, err := s.Delete("/foo", false, false)
 	assert.Nil(t, err, "")
 	assert.Equal(t, e.Action, "delete", "")
+	// check pervNode
+	assert.NotNil(t, e.PrevNode, "")
+	assert.Equal(t, e.PrevNode.Key, "/foo", "")
+	assert.Equal(t, e.PrevNode.Value, "bar", "")
 }
 
 // Ensure that the store can delete a directory if recursive is specified.
@@ -290,6 +304,10 @@ func TestStoreDeleteDiretory(t *testing.T) {
 	e, err := s.Delete("/foo", true, false)
 	assert.Nil(t, err, "")
 	assert.Equal(t, e.Action, "delete", "")
+	// check pervNode
+	assert.NotNil(t, e.PrevNode, "")
+	assert.Equal(t, e.PrevNode.Key, "/foo", "")
+	assert.Equal(t, e.PrevNode.Dir, true, "")
 
 	// create directory /foo and directory /foo/bar
 	s.Create("/foo/bar", true, "", false, Permanent)
@@ -346,6 +364,13 @@ func TestStoreCompareAndDeletePrevValue(t *testing.T) {
 	assert.Nil(t, err, "")
 	assert.Equal(t, e.Action, "compareAndDelete", "")
 	assert.Equal(t, e.Node.Key, "/foo", "")
+
+	// check pervNode
+	assert.NotNil(t, e.PrevNode, "")
+	assert.Equal(t, e.PrevNode.Key, "/foo", "")
+	assert.Equal(t, e.PrevNode.Value, "bar", "")
+	assert.Equal(t, e.PrevNode.ModifiedIndex, uint64(1), "")
+	assert.Equal(t, e.PrevNode.CreatedIndex, uint64(1), "")
 }
 
 func TestStoreCompareAndDeletePrevValueFailsIfNotMatch(t *testing.T) {
@@ -366,6 +391,12 @@ func TestStoreCompareAndDeletePrevIndex(t *testing.T) {
 	e, err := s.CompareAndDelete("/foo", "", 1)
 	assert.Nil(t, err, "")
 	assert.Equal(t, e.Action, "compareAndDelete", "")
+	// check pervNode
+	assert.NotNil(t, e.PrevNode, "")
+	assert.Equal(t, e.PrevNode.Key, "/foo", "")
+	assert.Equal(t, e.PrevNode.Value, "bar", "")
+	assert.Equal(t, e.PrevNode.ModifiedIndex, uint64(1), "")
+	assert.Equal(t, e.PrevNode.CreatedIndex, uint64(1), "")
 }
 
 func TestStoreCompareAndDeletePrevIndexFailsIfNotMatch(t *testing.T) {
@@ -398,8 +429,14 @@ func TestStoreCompareAndSwapPrevValue(t *testing.T) {
 	e, err := s.CompareAndSwap("/foo", "bar", 0, "baz", Permanent)
 	assert.Nil(t, err, "")
 	assert.Equal(t, e.Action, "compareAndSwap", "")
-	assert.Equal(t, e.Node.PrevValue, "bar", "")
 	assert.Equal(t, e.Node.Value, "baz", "")
+	// check pervNode
+	assert.NotNil(t, e.PrevNode, "")
+	assert.Equal(t, e.PrevNode.Key, "/foo", "")
+	assert.Equal(t, e.PrevNode.Value, "bar", "")
+	assert.Equal(t, e.PrevNode.ModifiedIndex, uint64(1), "")
+	assert.Equal(t, e.PrevNode.CreatedIndex, uint64(1), "")
+
 	e, _ = s.Get("/foo", false, false)
 	assert.Equal(t, e.Node.Value, "baz", "")
 }
@@ -424,8 +461,14 @@ func TestStoreCompareAndSwapPrevIndex(t *testing.T) {
 	e, err := s.CompareAndSwap("/foo", "", 1, "baz", Permanent)
 	assert.Nil(t, err, "")
 	assert.Equal(t, e.Action, "compareAndSwap", "")
-	assert.Equal(t, e.Node.PrevValue, "bar", "")
 	assert.Equal(t, e.Node.Value, "baz", "")
+	// check pervNode
+	assert.NotNil(t, e.PrevNode, "")
+	assert.Equal(t, e.PrevNode.Key, "/foo", "")
+	assert.Equal(t, e.PrevNode.Value, "bar", "")
+	assert.Equal(t, e.PrevNode.ModifiedIndex, uint64(1), "")
+	assert.Equal(t, e.PrevNode.CreatedIndex, uint64(1), "")
+
 	e, _ = s.Get("/foo", false, false)
 	assert.Equal(t, e.Node.Value, "baz", "")
 }