From b3c3e38574df1d53c37fbf9c7fe0128b2c76c35e Mon Sep 17 00:00:00 2001
From: crozzy <joseph.crosland@gmail.com>
Date: Thu, 19 Aug 2021 14:21:57 -0700
Subject: [PATCH] matcher: default garbage collection on

Change the GC logic to reflect the docs and
also update the docs with more details.
< 0 == GC off
0 or unspecified == GC on w/ default retention
< 2 == GC on w/ default retention
>= 2 == GC on w/ specified value

Signed-off-by: crozzy <joseph.crosland@gmail.com>
---
 Documentation/reference/config.md |  3 +-
 config/config_test.go             | 96 +++++++++++++++++++++++++++++++
 config/matcher.go                 |  7 ++-
 3 files changed, 104 insertions(+), 2 deletions(-)

diff --git a/Documentation/reference/config.md b/Documentation/reference/config.md
index 13c8086c52..67a487006e 100644
--- a/Documentation/reference/config.md
+++ b/Documentation/reference/config.md
@@ -216,7 +216,8 @@ constraints.
 
 Defaults to 10.
 
-If a value of 0 is provided, GC is disabled.
+If a value less than 0 is provided, GC is disabled. 2 is the minimum value to
+ensure updates can be compared for notifications. 
 
 ### `$.matchers`
 Matchers provides configuration for the in-tree Matchers and RemoteMatchers.
diff --git a/config/config_test.go b/config/config_test.go
index 4df24488fc..75916a4773 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -70,6 +70,102 @@ func Test_Config_Validate_Failure(t *testing.T) {
 	}
 }
 
+func TestConfigUpateRetention(t *testing.T) {
+	var table = []struct {
+		name              string
+		conf              config.Config
+		expectedRetention int
+	}{
+		{
+			name:              "Retention less than 0",
+			expectedRetention: 0,
+			conf: config.Config{
+				Mode:           config.ComboMode,
+				HTTPListenAddr: "localhost:8080",
+				Indexer: config.Indexer{
+					ConnString: "example@example/db",
+				},
+				Notifier: config.Notifier{
+					ConnString: "example@example/db",
+				},
+				Matcher: config.Matcher{
+					ConnString:      "example@example/db",
+					IndexerAddr:     "example@example/db",
+					UpdateRetention: -1,
+				},
+			},
+		},
+		{
+			name:              "Retention of 0",
+			expectedRetention: 10,
+			conf: config.Config{
+				Mode:           config.ComboMode,
+				HTTPListenAddr: "localhost:8080",
+				Indexer: config.Indexer{
+					ConnString: "example@example/db",
+				},
+				Notifier: config.Notifier{
+					ConnString: "example@example/db",
+				},
+				Matcher: config.Matcher{
+					ConnString:      "example@example/db",
+					IndexerAddr:     "example@example/db",
+					UpdateRetention: 0,
+				},
+			},
+		},
+		{
+			name:              "Retention less than 2",
+			expectedRetention: 10,
+			conf: config.Config{
+				Mode:           config.ComboMode,
+				HTTPListenAddr: "localhost:8080",
+				Indexer: config.Indexer{
+					ConnString: "example@example/db",
+				},
+				Notifier: config.Notifier{
+					ConnString: "example@example/db",
+				},
+				Matcher: config.Matcher{
+					ConnString:      "example@example/db",
+					IndexerAddr:     "example@example/db",
+					UpdateRetention: 1,
+				},
+			},
+		},
+		{
+			name:              "Retention of 2",
+			expectedRetention: 2,
+			conf: config.Config{
+				Mode:           config.ComboMode,
+				HTTPListenAddr: "localhost:8080",
+				Indexer: config.Indexer{
+					ConnString: "example@example/db",
+				},
+				Notifier: config.Notifier{
+					ConnString: "example@example/db",
+				},
+				Matcher: config.Matcher{
+					ConnString:      "example@example/db",
+					IndexerAddr:     "example@example/db",
+					UpdateRetention: 2,
+				},
+			},
+		},
+	}
+	for _, tab := range table {
+		t.Run(tab.name, func(t *testing.T) {
+			err := config.Validate(&tab.conf)
+			if err != nil {
+				log.Fatalf("expected no errors but got: %s, for test case: %s", err, tab.name)
+			}
+			if tab.conf.Matcher.UpdateRetention != tab.expectedRetention {
+				t.Fatalf("expected UpdateRetention of %d but got %d", tab.expectedRetention, tab.conf.Matcher.UpdateRetention)
+			}
+		})
+	}
+}
+
 func TestConfigDisableUpdaters(t *testing.T) {
 	var table = []struct {
 		name string
diff --git a/config/matcher.go b/config/matcher.go
index 304bc5ebae..e9bb151794 100644
--- a/config/matcher.go
+++ b/config/matcher.go
@@ -65,7 +65,12 @@ func (m *Matcher) Validate(combo bool) error {
 	if m.Period == 0 {
 		m.Period = DefaultPeriod
 	}
-	if m.UpdateRetention == 1 || m.UpdateRetention < 0 {
+	switch {
+	case m.UpdateRetention < 0:
+		// Less than 0 means GC is off.
+		m.UpdateRetention = 0
+	case m.UpdateRetention < 2:
+		// Anything less than 2 gets the default.
 		m.UpdateRetention = DefaultRetention
 	}
 	if m.CacheAge == 0 {