Skipped messages do not change total counts but shows as separate number

internal/transfer/message.go

@@ -58,6 +58,7 @@ type MessageStatus struct {
 	targetID    string    // Message ID at the target (if any).
 	bodyHash    string    // Hash of the message body.
 
+	skipped   bool
 	exported  bool
 	imported  bool
 	exportErr error
@@ -96,7 +97,7 @@ func (status *MessageStatus) setDetailsFromHeader(header mail.Header) {
 }
 
 func (status *MessageStatus) hasError(includeMissing bool) bool {
-	return status.exportErr != nil || status.importErr != nil || (includeMissing && !status.imported)
+	return status.exportErr != nil || status.importErr != nil || (includeMissing && !status.skipped && !status.imported)
 }
 
 // GetErrorMessage returns error message.
@@ -105,6 +106,9 @@ func (status *MessageStatus) GetErrorMessage() string {
 }
 
 func (status *MessageStatus) getErrorMessage(includeMissing bool) string {
+	if status.skipped {
+		return ""
+	}
 	if status.exportErr != nil {
 		return fmt.Sprintf("failed to export: %s", status.exportErr)
 	}

internal/transfer/progress.go

@@ -140,6 +140,19 @@ func (p *Progress) addMessage(messageID string, sourceNames, targetNames []strin
 	}
 }
 
+// messageSkipped should be called once the message is skipped due to some
+// filter such as time or folder and so on.
+func (p *Progress) messageSkipped(messageID string) {
+	p.lock.Lock()
+	defer p.lock.Unlock()
+	defer p.update()
+
+	p.log.WithField("id", messageID).Debug("Message skipped")
+
+	p.messageStatuses[messageID].skipped = true
+	p.logMessage(messageID)
+}
+
 // messageExported should be called right before message is exported.
 func (p *Progress) messageExported(messageID string, body []byte, err error) {
 	p.lock.Lock()
@@ -330,35 +343,40 @@ func (p *Progress) GetFailedMessages() []*MessageStatus {
 }
 
 // GetCounts returns counts of exported and imported messages.
-func (p *Progress) GetCounts() (failed, imported, exported, added, total uint) {
+func (p *Progress) GetCounts() ProgressCounts {
 	p.lock.Lock()
 	defer p.lock.Unlock()
 
+	counts := ProgressCounts{}
+
 	// Return counts only once total is estimated or the process already
 	// ended (for a case when it ended quickly to report it correctly).
 	if p.updateCh != nil && !p.messageCounted {
-		return
+		return counts
 	}
 
 	// Include lost messages in the process only when transfer is done.
 	includeMissing := p.updateCh == nil
 
 	for _, mailboxCount := range p.messageCounts {
-		total += mailboxCount
+		counts.Total += mailboxCount
 	}
 	for _, status := range p.messageStatuses {
-		added++
+		counts.Added++
+		if status.skipped {
+			counts.Skipped++
+		}
 		if status.exported {
-			exported++
+			counts.Exported++
 		}
 		if status.imported {
-			imported++
+			counts.Imported++
 		}
 		if status.hasError(includeMissing) {
-			failed++
+			counts.Failed++
 		}
 	}
-	return
+	return counts
 }
 
 // GenerateBugReport generates similar file to import log except private information.

internal/transfer/progress_counts.go

@@ -0,0 +1,28 @@
+// Copyright (c) 2020 Proton Technologies AG
+//
+// This file is part of ProtonMail Bridge.
+//
+// ProtonMail Bridge is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// ProtonMail Bridge is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with ProtonMail Bridge.  If not, see <https://www.gnu.org/licenses/>.
+
+package transfer
+
+// ProgressCounts holds counts counted by Progress.
+type ProgressCounts struct {
+	Failed,
+	Skipped,
+	Imported,
+	Exported,
+	Added,
+	Total uint
+}

internal/transfer/progress_test.go

@@ -39,8 +39,8 @@ func TestProgressUpdateCount(t *testing.T) {
 
 	progress.finish()
 
-	_, _, _, _, total := progress.GetCounts() //nolint[dogsled]
-	r.Equal(t, uint(42), total)
+	counts := progress.GetCounts()
+	r.Equal(t, uint(42), counts.Total)
 }
 
 func TestProgressAddingMessages(t *testing.T) {
@@ -66,13 +66,18 @@ func TestProgressAddingMessages(t *testing.T) {
 	progress.messageExported("msg4", []byte(""), errors.New("failed export"))
 	progress.messageImported("msg4", "", nil)
 
+	// msg5 is skipped.
+	progress.addMessage("msg5", []string{}, []string{})
+	progress.messageSkipped("msg5")
+
 	progress.finish()
 
-	failed, imported, exported, added, _ := progress.GetCounts()
-	a.Equal(t, uint(4), added)
-	a.Equal(t, uint(2), exported)
-	a.Equal(t, uint(2), imported)
-	a.Equal(t, uint(3), failed)
+	counts := progress.GetCounts()
+	a.Equal(t, uint(5), counts.Added)
+	a.Equal(t, uint(2), counts.Exported)
+	a.Equal(t, uint(2), counts.Imported)
+	a.Equal(t, uint(1), counts.Skipped)
+	a.Equal(t, uint(3), counts.Failed)
 
 	errorsMap := map[string]string{}
 	for _, status := range progress.GetFailedMessages() {

internal/transfer/provider_eml_source.go

@@ -82,8 +82,6 @@ func (p *EMLProvider) getFilePathsPerFolder(rules transferRules) (map[string][]s
 }
 
 func (p *EMLProvider) exportMessages(rule *Rule, filePaths []string, progress *Progress, ch chan<- Message) {
-	count := uint(len(filePaths))
-
 	for _, filePath := range filePaths {
 		if progress.shouldStop() {
 			break
@@ -91,6 +89,8 @@ func (p *EMLProvider) exportMessages(rule *Rule, filePaths []string, progress *P
 
 		msg, err := p.exportMessage(rule, filePath)
 
+		progress.addMessage(filePath, msg.sourceNames(), msg.targetNames())
+
 		// Read and check time in body only if the rule specifies it
 		// to not waste energy.
 		if err == nil && rule.HasTimeLimit() {
@@ -99,17 +99,11 @@ func (p *EMLProvider) exportMessages(rule *Rule, filePaths []string, progress *P
 				err = msgTimeErr
 			} else if !rule.isTimeInRange(msgTime) {
 				log.WithField("msg", filePath).Debug("Message skipped due to time")
-
-				count--
-				progress.updateCount(rule.SourceMailbox.Name, count)
+				progress.messageSkipped(filePath)
 				continue
 			}
 		}
 
-		// addMessage is called after time check to not report message
-		// which should not be exported but any error from reading body
-		// or parsing time is reported as an error.
-		progress.addMessage(filePath, msg.sourceNames(), msg.targetNames())
 		progress.messageExported(filePath, msg.Body, err)
 		if err == nil {
 			ch <- msg

internal/transfer/provider_mbox_source.go

@@ -114,7 +114,6 @@ func (p *MBOXProvider) transferTo(rules transferRules, progress *Progress, ch ch
 	}
 
 	index := 0
-	count := 0
 	for {
 		if progress.shouldStop() {
 			break
@@ -133,24 +132,18 @@ func (p *MBOXProvider) transferTo(rules transferRules, progress *Progress, ch ch
 
 		msg, err := p.exportMessage(rules, folderName, id, msgReader)
 
+		progress.addMessage(id, msg.sourceNames(), msg.targetNames())
+
 		if err == nil && len(msg.Targets) == 0 {
-			// Here should be called progress.messageSkipped(id) once we have
-			// this feature, and following progress.updateCount can be removed.
+			progress.messageSkipped(id)
 			continue
 		}
 
-		count++
-
-		// addMessage is called after time check to not report message
-		// which should not be exported but any error from reading body
-		// or parsing time is reported as an error.
-		progress.addMessage(id, msg.sourceNames(), msg.targetNames())
 		progress.messageExported(id, msg.Body, err)
 		if err == nil {
 			ch <- msg
 		}
 	}
-	progress.updateCount(filePath, uint(count))
 }
 
 func (p *MBOXProvider) exportMessage(rules transferRules, folderName, id string, msgReader io.Reader) (Message, error) {
@@ -177,7 +170,7 @@ func (p *MBOXProvider) getMessageRules(rules transferRules, folderName, id strin
 	folderRule, err := rules.getRuleBySourceMailboxName(folderName)
 	if err != nil {
 		log.WithField("msg", id).WithField("source", folderName).Debug("Message source doesn't have a rule")
-	} else {
+	} else if folderRule.Active {
 		msgRules = append(msgRules, folderRule)
 	}
 
@@ -191,7 +184,9 @@ func (p *MBOXProvider) getMessageRules(rules transferRules, folderName, id strin
 				log.WithField("msg", id).WithField("source", label).Debug("Message source doesn't have a rule")
 				continue
 			}
-			msgRules = append(msgRules, rule)
+			if rule.Active {
+				msgRules = append(msgRules, rule)
+			}
 		}
 	}
 

internal/transfer/provider_mbox_test.go

@@ -155,6 +155,29 @@ func TestMBOXProviderTransferFromTo(t *testing.T) {
 	})
 }
 
+func TestMBOXProviderGetMessageRules(t *testing.T) {
+	provider := newTestMBOXProvider("")
+
+	body := []byte(`Subject: Test
+X-Gmail-Labels: foo,bar
+
+`)
+	rules := transferRules{
+		rules: map[string]*Rule{
+			"1": {Active: true, SourceMailbox: Mailbox{Name: "folder"}},
+			"2": {Active: false, SourceMailbox: Mailbox{Name: "foo"}},
+			"3": {Active: true, SourceMailbox: Mailbox{Name: "bar"}},
+			"4": {Active: false, SourceMailbox: Mailbox{Name: "baz"}},
+			"5": {Active: true, SourceMailbox: Mailbox{Name: "other"}},
+		},
+	}
+
+	gotRules := provider.getMessageRules(rules, "folder", "id", body)
+	r.Equal(t, 2, len(gotRules))
+	r.Equal(t, "folder", gotRules[0].SourceMailbox.Name)
+	r.Equal(t, "bar", gotRules[1].SourceMailbox.Name)
+}
+
 func TestMBOXProviderGetMessageTargetsReturnsOnlyOneFolder(t *testing.T) {
 	provider := newTestMBOXProvider("")