Browse Source

fix(store): move logic to handle whether or not to notify (re: hidden keys) entirely into watcher hub

tobz 12 years ago
parent
commit
7a948746a8
3 changed files with 21 additions and 19 deletions
  1. 4 12
      store/store.go
  2. 15 6
      store/store_test.go
  3. 2 1
      store/watcher_hub.go

+ 4 - 12
store/store.go

@@ -289,9 +289,7 @@ func (s *store) Delete(nodePath string, dir, recursive bool) (*Event, error) {
 	// update etcd index
 	s.CurrentIndex++
 
-	if !n.IsHidden() {
-		s.WatcherHub.notify(e)
-	}
+	s.WatcherHub.notify(e)
 
 	s.Stats.Inc(DeleteSuccess)
 
@@ -432,9 +430,7 @@ func (s *store) Update(nodePath string, newValue string, expireTime time.Time) (
 
 	eNode.Expiration, eNode.TTL = n.ExpirationAndTTL()
 
-	if !n.IsHidden() {
-		s.WatcherHub.notify(e)
-	}
+	s.WatcherHub.notify(e)
 
 	s.Stats.Inc(UpdateSuccess)
 
@@ -518,9 +514,7 @@ func (s *store) internalCreate(nodePath string, dir bool, value string, unique,
 
 	s.CurrentIndex = nextIndex
 
-	if !n.IsHidden() {
-		s.WatcherHub.notify(e)
-	}
+	s.WatcherHub.notify(e)
 
 	return e, nil
 }
@@ -577,9 +571,7 @@ func (s *store) DeleteExpiredKeys(cutoff time.Time) {
 
 		s.Stats.Inc(ExpireCount)
 
-		if !node.IsHidden() {
-			s.WatcherHub.notify(e)
-		}
+		s.WatcherHub.notify(e)
 	}
 
 }

+ 15 - 6
store/store_test.go

@@ -499,12 +499,15 @@ func TestStoreWatchCreate(t *testing.T) {
 	assert.Nil(t, e, "")
 }
 
-// Ensure that the store doesn't see hidden key creations.
+// Ensure that the store can watch for hidden keys as long as it's an exact path match.
 func TestStoreWatchCreateWithHiddenKey(t *testing.T) {
 	s := newStore()
 	w, _ := s.Watch("/_foo", false, false, 0)
 	s.Create("/_foo", false, "bar", false, Permanent)
 	e := nbselect(w.EventChan)
+	assert.Equal(t, e.Action, "create", "")
+	assert.Equal(t, e.Node.Key, "/_foo", "")
+	e = nbselect(w.EventChan)
 	assert.Nil(t, e, "")
 }
 
@@ -518,7 +521,7 @@ func TestStoreWatchRecursiveCreate(t *testing.T) {
 	assert.Equal(t, e.Node.Key, "/foo/bar", "")
 }
 
-// Ensure that the store can watch for recursive key creation.
+// Ensure that the store doesn't see hidden key creates without an exact path match in recursive mode.
 func TestStoreWatchRecursiveCreateWithHiddenKey(t *testing.T) {
 	s := newStore()
 	w, _ := s.Watch("/foo", true, false, 0)
@@ -545,6 +548,9 @@ func TestStoreWatchUpdateWithHiddenKey(t *testing.T) {
 	w, _ := s.Watch("/_foo", false, false, 0)
 	s.Update("/_foo", "baz", Permanent)
 	e := nbselect(w.EventChan)
+	assert.Equal(t, e.Action, "update", "")
+	assert.Equal(t, e.Node.Key, "/_foo", "")
+	e = nbselect(w.EventChan)
 	assert.Nil(t, e, "")
 }
 
@@ -559,7 +565,7 @@ func TestStoreWatchRecursiveUpdate(t *testing.T) {
 	assert.Equal(t, e.Node.Key, "/foo/bar", "")
 }
 
-// Ensure that the store doesn't get recursive key updates for hidden keys.
+// Ensure that the store doesn't see hidden key updates without an exact path match in recursive mode.
 func TestStoreWatchRecursiveUpdateWithHiddenKey(t *testing.T) {
 	s := newStore()
 	s.Create("/foo/_bar", false, "baz", false, Permanent)
@@ -580,13 +586,16 @@ func TestStoreWatchDelete(t *testing.T) {
 	assert.Equal(t, e.Node.Key, "/foo", "")
 }
 
-// Ensure that the store doesn't see hidden key deletions.
+// Ensure that the store can watch for key deletions.
 func TestStoreWatchDeleteWithHiddenKey(t *testing.T) {
 	s := newStore()
 	s.Create("/_foo", false, "bar", false, Permanent)
-	w, _ := s.Watch("/foo", false, false, 0)
+	w, _ := s.Watch("/_foo", false, false, 0)
 	s.Delete("/_foo", false, false)
 	e := nbselect(w.EventChan)
+	assert.Equal(t, e.Action, "delete", "")
+	assert.Equal(t, e.Node.Key, "/_foo", "")
+	e = nbselect(w.EventChan)
 	assert.Nil(t, e, "")
 }
 
@@ -601,7 +610,7 @@ func TestStoreWatchRecursiveDelete(t *testing.T) {
 	assert.Equal(t, e.Node.Key, "/foo/bar", "")
 }
 
-// Ensure that the store can watch for recursive key deletions.
+// Ensure that the store doesn't see hidden key deletes without an exact path match in recursive mode.
 func TestStoreWatchRecursiveDeleteWithHiddenKey(t *testing.T) {
 	s := newStore()
 	s.Create("/foo/_bar", false, "baz", false, Permanent)

+ 2 - 1
store/watcher_hub.go

@@ -126,7 +126,8 @@ func (wh *watcherHub) notifyWatchers(e *Event, nodePath string, deleted bool) {
 
 			w, _ := curr.Value.(*Watcher)
 
-			if !isHidden(nodePath) && w.notify(e, e.Node.Key == nodePath, deleted) {
+			originalPath := (e.Node.Key == nodePath)
+			if (originalPath || !isHidden(e.Node.Key)) && w.notify(e, originalPath, deleted) {
 				if !w.stream { // do not remove the stream watcher
 					// if we successfully notify a watcher
 					// we need to remove the watcher from the list