refactor cache handling

- do not update cache when notifications could not be sent
- persist cache (in  separate yaml file)

cmd/config.go

@@ -2,69 +2,18 @@ package cmd
 
 import (
 	"os"
-	"path"
 	"strings"
 
+	"github.com/noerw/osem_notify/utils"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/viper"
 )
 
-/**
- * config file handling, as it is kinda broken in spf13/viper
- * mostly copied from https://github.com/TheThingsNetwork/ttn/blob/f623a6a/ttnctl/util/config.go
- */
-
-// GetConfigFile returns the location of the configuration file.
-// It checks the following (in this order):
-// the --config flag
-// $XDG_CONFIG_HOME/osem_notify/config.yml (if $XDG_CONFIG_HOME is set)
-// $HOME/.osem_notify.yml
-func getConfigFile() string {
-	flag := viper.GetString("config")
-
-	xdg := os.Getenv("XDG_CONFIG_HOME")
-	if xdg != "" {
-		xdg = path.Join(xdg, "osem_notify", "config.yml")
-	}
-
-	home := os.Getenv("HOME")
-	homeyml := ""
-	homeyaml := ""
-
-	if home != "" {
-		homeyml = path.Join(home, ".osem_notify.yml")
-		homeyaml = path.Join(home, ".osem_notify.yaml")
-	}
-
-	try_files := []string{
-		flag,
-		xdg,
-		homeyml,
-		homeyaml,
-	}
-
-	// find a file that exists, and use that
-	for _, file := range try_files {
-		if file != "" {
-			if _, err := os.Stat(file); err == nil {
-				return file
-			}
-		}
-	}
-
-	// no file found, set up correct fallback
-	if os.Getenv("XDG_CONFIG_HOME") != "" {
-		return xdg
-	} else {
-		return homeyml
-	}
-}
-
 // initConfig reads in config file and ENV variables if set.
 func initConfig() {
 	theConfig := cfgFile
 	if cfgFile == "" {
-		theConfig = getConfigFile()
+		theConfig = utils.GetConfigFile("osem_notify")
 	}
 
 	viper.SetConfigType("yaml")
@@ -109,15 +58,3 @@ func validateConfig() {
 		}
 	}
 }
-
-func printConfig() {
-	log.Debug("Using config:")
-	printKV("config file", viper.ConfigFileUsed())
-	for key, val := range viper.AllSettings() {
-		printKV(key, val)
-	}
-}
-
-func printKV(key, val interface{}) {
-	log.Debugf("%20s: %v", key, val)
-}

cmd/debug.go

@@ -4,10 +4,12 @@ import (
 	"fmt"
 	"os"
 
-	"github.com/noerw/osem_notify/core"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
+
+	"github.com/noerw/osem_notify/core"
+	"github.com/noerw/osem_notify/utils"
 )
 
 func init() {
@@ -23,7 +25,7 @@ var debugCmd = &cobra.Command{
 		log.SetLevel(log.DebugLevel)
 	},
 	PersistentPostRun: func(cmd *cobra.Command, args []string) {
-		printConfig()
+		utils.PrintConfig()
 	},
 	Run: func(cmd *cobra.Command, args []string) {
 		cmd.Help()

cmd/root.go

@@ -17,7 +17,7 @@ var rootCmd = &cobra.Command{
 		log.SetOutput(os.Stdout)
 		if viper.GetBool("debug") {
 			log.SetLevel(log.DebugLevel)
-			printConfig()
+			utils.PrintConfig()
 		} else {
 			log.SetLevel(log.InfoLevel)
 		}
@@ -60,7 +60,7 @@ You might want to run 'osem_notify debug notifications' first to verify everythi
 
 func Execute() {
 	// generate documentation
-	// err := doc.GenMarkdownTree(rootCmd, "./doc")
+	// err := doc.GenMarkdownTree(rootCmd, "./docs")
 	// if err != nil {
 	// 	log.Fatal(err)
 	// }

core/cache.go

@@ -0,0 +1,62 @@
+package core
+
+import (
+	"fmt"
+	"os"
+
+	log "github.com/sirupsen/logrus"
+	"github.com/spf13/viper"
+
+	"github.com/noerw/osem_notify/utils"
+)
+
+/**
+ * in memory + yaml persisted cache for check results, ensuring we don't resend
+ * notifications on every check
+
+ * TODO: reminder functionality: extract additional results with Status ERR
+ * from cache with time.Since(lastNotifyDate) > remindAfter.
+ * would require to serialize the full result..
+ */
+
+var cache = viper.New()
+
+func init() {
+	fileName := utils.GetConfigFile("osem_notify_cache")
+
+	cache.SetConfigType("yaml")
+	cache.SetConfigFile(fileName)
+
+	if _, err := os.Stat(fileName); err == nil {
+		err := cache.ReadInConfig()
+		if err != nil {
+			log.Error("Error when reading cache file:", err)
+		}
+	}
+}
+
+func (results BoxCheckResults) filterChangedFromCache() BoxCheckResults {
+	remaining := BoxCheckResults{}
+
+	for box, boxResults := range results {
+		// get results from cache. they are indexed by an event ID per boxId
+		// filter, so that only changed result.Status remain
+		remaining[box] = []CheckResult{}
+		for _, result := range boxResults {
+			cached := cache.GetStringMap(fmt.Sprintf("watchcache.%s.%s", box.Id, result.EventID()))
+			if result.Status != cached["laststatus"] {
+				remaining[box] = append(remaining[box], result)
+			}
+		}
+	}
+
+	return remaining
+}
+
+func updateCache(box *Box, results []CheckResult) error {
+	for _, result := range results {
+		key := fmt.Sprintf("watchcache.%s.%s", box.Id, result.EventID())
+		cache.Set(key+".laststatus", result.Status)
+	}
+	return cache.WriteConfig()
+}

core/checkrunner.go

@@ -10,10 +10,14 @@ import (
 
 type BoxCheckResults map[*Box][]CheckResult
 
-func (results BoxCheckResults) Size() int {
+func (results BoxCheckResults) Size(status string) int {
 	size := 0
 	for _, boxResults := range results {
-		size += len(boxResults)
+		for _, result := range boxResults {
+			if status == result.Status || status == "" {
+				size++
+			}
+		}
 	}
 	return size
 }
@@ -71,7 +75,6 @@ func CheckBoxes(boxIds []string, defaultConf *NotifyConfig) (BoxCheckResults, er
 }
 
 func checkBox(boxId string, defaultConf *NotifyConfig) (*Box, []CheckResult, error) {
-
 	osem := NewOsemClient(viper.GetString("api"))
 
 	// get box data

core/notifiers.go

@@ -6,7 +6,6 @@ import (
 	"time"
 
 	log "github.com/sirupsen/logrus"
-	"github.com/spf13/viper"
 )
 
 var Notifiers = map[string]AbstractNotifier{
@@ -41,21 +40,26 @@ func (box Box) GetNotifier() (AbstractNotifier, error) {
 }
 
 func (results BoxCheckResults) SendNotifications() error {
-	results = results.FilterChangedFromCache(false)
+	// TODO: expose flags to not use cache, and to notify for checks turned CheckOk as well
-	errs := []string{}
 
-	n := results.Size()
+	results = results.filterChangedFromCache()
-	if n == 0 {
+
+	nErr := results.Size(CheckErr)
+	if nErr == 0 {
 		log.Info("No notifications due.")
-		return nil
 	} else {
-		log.Infof("Notifying for %v checks turned bad in total...", results.Size())
+		log.Infof("Notifying for %v checks turned bad in total...", nErr)
 	}
+	log.Debugf("%v checks turned OK!", results.Size(CheckOk))
 
-	// FIXME: only update cache when notifications sent successfully
+	errs := []string{}
-	for box, resultsDue := range results {
+	for box, resultsBox := range results {
-		if len(resultsDue) == 0 {
+		// only submit results which are errors
-			continue
+		resultsDue := []CheckResult{}
+		for _, result := range resultsBox {
+			if result.Status != CheckOk {
+				resultsDue = append(resultsDue, result)
+			}
 		}
 
 		transport := box.NotifyConf.Notifications.Transport
@@ -64,28 +68,40 @@ func (results BoxCheckResults) SendNotifications() error {
 			"transport": transport,
 		})
 
-		notifier, err := box.GetNotifier()
+		if len(resultsDue) != 0 {
-		if err != nil {
+			notifier, err := box.GetNotifier()
-			notifyLog.Error(err)
+			if err != nil {
-			errs = append(errs, err.Error())
+				notifyLog.Error(err)
-			continue
+				errs = append(errs, err.Error())
+				continue
+			}
+
+			notification := notifier.ComposeNotification(box, resultsDue)
+
+			var submitErr error
+			submitErr = notifier.Submit(notification)
+			for retry := 1; submitErr != nil && retry < 3; retry++ {
+				time.Sleep(10 * time.Second)
+				notifyLog.Infof("trying to submit (retry %v)", retry)
+			}
+			if submitErr != nil {
+				notifyLog.Error(submitErr)
+				errs = append(errs, submitErr.Error())
+				continue
+			}
 		}
 
-		notification := notifier.ComposeNotification(box, resultsDue)
+		// update cache (also with CheckOk results to reset status)
-
+		notifyLog.Debug("updating cache")
-		var submitErr error
+		cacheError := updateCache(box, resultsBox)
-		submitErr = notifier.Submit(notification)
+		if cacheError != nil {
-		for retry := 1; submitErr != nil && retry < 3; retry++ {
+			notifyLog.Error("could not cache notification results: ", cacheError)
-			time.Sleep(10 * time.Second)
+			errs = append(errs, cacheError.Error())
-			notifyLog.Debugf("trying to submit (retry %v)", retry)
-		}
-		if submitErr != nil {
-			notifyLog.Error(submitErr)
-			errs = append(errs, submitErr.Error())
-			continue
 		}
 
-		notifyLog.Infof("Sent notification for %s via %s with %v new issues", box.Name, transport, len(resultsDue))
+		if len(resultsDue) != 0 {
+			notifyLog.Infof("Sent notification for %s via %s with %v new issues", box.Name, transport, len(resultsDue))
+		}
 	}
 
 	if len(errs) != 0 {
@@ -93,36 +109,3 @@ func (results BoxCheckResults) SendNotifications() error {
 	}
 	return nil
 }
-
-func (results BoxCheckResults) FilterChangedFromCache(keepOk bool) BoxCheckResults {
-	remaining := BoxCheckResults{}
-
-	for box, boxResults := range results {
-		// get results from cache. they are indexed by an event ID per boxId
-		// filter, so that only changed result.Status remain
-		remaining[box] = []CheckResult{}
-		for _, result := range boxResults {
-			cached := viper.GetStringMap(fmt.Sprintf("watchcache.%s.%s", box.Id, result.EventID()))
-			if result.Status != cached["laststatus"] {
-				if result.Status != CheckOk || keepOk {
-					remaining[box] = append(remaining[box], result)
-				}
-			}
-		}
-
-		// TODO: reminder functionality: extract additional results with Status ERR
-		// from cache with time.Since(lastNotifyDate) > remindAfter.
-		// would require to serialize the full result..
-	}
-
-	// upate cache, setting lastNotifyDate to Now()
-	for box, boxResults := range results {
-		for _, result := range boxResults {
-			// FIXME: somehow this is not persisted?
-			key := fmt.Sprintf("watchcache.%s.%s", box.Id, result.EventID())
-			viper.Set(key+".laststatus", result.Status)
-		}
-	}
-
-	return remaining
-}

utils/config.go

@@ -0,0 +1,72 @@
+package utils
+
+import (
+	"os"
+	"path"
+
+	log "github.com/sirupsen/logrus"
+	"github.com/spf13/viper"
+)
+
+/**
+ * config file handling, as it is kinda broken in spf13/viper
+ * mostly copied from https://github.com/TheThingsNetwork/ttn/blob/f623a6a/ttnctl/util/config.go
+ */
+
+// GetConfigFile returns the location of the configuration file.
+// It checks the following (in this order):
+// the --config flag
+// $XDG_CONFIG_HOME/osem_notify/config.yml (if $XDG_CONFIG_HOME is set)
+// $HOME/.osem_notify.yml
+func GetConfigFile(name string) string {
+	flag := viper.GetString("config")
+
+	xdg := os.Getenv("XDG_CONFIG_HOME")
+	if xdg != "" {
+		xdg = path.Join(xdg, name, "config.yml")
+	}
+
+	home := os.Getenv("HOME")
+	homeyml := ""
+	homeyaml := ""
+
+	if home != "" {
+		homeyml = path.Join(home, "."+name+".yml")
+		homeyaml = path.Join(home, "."+name+".yaml")
+	}
+
+	tryFiles := []string{
+		flag,
+		xdg,
+		homeyml,
+		homeyaml,
+	}
+
+	// find a file that exists, and use that
+	for _, file := range tryFiles {
+		if file != "" {
+			if _, err := os.Stat(file); err == nil {
+				return file
+			}
+		}
+	}
+
+	// no file found, set up correct fallback
+	if os.Getenv("XDG_CONFIG_HOME") != "" {
+		return xdg
+	} else {
+		return homeyml
+	}
+}
+
+func PrintConfig() {
+	log.Debug("Using config:")
+	printKV("config file", viper.ConfigFileUsed())
+	for key, val := range viper.AllSettings() {
+		printKV(key, val)
+	}
+}
+
+func printKV(key, val interface{}) {
+	log.Debugf("%20s: %v", key, val)
+}