From fc173bf679dbe4058c9a695933b13e7fd95af511 Mon Sep 17 00:00:00 2001
From: Audrius Butkevicius <audrius.butkevicius@gmail.com>
Date: Thu, 26 May 2016 06:53:27 +0000
Subject: [PATCH] lib/model: Fix wild completion percentages

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3188
---
 lib/model/devicedownloadstate.go | 37 ++++++++++++++++----------------
 lib/model/model.go               | 27 +++++++++++++++--------
 2 files changed, 36 insertions(+), 28 deletions(-)

diff --git a/lib/model/devicedownloadstate.go b/lib/model/devicedownloadstate.go
index e6354140a..e0932bb28 100644
--- a/lib/model/devicedownloadstate.go
+++ b/lib/model/devicedownloadstate.go
@@ -23,9 +23,8 @@ type deviceFolderFileDownloadState struct {
 // deviceFolderDownloadState holds current download state of all files that
 // a remote device is currently downloading in a specific folder.
 type deviceFolderDownloadState struct {
-	mut                      sync.RWMutex
-	files                    map[string]deviceFolderFileDownloadState
-	numberOfBlocksInProgress int
+	mut   sync.RWMutex
+	files map[string]deviceFolderFileDownloadState
 }
 
 // Has returns whether a block at that specific index, and that specific version of the file
@@ -57,7 +56,6 @@ func (p *deviceFolderDownloadState) Update(updates []protocol.FileDownloadProgre
 	for _, update := range updates {
 		local, ok := p.files[update.Name]
 		if update.UpdateType == protocol.UpdateTypeForget && ok && local.version.Equal(update.Version) {
-			p.numberOfBlocksInProgress -= len(local.blockIndexes)
 			delete(p.files, update.Name)
 		} else if update.UpdateType == protocol.UpdateTypeAppend {
 			if !ok {
@@ -66,25 +64,25 @@ func (p *deviceFolderDownloadState) Update(updates []protocol.FileDownloadProgre
 					version:      update.Version,
 				}
 			} else if !local.version.Equal(update.Version) {
-				p.numberOfBlocksInProgress -= len(local.blockIndexes)
 				local.blockIndexes = append(local.blockIndexes[:0], update.BlockIndexes...)
 				local.version = update.Version
 			} else {
 				local.blockIndexes = append(local.blockIndexes, update.BlockIndexes...)
 			}
 			p.files[update.Name] = local
-			p.numberOfBlocksInProgress += len(update.BlockIndexes)
 		}
 	}
 }
 
-// NumberOfBlocksInProgress returns the number of blocks the device has downloaded
-// for a specific folder.
-func (p *deviceFolderDownloadState) NumberOfBlocksInProgress() int {
+// GetBlockCounts returns a map filename -> number of blocks downloaded.
+func (p *deviceFolderDownloadState) GetBlockCounts() map[string]int {
 	p.mut.RLock()
-	n := p.numberOfBlocksInProgress
+	res := make(map[string]int, len(p.files))
+	for name, state := range p.files {
+		res[name] = len(state.blockIndexes)
+	}
 	p.mut.RUnlock()
-	return n
+	return res
 }
 
 // deviceDownloadState represents the state of all in progress downloads
@@ -134,20 +132,21 @@ func (t *deviceDownloadState) Has(folder, file string, version protocol.Vector,
 	return f.Has(file, version, index)
 }
 
-// NumberOfBlocksInProgress returns the number of blocks the device has downloaded
-// for all folders.
-func (t *deviceDownloadState) NumberOfBlocksInProgress() int {
+// GetBlockCounts returns a map filename -> number of blocks downloaded for the
+// given folder.
+func (t *deviceDownloadState) GetBlockCounts(folder string) map[string]int {
 	if t == nil {
-		return 0
+		return nil
 	}
 
-	n := 0
 	t.mut.RLock()
-	for _, folder := range t.folders {
-		n += folder.NumberOfBlocksInProgress()
+	for name, state := range t.folders {
+		if name == folder {
+			return state.GetBlockCounts()
+		}
 	}
 	t.mut.RUnlock()
-	return n
+	return nil
 }
 
 func newDeviceDownloadState() *deviceDownloadState {
diff --git a/lib/model/model.go b/lib/model/model.go
index 477ecea3e..8af94770f 100644
--- a/lib/model/model.go
+++ b/lib/model/model.go
@@ -374,17 +374,26 @@ func (m *Model) Completion(device protocol.DeviceID, folder string) float64 {
 		return 100 // Folder is empty, so we have all of it
 	}
 
-	var need int64
+	m.pmut.RLock()
+	counts := m.deviceDownloads[device].GetBlockCounts(folder)
+	m.pmut.RUnlock()
+
+	var need, fileNeed, downloaded int64
 	rf.WithNeedTruncated(device, func(f db.FileIntf) bool {
-		need += f.Size()
+		ft := f.(db.FileInfoTruncated)
+
+		// This might might be more than it really is, because some blocks can be of a smaller size.
+		downloaded = int64(counts[ft.Name] * protocol.BlockSize)
+
+		fileNeed = ft.Size() - downloaded
+		if fileNeed < 0 {
+			fileNeed = 0
+		}
+
+		need += fileNeed
 		return true
 	})
 
-	// This might might be more than it really is, because some blocks can be of a smaller size.
-	m.pmut.RLock()
-	need -= int64(m.deviceDownloads[device].NumberOfBlocksInProgress() * protocol.BlockSize)
-	m.pmut.RUnlock()
-
 	needRatio := float64(need) / float64(tot)
 	completionPct := 100 * (1 - needRatio)
 	l.Debugf("%v Completion(%s, %q): %f (%d / %d = %f)", m, device, folder, completionPct, need, tot, needRatio)
@@ -1083,13 +1092,13 @@ func (m *Model) DownloadProgress(device protocol.DeviceID, folder string, update
 
 	m.pmut.RLock()
 	m.deviceDownloads[device].Update(folder, updates)
-	blocks := m.deviceDownloads[device].NumberOfBlocksInProgress()
+	state := m.deviceDownloads[device].GetBlockCounts(folder)
 	m.pmut.RUnlock()
 
 	events.Default.Log(events.RemoteDownloadProgress, map[string]interface{}{
 		"device": device.String(),
 		"folder": folder,
-		"blocks": blocks,
+		"state":  state,
 	})
 }