From d62a0cf6929055213b81a1feeaec1f83f1db5a6d Mon Sep 17 00:00:00 2001
From: Jakob Borg <jakob@kastelo.net>
Date: Mon, 20 Jan 2020 21:14:29 +0100
Subject: [PATCH] lib/model: Handle progress emitter zero interval (fixes
 #6281) (#6282)

Makes the logic a bit clearer and safer. This also sneakily redefines
the 0 interval to also mean disabled, whereas it previously meant ...
sometimes default to 1s, sometimes just spin.
---
 lib/model/progressemitter.go      | 27 +++++++++++++++------------
 lib/model/progressemitter_test.go |  4 ++--
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/lib/model/progressemitter.go b/lib/model/progressemitter.go
index 7d340a824..d08179372 100644
--- a/lib/model/progressemitter.go
+++ b/lib/model/progressemitter.go
@@ -197,27 +197,30 @@ func (t *ProgressEmitter) VerifyConfiguration(from, to config.Configuration) err
 }
 
 // CommitConfiguration implements the config.Committer interface
-func (t *ProgressEmitter) CommitConfiguration(from, to config.Configuration) bool {
+func (t *ProgressEmitter) CommitConfiguration(_, to config.Configuration) bool {
 	t.mut.Lock()
 	defer t.mut.Unlock()
 
-	switch {
-	case t.disabled && to.Options.ProgressUpdateIntervalS >= 0:
-		t.disabled = false
-		l.Debugln("progress emitter: enabled")
-		fallthrough
-	case !t.disabled && from.Options.ProgressUpdateIntervalS != to.Options.ProgressUpdateIntervalS:
-		t.interval = time.Duration(to.Options.ProgressUpdateIntervalS) * time.Second
-		if t.interval < time.Second {
-			t.interval = time.Second
+	newInterval := time.Duration(to.Options.ProgressUpdateIntervalS) * time.Second
+	if newInterval > 0 {
+		if t.disabled {
+			t.disabled = false
+			l.Debugln("progress emitter: enabled")
 		}
-		l.Debugln("progress emitter: updated interval", t.interval)
-	case !t.disabled && to.Options.ProgressUpdateIntervalS < 0:
+		if t.interval != newInterval {
+			t.interval = newInterval
+			l.Debugln("progress emitter: updated interval", t.interval)
+		}
+	} else if !t.disabled {
 		t.clearLocked()
 		t.disabled = true
 		l.Debugln("progress emitter: disabled")
 	}
 	t.minBlocks = to.Options.TempIndexMinBlocks
+	if t.interval < time.Second {
+		// can't happen when we're not disabled, but better safe than sorry.
+		t.interval = time.Second
+	}
 
 	return true
 }
diff --git a/lib/model/progressemitter_test.go b/lib/model/progressemitter_test.go
index 4ecca5aba..b8ce2826b 100644
--- a/lib/model/progressemitter_test.go
+++ b/lib/model/progressemitter_test.go
@@ -62,7 +62,7 @@ func TestProgressEmitter(t *testing.T) {
 	c := createTmpWrapper(config.Configuration{})
 	defer os.Remove(c.ConfigPath())
 	c.SetOptions(config.OptionsConfiguration{
-		ProgressUpdateIntervalS: 0,
+		ProgressUpdateIntervalS: 60, // irrelevant, but must be positive
 	})
 
 	p := NewProgressEmitter(c, evLogger)
@@ -112,7 +112,7 @@ func TestSendDownloadProgressMessages(t *testing.T) {
 	c := createTmpWrapper(config.Configuration{})
 	defer os.Remove(c.ConfigPath())
 	c.SetOptions(config.OptionsConfiguration{
-		ProgressUpdateIntervalS: 0,
+		ProgressUpdateIntervalS: 60, // irrelevant, but must be positive
 		TempIndexMinBlocks:      10,
 	})