From 76b65072de75ddd3f10e4ea3967161d865e37b9a Mon Sep 17 00:00:00 2001
From: Victor Agababov <vagababov@gmail.com>
Date: Mon, 22 Jun 2020 14:39:26 -0700
Subject: [PATCH] Improve manual watcher a bit (#1435)

- remove redundant checks (iterating nil/empty slice works)
- fix comments
- remove duplicate test
- map scale size fix.
---
 configmap/manual_watcher.go      | 14 ++++----------
 configmap/manual_watcher_test.go | 28 +++++++---------------------
 2 files changed, 11 insertions(+), 31 deletions(-)

diff --git a/configmap/manual_watcher.go b/configmap/manual_watcher.go
index ad39bb8b9..b1eb27633 100644
--- a/configmap/manual_watcher.go
+++ b/configmap/manual_watcher.go
@@ -26,9 +26,8 @@ import (
 type ManualWatcher struct {
 	Namespace string
 
-	// Guards mutations to defaultImpl fields
-	m sync.RWMutex
-
+	// Guards observers
+	m         sync.RWMutex
 	observers map[string][]Observer
 }
 
@@ -40,7 +39,7 @@ func (w *ManualWatcher) Watch(name string, o ...Observer) {
 	defer w.m.Unlock()
 
 	if w.observers == nil {
-		w.observers = make(map[string][]Observer, len(o))
+		w.observers = make(map[string][]Observer, 1)
 	}
 	w.observers[name] = append(w.observers[name], o...)
 }
@@ -58,13 +57,8 @@ func (w *ManualWatcher) OnChange(configMap *corev1.ConfigMap) {
 	// Within our namespace, take the lock and see if there are any registered observers.
 	w.m.RLock()
 	defer w.m.RUnlock()
-	observers, ok := w.observers[configMap.Name]
-	if !ok {
-		return // No observers.
-	}
-
 	// Iterate over the observers and invoke their callbacks.
-	for _, o := range observers {
+	for _, o := range w.observers[configMap.Name] {
 		o(configMap)
 	}
 }
diff --git a/configmap/manual_watcher_test.go b/configmap/manual_watcher_test.go
index 72f35ee37..dfe0c64e9 100644
--- a/configmap/manual_watcher_test.go
+++ b/configmap/manual_watcher_test.go
@@ -37,6 +37,13 @@ func TestCallbackInvoked(t *testing.T) {
 		Namespace: "default",
 	}
 
+	// Verify empty works as designed.
+	watcher.OnChange(&corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "default",
+			Name:      "foo",
+		},
+	})
 	observer := counter{}
 
 	watcher.Watch("foo", observer.callback)
@@ -72,27 +79,6 @@ func TestDifferentNamespace(t *testing.T) {
 	}
 }
 
-func TestLateRegistration(t *testing.T) {
-	watcher := ManualWatcher{
-		Namespace: "default",
-	}
-
-	observer := counter{}
-
-	watcher.OnChange(&corev1.ConfigMap{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: "default",
-			Name:      "foo",
-		},
-	})
-
-	watcher.Watch("foo", observer.callback)
-
-	if observer.count() != 0 {
-		t.Errorf("Expected callback to be not be invoked - got invocations %v", observer.count())
-	}
-}
-
 func TestDifferentConfigName(t *testing.T) {
 	watcher := ManualWatcher{
 		Namespace: "default",