From d2d7e37990e33a0351eafbbfc13f48e519232e4f Mon Sep 17 00:00:00 2001 From: rick Date: Sun, 1 Dec 2013 13:38:09 -0700 Subject: [PATCH] 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)