fix(upgrade): avoid duplicate HTTP requests

src/segments/upgrade.go

@@ -2,8 +2,6 @@ package segments
 
 import (
 	"encoding/json"
-	"errors"
-	"fmt"
 
 	"github.com/jandedobbeleer/oh-my-posh/src/platform"
 	"github.com/jandedobbeleer/oh-my-posh/src/properties"
@@ -34,53 +32,59 @@ func (u *Upgrade) Init(props properties.Properties, env platform.Environment) {
 }
 
 func (u *Upgrade) Enabled() bool {
-	version := fmt.Sprintf("v%s", u.env.Flags().Version)
-
-	if shouldDisplay, err := u.ShouldDisplay(version); err == nil {
-		return shouldDisplay
+	current := u.env.Flags().Version
+	latest := u.cachedLatest(current)
+	if len(latest) == 0 {
+		latest = u.checkUpdate(current)
 	}
 
-	latest, err := upgrade.Latest(u.env)
-	if err != nil {
+	if len(latest) == 0 || current == latest {
 		return false
 	}
 
-	if latest == version {
-		return false
-	}
-
-	cacheData := &UpgradeCache{
-		Latest:  latest,
-		Current: version,
-	}
-	cacheJSON, err := json.Marshal(cacheData)
-	if err != nil {
-		return false
-	}
-
-	oneWeek := 10080
-	cacheTimeout := u.props.GetInt(properties.CacheTimeout, oneWeek)
-	u.env.Cache().Set(UPGRADECACHEKEY, string(cacheJSON), cacheTimeout)
-
 	u.Version = latest
 	return true
 }
 
-func (u *Upgrade) ShouldDisplay(version string) (bool, error) {
-	data, OK := u.env.Cache().Get(UPGRADECACHEKEY)
-	if !OK {
-		return false, errors.New("no cache")
+func (u *Upgrade) cachedLatest(current string) string {
+	data, ok := u.env.Cache().Get(UPGRADECACHEKEY)
+	if !ok {
+		return "" // no cache
 	}
 
 	var cacheJSON UpgradeCache
 	err := json.Unmarshal([]byte(data), &cacheJSON)
 	if err != nil {
-		return false, errors.New("invalid cache data")
+		return "" // invalid cache data
 	}
 
-	if version == cacheJSON.Current {
-		return cacheJSON.Current != cacheJSON.Latest, nil
+	if current != cacheJSON.Current {
+		return "" // version changed, run the check again
 	}
 
-	return false, errors.New("version changed, run the check again")
+	return cacheJSON.Latest
+}
+
+func (u *Upgrade) checkUpdate(current string) string {
+	tag, err := upgrade.Latest(u.env)
+	if err != nil {
+		return ""
+	}
+
+	latest := tag[1:]
+	cacheData := &UpgradeCache{
+		Latest:  latest,
+		Current: current,
+	}
+	cacheJSON, err := json.Marshal(cacheData)
+	if err != nil {
+		return ""
+	}
+
+	oneWeek := 10080
+	cacheTimeout := u.props.GetInt(properties.CacheTimeout, oneWeek)
+	// update cache
+	u.env.Cache().Set(UPGRADECACHEKEY, string(cacheJSON), cacheTimeout)
+
+	return latest
 }

src/segments/upgrade_test.go

@@ -21,7 +21,7 @@ func TestUpgrade(t *testing.T) {
 		HasCache        bool
 		CurrentVersion  string
 		LatestVersion   string
-		CacheVersion    string
+		CachedVersion   string
 		Error           error
 	}{
 		{
@@ -36,7 +36,7 @@ func TestUpgrade(t *testing.T) {
 			LatestVersion:  "1.0.1",
 		},
 		{
-			Case:  "Version error",
+			Case:  "Error on update check",
 			Error: errors.New("error"),
 		},
 		{
@@ -44,7 +44,7 @@ func TestUpgrade(t *testing.T) {
 			HasCache:        true,
 			CurrentVersion:  "1.0.2",
 			LatestVersion:   "1.0.3",
-			CacheVersion:    "1.0.2",
+			CachedVersion:   "1.0.2",
 			ExpectedEnabled: true,
 		},
 		{
@@ -52,14 +52,14 @@ func TestUpgrade(t *testing.T) {
 			HasCache:       true,
 			CurrentVersion: "1.0.2",
 			LatestVersion:  "1.0.2",
-			CacheVersion:   "1.0.1",
+			CachedVersion:  "1.0.1",
 		},
 		{
 			Case:            "On previous, version changed",
 			HasCache:        true,
 			CurrentVersion:  "1.0.2",
 			LatestVersion:   "1.0.3",
-			CacheVersion:    "1.0.1",
+			CachedVersion:   "1.0.1",
 			ExpectedEnabled: true,
 		},
 	}
@@ -69,10 +69,10 @@ func TestUpgrade(t *testing.T) {
 		cache := &mock.MockedCache{}
 
 		env.On("Cache").Return(cache)
-		if len(tc.CacheVersion) == 0 {
-			tc.CacheVersion = tc.CurrentVersion
+		if len(tc.CachedVersion) == 0 {
+			tc.CachedVersion = tc.CurrentVersion
 		}
-		cacheData := fmt.Sprintf(`{"latest":"v%s", "current": "v%s"}`, tc.LatestVersion, tc.CacheVersion)
+		cacheData := fmt.Sprintf(`{"latest":"%s", "current": "%s"}`, tc.LatestVersion, tc.CachedVersion)
 		cache.On("Get", UPGRADECACHEKEY).Return(cacheData, tc.HasCache)
 		cache.On("Set", mock2.Anything, mock2.Anything, mock2.Anything)