fix(upgrade): only run when cache expires

resolves #6071

src/cli/init.go

@@ -78,10 +78,11 @@ func runInit(sh string) {
 	cfg := config.Load(configFile, sh, false)
 
 	flags := &runtime.Flags{
-		Shell:  sh,
-		Config: configFile,
-		Strict: strict,
-		Debug:  debug,
+		Shell:     sh,
+		Config:    configFile,
+		Strict:    strict,
+		Debug:     debug,
+		SaveCache: true,
 	}
 
 	env := &runtime.Terminal{}

src/config/config.go

@@ -100,23 +100,7 @@ func (cfg *Config) Features(env runtime.Environment) shell.Features {
 		feats = append(feats, shell.FTCSMarks)
 	}
 
-	autoUpgrade := cfg.Upgrade.Auto
-	if _, OK := env.Cache().Get(AUTOUPGRADE); OK {
-		autoUpgrade = true
-	}
-
-	upgradeNotice := cfg.Upgrade.DisplayNotice
-	if _, OK := env.Cache().Get(UPGRADENOTICE); OK {
-		upgradeNotice = true
-	}
-
-	if upgradeNotice && !autoUpgrade {
-		feats = append(feats, shell.Notice)
-	}
-
-	if autoUpgrade {
-		feats = append(feats, shell.Upgrade)
-	}
+	feats = append(feats, cfg.UpgradeFeatures(env)...)
 
 	if cfg.ErrorLine != nil || cfg.ValidLine != nil {
 		feats = append(feats, shell.LineError)
@@ -158,3 +142,34 @@ func (cfg *Config) Features(env runtime.Environment) shell.Features {
 
 	return feats
 }
+
+func (cfg *Config) UpgradeFeatures(env runtime.Environment) shell.Features {
+	feats := shell.Features{}
+
+	if _, OK := env.Cache().Get(upgrade.CACHEKEY); OK && !cfg.Upgrade.Force {
+		return feats
+	}
+
+	// always reset the cache key so we respect the interval no matter what the outcome
+	env.Cache().Set(upgrade.CACHEKEY, "", cfg.Upgrade.Interval)
+
+	autoUpgrade := cfg.Upgrade.Auto
+	if _, OK := env.Cache().Get(AUTOUPGRADE); OK {
+		autoUpgrade = true
+	}
+
+	upgradeNotice := cfg.Upgrade.DisplayNotice
+	if _, OK := env.Cache().Get(UPGRADENOTICE); OK {
+		upgradeNotice = true
+	}
+
+	if upgradeNotice && !autoUpgrade {
+		feats = append(feats, shell.Notice)
+	}
+
+	if autoUpgrade {
+		feats = append(feats, shell.Upgrade)
+	}
+
+	return feats
+}

src/config/config_test.go

@@ -4,11 +4,15 @@ import (
 	"testing"
 
 	"github.com/jandedobbeleer/oh-my-posh/src/cache"
+	cache_ "github.com/jandedobbeleer/oh-my-posh/src/cache/mock"
 	"github.com/jandedobbeleer/oh-my-posh/src/color"
 	"github.com/jandedobbeleer/oh-my-posh/src/runtime/mock"
+	"github.com/jandedobbeleer/oh-my-posh/src/shell"
 	"github.com/jandedobbeleer/oh-my-posh/src/template"
+	"github.com/jandedobbeleer/oh-my-posh/src/upgrade"
 
 	"github.com/stretchr/testify/assert"
+	mock_ "github.com/stretchr/testify/mock"
 )
 
 func TestGetPalette(t *testing.T) {
@@ -107,3 +111,85 @@ func TestGetPalette(t *testing.T) {
 		assert.Equal(t, tc.ExpectedPalette, got, tc.Case)
 	}
 }
+func TestUpgradeFeatures(t *testing.T) {
+	cases := []struct {
+		Case                  string
+		ExpectedFeats         shell.Features
+		UpgradeCacheKeyExists bool
+		AutoUpgrade           bool
+		Force                 bool
+		DisplayNotice         bool
+		AutoUpgradeKey        bool
+		NoticeKey             bool
+	}{
+		{
+			Case:                  "cache exists, no force",
+			UpgradeCacheKeyExists: true,
+			ExpectedFeats:         shell.Features{},
+		},
+		{
+			Case:          "auto upgrade enabled",
+			AutoUpgrade:   true,
+			ExpectedFeats: shell.Features{shell.Upgrade},
+		},
+		{
+			Case:           "auto upgrade via cache",
+			AutoUpgradeKey: true,
+			ExpectedFeats:  shell.Features{shell.Upgrade},
+		},
+		{
+			Case:          "notice enabled, no auto upgrade",
+			DisplayNotice: true,
+			ExpectedFeats: shell.Features{shell.Notice},
+		},
+		{
+			Case:          "notice via cache, no auto upgrade",
+			NoticeKey:     true,
+			ExpectedFeats: shell.Features{shell.Notice},
+		},
+		{
+			Case:                  "force upgrade ignores cache",
+			UpgradeCacheKeyExists: true,
+			Force:                 true,
+			AutoUpgrade:           true,
+			ExpectedFeats:         shell.Features{shell.Upgrade},
+		},
+	}
+
+	for _, tc := range cases {
+		env := &mock.Environment{}
+		cache := &cache_.Cache{}
+		env.On("Cache").Return(cache)
+
+		if tc.UpgradeCacheKeyExists {
+			cache.On("Get", upgrade.CACHEKEY).Return("", true)
+		} else {
+			cache.On("Get", upgrade.CACHEKEY).Return("", false)
+		}
+
+		cache.On("Set", upgrade.CACHEKEY, "", mock_.Anything).Return()
+
+		if tc.AutoUpgradeKey {
+			cache.On("Get", AUTOUPGRADE).Return("", true)
+		} else {
+			cache.On("Get", AUTOUPGRADE).Return("", false)
+		}
+
+		if tc.NoticeKey {
+			cache.On("Get", UPGRADENOTICE).Return("", true)
+		} else {
+			cache.On("Get", UPGRADENOTICE).Return("", false)
+		}
+
+		cfg := &Config{
+			Upgrade: &upgrade.Config{
+				Auto:          tc.AutoUpgrade,
+				Force:         tc.Force,
+				DisplayNotice: tc.DisplayNotice,
+			},
+		}
+
+		got := cfg.UpgradeFeatures(env)
+		assert.Equal(t, tc.ExpectedFeats, got, tc.Case)
+	}
+}

src/upgrade/notice.go

@@ -31,11 +31,6 @@ func (cfg *Config) Notice() (string, bool) {
 		return "", false
 	}
 
-	// do not check when last validation was < 1 week ago
-	if _, OK := cfg.Cache.Get(CACHEKEY); OK && !cfg.Force {
-		return "", false
-	}
-
 	if !http.IsConnected() {
 		return "", false
 	}
@@ -45,8 +40,6 @@ func (cfg *Config) Notice() (string, bool) {
 		return "", false
 	}
 
-	cfg.Cache.Set(CACHEKEY, latest, cfg.Interval)
-
 	if latest == build.Version {
 		return "", false
 	}