From 702cf1cc36f0a8b66ccce7e57f2e4977074e67b0 Mon Sep 17 00:00:00 2001 From: rick Date: Sat, 30 Nov 2013 10:02:03 -0700 Subject: [PATCH 01/12] teach store.Store about CompareAndDelete --- store/store.go | 55 +++++++++++++++++++++++++++++++++++++++++++-- store/store_test.go | 40 +++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/store/store.go b/store/store.go index 80f9622af..39ee3b136 100644 --- a/store/store.go +++ b/store/store.go @@ -260,7 +260,7 @@ func (s *store) Delete(nodePath string, recursive bool) (*Event, error) { } callback := func(path string) { // notify function - // notify the watchers with delted set true + // notify the watchers with deleted set true s.WatcherHub.notifyWatchers(e, path, true) } @@ -280,6 +280,58 @@ func (s *store) Delete(nodePath string, recursive bool) (*Event, error) { return e, nil } +func (s *store) CompareAndDelete(nodePath string, prevValue string, prevIndex uint64, +) (*Event, error) { + + nodePath = path.Clean(path.Join("/", nodePath)) + + s.worldLock.Lock() + defer s.worldLock.Unlock() + + n, err := s.internalGet(nodePath) + + if err != nil { // if the node does not exist, return error + s.Stats.Inc(DeleteFail) + return nil, err + } + + if n.IsDir() { // can only test and set file + s.Stats.Inc(DeleteFail) + return nil, etcdErr.NewError(etcdErr.EcodeNotFile, nodePath, s.CurrentIndex) + } + + // If both of the prevValue and prevIndex are given, we will test both of them. + // Command will be executed, only if both of the tests are successful. + if (prevValue == "" || n.Value == prevValue) && (prevIndex == 0 || n.ModifiedIndex == prevIndex) { + + e := newEvent(Delete, nodePath, s.CurrentIndex+1) + e.PrevValue = n.Value + + callback := func(path string) { // notify function + // notify the watchers with deleted set true + s.WatcherHub.notifyWatchers(e, path, true) + } + + err = n.Remove(false, callback) + + if err != nil { + s.Stats.Inc(DeleteFail) + return nil, err + } + + // update etcd index + s.CurrentIndex++ + + s.WatcherHub.notify(e) + s.Stats.Inc(DeleteSuccess) + return e, nil + } + + cause := fmt.Sprintf("[%v != %v] [%v != %v]", prevValue, n.Value, prevIndex, n.ModifiedIndex) + s.Stats.Inc(DeleteFail) + return nil, etcdErr.NewError(etcdErr.EcodeTestFailed, cause, s.CurrentIndex) +} + func (s *store) Watch(prefix string, recursive bool, sinceIndex uint64) (<-chan *Event, error) { prefix = path.Clean(path.Join("/", prefix)) @@ -396,7 +448,6 @@ func (s *store) internalCreate(nodePath string, value string, unique bool, repla expireTime = Permanent } - dir, newNodeName := path.Split(nodePath) // walk through the nodePath, create dirs and get the last directory node diff --git a/store/store_test.go b/store/store_test.go index 29b2986b9..8699e1e69 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -204,6 +204,46 @@ func TestStoreDeleteValue(t *testing.T) { assert.Equal(t, e.Action, "delete", "") } +func TestStoreCompareAndDeletePrevValue(t *testing.T) { + s := newStore() + s.Create("/foo", "bar", false, Permanent) + e, err := s.CompareAndDelete("/foo", "bar", 0) + assert.Nil(t, err, "") + assert.Equal(t, e.Action, "delete", "") +} + +func TestStoreCompareAndDeletePrevValueFailsIfNotMatch(t *testing.T) { + s := newStore() + s.Create("/foo", "bar", false, Permanent) + e, _err := s.CompareAndDelete("/foo", "baz", 0) + err := _err.(*etcdErr.Error) + assert.Equal(t, err.ErrorCode, etcdErr.EcodeTestFailed, "") + assert.Equal(t, err.Message, "Test Failed", "") + assert.Nil(t, e, "") + e, _ = s.Get("/foo", false, false) + assert.Equal(t, e.Value, "bar", "") +} + +func TestStoreCompareAndDeletePrevIndex(t *testing.T) { + s := newStore() + s.Create("/foo", "bar", false, Permanent) + e, err := s.CompareAndDelete("/foo", "", 1) + assert.Nil(t, err, "") + assert.Equal(t, e.Action, "delete", "") +} + +func TestStoreCompareAndDeletePrevIndexFailsIfNotMatch(t *testing.T) { + s := newStore() + s.Create("/foo", "bar", false, Permanent) + e, _err := s.CompareAndDelete("/foo", "baz", 100) + err := _err.(*etcdErr.Error) + assert.Equal(t, err.ErrorCode, etcdErr.EcodeTestFailed, "") + assert.Equal(t, err.Message, "Test Failed", "") + assert.Nil(t, e, "") + e, _ = s.Get("/foo", false, false) + assert.Equal(t, e.Value, "bar", "") +} + // Ensure that the store can delete a directory if recursive is specified. func TestStoreDeleteDiretory(t *testing.T) { s := newStore() From 5b739f6166c5f111f67d5272cd3621a2da7626c0 Mon Sep 17 00:00:00 2001 From: rick Date: Sat, 30 Nov 2013 10:05:48 -0700 Subject: [PATCH 02/12] track CompareAndDelete stats --- store/stats.go | 14 +++++++++++++- store/store.go | 10 +++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/store/stats.go b/store/stats.go index 2eb271d73..5f3cd62a3 100644 --- a/store/stats.go +++ b/store/stats.go @@ -35,6 +35,8 @@ const ( GetSuccess GetFail ExpireCount + CompareAndDeleteSuccess + CompareAndDeleteFail ) type Stats struct { @@ -63,6 +65,10 @@ type Stats struct { CompareAndSwapSuccess uint64 `json:"compareAndSwapSuccess"` CompareAndSwapFail uint64 `json:"compareAndSwapFail"` + // Number of compareAndDelete requests + CompareAndDeleteSuccess uint64 `json:"compareAndDeleteSuccess"` + CompareAndDeleteFail uint64 `json:"compareAndDeleteFail"` + ExpireCount uint64 `json:"expireCount"` Watchers uint64 `json:"watchers"` @@ -76,7 +82,8 @@ func newStats() *Stats { func (s *Stats) clone() *Stats { return &Stats{s.GetSuccess, s.GetFail, s.SetSuccess, s.SetFail, s.DeleteSuccess, s.DeleteFail, s.UpdateSuccess, s.UpdateFail, s.CreateSuccess, - s.CreateFail, s.CompareAndSwapSuccess, s.CompareAndSwapFail, s.Watchers, s.ExpireCount} + s.CreateFail, s.CompareAndSwapSuccess, s.CompareAndSwapFail, + s.CompareAndDeleteSuccess, s.CompareAndDeleteFail, s.Watchers, s.ExpireCount} } // Status() return the statistics info of etcd storage its recent start @@ -93,6 +100,7 @@ func (s *Stats) TotalTranscations() uint64 { return s.SetSuccess + s.SetFail + s.DeleteSuccess + s.DeleteFail + s.CompareAndSwapSuccess + s.CompareAndSwapFail + + s.CompareAndDeleteSuccess + s.CompareAndDeleteFail + s.UpdateSuccess + s.UpdateFail } @@ -122,6 +130,10 @@ func (s *Stats) Inc(field int) { atomic.AddUint64(&s.CompareAndSwapSuccess, 1) case CompareAndSwapFail: atomic.AddUint64(&s.CompareAndSwapFail, 1) + case CompareAndDeleteSuccess: + atomic.AddUint64(&s.CompareAndDeleteSuccess, 1) + case CompareAndDeleteFail: + atomic.AddUint64(&s.CompareAndDeleteFail, 1) case ExpireCount: atomic.AddUint64(&s.ExpireCount, 1) } diff --git a/store/store.go b/store/store.go index 39ee3b136..08a9e1702 100644 --- a/store/store.go +++ b/store/store.go @@ -291,12 +291,12 @@ func (s *store) CompareAndDelete(nodePath string, prevValue string, prevIndex ui n, err := s.internalGet(nodePath) if err != nil { // if the node does not exist, return error - s.Stats.Inc(DeleteFail) + s.Stats.Inc(CompareAndDeleteFail) return nil, err } if n.IsDir() { // can only test and set file - s.Stats.Inc(DeleteFail) + s.Stats.Inc(CompareAndDeleteFail) return nil, etcdErr.NewError(etcdErr.EcodeNotFile, nodePath, s.CurrentIndex) } @@ -315,7 +315,7 @@ func (s *store) CompareAndDelete(nodePath string, prevValue string, prevIndex ui err = n.Remove(false, callback) if err != nil { - s.Stats.Inc(DeleteFail) + s.Stats.Inc(CompareAndDeleteFail) return nil, err } @@ -323,12 +323,12 @@ func (s *store) CompareAndDelete(nodePath string, prevValue string, prevIndex ui s.CurrentIndex++ s.WatcherHub.notify(e) - s.Stats.Inc(DeleteSuccess) + s.Stats.Inc(CompareAndDeleteSuccess) return e, nil } cause := fmt.Sprintf("[%v != %v] [%v != %v]", prevValue, n.Value, prevIndex, n.ModifiedIndex) - s.Stats.Inc(DeleteFail) + s.Stats.Inc(CompareAndDeleteFail) return nil, etcdErr.NewError(etcdErr.EcodeTestFailed, cause, s.CurrentIndex) } From 90a8f56c963398bcb05141a40b444370adcc7796 Mon Sep 17 00:00:00 2001 From: rick Date: Sat, 30 Nov 2013 10:08:25 -0700 Subject: [PATCH 03/12] add compareAndDelete event action --- store/event.go | 1 + store/store.go | 2 +- store/store_test.go | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/store/event.go b/store/event.go index f3d607e0b..f7a846717 100644 --- a/store/event.go +++ b/store/event.go @@ -11,6 +11,7 @@ const ( Update = "update" Delete = "delete" CompareAndSwap = "compareAndSwap" + CompareAndDelete = "compareAndDelete" Expire = "expire" ) diff --git a/store/store.go b/store/store.go index 08a9e1702..eb88f5c82 100644 --- a/store/store.go +++ b/store/store.go @@ -304,7 +304,7 @@ func (s *store) CompareAndDelete(nodePath string, prevValue string, prevIndex ui // Command will be executed, only if both of the tests are successful. if (prevValue == "" || n.Value == prevValue) && (prevIndex == 0 || n.ModifiedIndex == prevIndex) { - e := newEvent(Delete, nodePath, s.CurrentIndex+1) + e := newEvent(CompareAndDelete, nodePath, s.CurrentIndex+1) e.PrevValue = n.Value callback := func(path string) { // notify function diff --git a/store/store_test.go b/store/store_test.go index 8699e1e69..e70cc5419 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -209,7 +209,7 @@ func TestStoreCompareAndDeletePrevValue(t *testing.T) { s.Create("/foo", "bar", false, Permanent) e, err := s.CompareAndDelete("/foo", "bar", 0) assert.Nil(t, err, "") - assert.Equal(t, e.Action, "delete", "") + assert.Equal(t, e.Action, "compareAndDelete", "") } func TestStoreCompareAndDeletePrevValueFailsIfNotMatch(t *testing.T) { @@ -229,7 +229,7 @@ func TestStoreCompareAndDeletePrevIndex(t *testing.T) { s.Create("/foo", "bar", false, Permanent) e, err := s.CompareAndDelete("/foo", "", 1) assert.Nil(t, err, "") - assert.Equal(t, e.Action, "delete", "") + assert.Equal(t, e.Action, "compareAndDelete", "") } func TestStoreCompareAndDeletePrevIndexFailsIfNotMatch(t *testing.T) { From 171072c736f587579c3a995a0d72ba9edb8a2d70 Mon Sep 17 00:00:00 2001 From: rick Date: Sat, 30 Nov 2013 16:24:23 -0700 Subject: [PATCH 04/12] add the CompareAndDelete command --- store/command_factory.go | 1 + store/event.go | 14 +++++----- store/store.go | 1 + store/v2/command_factory.go | 9 +++++++ store/v2/compare_and_delete_command.go | 37 ++++++++++++++++++++++++++ 5 files changed, 55 insertions(+), 7 deletions(-) create mode 100644 store/v2/compare_and_delete_command.go diff --git a/store/command_factory.go b/store/command_factory.go index 99f2affed..eeab8803b 100644 --- a/store/command_factory.go +++ b/store/command_factory.go @@ -21,6 +21,7 @@ type CommandFactory interface { CreateUpdateCommand(key string, value string, expireTime time.Time) raft.Command CreateDeleteCommand(key string, recursive bool) raft.Command CreateCompareAndSwapCommand(key string, value string, prevValue string, prevIndex uint64, expireTime time.Time) raft.Command + CreateCompareAndDeleteCommand(key string, prevValue string, prevIndex uint64) raft.Command CreateSyncCommand(now time.Time) raft.Command } diff --git a/store/event.go b/store/event.go index f7a846717..aca446393 100644 --- a/store/event.go +++ b/store/event.go @@ -5,14 +5,14 @@ import ( ) const ( - Get = "get" - Create = "create" - Set = "set" - Update = "update" - Delete = "delete" - CompareAndSwap = "compareAndSwap" + Get = "get" + Create = "create" + Set = "set" + Update = "update" + Delete = "delete" + CompareAndSwap = "compareAndSwap" CompareAndDelete = "compareAndDelete" - Expire = "expire" + Expire = "expire" ) type Event struct { diff --git a/store/store.go b/store/store.go index eb88f5c82..bda244813 100644 --- a/store/store.go +++ b/store/store.go @@ -50,6 +50,7 @@ type Store interface { CompareAndSwap(nodePath string, prevValue string, prevIndex uint64, value string, expireTime time.Time) (*Event, error) Delete(nodePath string, recursive bool) (*Event, error) + CompareAndDelete(nodePath string, prevValue string, prevIndex uint64) (*Event, error) Watch(prefix string, recursive bool, sinceIndex uint64) (<-chan *Event, error) Save() ([]byte, error) Recovery(state []byte) error diff --git a/store/v2/command_factory.go b/store/v2/command_factory.go index 70b03cded..6f3707929 100644 --- a/store/v2/command_factory.go +++ b/store/v2/command_factory.go @@ -72,6 +72,15 @@ func (f *CommandFactory) CreateCompareAndSwapCommand(key string, value string, p } } +// CreateCompareAndDeleteCommand creates a version 2 command to conditionally delete a key from the store. +func (f *CommandFactory) CreateCompareAndDeleteCommand(key string, prevValue string, prevIndex uint64) raft.Command { + return &CompareAndDeleteCommand{ + Key: key, + PrevValue: prevValue, + PrevIndex: prevIndex, + } +} + func (f *CommandFactory) CreateSyncCommand(now time.Time) raft.Command { return &SyncCommand{ Time: time.Now(), diff --git a/store/v2/compare_and_delete_command.go b/store/v2/compare_and_delete_command.go new file mode 100644 index 000000000..8a518c5a6 --- /dev/null +++ b/store/v2/compare_and_delete_command.go @@ -0,0 +1,37 @@ +package v2 + +import ( + "github.com/coreos/etcd/log" + "github.com/coreos/etcd/store" + "github.com/coreos/raft" +) + +func init() { + raft.RegisterCommand(&CompareAndDeleteCommand{}) +} + +// The CompareAndDelete performs a conditional delete on a key in the store. +type CompareAndDeleteCommand struct { + Key string `json:"key"` + PrevValue string `json:"prevValue"` + PrevIndex uint64 `json:"prevIndex"` +} + +// The name of the compareAndDelete command in the log +func (c *CompareAndDeleteCommand) CommandName() string { + return "etcd:compareAndDelete" +} + +// Set the key-value pair if the current value of the key equals to the given prevValue +func (c *CompareAndDeleteCommand) Apply(server raft.Server) (interface{}, error) { + s, _ := server.StateMachine().(store.Store) + + e, err := s.CompareAndDelete(c.Key, c.PrevValue, c.PrevIndex) + + if err != nil { + log.Debug(err) + return nil, err + } + + return e, nil +} From 373199fe468b017071d527c39641bfcdf7313af3 Mon Sep 17 00:00:00 2001 From: rick Date: Sat, 30 Nov 2013 16:25:26 -0700 Subject: [PATCH 05/12] support CreateAndDelete through the v2 server --- server/v2/delete_handler.go | 36 ++++++++- server/v2/tests/delete_handler_test.go | 107 +++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 2 deletions(-) diff --git a/server/v2/delete_handler.go b/server/v2/delete_handler.go index 7afc02f04..c98c2768e 100644 --- a/server/v2/delete_handler.go +++ b/server/v2/delete_handler.go @@ -2,15 +2,47 @@ package v2 import ( "net/http" + "strconv" + etcdErr "github.com/coreos/etcd/error" "github.com/gorilla/mux" ) func DeleteHandler(w http.ResponseWriter, req *http.Request, s Server) error { vars := mux.Vars(req) key := "/" + vars["key"] - recursive := (req.FormValue("recursive") == "true") - c := s.Store().CommandFactory().CreateDeleteCommand(key, recursive) + req.ParseForm() + _, valueOk := req.Form["prevValue"] + _, indexOk := req.Form["prevIndex"] + + if !valueOk && !indexOk { + recursive := (req.Form.Get("recursive") == "true") + + c := s.Store().CommandFactory().CreateDeleteCommand(key, recursive) + return s.Dispatch(c, w, req) + } + + var err error + prevIndex := uint64(0) + prevValue := req.Form.Get("prevValue") + + if indexOk { + prevIndexStr := req.Form.Get("prevIndex") + prevIndex, err = strconv.ParseUint(prevIndexStr, 10, 64) + + // bad previous index + if err != nil { + return etcdErr.NewError(etcdErr.EcodeIndexNaN, "CompareAndDelete", s.Store().Index()) + } + } + + if valueOk { + if prevValue == "" { + return etcdErr.NewError(etcdErr.EcodePrevValueRequired, "CompareAndDelete", s.Store().Index()) + } + } + + c := s.Store().CommandFactory().CreateCompareAndDeleteCommand(key, prevValue, prevIndex) return s.Dispatch(c, w, req) } diff --git a/server/v2/tests/delete_handler_test.go b/server/v2/tests/delete_handler_test.go index 997127a9e..b602e0764 100644 --- a/server/v2/tests/delete_handler_test.go +++ b/server/v2/tests/delete_handler_test.go @@ -27,3 +27,110 @@ func TestV2DeleteKey(t *testing.T) { assert.Equal(t, string(body), `{"action":"delete","key":"/foo/bar","prevValue":"XXX","modifiedIndex":2}`, "") }) } + +// Ensures that a key is deleted only if the previous index matches. +// +// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX +// $ curl -X DELETE localhost:4001/v2/keys/foo/bar?prevIndex=1 +// +func TestV2DeleteKeyCADOnIndexSuccess(t *testing.T) { + tests.RunServer(func(s *server.Server) { + v := url.Values{} + v.Set("value", "XXX") + resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + tests.ReadBody(resp) + resp, _ = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?prevIndex=1"), v) + body := tests.ReadBodyJSON(resp) + assert.Equal(t, body["action"], "compareAndDelete", "") + assert.Equal(t, body["prevValue"], "XXX", "") + assert.Equal(t, body["modifiedIndex"], 2, "") + }) +} + +// Ensures that a key is not deleted if the previous index does not matche. +// +// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX +// $ curl -X DELETE localhost:4001/v2/keys/foo/bar?prevIndex=2 +// +func TestV2DeleteKeyCADOnIndexFail(t *testing.T) { + tests.RunServer(func(s *server.Server) { + v := url.Values{} + v.Set("value", "XXX") + resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + tests.ReadBody(resp) + resp, _ = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?prevIndex=100"), v) + body := tests.ReadBodyJSON(resp) + fmt.Println(body) + assert.Equal(t, body["errorCode"], 101) + }) +} + +// Ensures that an error is thrown if an invalid previous index is provided. +// +// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX +// $ curl -X DELETE localhost:4001/v2/keys/foo/bar?prevIndex=bad_index +// +func TestV2DeleteKeyCADWithInvalidIndex(t *testing.T) { + tests.RunServer(func(s *server.Server) { + v := url.Values{} + v.Set("value", "XXX") + resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + tests.ReadBody(resp) + resp, _ = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?prevIndex=bad_index"), v) + body := tests.ReadBodyJSON(resp) + assert.Equal(t, body["errorCode"], 203) + }) +} + +// Ensures that a key is deleted only if the previous value matches. +// +// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX +// $ curl -X DELETE localhost:4001/v2/keys/foo/bar?prevValue=XXX +// +func TestV2DeleteKeyCADOnValueSuccess(t *testing.T) { + tests.RunServer(func(s *server.Server) { + v := url.Values{} + v.Set("value", "XXX") + resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + tests.ReadBody(resp) + resp, _ = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?prevValue=XXX"), v) + body := tests.ReadBodyJSON(resp) + assert.Equal(t, body["action"], "compareAndDelete", "") + assert.Equal(t, body["prevValue"], "XXX", "") + assert.Equal(t, body["modifiedIndex"], 2, "") + }) +} + +// Ensures that a key is not deleted if the previous value does not matche. +// +// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX +// $ curl -X DELETE localhost:4001/v2/keys/foo/bar?prevValue=YYY +// +func TestV2DeleteKeyCADOnValueFail(t *testing.T) { + tests.RunServer(func(s *server.Server) { + v := url.Values{} + v.Set("value", "XXX") + resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + tests.ReadBody(resp) + resp, _ = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?prevValue=YYY"), v) + body := tests.ReadBodyJSON(resp) + assert.Equal(t, body["errorCode"], 101) + }) +} + +// Ensures that an error is thrown if an invalid previous value is provided. +// +// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX +// $ curl -X DELETE localhost:4001/v2/keys/foo/bar?prevIndex= +// +func TestV2DeleteKeyCADWithInvalidValue(t *testing.T) { + tests.RunServer(func(s *server.Server) { + v := url.Values{} + v.Set("value", "XXX") + resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + tests.ReadBody(resp) + resp, _ = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?prevValue="), v) + body := tests.ReadBodyJSON(resp) + assert.Equal(t, body["errorCode"], 201) + }) +} From f8985d731f2357db2fe36336c72072531fca569f Mon Sep 17 00:00:00 2001 From: rick Date: Sun, 1 Dec 2013 13:28:14 -0700 Subject: [PATCH 06/12] keep the Delete tests together --- store/store_test.go | 80 ++++++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/store/store_test.go b/store/store_test.go index e70cc5419..8cc65efd9 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -204,46 +204,6 @@ func TestStoreDeleteValue(t *testing.T) { assert.Equal(t, e.Action, "delete", "") } -func TestStoreCompareAndDeletePrevValue(t *testing.T) { - s := newStore() - s.Create("/foo", "bar", false, Permanent) - e, err := s.CompareAndDelete("/foo", "bar", 0) - assert.Nil(t, err, "") - assert.Equal(t, e.Action, "compareAndDelete", "") -} - -func TestStoreCompareAndDeletePrevValueFailsIfNotMatch(t *testing.T) { - s := newStore() - s.Create("/foo", "bar", false, Permanent) - e, _err := s.CompareAndDelete("/foo", "baz", 0) - err := _err.(*etcdErr.Error) - assert.Equal(t, err.ErrorCode, etcdErr.EcodeTestFailed, "") - assert.Equal(t, err.Message, "Test Failed", "") - assert.Nil(t, e, "") - e, _ = s.Get("/foo", false, false) - assert.Equal(t, e.Value, "bar", "") -} - -func TestStoreCompareAndDeletePrevIndex(t *testing.T) { - s := newStore() - s.Create("/foo", "bar", false, Permanent) - e, err := s.CompareAndDelete("/foo", "", 1) - assert.Nil(t, err, "") - assert.Equal(t, e.Action, "compareAndDelete", "") -} - -func TestStoreCompareAndDeletePrevIndexFailsIfNotMatch(t *testing.T) { - s := newStore() - s.Create("/foo", "bar", false, Permanent) - e, _err := s.CompareAndDelete("/foo", "baz", 100) - err := _err.(*etcdErr.Error) - assert.Equal(t, err.ErrorCode, etcdErr.EcodeTestFailed, "") - assert.Equal(t, err.Message, "Test Failed", "") - assert.Nil(t, e, "") - e, _ = s.Get("/foo", false, false) - assert.Equal(t, e.Value, "bar", "") -} - // Ensure that the store can delete a directory if recursive is specified. func TestStoreDeleteDiretory(t *testing.T) { s := newStore() @@ -264,6 +224,46 @@ func TestStoreDeleteDiretoryFailsIfNonRecursive(t *testing.T) { assert.Nil(t, e, "") } +func TestStoreCompareAndDeletePrevValue(t *testing.T) { + s := newStore() + s.Create("/foo", "bar", false, Permanent) + e, err := s.CompareAndDelete("/foo", false, "bar", 0) + assert.Nil(t, err, "") + assert.Equal(t, e.Action, "compareAndDelete", "") +} + +func TestStoreCompareAndDeletePrevValueFailsIfNotMatch(t *testing.T) { + s := newStore() + s.Create("/foo", "bar", false, Permanent) + e, _err := s.CompareAndDelete("/foo", false, "baz", 0) + err := _err.(*etcdErr.Error) + assert.Equal(t, err.ErrorCode, etcdErr.EcodeTestFailed, "") + assert.Equal(t, err.Message, "Test Failed", "") + assert.Nil(t, e, "") + e, _ = s.Get("/foo", false, false) + assert.Equal(t, e.Value, "bar", "") +} + +func TestStoreCompareAndDeletePrevIndex(t *testing.T) { + s := newStore() + s.Create("/foo", "bar", false, Permanent) + e, err := s.CompareAndDelete("/foo", false, "", 1) + assert.Nil(t, err, "") + assert.Equal(t, e.Action, "compareAndDelete", "") +} + +func TestStoreCompareAndDeletePrevIndexFailsIfNotMatch(t *testing.T) { + s := newStore() + s.Create("/foo", "bar", false, Permanent) + e, _err := s.CompareAndDelete("/foo", false, "baz", 100) + err := _err.(*etcdErr.Error) + assert.Equal(t, err.ErrorCode, etcdErr.EcodeTestFailed, "") + assert.Equal(t, err.Message, "Test Failed", "") + assert.Nil(t, e, "") + e, _ = s.Get("/foo", false, false) + assert.Equal(t, e.Value, "bar", "") +} + // Ensure that the store can conditionally update a key if it has a previous value. func TestStoreCompareAndSwapPrevValue(t *testing.T) { s := newStore() From d2d7e37990e33a0351eafbbfc13f48e519232e4f Mon Sep 17 00:00:00 2001 From: rick Date: Sun, 1 Dec 2013 13:38:09 -0700 Subject: [PATCH 07/12] implement recursive for CompareAndDelete in the store --- store/command_factory.go | 2 +- store/store.go | 21 +++++----- store/store_test.go | 55 +++++++++++++++++++++++++- store/v2/command_factory.go | 3 +- store/v2/compare_and_delete_command.go | 3 +- 5 files changed, 69 insertions(+), 15 deletions(-) diff --git a/store/command_factory.go b/store/command_factory.go index eeab8803b..9cb3760a0 100644 --- a/store/command_factory.go +++ b/store/command_factory.go @@ -21,7 +21,7 @@ type CommandFactory interface { CreateUpdateCommand(key string, value string, expireTime time.Time) raft.Command CreateDeleteCommand(key string, recursive bool) raft.Command CreateCompareAndSwapCommand(key string, value string, prevValue string, prevIndex uint64, expireTime time.Time) raft.Command - CreateCompareAndDeleteCommand(key string, prevValue string, prevIndex uint64) raft.Command + CreateCompareAndDeleteCommand(key string, recursive bool, prevValue string, prevIndex uint64) raft.Command CreateSyncCommand(now time.Time) raft.Command } diff --git a/store/store.go b/store/store.go index bda244813..4a388a875 100644 --- a/store/store.go +++ b/store/store.go @@ -50,7 +50,7 @@ type Store interface { CompareAndSwap(nodePath string, prevValue string, prevIndex uint64, value string, expireTime time.Time) (*Event, error) Delete(nodePath string, recursive bool) (*Event, error) - CompareAndDelete(nodePath string, prevValue string, prevIndex uint64) (*Event, error) + CompareAndDelete(nodePath string, recursive bool, prevValue string, prevIndex uint64) (*Event, error) Watch(prefix string, recursive bool, sinceIndex uint64) (<-chan *Event, error) Save() ([]byte, error) Recovery(state []byte) error @@ -281,9 +281,7 @@ func (s *store) Delete(nodePath string, recursive bool) (*Event, error) { return e, nil } -func (s *store) CompareAndDelete(nodePath string, prevValue string, prevIndex uint64, -) (*Event, error) { - +func (s *store) CompareAndDelete(nodePath string, recursive bool, prevValue string, prevIndex uint64) (*Event, error) { nodePath = path.Clean(path.Join("/", nodePath)) s.worldLock.Lock() @@ -296,24 +294,25 @@ func (s *store) CompareAndDelete(nodePath string, prevValue string, prevIndex ui return nil, err } - if n.IsDir() { // can only test and set file - s.Stats.Inc(CompareAndDeleteFail) - return nil, etcdErr.NewError(etcdErr.EcodeNotFile, nodePath, s.CurrentIndex) - } + isDir := n.IsDir() // If both of the prevValue and prevIndex are given, we will test both of them. // Command will be executed, only if both of the tests are successful. - if (prevValue == "" || n.Value == prevValue) && (prevIndex == 0 || n.ModifiedIndex == prevIndex) { + if (isDir || prevValue == "" || n.Value == prevValue) && (prevIndex == 0 || n.ModifiedIndex == prevIndex) { e := newEvent(CompareAndDelete, nodePath, s.CurrentIndex+1) - e.PrevValue = n.Value + if isDir { + e.Dir = true + } else { + e.PrevValue = n.Value + } callback := func(path string) { // notify function // notify the watchers with deleted set true s.WatcherHub.notifyWatchers(e, path, true) } - err = n.Remove(false, callback) + err = n.Remove(recursive, callback) if err != nil { s.Stats.Inc(CompareAndDeleteFail) diff --git a/store/store_test.go b/store/store_test.go index 8cc65efd9..74425a26e 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -230,6 +230,7 @@ func TestStoreCompareAndDeletePrevValue(t *testing.T) { e, err := s.CompareAndDelete("/foo", false, "bar", 0) assert.Nil(t, err, "") assert.Equal(t, e.Action, "compareAndDelete", "") + assert.Equal(t, e.Dir, false, "") } func TestStoreCompareAndDeletePrevValueFailsIfNotMatch(t *testing.T) { @@ -255,7 +256,7 @@ func TestStoreCompareAndDeletePrevIndex(t *testing.T) { func TestStoreCompareAndDeletePrevIndexFailsIfNotMatch(t *testing.T) { s := newStore() s.Create("/foo", "bar", false, Permanent) - e, _err := s.CompareAndDelete("/foo", false, "baz", 100) + e, _err := s.CompareAndDelete("/foo", false, "", 100) err := _err.(*etcdErr.Error) assert.Equal(t, err.ErrorCode, etcdErr.EcodeTestFailed, "") assert.Equal(t, err.Message, "Test Failed", "") @@ -264,6 +265,58 @@ func TestStoreCompareAndDeletePrevIndexFailsIfNotMatch(t *testing.T) { assert.Equal(t, e.Value, "bar", "") } +// Ensure that the store can delete a directory if recursive is specified. +func TestStoreCompareAndDeleteDiretory(t *testing.T) { + s := newStore() + s.Create("/foo", "", false, Permanent) + e, err := s.CompareAndDelete("/foo", true, "", 0) + assert.Nil(t, err, "") + assert.Equal(t, e.Action, "compareAndDelete", "") + assert.Equal(t, e.Dir, true, "") +} + +// Ensure that the store can delete a directory if recursive is specified. +func TestStoreCompareAndDeleteDiretoryIgnoringPrevValue(t *testing.T) { + s := newStore() + s.Create("/foo", "", false, Permanent) + e, err := s.CompareAndDelete("/foo", true, "baz", 0) + assert.Nil(t, err, "") + assert.Equal(t, e.Action, "compareAndDelete", "") +} + +// Ensure that the store can delete a directory with a prev index. +func TestStoreCompareAndDeleteDirectoryPrevIndex(t *testing.T) { + s := newStore() + s.Create("/foo", "", false, Permanent) + e, err := s.CompareAndDelete("/foo", true, "", 1) + assert.Nil(t, err, "") + assert.Equal(t, e.Action, "compareAndDelete", "") +} + +// Ensure that the store won't delete a directory if prevIndex does not match +func TestStoreCompareAndDeleteDirectoryPrevIndexFailsIfNotMatch(t *testing.T) { + s := newStore() + s.Create("/foo", "", false, Permanent) + e, _err := s.CompareAndDelete("/foo", true, "", 100) + err := _err.(*etcdErr.Error) + assert.Equal(t, err.ErrorCode, etcdErr.EcodeTestFailed, "") + assert.Equal(t, err.Message, "Test Failed", "") + assert.Nil(t, e, "") + e, _ = s.Get("/foo", false, false) + assert.Equal(t, e.Value, "", "") +} + +// Ensure that the store cannot delete a directory if recursive is not specified. +func TestStoreCompareAndDeleteDiretoryFailsIfNonRecursive(t *testing.T) { + s := newStore() + s.Create("/foo", "", false, Permanent) + e, _err := s.CompareAndDelete("/foo", false, "", 0) + err := _err.(*etcdErr.Error) + assert.Equal(t, err.ErrorCode, etcdErr.EcodeNotFile, "") + assert.Equal(t, err.Message, "Not A File", "") + assert.Nil(t, e, "") +} + // Ensure that the store can conditionally update a key if it has a previous value. func TestStoreCompareAndSwapPrevValue(t *testing.T) { s := newStore() diff --git a/store/v2/command_factory.go b/store/v2/command_factory.go index 6f3707929..e0106562a 100644 --- a/store/v2/command_factory.go +++ b/store/v2/command_factory.go @@ -73,9 +73,10 @@ func (f *CommandFactory) CreateCompareAndSwapCommand(key string, value string, p } // CreateCompareAndDeleteCommand creates a version 2 command to conditionally delete a key from the store. -func (f *CommandFactory) CreateCompareAndDeleteCommand(key string, prevValue string, prevIndex uint64) raft.Command { +func (f *CommandFactory) CreateCompareAndDeleteCommand(key string, recursive bool, prevValue string, prevIndex uint64) raft.Command { return &CompareAndDeleteCommand{ Key: key, + Recursive: recursive, PrevValue: prevValue, PrevIndex: prevIndex, } diff --git a/store/v2/compare_and_delete_command.go b/store/v2/compare_and_delete_command.go index 8a518c5a6..7939f253d 100644 --- a/store/v2/compare_and_delete_command.go +++ b/store/v2/compare_and_delete_command.go @@ -15,6 +15,7 @@ type CompareAndDeleteCommand struct { Key string `json:"key"` PrevValue string `json:"prevValue"` PrevIndex uint64 `json:"prevIndex"` + Recursive bool `json:"recursive"` } // The name of the compareAndDelete command in the log @@ -26,7 +27,7 @@ func (c *CompareAndDeleteCommand) CommandName() string { func (c *CompareAndDeleteCommand) Apply(server raft.Server) (interface{}, error) { s, _ := server.StateMachine().(store.Store) - e, err := s.CompareAndDelete(c.Key, c.PrevValue, c.PrevIndex) + e, err := s.CompareAndDelete(c.Key, c.Recursive, c.PrevValue, c.PrevIndex) if err != nil { log.Debug(err) From 3d16633a9436709a96d59ec50c7af71193376a22 Mon Sep 17 00:00:00 2001 From: rick Date: Sun, 1 Dec 2013 13:56:32 -0700 Subject: [PATCH 08/12] update the v2 server to support recursive on CompareAndDelete events --- server/v2/delete_handler.go | 5 +-- server/v2/tests/delete_handler_test.go | 59 +++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/server/v2/delete_handler.go b/server/v2/delete_handler.go index c98c2768e..c70fdb6ab 100644 --- a/server/v2/delete_handler.go +++ b/server/v2/delete_handler.go @@ -15,10 +15,9 @@ func DeleteHandler(w http.ResponseWriter, req *http.Request, s Server) error { req.ParseForm() _, valueOk := req.Form["prevValue"] _, indexOk := req.Form["prevIndex"] + recursive := (req.Form.Get("recursive") == "true") if !valueOk && !indexOk { - recursive := (req.Form.Get("recursive") == "true") - c := s.Store().CommandFactory().CreateDeleteCommand(key, recursive) return s.Dispatch(c, w, req) } @@ -43,6 +42,6 @@ func DeleteHandler(w http.ResponseWriter, req *http.Request, s Server) error { } } - c := s.Store().CommandFactory().CreateCompareAndDeleteCommand(key, prevValue, prevIndex) + c := s.Store().CommandFactory().CreateCompareAndDeleteCommand(key, recursive, prevValue, prevIndex) return s.Dispatch(c, w, req) } diff --git a/server/v2/tests/delete_handler_test.go b/server/v2/tests/delete_handler_test.go index b602e0764..7fcd4e1f4 100644 --- a/server/v2/tests/delete_handler_test.go +++ b/server/v2/tests/delete_handler_test.go @@ -28,6 +28,64 @@ func TestV2DeleteKey(t *testing.T) { }) } +// Ensures that a directory is deleted. +// +// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX +// $ curl -X DELETE localhost:4001/v2/keys/foo?recursive=true +// +func TestV2DeleteDirectory(t *testing.T) { + tests.RunServer(func(s *server.Server) { + v := url.Values{} + v.Set("value", "XXX") + resp, err := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + tests.ReadBody(resp) + resp, err = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo?recursive=true"), url.Values{}) + assert.Nil(t, err, "") + body := tests.ReadBodyJSON(resp) + assert.Equal(t, body["action"], "delete", "") + assert.Equal(t, body["dir"], true, "") + assert.Equal(t, body["modifiedIndex"], 2, "") + }) +} + +// Ensures that a directory is deleted if the previous index matches +// +// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX +// $ curl -X DELETE localhost:4001/v2/keys/foo?recursive=true&prevIndex=1 +// +func TestV2DeleteDirectoryCADOnIndexSuccess(t *testing.T) { + tests.RunServer(func(s *server.Server) { + v := url.Values{} + v.Set("value", "XXX") + resp, err := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + tests.ReadBody(resp) + resp, err = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo?recursive=true&prevIndex=1"), url.Values{}) + assert.Nil(t, err, "") + body := tests.ReadBodyJSON(resp) + assert.Equal(t, body["action"], "compareAndDelete", "") + assert.Equal(t, body["dir"], true, "") + assert.Equal(t, body["modifiedIndex"], 2, "") + }) +} + +// Ensures that a directory is not deleted if the previous index does not match +// +// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX +// $ curl -X DELETE localhost:4001/v2/keys/foo?recursive=true&prevIndex=100 +// +func TestV2DeleteDirectoryCADOnIndexFail(t *testing.T) { + tests.RunServer(func(s *server.Server) { + v := url.Values{} + v.Set("value", "XXX") + resp, err := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + tests.ReadBody(resp) + resp, err = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo?recursive=true&prevIndex=100"), url.Values{}) + assert.Nil(t, err, "") + body := tests.ReadBodyJSON(resp) + assert.Equal(t, body["errorCode"], 101) + }) +} + // Ensures that a key is deleted only if the previous index matches. // // $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX @@ -60,7 +118,6 @@ func TestV2DeleteKeyCADOnIndexFail(t *testing.T) { tests.ReadBody(resp) resp, _ = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?prevIndex=100"), v) body := tests.ReadBodyJSON(resp) - fmt.Println(body) assert.Equal(t, body["errorCode"], 101) }) } From 9cf1fcc987436880f50bd869e3118e10574489b6 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Fri, 20 Dec 2013 05:10:22 +0800 Subject: [PATCH 09/12] refactor(compareAndDelete) --- server/v2/delete_handler.go | 2 +- server/v2/tests/delete_handler_test.go | 110 +++++++------------------ store/command_factory.go | 2 +- store/node.go | 7 ++ store/store.go | 36 ++++---- store/store_test.go | 79 +++++------------- store/v2/command_factory.go | 3 +- store/v2/compare_and_delete_command.go | 3 +- 8 files changed, 74 insertions(+), 168 deletions(-) diff --git a/server/v2/delete_handler.go b/server/v2/delete_handler.go index 33db3268a..a9f2a4122 100644 --- a/server/v2/delete_handler.go +++ b/server/v2/delete_handler.go @@ -44,6 +44,6 @@ func DeleteHandler(w http.ResponseWriter, req *http.Request, s Server) error { } } - c := s.Store().CommandFactory().CreateCompareAndDeleteCommand(key, recursive, prevValue, prevIndex) + c := s.Store().CommandFactory().CreateCompareAndDeleteCommand(key, prevValue, prevIndex) return s.Dispatch(c, w, req) } diff --git a/server/v2/tests/delete_handler_test.go b/server/v2/tests/delete_handler_test.go index aed48f76d..1169ab0c1 100644 --- a/server/v2/tests/delete_handler_test.go +++ b/server/v2/tests/delete_handler_test.go @@ -84,95 +84,42 @@ func TestV2DeleteDirectoryRecursiveImpliesDir(t *testing.T) { }) } -// Ensures that a directory is deleted. +// Ensures that a key is deleted if the previous index matches // -// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX -// $ curl -X DELETE localhost:4001/v2/keys/foo?recursive=true -// -func TestV2DeleteDirectory(t *testing.T) { - tests.RunServer(func(s *server.Server) { - v := url.Values{} - v.Set("value", "XXX") - resp, err := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) - tests.ReadBody(resp) - resp, err = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo?recursive=true"), url.Values{}) - assert.Nil(t, err, "") - body := tests.ReadBodyJSON(resp) - assert.Equal(t, body["action"], "delete", "") - assert.Equal(t, body["dir"], true, "") - assert.Equal(t, body["modifiedIndex"], 2, "") - }) -} - -// Ensures that a directory is deleted if the previous index matches -// -// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX -// $ curl -X DELETE localhost:4001/v2/keys/foo?recursive=true&prevIndex=1 -// -func TestV2DeleteDirectoryCADOnIndexSuccess(t *testing.T) { - tests.RunServer(func(s *server.Server) { - v := url.Values{} - v.Set("value", "XXX") - resp, err := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) - tests.ReadBody(resp) - resp, err = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo?recursive=true&prevIndex=1"), url.Values{}) - assert.Nil(t, err, "") - body := tests.ReadBodyJSON(resp) - assert.Equal(t, body["action"], "compareAndDelete", "") - assert.Equal(t, body["dir"], true, "") - assert.Equal(t, body["modifiedIndex"], 2, "") - }) -} - -// Ensures that a directory is not deleted if the previous index does not match -// -// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX -// $ curl -X DELETE localhost:4001/v2/keys/foo?recursive=true&prevIndex=100 -// -func TestV2DeleteDirectoryCADOnIndexFail(t *testing.T) { - tests.RunServer(func(s *server.Server) { - v := url.Values{} - v.Set("value", "XXX") - resp, err := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) - tests.ReadBody(resp) - resp, err = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo?recursive=true&prevIndex=100"), url.Values{}) - assert.Nil(t, err, "") - body := tests.ReadBodyJSON(resp) - assert.Equal(t, body["errorCode"], 101) - }) -} - -// Ensures that a key is deleted only if the previous index matches. -// -// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX -// $ curl -X DELETE localhost:4001/v2/keys/foo/bar?prevIndex=1 +// $ curl -X PUT localhost:4001/v2/keys/foo -d value=XXX +// $ curl -X DELETE localhost:4001/v2/keys/foo?prevIndex=1 // func TestV2DeleteKeyCADOnIndexSuccess(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, err := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo"), v) tests.ReadBody(resp) - resp, _ = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?prevIndex=1"), v) + resp, err = tests.DeleteForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo?prevIndex=2"), url.Values{}) + assert.Nil(t, err, "") + fmt.Println(resp) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["action"], "compareAndDelete", "") - assert.Equal(t, body["prevValue"], "XXX", "") - assert.Equal(t, body["modifiedIndex"], 2, "") + + node := body["node"].(map[string]interface{}) + assert.Equal(t, node["key"], "/foo", "") + assert.Equal(t, node["modifiedIndex"], 3, "") }) } -// Ensures that a key is not deleted if the previous index does not matche. +// Ensures that a key is not deleted if the previous index does not match // -// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX -// $ curl -X DELETE localhost:4001/v2/keys/foo/bar?prevIndex=2 +// $ curl -X PUT localhost:4001/v2/keys/foo -d value=XXX +// $ curl -X DELETE localhost:4001/v2/keys/foo?prevIndex=100 // func TestV2DeleteKeyCADOnIndexFail(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, err := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo"), v) tests.ReadBody(resp) - resp, _ = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?prevIndex=100"), v) + resp, err = tests.DeleteForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo?prevIndex=100"), url.Values{}) + assert.Nil(t, err, "") body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 101) }) @@ -187,9 +134,9 @@ func TestV2DeleteKeyCADWithInvalidIndex(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) tests.ReadBody(resp) - resp, _ = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?prevIndex=bad_index"), v) + resp, _ = tests.DeleteForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar?prevIndex=bad_index"), v) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 203) }) @@ -204,17 +151,18 @@ func TestV2DeleteKeyCADOnValueSuccess(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) tests.ReadBody(resp) - resp, _ = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?prevValue=XXX"), v) + resp, _ = tests.DeleteForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar?prevValue=XXX"), v) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["action"], "compareAndDelete", "") - assert.Equal(t, body["prevValue"], "XXX", "") - assert.Equal(t, body["modifiedIndex"], 2, "") + + node := body["node"].(map[string]interface{}) + assert.Equal(t, node["modifiedIndex"], 3, "") }) } -// Ensures that a key is not deleted if the previous value does not matche. +// Ensures that a key is not deleted if the previous value does not match. // // $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX // $ curl -X DELETE localhost:4001/v2/keys/foo/bar?prevValue=YYY @@ -223,9 +171,9 @@ func TestV2DeleteKeyCADOnValueFail(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) tests.ReadBody(resp) - resp, _ = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?prevValue=YYY"), v) + resp, _ = tests.DeleteForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar?prevValue=YYY"), v) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 101) }) @@ -240,9 +188,9 @@ func TestV2DeleteKeyCADWithInvalidValue(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) tests.ReadBody(resp) - resp, _ = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?prevValue="), v) + resp, _ = tests.DeleteForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar?prevValue="), v) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 201) }) diff --git a/store/command_factory.go b/store/command_factory.go index caff8e381..9eabffe27 100644 --- a/store/command_factory.go +++ b/store/command_factory.go @@ -22,7 +22,7 @@ type CommandFactory interface { CreateDeleteCommand(key string, dir, recursive bool) raft.Command CreateCompareAndSwapCommand(key string, value string, prevValue string, prevIndex uint64, expireTime time.Time) raft.Command - CreateCompareAndDeleteCommand(key string, recursive bool, prevValue string, prevIndex uint64) raft.Command + CreateCompareAndDeleteCommand(key string, prevValue string, prevIndex uint64) raft.Command CreateSyncCommand(now time.Time) raft.Command } diff --git a/store/node.go b/store/node.go index f036a594e..71c0a64d7 100644 --- a/store/node.go +++ b/store/node.go @@ -306,6 +306,13 @@ func (n *node) UpdateTTL(expireTime time.Time) { } } +func (n *node) Compare(prevValue string, prevIndex uint64) bool { + compareValue := (prevValue == "" || n.Value == prevValue) + compareIndex := (prevIndex == 0 || n.ModifiedIndex == prevIndex) + + return compareValue && compareIndex +} + // Clone function clone the node recursively and return the new node. // If the node is a directory, it will clone all the content under this directory. // If the node is a key-value pair, it will clone the pair. diff --git a/store/store.go b/store/store.go index d6d17f422..0451cc6e7 100644 --- a/store/store.go +++ b/store/store.go @@ -51,7 +51,7 @@ type Store interface { CompareAndSwap(nodePath string, prevValue string, prevIndex uint64, value string, expireTime time.Time) (*Event, error) Delete(nodePath string, recursive, dir bool) (*Event, error) - CompareAndDelete(nodePath string, recursive bool, prevValue string, prevIndex uint64) (*Event, error) + CompareAndDelete(nodePath string, prevValue string, prevIndex uint64) (*Event, error) Watch(prefix string, recursive bool, sinceIndex uint64) (<-chan *Event, error) Save() ([]byte, error) @@ -208,14 +208,14 @@ func (s *store) CompareAndSwap(nodePath string, prevValue string, prevIndex uint return nil, err } - if n.IsDir() { // can only test and set file + if n.IsDir() { // can only compare and swap file s.Stats.Inc(CompareAndSwapFail) return nil, etcdErr.NewError(etcdErr.EcodeNotFile, nodePath, s.CurrentIndex) } // If both of the prevValue and prevIndex are given, we will test both of them. // Command will be executed, only if both of the tests are successful. - if (prevValue == "" || n.Value == prevValue) && (prevIndex == 0 || n.ModifiedIndex == prevIndex) { + if n.Compare(prevValue, prevIndex) { // update etcd index s.CurrentIndex++ @@ -258,8 +258,6 @@ func (s *store) Delete(nodePath string, dir, recursive bool) (*Event, error) { dir = true } - nextIndex := s.CurrentIndex + 1 - n, err := s.internalGet(nodePath) if err != nil { // if the node does not exist, return error @@ -267,6 +265,7 @@ func (s *store) Delete(nodePath string, dir, recursive bool) (*Event, error) { return nil, err } + nextIndex := s.CurrentIndex + 1 e := newEvent(Delete, nodePath, nextIndex, n.CreatedIndex) eNode := e.Node @@ -297,7 +296,7 @@ func (s *store) Delete(nodePath string, dir, recursive bool) (*Event, error) { return e, nil } -func (s *store) CompareAndDelete(nodePath string, recursive bool, prevValue string, prevIndex uint64) (*Event, error) { +func (s *store) CompareAndDelete(nodePath string, prevValue string, prevIndex uint64) (*Event, error) { nodePath = path.Clean(path.Join("/", nodePath)) s.worldLock.Lock() @@ -310,31 +309,26 @@ func (s *store) CompareAndDelete(nodePath string, recursive bool, prevValue stri return nil, err } - isDir := n.IsDir() + if n.IsDir() { // can only compare and delete file + s.Stats.Inc(CompareAndSwapFail) + return nil, etcdErr.NewError(etcdErr.EcodeNotFile, nodePath, s.CurrentIndex) + } // If both of the prevValue and prevIndex are given, we will test both of them. // Command will be executed, only if both of the tests are successful. - if (isDir || prevValue == "" || n.Value == prevValue) && (prevIndex == 0 || n.ModifiedIndex == prevIndex) { + if n.Compare(prevValue, prevIndex) { + // update etcd index + s.CurrentIndex++ - e := newEvent(CompareAndDelete, nodePath, s.CurrentIndex+1, n.CreatedIndex) - if isDir { - e.Node.Dir = true - } + e := newEvent(CompareAndDelete, nodePath, s.CurrentIndex, n.CreatedIndex) callback := func(path string) { // notify function // notify the watchers with deleted set true s.WatcherHub.notifyWatchers(e, path, true) } - err = n.Remove(true, recursive, callback) - - if err != nil { - s.Stats.Inc(CompareAndDeleteFail) - return nil, err - } - - // update etcd index - s.CurrentIndex++ + // delete a key-value pair, no error should happen + n.Remove(false, false, callback) s.WatcherHub.notify(e) s.Stats.Inc(CompareAndDeleteSuccess) diff --git a/store/store_test.go b/store/store_test.go index ae71aa6d0..b29fea627 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -341,95 +341,54 @@ func TestRootRdOnly(t *testing.T) { func TestStoreCompareAndDeletePrevValue(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent) - e, err := s.CompareAndDelete("/foo", false, "bar", 0) + s.Create("/foo", false, "bar", false, Permanent) + e, err := s.CompareAndDelete("/foo", "bar", 0) assert.Nil(t, err, "") assert.Equal(t, e.Action, "compareAndDelete", "") - assert.Equal(t, e.Dir, false, "") + assert.Equal(t, e.Node.Key, "/foo", "") } func TestStoreCompareAndDeletePrevValueFailsIfNotMatch(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent) - e, _err := s.CompareAndDelete("/foo", false, "baz", 0) + s.Create("/foo", false, "bar", false, Permanent) + e, _err := s.CompareAndDelete("/foo", "baz", 0) err := _err.(*etcdErr.Error) assert.Equal(t, err.ErrorCode, etcdErr.EcodeTestFailed, "") - assert.Equal(t, err.Message, "Test Failed", "") + assert.Equal(t, err.Message, "Compare failed", "") assert.Nil(t, e, "") e, _ = s.Get("/foo", false, false) - assert.Equal(t, e.Value, "bar", "") + assert.Equal(t, e.Node.Value, "bar", "") } func TestStoreCompareAndDeletePrevIndex(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent) - e, err := s.CompareAndDelete("/foo", false, "", 1) + s.Create("/foo", false, "bar", false, Permanent) + e, err := s.CompareAndDelete("/foo", "", 1) assert.Nil(t, err, "") assert.Equal(t, e.Action, "compareAndDelete", "") } func TestStoreCompareAndDeletePrevIndexFailsIfNotMatch(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent) - e, _err := s.CompareAndDelete("/foo", false, "", 100) + s.Create("/foo", false, "bar", false, Permanent) + e, _err := s.CompareAndDelete("/foo", "", 100) + assert.NotNil(t, _err, "") err := _err.(*etcdErr.Error) assert.Equal(t, err.ErrorCode, etcdErr.EcodeTestFailed, "") - assert.Equal(t, err.Message, "Test Failed", "") + assert.Equal(t, err.Message, "Compare failed", "") assert.Nil(t, e, "") e, _ = s.Get("/foo", false, false) - assert.Equal(t, e.Value, "bar", "") + assert.Equal(t, e.Node.Value, "bar", "") } -// Ensure that the store can delete a directory if recursive is specified. -func TestStoreCompareAndDeleteDiretory(t *testing.T) { +// Ensure that the store cannot delete a directory. +func TestStoreCompareAndDeleteDiretoryFail(t *testing.T) { s := newStore() - s.Create("/foo", "", false, Permanent) - e, err := s.CompareAndDelete("/foo", true, "", 0) - assert.Nil(t, err, "") - assert.Equal(t, e.Action, "compareAndDelete", "") - assert.Equal(t, e.Dir, true, "") -} - -// Ensure that the store can delete a directory if recursive is specified. -func TestStoreCompareAndDeleteDiretoryIgnoringPrevValue(t *testing.T) { - s := newStore() - s.Create("/foo", "", false, Permanent) - e, err := s.CompareAndDelete("/foo", true, "baz", 0) - assert.Nil(t, err, "") - assert.Equal(t, e.Action, "compareAndDelete", "") -} - -// Ensure that the store can delete a directory with a prev index. -func TestStoreCompareAndDeleteDirectoryPrevIndex(t *testing.T) { - s := newStore() - s.Create("/foo", "", false, Permanent) - e, err := s.CompareAndDelete("/foo", true, "", 1) - assert.Nil(t, err, "") - assert.Equal(t, e.Action, "compareAndDelete", "") -} - -// Ensure that the store won't delete a directory if prevIndex does not match -func TestStoreCompareAndDeleteDirectoryPrevIndexFailsIfNotMatch(t *testing.T) { - s := newStore() - s.Create("/foo", "", false, Permanent) - e, _err := s.CompareAndDelete("/foo", true, "", 100) - err := _err.(*etcdErr.Error) - assert.Equal(t, err.ErrorCode, etcdErr.EcodeTestFailed, "") - assert.Equal(t, err.Message, "Test Failed", "") - assert.Nil(t, e, "") - e, _ = s.Get("/foo", false, false) - assert.Equal(t, e.Value, "", "") -} - -// Ensure that the store cannot delete a directory if recursive is not specified. -func TestStoreCompareAndDeleteDiretoryFailsIfNonRecursive(t *testing.T) { - s := newStore() - s.Create("/foo", "", false, Permanent) - e, _err := s.CompareAndDelete("/foo", false, "", 0) + s.Create("/foo", true, "", false, Permanent) + _, _err := s.CompareAndDelete("/foo", "", 0) + assert.NotNil(t, _err, "") err := _err.(*etcdErr.Error) assert.Equal(t, err.ErrorCode, etcdErr.EcodeNotFile, "") - assert.Equal(t, err.Message, "Not A File", "") - assert.Nil(t, e, "") } // Ensure that the store can conditionally update a key if it has a previous value. diff --git a/store/v2/command_factory.go b/store/v2/command_factory.go index bcd5fe226..bc06661e7 100644 --- a/store/v2/command_factory.go +++ b/store/v2/command_factory.go @@ -76,10 +76,9 @@ func (f *CommandFactory) CreateCompareAndSwapCommand(key string, value string, p } // CreateCompareAndDeleteCommand creates a version 2 command to conditionally delete a key from the store. -func (f *CommandFactory) CreateCompareAndDeleteCommand(key string, recursive bool, prevValue string, prevIndex uint64) raft.Command { +func (f *CommandFactory) CreateCompareAndDeleteCommand(key string, prevValue string, prevIndex uint64) raft.Command { return &CompareAndDeleteCommand{ Key: key, - Recursive: recursive, PrevValue: prevValue, PrevIndex: prevIndex, } diff --git a/store/v2/compare_and_delete_command.go b/store/v2/compare_and_delete_command.go index 7939f253d..8a518c5a6 100644 --- a/store/v2/compare_and_delete_command.go +++ b/store/v2/compare_and_delete_command.go @@ -15,7 +15,6 @@ type CompareAndDeleteCommand struct { Key string `json:"key"` PrevValue string `json:"prevValue"` PrevIndex uint64 `json:"prevIndex"` - Recursive bool `json:"recursive"` } // The name of the compareAndDelete command in the log @@ -27,7 +26,7 @@ func (c *CompareAndDeleteCommand) CommandName() string { func (c *CompareAndDeleteCommand) Apply(server raft.Server) (interface{}, error) { s, _ := server.StateMachine().(store.Store) - e, err := s.CompareAndDelete(c.Key, c.Recursive, c.PrevValue, c.PrevIndex) + e, err := s.CompareAndDelete(c.Key, c.PrevValue, c.PrevIndex) if err != nil { log.Debug(err) From c4179829d68394c184ed4a1578960f74faefb701 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Fri, 20 Dec 2013 08:23:50 +0800 Subject: [PATCH 10/12] tests(get_handler) loosen the time assumption for the un --- server/v2/tests/get_handler_test.go | 2 +- tests/functional/simple_snapshot_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/v2/tests/get_handler_test.go b/server/v2/tests/get_handler_test.go index a8dd0e1d3..b9f4979db 100644 --- a/server/v2/tests/get_handler_test.go +++ b/server/v2/tests/get_handler_test.go @@ -205,7 +205,7 @@ func TestV2WatchKeyInDir(t *testing.T) { }() // wait for expiration, we do have a up to 500 millisecond delay - time.Sleep(1500 * time.Millisecond) + time.Sleep(2000 * time.Millisecond) select { case <-c: diff --git a/tests/functional/simple_snapshot_test.go b/tests/functional/simple_snapshot_test.go index 29872d648..b47be3e65 100644 --- a/tests/functional/simple_snapshot_test.go +++ b/tests/functional/simple_snapshot_test.go @@ -56,7 +56,7 @@ func TestSimpleSnapshot(t *testing.T) { index, _ := strconv.Atoi(snapshots[0].Name()[2:5]) - if index <= 507 || index >= 510 { + if index < 507 || index > 510 { t.Fatal("wrong name of snapshot :", snapshots[0].Name()) } @@ -89,7 +89,7 @@ func TestSimpleSnapshot(t *testing.T) { index, _ = strconv.Atoi(snapshots[0].Name()[2:6]) - if index <= 1014 || index >= 1017 { + if index < 1014 || index > 1017 { t.Fatal("wrong name of snapshot :", snapshots[0].Name()) } } From bfa7d54b028f5c6614b22b9a4626759554bcce1c Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Wed, 25 Dec 2013 19:01:04 +0800 Subject: [PATCH 11/12] refactor(store.go) handle short condition first --- store/store.go | 84 +++++++++++++++++++++++++------------------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/store/store.go b/store/store.go index 0451cc6e7..08d585056 100644 --- a/store/store.go +++ b/store/store.go @@ -215,30 +215,30 @@ func (s *store) CompareAndSwap(nodePath string, prevValue string, prevIndex uint // If both of the prevValue and prevIndex are given, we will test both of them. // Command will be executed, only if both of the tests are successful. - if n.Compare(prevValue, prevIndex) { - // update etcd index - s.CurrentIndex++ - - e := newEvent(CompareAndSwap, nodePath, s.CurrentIndex, n.CreatedIndex) - eNode := e.Node - - eNode.PrevValue = n.Value - - // if test succeed, write the value - n.Write(value, s.CurrentIndex) - n.UpdateTTL(expireTime) - - eNode.Value = value - eNode.Expiration, eNode.TTL = n.ExpirationAndTTL() - - s.WatcherHub.notify(e) - s.Stats.Inc(CompareAndSwapSuccess) - return e, nil + if !n.Compare(prevValue, prevIndex) { + cause := fmt.Sprintf("[%v != %v] [%v != %v]", prevValue, n.Value, prevIndex, n.ModifiedIndex) + s.Stats.Inc(CompareAndSwapFail) + return nil, etcdErr.NewError(etcdErr.EcodeTestFailed, cause, s.CurrentIndex) } - cause := fmt.Sprintf("[%v != %v] [%v != %v]", prevValue, n.Value, prevIndex, n.ModifiedIndex) - s.Stats.Inc(CompareAndSwapFail) - return nil, etcdErr.NewError(etcdErr.EcodeTestFailed, cause, s.CurrentIndex) + // update etcd index + s.CurrentIndex++ + + e := newEvent(CompareAndSwap, nodePath, s.CurrentIndex, n.CreatedIndex) + eNode := e.Node + + eNode.PrevValue = n.Value + + // if test succeed, write the value + n.Write(value, s.CurrentIndex) + n.UpdateTTL(expireTime) + + eNode.Value = value + eNode.Expiration, eNode.TTL = n.ExpirationAndTTL() + + s.WatcherHub.notify(e) + s.Stats.Inc(CompareAndSwapSuccess) + return e, nil } // Delete function deletes the node at the given path. @@ -316,28 +316,28 @@ func (s *store) CompareAndDelete(nodePath string, prevValue string, prevIndex ui // If both of the prevValue and prevIndex are given, we will test both of them. // Command will be executed, only if both of the tests are successful. - if n.Compare(prevValue, prevIndex) { - // update etcd index - s.CurrentIndex++ - - e := newEvent(CompareAndDelete, nodePath, s.CurrentIndex, n.CreatedIndex) - - callback := func(path string) { // notify function - // notify the watchers with deleted set true - s.WatcherHub.notifyWatchers(e, path, true) - } - - // delete a key-value pair, no error should happen - n.Remove(false, false, callback) - - s.WatcherHub.notify(e) - s.Stats.Inc(CompareAndDeleteSuccess) - return e, nil + if !n.Compare(prevValue, prevIndex) { + cause := fmt.Sprintf("[%v != %v] [%v != %v]", prevValue, n.Value, prevIndex, n.ModifiedIndex) + s.Stats.Inc(CompareAndDeleteFail) + return nil, etcdErr.NewError(etcdErr.EcodeTestFailed, cause, s.CurrentIndex) } - cause := fmt.Sprintf("[%v != %v] [%v != %v]", prevValue, n.Value, prevIndex, n.ModifiedIndex) - s.Stats.Inc(CompareAndDeleteFail) - return nil, etcdErr.NewError(etcdErr.EcodeTestFailed, cause, s.CurrentIndex) + // update etcd index + s.CurrentIndex++ + + e := newEvent(CompareAndDelete, nodePath, s.CurrentIndex, n.CreatedIndex) + + callback := func(path string) { // notify function + // notify the watchers with deleted set true + s.WatcherHub.notifyWatchers(e, path, true) + } + + // delete a key-value pair, no error should happen + n.Remove(false, false, callback) + + s.WatcherHub.notify(e) + s.Stats.Inc(CompareAndDeleteSuccess) + return e, nil } func (s *store) Watch(key string, recursive bool, sinceIndex uint64) (<-chan *Event, error) { From c36f306a1da4e6b09bb472282adcb06fe71aa975 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Wed, 25 Dec 2013 19:05:40 +0800 Subject: [PATCH 12/12] test(delete_handler_test.go) fix inconsistent between test case and comments --- server/v2/tests/delete_handler_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/v2/tests/delete_handler_test.go b/server/v2/tests/delete_handler_test.go index 1169ab0c1..58702e68a 100644 --- a/server/v2/tests/delete_handler_test.go +++ b/server/v2/tests/delete_handler_test.go @@ -87,7 +87,7 @@ func TestV2DeleteDirectoryRecursiveImpliesDir(t *testing.T) { // Ensures that a key is deleted if the previous index matches // // $ curl -X PUT localhost:4001/v2/keys/foo -d value=XXX -// $ curl -X DELETE localhost:4001/v2/keys/foo?prevIndex=1 +// $ curl -X DELETE localhost:4001/v2/keys/foo?prevIndex=2 // func TestV2DeleteKeyCADOnIndexSuccess(t *testing.T) { tests.RunServer(func(s *server.Server) { @@ -97,7 +97,6 @@ func TestV2DeleteKeyCADOnIndexSuccess(t *testing.T) { tests.ReadBody(resp) resp, err = tests.DeleteForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo?prevIndex=2"), url.Values{}) assert.Nil(t, err, "") - fmt.Println(resp) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["action"], "compareAndDelete", "")