From e398da13caea1c609a5575d9c44ec76d085af08c Mon Sep 17 00:00:00 2001
From: Russell Troxel <russelltroxel@gmail.com>
Date: Thu, 25 Jan 2024 10:29:47 -0800
Subject: [PATCH] fix(#252): Handle an empty Server Stat map returned from Sab.
 (#259)

Signed-off-by: Russell Troxel <russell@troxel.io>
---
 internal/sabnzbd/collector/cache.go      | 21 ++++++---
 internal/sabnzbd/collector/cache_test.go | 55 ++++++++++++++++++++++++
 internal/sabnzbd/collector/collector.go  |  4 +-
 internal/sabnzbd/model/model.go          |  4 ++
 4 files changed, 76 insertions(+), 8 deletions(-)

diff --git a/internal/sabnzbd/collector/cache.go b/internal/sabnzbd/collector/cache.go
index 04f09a9..3e5341b 100644
--- a/internal/sabnzbd/collector/cache.go
+++ b/internal/sabnzbd/collector/cache.go
@@ -1,13 +1,14 @@
 package collector
 
 import (
+	"errors"
 	"sync"
 
 	"github.com/onedr0p/exportarr/internal/sabnzbd/model"
 )
 
 type ServerStats interface {
-	Update(stat model.ServerStat) ServerStats
+	Update(stat model.ServerStat) (ServerStats, error)
 	GetTotal() int
 	GetArticlesTried() int
 	GetArticlesSuccess() int
@@ -22,7 +23,12 @@ type serverStatCache struct {
 	todayKey                  string
 }
 
-func (s serverStatCache) Update(stat model.ServerStat) ServerStats {
+func (s serverStatCache) Update(stat model.ServerStat) (ServerStats, error) {
+	if stat.DayParsed == "" && s.todayKey != "" {
+		// If the day parsed is empty, it means there are no server side stats.
+		// If we have exportarr stats, something likely went wrong,
+		return s, errors.New("No Parsed Dates from Server, but cache is not empty")
+	}
 	s.total = stat.Total
 
 	if stat.DayParsed != s.todayKey {
@@ -36,7 +42,7 @@ func (s serverStatCache) Update(stat model.ServerStat) ServerStats {
 	s.articlesTriedToday = stat.ArticlesTried
 	s.articlesSuccessToday = stat.ArticlesSuccess
 
-	return s
+	return s, nil
 }
 
 func (s serverStatCache) GetTotal() int {
@@ -63,7 +69,7 @@ func NewServersStatsCache() *ServersStatsCache {
 	}
 }
 
-func (c *ServersStatsCache) Update(stats model.ServerStats) {
+func (c *ServersStatsCache) Update(stats model.ServerStats) error {
 	c.lock.Lock()
 	defer c.lock.Unlock()
 
@@ -75,8 +81,13 @@ func (c *ServersStatsCache) Update(stats model.ServerStats) {
 			toCache = cached
 		}
 
-		c.Servers[name] = toCache.Update(srv).(serverStatCache)
+		updated, err := toCache.Update(srv)
+		if err != nil {
+			return err
+		}
+		c.Servers[name] = updated.(serverStatCache)
 	}
+	return nil
 }
 
 func (c *ServersStatsCache) GetTotal() int {
diff --git a/internal/sabnzbd/collector/cache_test.go b/internal/sabnzbd/collector/cache_test.go
index 6ad26fd..5993caf 100644
--- a/internal/sabnzbd/collector/cache_test.go
+++ b/internal/sabnzbd/collector/cache_test.go
@@ -137,6 +137,61 @@ func TestUpdateServerStatsCache_DifferentDay(t *testing.T) {
 	require.Equal(12, server2.GetArticlesSuccess())
 }
 
+func TestUpdateServerStatsCache_EmptyServerStats(t *testing.T) {
+	tests := []struct {
+		name          string
+		startingStats model.ServerStats
+		endingStats   model.ServerStats
+		shouldError   bool
+	}{
+		{
+			name: "Empty Starting Date",
+			startingStats: model.ServerStats{
+				Total:   1,
+				Servers: map[string]model.ServerStat{},
+			},
+		},
+		{
+			name: "Non-Empty Starting Date",
+			startingStats: model.ServerStats{
+				Total: 1,
+				Servers: map[string]model.ServerStat{
+					"server1": {
+						Total:           1,
+						ArticlesTried:   2,
+						ArticlesSuccess: 2,
+						DayParsed:       "2020-01-01",
+					},
+				},
+			},
+			shouldError: true,
+		},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			require := require.New(t)
+			cache := NewServersStatsCache()
+			err := cache.Update(tt.startingStats)
+			require.NoError(err)
+			err = cache.Update(model.ServerStats{
+				Total: 1,
+				Servers: map[string]model.ServerStat{
+					"server1": {
+						Total:     1,
+						DayParsed: "",
+					},
+				},
+			})
+			if tt.shouldError {
+				require.Error(err)
+			} else {
+				require.NoError(err)
+			}
+		})
+	}
+}
+
 func TestNewServerStatsCache_SetsServers(t *testing.T) {
 	require := require.New(t)
 	cache := NewServersStatsCache()
diff --git a/internal/sabnzbd/collector/collector.go b/internal/sabnzbd/collector/collector.go
index 49827d8..af31f5e 100644
--- a/internal/sabnzbd/collector/collector.go
+++ b/internal/sabnzbd/collector/collector.go
@@ -276,9 +276,7 @@ func (e *SabnzbdCollector) Collect(ch chan<- prometheus.Metric) {
 			return fmt.Errorf("failed to get server stats: %w", err)
 		}
 
-		e.cache.Update(*serverStats)
-
-		return nil
+		return e.cache.Update(*serverStats)
 	})
 
 	if err := g.Wait(); err != nil {
diff --git a/internal/sabnzbd/model/model.go b/internal/sabnzbd/model/model.go
index dbe59cb..274ea92 100644
--- a/internal/sabnzbd/model/model.go
+++ b/internal/sabnzbd/model/model.go
@@ -166,6 +166,10 @@ func (q *QueueStats) UnmarshalJSON(data []byte) error {
 
 // latestStat gets the most recent date's value from a map of dates to values
 func latestStat(m map[string]int) (string, int) {
+	if len(m) == 0 {
+		return "", 0
+	}
+
 	keys := make([]string, 0, len(m))
 	for k := range m {
 		keys = append(keys, k)