fix(caller): ensure the caller is appropriately defined (#108)

internal/cache/cache.go

@@ -12,7 +12,7 @@ import (
 type ExpiringReadWriter interface {
 	Read(any) error
 	Write(any) error
-	Expired() (bool, error)
+	Expired() bool
 }
 
 type FileCache struct {
@@ -54,18 +54,16 @@ func (c *FileCache) Write(in any) error {
 	return os.WriteFile(c.path, marshalled, 0644)
 }
 
-func (c *FileCache) Expired() (bool, error) {
+func (c *FileCache) Expired() bool {
 	info, err := os.Stat(c.path)
 
 	if err != nil {
-		if errors.Is(err, os.ErrNotExist) {
-			return true, nil
-		}
-		return false, err
+		// We could return the error here, but we will get it again when we try
+		// to read/write the cache, so we can ignore it.
+		return true
 	}
 
 	expiration := info.ModTime().Add(c.ttl)
-	expired := time.Now().After(expiration)
 
-	return expired, nil
+	return time.Now().After(expiration)
 }

internal/cmd/root.go

@@ -126,7 +126,7 @@ func displayRepl(renderCache string, n notifications.Notifications) error {
 		slog.Error("Failed to create an API REST client", "err", err)
 		return err
 	}
-	manager.WithCaller(caller)
+	manager.SetCaller(caller)
 
 	model := normal.New(manager.Actors, n, renderCache, config.Data.Keymap, config.Data.View)
 

internal/cmd/sync.go

@@ -13,7 +13,6 @@ import (
 
 var (
 	noop                 bool
-	force                bool
 	notificationDumpPath string
 
 	refreshStrategy managerPkg.RefreshStrategy
@@ -27,6 +26,12 @@ var (
 
 Use this command when you want to make sure that your notification list is up to
 date with your ruleset.
+
+E.g.:
+  gh-not sync
+  gh-not sync --force-strategy=noop,enrich
+  gh-not sync --refresh-strategy=prevent
+  gh-not sync --from-file=notifications.json
 `,
 		RunE: runSync,
 	}
@@ -35,8 +40,6 @@ date with your ruleset.
 func init() {
 	rootCmd.AddCommand(syncCmd)
 
-	syncCmd.Flags().BoolVarP(&noop, "noop", "n", false, "Doesn't execute any action")
-
 	syncCmd.Flags().VarP(&forceStrategy, "force-strategy", "f", fmt.Sprintf("Force strategy: %s", forceStrategy.Allowed()))
 	syncCmd.Flags().VarP(&refreshStrategy, "refresh-strategy", "r", fmt.Sprintf("Refresh strategy: %s", refreshStrategy.Allowed()))
 
@@ -56,16 +59,22 @@ func runSync(cmd *cobra.Command, args []string) error {
 			return err
 		}
 	}
-	manager.Force = forceStrategy
-	manager.Refresh = refreshStrategy
-	manager.WithCaller(caller)
+	manager.SetCaller(caller)
+
+	manager.ForceStrategy = forceStrategy
+	manager.RefreshStrategy = refreshStrategy
 
 	if err := manager.Load(); err != nil {
 		slog.Error("Failed to load the notifications", "err", err)
 		return err
 	}
 
-	if err := manager.Apply(noop, force); err != nil {
+	if err := manager.Refresh(); err != nil {
+		slog.Error("Failed to refresh the notifications", "err", err)
+		return err
+	}
+
+	if err := manager.Apply(); err != nil {
 		slog.Error("Failed to applying rules", "err", err)
 		return err
 	}

internal/manager/manager.go

@@ -20,8 +20,8 @@ type Manager struct {
 	client        *gh.Client
 	Actors        actors.ActorsMap
 
-	Refresh RefreshStrategy
-	Force   ForceStrategy
+	RefreshStrategy RefreshStrategy
+	ForceStrategy   ForceStrategy
 }
 
 func New(config *config.Data) *Manager {
@@ -33,45 +33,29 @@ func New(config *config.Data) *Manager {
 	return m
 }
 
-func (m *Manager) WithCaller(caller api.Caller) *Manager {
+func (m *Manager) SetCaller(caller api.Caller) {
 	m.client = gh.NewClient(caller, m.cache, m.config.Endpoint)
 	m.Actors = actors.Map(m.client)
-
-	return m
 }
 
 func (m *Manager) Load() error {
-	m.Notifications = notifications.Notifications{}
-
-	cachedNotifications, expired, err := m.loadCache()
-	if err != nil {
+	if err := m.cache.Read(&m.Notifications); err != nil {
 		slog.Warn("cannot read the cache: %#v\n", err)
-	} else if cachedNotifications != nil {
-		m.Notifications = cachedNotifications
-	}
-
-	if m.shouldRefresh(expired) {
-		return m.refreshNotifications()
 	}
 
 	slog.Info("Loaded notifications", "count", len(m.Notifications))
 
 	return nil
 }
 
-func (m *Manager) shouldRefresh(expired bool) bool {
-	if !expired && m.Refresh == ForceRefresh {
-		slog.Info("forcing a refresh")
-		return true
+func (m *Manager) Refresh() error {
+	if m.RefreshStrategy.ShouldRefresh(m.cache.Expired()) {
+		return m.refreshNotifications()
 	}
 
-	if expired && m.Refresh == PreventRefresh {
-		slog.Info("preventing a refresh")
-		return false
-	}
+	slog.Info("Refreshed notifications", "count", len(m.Notifications))
 
-	slog.Debug("refresh", "refresh", expired)
-	return expired
+	return nil
 }
 
 func (m *Manager) refreshNotifications() error {
@@ -87,13 +71,7 @@ func (m *Manager) refreshNotifications() error {
 	}
 
 	m.Notifications = notifications.Sync(m.Notifications, remoteNotifications)
-
-	if err := m.cache.Write(m.Notifications); err != nil {
-		slog.Error("Error while writing the cache: %#v", err)
-	}
-
 	m.Notifications = m.Notifications.Uniq()
-
 	m.Notifications, err = m.Enrich(m.Notifications)
 
 	return err
@@ -105,7 +83,7 @@ func (m *Manager) Save() error {
 
 func (m *Manager) Enrich(ns notifications.Notifications) (notifications.Notifications, error) {
 	for i, n := range ns {
-		if n.Meta.Done && !m.Force.Has(ForceEnrich) {
+		if n.Meta.Done && !m.ForceStrategy.Has(ForceEnrich) {
 			continue
 		}
 
@@ -119,21 +97,7 @@ func (m *Manager) Enrich(ns notifications.Notifications) (notifications.Notifica
 	return ns, nil
 }
 
-func (m *Manager) loadCache() (notifications.Notifications, bool, error) {
-	expired, err := m.cache.Expired()
-	if err != nil {
-		return nil, false, err
-	}
-
-	n := notifications.Notifications{}
-	if err := m.cache.Read(&n); err != nil {
-		return nil, expired, err
-	}
-
-	return n, expired, nil
-}
-
-func (m *Manager) Apply(noop, force bool) error {
+func (m *Manager) Apply() error {
 	for _, rule := range m.config.Rules {
 		actor, ok := m.Actors[rule.Action]
 		if !ok {
@@ -149,12 +113,12 @@ func (m *Manager) Apply(noop, force bool) error {
 		slog.Debug("apply rule", "name", rule.Name, "count", len(selectedIds))
 
 		for _, notification := range m.Notifications.FilterFromIds(selectedIds) {
-			if notification.Meta.Done && !m.Force.Has(ForceApply) {
+			if notification.Meta.Done && !m.ForceStrategy.Has(ForceApply) {
 				slog.Debug("skipping done notification", "id", notification.Id)
 				continue
 			}
 
-			if noop {
+			if m.ForceStrategy.Has(ForceNoop) {
 				fmt.Printf("NOOP'ing action %s on notification %s\n", rule.Action, notification.String())
 				continue
 			}

internal/manager/strategies.go

@@ -2,6 +2,7 @@ package manager
 
 import (
 	"fmt"
+	"log/slog"
 	"strings"
 )
 
@@ -36,6 +37,23 @@ func (r *RefreshStrategy) Allowed() string {
 	return "auto, force, prevent"
 }
 
+func (r RefreshStrategy) ShouldRefresh(expired bool) bool {
+	switch r {
+
+	case ForceRefresh:
+		slog.Info("forcing a refresh")
+		return true
+
+	case PreventRefresh:
+		slog.Info("preventing a refresh")
+		return false
+
+	default:
+		slog.Debug("refresh based on cache expiration", "expired", expired)
+		return expired
+	}
+}
+
 func (r *RefreshStrategy) Set(value string) error {
 	switch value {
 	case "auto":
@@ -64,6 +82,9 @@ const (
 	// even the ones marked as Done.
 	ForceApply ForceStrategy = 1 << iota
 
+	// ForceNoop prevents any Action from being executed.
+	ForceNoop
+
 	// ForceApply forces the enrichment of all notifications, even the ones
 	// marked as Done.
 	ForceEnrich
@@ -80,6 +101,10 @@ func (r ForceStrategy) String() string {
 		s = append(s, "apply")
 	}
 
+	if r.Has(ForceNoop) {
+		s = append(s, "noop")
+	}
+
 	if r.Has(ForceEnrich) {
 		s = append(s, "enrich")
 	}
@@ -88,7 +113,7 @@ func (r ForceStrategy) String() string {
 }
 
 func (r ForceStrategy) Allowed() string {
-	return "apply, enrich"
+	return "apply, noop, enrich"
 }
 
 func (r *ForceStrategy) Set(value string) error {
@@ -98,6 +123,8 @@ func (r *ForceStrategy) Set(value string) error {
 		switch s {
 		case "apply":
 			*r |= ForceApply
+		case "noop":
+			*r |= ForceNoop
 		case "enrich":
 			*r |= ForceEnrich
 		default:

internal/notifications/sync.go

@@ -55,5 +55,7 @@ func Sync(local, remote Notifications) Notifications {
 
 	n.Sort()
 
+	// TODO: add uniq here
+
 	return n
 }