Simplify CheckForUpdate handling of state file

samcoe · mislav · commit 42c97509ca18 · 2020-12-15T16:03:37.000+01:00
diff --git a/update/update.go b/update/update.go
@@ -24,45 +24,36 @@ type StateEntry struct {
 
 // CheckForUpdate checks whether this software has had a newer release on GitHub
 func CheckForUpdate(client *api.Client, stateFilePath, repo, currentVersion string) (*ReleaseInfo, error) {
-	releaseInfo, err := getLatestReleaseInfo(client, stateFilePath, repo)
+	stateEntry, _ := getStateEntry(stateFilePath)
+	if stateEntry != nil && time.Since(stateEntry.CheckedForUpdateAt).Hours() < 24 {
+		return nil, nil
+	}
+
+	releaseInfo, err := getLatestReleaseInfo(client, repo)
 	if err != nil {
 		return nil, err
 	}
 
-	if releaseInfo == nil {
-		return nil, nil
+	err = setStateEntry(stateFilePath, time.Now(), *releaseInfo)
+	if err != nil {
+		return nil, err
 	}
 
-	if versionGreaterThan(releaseInfo.LatestRelease.Version, currentVersion) {
-		return &releaseInfo.LatestRelease, nil
+	if versionGreaterThan(releaseInfo.Version, currentVersion) {
+		return releaseInfo, nil
 	}
 
 	return nil, nil
 }
 
-func getLatestReleaseInfo(client *api.Client, stateFilePath, repo string) (*StateEntry, error) {
-	stateEntry, err := getStateEntry(stateFilePath)
-	if err == nil && time.Since(stateEntry.CheckedForUpdateAt).Hours() < 24 {
-		return nil, nil
-	}
-
+func getLatestReleaseInfo(client *api.Client, repo string) (*ReleaseInfo, error) {
 	var latestRelease ReleaseInfo
-	err = client.REST(ghinstance.Default(), "GET", fmt.Sprintf("repos/%s/releases/latest", repo), nil, &latestRelease)
-	if err != nil {
-		return nil, err
-	}
-
-	err = setStateEntry(stateFilePath, time.Now(), latestRelease)
-	if err != nil {
-		return nil, err
-	}
-
-	stateEntry, err = getStateEntry(stateFilePath)
+	err := client.REST(ghinstance.Default(), "GET", fmt.Sprintf("repos/%s/releases/latest", repo), nil, &latestRelease)
 	if err != nil {
 		return nil, err
 	}
 
-	return stateEntry, nil
+	return &latestRelease, nil
 }
 
 func getStateEntry(stateFilePath string) (*StateEntry, error) {
diff --git a/update/update_test.go b/update/update_test.go
@@ -52,17 +52,14 @@ func TestCheckForUpdate(t *testing.T) {
 
 	for _, s := range scenarios {
 		t.Run(s.Name, func(t *testing.T) {
-			stateFilePath := tempFilePath()
-			defer os.Remove(stateFilePath)
-
 			http := &httpmock.Registry{}
 			client := api.NewClient(api.ReplaceTripper(http))
 			http.StubResponse(200, bytes.NewBufferString(fmt.Sprintf(`{
 				"tag_name": "%s",
 				"html_url": "%s"
 			}`, s.LatestVersion, s.LatestURL)))
 
-			rel, err := CheckForUpdate(client, stateFilePath, "OWNER/REPO", s.CurrentVersion)
+			rel, err := CheckForUpdate(client, tempFilePath(), "OWNER/REPO", s.CurrentVersion)
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -96,18 +93,10 @@ func TestCheckForUpdate(t *testing.T) {
 }
 
 func tempFilePath() string {
-	content := []byte(
-		`checked_for_update_at: 2020-11-22T09:53:32.609610344-03:00
-latest_release:
-    version: v1.1.0
-    url: https://github.com/cli/cli/releases/tag/v1.1.0`)
-	file, err := ioutil.TempFile("", "state*.yml")
+	file, err := ioutil.TempFile("", "")
 	if err != nil {
 		log.Fatal(err)
 	}
-
-	if _, err := file.Write(content); err != nil {
-		log.Fatal(err)
-	}
+	os.Remove(file.Name())
 	return file.Name()
 }
