From f5a20087fe690b61baab0e058190b739301085cc Mon Sep 17 00:00:00 2001
From: Jono Wenger <jono@elixxir.io>
Date: Mon, 13 Dec 2021 16:08:22 -0800
Subject: [PATCH] Remove unused code, minor test fixes, and fix integration
 lagging once file is received

---
 cmd/fileTransfer.go          | 40 ++++++++++++++--------------
 fileTransfer/manager.go      | 51 ------------------------------------
 fileTransfer/manager_test.go | 27 +++++++++----------
 fileTransfer/send_test.go    | 10 +++----
 fileTransfer/utils_test.go   | 10 ++++---
 5 files changed, 44 insertions(+), 94 deletions(-)

diff --git a/cmd/fileTransfer.go b/cmd/fileTransfer.go
index 17e2a0d7c..df6ba635e 100644
--- a/cmd/fileTransfer.go
+++ b/cmd/fileTransfer.go
@@ -95,11 +95,11 @@ var ftCmd = &cobra.Command{
 		for done := false; !done; {
 			select {
 			case <-sendDone:
-				jww.DEBUG.Printf("Finished sending message. Stopping threads " +
+				jww.DEBUG.Printf("Finished sending file. Stopping threads " +
 					"and network follower.")
 				done = true
 			case <-receiveDone:
-				jww.DEBUG.Printf("Finished receiving message. Stopping " +
+				jww.DEBUG.Printf("Finished receiving file. Stopping " +
 					"threads and network follower.")
 				done = true
 			}
@@ -229,23 +229,25 @@ func receiveNewFileTransfers(receive chan receivedFtResults, done,
 	quit chan struct{}, m *ft.Manager) {
 	jww.DEBUG.Print("Starting thread waiting to receive NewFileTransfer " +
 		"E2E message.")
-	select {
-	case <-quit:
-		jww.DEBUG.Print("Quitting thread waiting for NewFileTransfer E2E " +
-			"message.")
-		return
-	case r := <-receive:
-		jww.DEBUG.Printf("Received new file %q transfer %s from %s of size %d "+
-			"bytes with preview: %q",
-			r.fileName, r.tid, r.sender, r.size, r.preview)
-		fmt.Printf("Received new file transfer %q of size %d "+
-			"bytes with preview: %q\n", r.fileName, r.size, r.preview)
-
-		cb := newReceiveProgressCB(r.tid, done, m)
-		err := m.RegisterReceiveProgressCallback(r.tid, cb, callbackPeriod)
-		if err != nil {
-			jww.FATAL.Panicf("Failed to register new receive progress "+
-				"callback for transfer %s: %+v", r.tid, err)
+	for {
+		select {
+		case <-quit:
+			jww.DEBUG.Print("Quitting thread waiting for NewFileTransfer E2E " +
+				"message.")
+			return
+		case r := <-receive:
+			jww.DEBUG.Printf("Received new file %q transfer %s from %s of size %d "+
+				"bytes with preview: %q",
+				r.fileName, r.tid, r.sender, r.size, r.preview)
+			fmt.Printf("Received new file transfer %q of size %d "+
+				"bytes with preview: %q\n", r.fileName, r.size, r.preview)
+
+			cb := newReceiveProgressCB(r.tid, done, m)
+			err := m.RegisterReceiveProgressCallback(r.tid, cb, callbackPeriod)
+			if err != nil {
+				jww.FATAL.Panicf("Failed to register new receive progress "+
+					"callback for transfer %s: %+v", r.tid, err)
+			}
 		}
 	}
 }
diff --git a/fileTransfer/manager.go b/fileTransfer/manager.go
index 994a072b8..c63d8eb8d 100644
--- a/fileTransfer/manager.go
+++ b/fileTransfer/manager.go
@@ -52,14 +52,6 @@ const (
 	networkHealthBuffLen = 10_000
 )
 
-// Part status constants.
-const (
-	unsent   = 0
-	sent     = 1
-	arrived  = 2
-	received = 1
-)
-
 // Error messages.
 const (
 	// newManager
@@ -321,29 +313,6 @@ func (m Manager) RegisterSendProgressCallback(tid ftCrypto.TransferID,
 	return nil
 }
 
-// GetSentPartStatus returns the status of the sent file part number for the
-// given transfer ID. An error is returned if the sent transfer does not
-// exist. The possible values for the status are:
-// 0 = unsent
-// 1 = sent
-// 2 = arrived
-// TODO: test
-func (m Manager) GetSentPartStatus(tid ftCrypto.TransferID, partNum uint16) (int, error) {
-	// Get the transfer for the given ID
-	transfer, err := m.sent.GetTransfer(tid)
-	if err != nil {
-		return unsent, err
-	}
-
-	if transfer.IsPartInProgress(partNum) {
-		return sent, nil
-	} else if transfer.IsPartFinished(partNum) {
-		return arrived, nil
-	} else {
-		return unsent, nil
-	}
-}
-
 // Resend resends a file if sending fails. Returns an error if CloseSend
 // was already called or if the transfer did not run out of retries. This
 // function should only be called if the interfaces.SentProgressCallback returns
@@ -428,26 +397,6 @@ func (m Manager) RegisterReceiveProgressCallback(tid ftCrypto.TransferID,
 	return nil
 }
 
-// GetReceivedPartStatus returns the status of the received file part number
-// for the given transfer ID. An error is returned if the received transfer
-// does not exist. The possible values for the status are:
-// 0 = unsent
-// 1 = received
-// TODO: test
-func (m Manager) GetReceivedPartStatus(tid ftCrypto.TransferID, partNum uint16) (int, error) {
-	// Get the transfer for the given ID
-	transfer, err := m.received.GetTransfer(tid)
-	if err != nil {
-		return unsent, err
-	}
-
-	if transfer.IsPartReceived(partNum) {
-		return received, nil
-	} else {
-		return unsent, nil
-	}
-}
-
 // calcNumberOfFingerprints is the formula used to calculate the number of
 // fingerprints to generate, which is based off the number of file parts and the
 // retry float.
diff --git a/fileTransfer/manager_test.go b/fileTransfer/manager_test.go
index 8d88f058a..59c93026b 100644
--- a/fileTransfer/manager_test.go
+++ b/fileTransfer/manager_test.go
@@ -241,8 +241,8 @@ func TestManager_RegisterSendProgressCallback(t *testing.T) {
 	// Create new callback and channel for the callback to trigger
 	cbChan := make(chan sentProgressResults, 6)
 	cb := func(completed bool, sent, arrived, total uint16,
-		t interfaces.FilePartTracker, err error) {
-		cbChan <- sentProgressResults{completed, sent, arrived, total, err}
+		tr interfaces.FilePartTracker, err error) {
+		cbChan <- sentProgressResults{completed, sent, arrived, total, tr, err}
 	}
 
 	// Start thread waiting for callback to be called
@@ -452,8 +452,8 @@ func TestManager_RegisterReceiveProgressCallback(t *testing.T) {
 	// Create new callback and channel for the callback to trigger
 	cbChan := make(chan receivedProgressResults, 6)
 	cb := func(completed bool, received, total uint16,
-		t interfaces.FilePartTracker, err error) {
-		cbChan <- receivedProgressResults{completed, received, total, err}
+		tr interfaces.FilePartTracker, err error) {
+		cbChan <- receivedProgressResults{completed, received, total, tr, err}
 	}
 
 	// Start thread waiting for callback to be called
@@ -579,8 +579,8 @@ func Test_FileTransfer(t *testing.T) {
 	// Create progress tracker for sending
 	sentCbChan := make(chan sentProgressResults, 20)
 	sentCb := func(completed bool, sent, arrived, total uint16,
-		t interfaces.FilePartTracker, err error) {
-		sentCbChan <- sentProgressResults{completed, sent, arrived, total, err}
+		tr interfaces.FilePartTracker, err error) {
+		sentCbChan <- sentProgressResults{completed, sent, arrived, total, tr, err}
 	}
 
 	// Start threads that tracks sent progress until complete
@@ -637,8 +637,8 @@ func Test_FileTransfer(t *testing.T) {
 	// Register progress callback with receiving manager
 	receiveCbChan := make(chan receivedProgressResults, 100)
 	receiveCb := func(completed bool, received, total uint16,
-		t interfaces.FilePartTracker, err error) {
-		receiveCbChan <- receivedProgressResults{completed, received, total, err}
+		tr interfaces.FilePartTracker, err error) {
+		receiveCbChan <- receivedProgressResults{completed, received, total, tr, err}
 	}
 
 	// Start threads that tracks received progress until complete
@@ -647,18 +647,15 @@ func Test_FileTransfer(t *testing.T) {
 		defer wg.Done()
 		for i := 0; i < 20; i++ {
 			select {
-			case <-time.NewTimer(250 * time.Millisecond).C:
+			case <-time.NewTimer(350 * time.Millisecond).C:
 				t.Errorf("Timed out waiting for receive progress callback %d.", i)
 			case r := <-receiveCbChan:
 				if r.completed {
 					// Count the number of parts marked as received
 					count := 0
 					for j := uint16(0); j < r.total; j++ {
-						status, err2 := m2.GetReceivedPartStatus(receiveTid, j)
-						if err2 != nil {
-							t.Errorf("Failed to get part %d status: %+v", j, err2)
-						}
-						if status == received {
+						status := r.tracker.GetPartStatus(j)
+						if status == 3 {
 							count++
 						}
 					}
@@ -667,7 +664,7 @@ func Test_FileTransfer(t *testing.T) {
 					// callback matches the number marked received
 					if count != int(r.received) {
 						t.Errorf("Number of parts marked received does not match "+
-							"number reported by callback.\nmarked: %d\ncallback: %d",
+							"number reported by callback.\nmarked:   %d\ncallback: %d",
 							count, r.received)
 					}
 
diff --git a/fileTransfer/send_test.go b/fileTransfer/send_test.go
index 29172e752..56d6b2ef5 100644
--- a/fileTransfer/send_test.go
+++ b/fileTransfer/send_test.go
@@ -751,10 +751,10 @@ func TestManager_makeRoundEventCallback_RoundFailure(t *testing.T) {
 	<-done1
 }
 
-// Error path: tests that Manager.makeRoundEventCallback panics when
+// Panic path: tests that Manager.makeRoundEventCallback panics when
 // SentTransfer.FinishTransfer returns an error because the file parts had not
 // previously been set to in-progress.
-func TestManager_makeRoundEventCallback_FinishTransferError(t *testing.T) {
+func TestManager_makeRoundEventCallback_FinishTransferPanic(t *testing.T) {
 	m := newTestManager(false, nil, nil, nil, t)
 
 	prng := NewPrng(42)
@@ -775,10 +775,10 @@ func TestManager_makeRoundEventCallback_FinishTransferError(t *testing.T) {
 
 	expectedErr := strings.Split(finishTransferPanic, "%")[0]
 	defer func() {
-		err := recover()
-		if err == nil || !strings.Contains(err.(string), expectedErr) {
+		err2 := recover()
+		if err2 == nil || !strings.Contains(err2.(string), expectedErr) {
 			t.Errorf("makeRoundEventCallback failed to panic or returned the "+
-				"wrong error.\nexpected: %s\nreceived: %+v", expectedErr, err)
+				"wrong error.\nexpected: %s\nreceived: %+v", expectedErr, err2)
 		}
 	}()
 
diff --git a/fileTransfer/utils_test.go b/fileTransfer/utils_test.go
index 46eb20f72..374082112 100644
--- a/fileTransfer/utils_test.go
+++ b/fileTransfer/utils_test.go
@@ -232,8 +232,8 @@ func newTestManagerWithTransfers(numParts []uint16, sendErr bool,
 		cbChan := make(chan sentProgressResults, 6)
 
 		cb := func(completed bool, sent, arrived, total uint16,
-			t interfaces.FilePartTracker, err error) {
-			cbChan <- sentProgressResults{completed, sent, arrived, total, err}
+			tr interfaces.FilePartTracker, err error) {
+			cbChan <- sentProgressResults{completed, sent, arrived, total, tr, err}
 		}
 
 		sti[i].cbChan = cbChan
@@ -267,8 +267,8 @@ func newTestManagerWithTransfers(numParts []uint16, sendErr bool,
 		cbChan := make(chan receivedProgressResults, 6)
 
 		cb := func(completed bool, received, total uint16,
-			t interfaces.FilePartTracker, err error) {
-			cbChan <- receivedProgressResults{completed, received, total, err}
+			tr interfaces.FilePartTracker, err error) {
+			cbChan <- receivedProgressResults{completed, received, total, tr, err}
 		}
 
 		rti[i].cbChan = cbChan
@@ -300,6 +300,7 @@ type receivedFtResults struct {
 type sentProgressResults struct {
 	completed            bool
 	sent, arrived, total uint16
+	tracker              interfaces.FilePartTracker
 	err                  error
 }
 
@@ -324,6 +325,7 @@ type sentTransferInfo struct {
 type receivedProgressResults struct {
 	completed       bool
 	received, total uint16
+	tracker         interfaces.FilePartTracker
 	err             error
 }
 
-- 
GitLab