From 64e182c69e30ed6cd51871d59219da5f7b0b3efc Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Fri, 25 Dec 2015 02:58:46 -0800 Subject: [PATCH] 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 | 7 +- store/node.go | 72 +++++++------ store/node_test.go | 247 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 282 insertions(+), 44 deletions(-) create mode 100644 store/node_test.go diff --git a/store/event.go b/store/event.go index 59a1dc654..8b1cfc02a 100644 --- a/store/event.go +++ b/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 { diff --git a/store/node.go b/store/node.go index 0ad9abd4c..34cff727c 100644 --- a/store/node.go +++ b/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) } - } diff --git a/store/node_test.go b/store/node_test.go new file mode 100644 index 000000000..42934639c --- /dev/null +++ b/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 +}