Fix and cleanup update message and improve related tests (#4361)

Fixes #4232

cmd/server-main.go

@@ -81,8 +81,8 @@ EXAMPLES:
 func checkUpdate(mode string) {
 	// Its OK to ignore any errors during getUpdateInfo() here.
 	if older, downloadURL, err := getUpdateInfo(1*time.Second, mode); err == nil {
-		if older > time.Duration(0) {
-			log.Println(colorizeUpdateMessage(downloadURL, older))
+		if updateMsg := computeUpdateMessage(downloadURL, older); updateMsg != "" {
+			log.Println(updateMsg)
 		}
 	}
 }

cmd/update-main.go

@@ -238,13 +238,16 @@ func getDownloadURL() (downloadURL string) {
 	return minioReleaseURL + "minio"
 }
 
-func getUpdateInfo(timeout time.Duration, mode string) (older time.Duration, downloadURL string, err error) {
-	currentReleaseTime, err := GetCurrentReleaseTime()
+func getUpdateInfo(timeout time.Duration, mode string) (older time.Duration,
+	downloadURL string, err error) {
+
+	var currentReleaseTime, latestReleaseTime time.Time
+	currentReleaseTime, err = GetCurrentReleaseTime()
 	if err != nil {
 		return older, downloadURL, err
 	}
 
-	latestReleaseTime, err := getLatestReleaseTime(timeout, mode)
+	latestReleaseTime, err = getLatestReleaseTime(timeout, mode)
 	if err != nil {
 		return older, downloadURL, err
 	}
@@ -274,8 +277,8 @@ func mainUpdate(ctx *cli.Context) {
 		os.Exit(-1)
 	}
 
-	if older != time.Duration(0) {
-		log.Println(colorizeUpdateMessage(downloadURL, older))
+	if updateMsg := computeUpdateMessage(downloadURL, older); updateMsg != "" {
+		log.Println(updateMsg)
 		os.Exit(1)
 	}
 

cmd/update-notifier.go

@@ -28,25 +28,39 @@ import (
 	"github.com/fatih/color"
 )
 
+// computeUpdateMessage - calculates the update message, only if a
+// newer version is available.
+func computeUpdateMessage(downloadURL string, older time.Duration) string {
+	if downloadURL == "" || older <= 0 {
+		return ""
+	}
+
+	// Compute friendly duration string to indicate time
+	// difference between newer and current release.
+	t := time.Time{}
+	newerThan := humanize.RelTime(t, t.Add(older), "ago", "")
+
+	// Return the nicely colored and formatted update message.
+	return colorizeUpdateMessage(downloadURL, newerThan)
+}
+
 // colorizeUpdateMessage - inspired from Yeoman project npm package https://github.com/yeoman/update-notifier
-func colorizeUpdateMessage(updateString string, newerThan time.Duration) string {
+func colorizeUpdateMessage(updateString string, newerThan string) string {
 	// Initialize coloring.
 	cyan := color.New(color.FgCyan, color.Bold).SprintFunc()
 	yellow := color.New(color.FgYellow, color.Bold).SprintfFunc()
 
-	// Calculate length without color coding, due to ANSI color
-	// characters padded to actual string the final length is wrong
-	// than the original string length.
-	newerThanStr := humanize.Time(UTCNow().Add(newerThan))
+	msgLine1Fmt := " You are running an older version of Minio released %s "
+	msgLine2Fmt := " Update: %s "
 
-	line1Str := fmt.Sprintf(" You are running an older version of Minio released %s ", newerThanStr)
-	line2Str := fmt.Sprintf(" Update: %s ", updateString)
-	line1Length := len(line1Str)
-	line2Length := len(line2Str)
+	// Calculate length *without* color coding: with ANSI terminal
+	// color characters, the result is incorrect.
+	line1Length := len(fmt.Sprintf(msgLine1Fmt, newerThan))
+	line2Length := len(fmt.Sprintf(msgLine2Fmt, updateString))
 
 	// Populate lines with color coding.
-	line1InColor := fmt.Sprintf(" You are running an older version of Minio released %s ", yellow(newerThanStr))
-	line2InColor := fmt.Sprintf(" Update: %s ", cyan(updateString))
+	line1InColor := fmt.Sprintf(msgLine1Fmt, yellow(newerThan))
+	line2InColor := fmt.Sprintf(msgLine2Fmt, cyan(updateString))
 
 	// calculate the rectangular box size.
 	maxContentWidth := int(math.Max(float64(line1Length), float64(line2Length)))
@@ -60,7 +74,7 @@ func colorizeUpdateMessage(updateString string, newerThan time.Duration) string
 
 	// Box cannot be printed if terminal width is small than maxContentWidth
 	if maxContentWidth > termWidth {
-		return "\n" + line1InColor + "\n" + line2InColor + "\n" + "\n"
+		return "\n" + line1InColor + "\n" + line2InColor + "\n\n"
 	}
 
 	topLeftChar := "┏"
@@ -79,14 +93,11 @@ func colorizeUpdateMessage(updateString string, newerThan time.Duration) string
 		vertBarChar = "|"
 	}
 
-	message := "\n"
-	// Add top line
-	message += yellow(topLeftChar+strings.Repeat(horizBarChar, maxContentWidth)+topRightChar) + "\n"
-	// Add message lines
-	message += vertBarChar + line1InColor + strings.Repeat(" ", maxContentWidth-line1Length) + vertBarChar + "\n"
-	message += vertBarChar + line2InColor + strings.Repeat(" ", maxContentWidth-line2Length) + vertBarChar + "\n"
-	// Add bottom line
-	message += yellow(bottomLeftChar+strings.Repeat(horizBarChar, maxContentWidth)+bottomRightChar) + "\n"
-
-	return message
+	lines := []string{
+		yellow(topLeftChar + strings.Repeat(horizBarChar, maxContentWidth) + topRightChar),
+		vertBarChar + line1InColor + strings.Repeat(" ", maxContentWidth-line1Length) + vertBarChar,
+		vertBarChar + line2InColor + strings.Repeat(" ", maxContentWidth-line2Length) + vertBarChar,
+		yellow(bottomLeftChar + strings.Repeat(horizBarChar, maxContentWidth) + bottomRightChar),
+	}
+	return "\n" + strings.Join(lines, "\n") + "\n"
 }

cmd/update-notifier_test.go

@@ -17,7 +17,7 @@
 package cmd
 
 import (
-	"runtime"
+	"fmt"
 	"strings"
 	"testing"
 	"time"
@@ -26,25 +26,69 @@ import (
 )
 
 // Tests update notifier string builder.
-func TestUpdateNotifier(t *testing.T) {
+func TestComputeUpdateMessage(t *testing.T) {
+	testCases := []struct {
+		older time.Duration
+		dlURL string
+
+		expectedSubStr string
+	}{
+		// Testcase index 0
+		{72 * time.Hour, "my_download_url", "3 days ago"},
+		{3 * time.Hour, "https://my_download_url_is_huge/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "3 hours ago"},
+		{-72 * time.Hour, "another_update_url", ""},
+		{0, "another_update_url", ""},
+		{time.Hour, "", ""},
+		{1 * time.Second, "my_download_url", "now"},
+		{2 * time.Second, "my_download_url", "1 second ago"},
+		{37 * time.Second, "my_download_url", "37 seconds ago"},
+		{60 * time.Second, "my_download_url", "60 seconds ago"},
+		{61 * time.Second, "my_download_url", "1 minute ago"},
+
+		// Testcase index 10
+		{37 * time.Minute, "my_download_url", "37 minutes ago"},
+		{1 * time.Hour, "my_download_url", "60 minutes ago"},
+		{61 * time.Minute, "my_download_url", "1 hour ago"},
+		{122 * time.Minute, "my_download_url", "2 hours ago"},
+		{24 * time.Hour, "my_download_url", "24 hours ago"},
+		{25 * time.Hour, "my_download_url", "1 day ago"},
+		{49 * time.Hour, "my_download_url", "2 days ago"},
+		{7 * 24 * time.Hour, "my_download_url", "7 days ago"},
+		{8 * 24 * time.Hour, "my_download_url", "1 week ago"},
+		{15 * 24 * time.Hour, "my_download_url", "2 weeks ago"},
+
+		// Testcase index 20
+		{30 * 24 * time.Hour, "my_download_url", "4 weeks ago"},
+		{31 * 24 * time.Hour, "my_download_url", "1 month ago"},
+		{61 * 24 * time.Hour, "my_download_url", "2 months ago"},
+		{360 * 24 * time.Hour, "my_download_url", "12 months ago"},
+		{361 * 24 * time.Hour, "my_download_url", "1 year ago"},
+		{2 * 365 * 24 * time.Hour, "my_download_url", "2 years ago"},
+	}
+
 	plainMsg := "You are running an older version of Minio released"
-	colorMsg := plainMsg
 	yellow := color.New(color.FgYellow, color.Bold).SprintfFunc()
-	if runtime.GOOS == "windows" {
-		plainMsg += "3 days from now"
-		colorMsg += yellow("3 days from now")
-	} else {
-		plainMsg += "2 days from now"
-		colorMsg += yellow("2 days from now")
-	}
+	cyan := color.New(color.FgCyan, color.Bold).SprintFunc()
 
-	updateMsg := colorizeUpdateMessage(minioReleaseURL, time.Duration(72*time.Hour))
-
-	if !(strings.Contains(updateMsg, plainMsg) || strings.Contains(updateMsg, colorMsg)) {
-		t.Fatal("Duration string not found in colorized update message", updateMsg)
-	}
-
-	if !strings.Contains(updateMsg, minioReleaseURL) {
-		t.Fatal("Update message not found in colorized update message", minioReleaseURL)
+	for i, testCase := range testCases {
+		output := computeUpdateMessage(testCase.dlURL, testCase.older)
+		line1 := fmt.Sprintf("%s %s", plainMsg, yellow(testCase.expectedSubStr))
+		line2 := fmt.Sprintf("Update: %s", cyan(testCase.dlURL))
+		// Uncomment below to see message appearance:
+		// fmt.Println(output)
+		switch {
+		case testCase.dlURL == "" && output != "":
+			t.Errorf("Testcase %d: no newer release available but got an update message: %s", i, output)
+		case output == "" && testCase.dlURL != "" && testCase.older > 0:
+			t.Errorf("Testcase %d: newer release is available but got empty update message!", i)
+		case output == "" && (testCase.dlURL == "" || testCase.older <= 0):
+			// Valid no update message case. No further
+			// validation needed.
+			continue
+		case !strings.Contains(output, line1):
+			t.Errorf("Testcase %d: output '%s' did not contain line 1: '%s'", i, output, line1)
+		case !strings.Contains(output, line2):
+			t.Errorf("Testcase %d: output '%s' did not contain line 2: '%s'", i, output, line2)
+		}
 	}
 }