chore: Umshiang Bridge 3.5.1 changelog.

Do not abort removing files on first error. Collect errors and try to
remove as many as possible. This would cause some state files to not be
removed on windows.

Changelog.md

@@ -3,6 +3,16 @@
 Changelog [format](http://keepachangelog.com/en/1.0.0/)
 
 
+## Umshiang Bridge 3.5.1
+
+### Fixed
+* GODT-2963: Use multi error to report file removal errors.
+* GODT-2956: Restore old deletion rules.
+* GODT-2951: Negative WaitGroup Counter.
+* GODT-2590: Fix send on closed channel.
+* GODT-2949: Fix close of close channel in event service.
+
+
 ## Umshiang Bridge 3.5.0
 
 ### Added
@@ -75,6 +85,7 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/)
 * GODT-2780: Fix 'QSystemTrayIcon::setVisible: No Icon set' warning in bridge-gui log on startup.
 * GODT-2778: Fix login screen being disabled after an 'already logged in' error.
 * Fix typos found by codespell.
+* GODT-2577: Answered flag should only be applied to replied messages.
 
 
 ## Trift Bridge 3.4.1

Makefile

@@ -11,7 +11,7 @@ ROOT_DIR:=$(shell dirname $(realpath $(firstword $(MAKEFILE_LIST))))
 .PHONY: build build-gui build-nogui build-launcher versioner hasher
 
 # Keep version hardcoded so app build works also without Git repository.
-BRIDGE_APP_VERSION?=3.5.0+git
+BRIDGE_APP_VERSION?=3.5.1+git
 APP_VERSION:=${BRIDGE_APP_VERSION}
 APP_FULL_NAME:=Proton Mail Bridge
 APP_VENDOR:=Proton AG

internal/services/imapservice/connector.go

@@ -37,6 +37,7 @@ import (
 	"github.com/ProtonMail/proton-bridge/v3/pkg/message"
 	"github.com/ProtonMail/proton-bridge/v3/pkg/message/parser"
 	"github.com/bradenaw/juniper/stream"
+	"github.com/bradenaw/juniper/xslices"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/exp/slices"
 )
@@ -334,8 +335,69 @@ func (s *Connector) RemoveMessagesFromMailbox(ctx context.Context, _ connector.I
 	}
 
 	if mboxID == proton.TrashLabel || mboxID == proton.DraftsLabel {
-		if err := s.client.DeleteMessage(ctx, msgIDs...); err != nil {
-			return err
+		const ChunkSize = 150
+		var msgToPermaDelete []string
+
+		rdLabels := s.labels.Read()
+		defer rdLabels.Close()
+
+		// There's currently no limit on how many IDs we can filter on,
+		// but to be nice to API, let's chunk it by 150.
+		for _, messageIDs := range xslices.Chunk(messageIDs, ChunkSize) {
+			metadata, err := s.client.GetMessageMetadataPage(ctx, 0, ChunkSize, proton.MessageFilter{
+				ID: usertypes.MapTo[imap.MessageID, string](messageIDs),
+			})
+			if err != nil {
+				return err
+			}
+
+			// If a message is not preset in any other label other than AllMail, AllDrafts and AllSent, it can be
+			// permanently deleted.
+			for _, m := range metadata {
+				var remainingLabels []string
+
+				for _, id := range m.LabelIDs {
+					label, ok := rdLabels.GetLabel(id)
+					if !ok {
+						// Handle case where this label was newly introduced and we do not yet know about it.
+						logrus.WithField("labelID", id).Warnf("Unknown label found during expung from Trash, attempting to locate it")
+						label, err = s.client.GetLabel(ctx, id, proton.LabelTypeFolder, proton.LabelTypeSystem, proton.LabelTypeSystem)
+						if err != nil {
+							if errors.Is(err, proton.ErrNoSuchLabel) {
+								logrus.WithField("labelID", id).Warn("Label does not exist, ignoring")
+								continue
+							}
+
+							logrus.WithField("labelID", id).Errorf("Failed to resolve label: %v", err)
+							return fmt.Errorf("failed to resolve label: %w", err)
+						}
+					}
+					if !WantLabel(label) {
+						continue
+					}
+
+					if label.Type == proton.LabelTypeSystem && (id == proton.AllDraftsLabel ||
+						id == proton.AllMailLabel ||
+						id == proton.AllSentLabel ||
+						id == proton.AllScheduledLabel) {
+						continue
+					}
+
+					remainingLabels = append(remainingLabels, m.ID)
+				}
+
+				if len(remainingLabels) == 0 {
+					msgToPermaDelete = append(msgToPermaDelete, m.ID)
+				}
+			}
+		}
+
+		if len(msgToPermaDelete) != 0 {
+			logrus.Debugf("Following message(s) will be perma-deleted: %v", msgToPermaDelete)
+
+			if err := s.client.DeleteMessage(ctx, msgToPermaDelete...); err != nil {
+				return err
+			}
 		}
 	}
 

internal/services/syncservice/job.go

@@ -113,13 +113,14 @@ func (j *Job) onStageCompleted(ctx context.Context, count int64) {
 }
 
 func (j *Job) onJobFinished(ctx context.Context, lastMessageID string, count int64) {
-	defer j.wg.Done()
-
 	if err := j.state.SetLastMessageID(ctx, lastMessageID, count); err != nil {
 		j.log.WithError(err).Error("Failed to store last synced message id")
 		j.onError(err)
 		return
 	}
+
+	// j.onError() also calls j.wg.Done().
+	j.wg.Done()
 	j.syncReporter.OnProgress(ctx, count)
 }
 

internal/services/userevents/service.go

@@ -192,7 +192,6 @@ func (s *Service) run(ctx context.Context, lastEventID string) {
 	defer s.cpc.Close()
 	defer s.timer.Stop()
 	defer s.log.Info("Exiting service")
-	defer s.Close()
 
 	client := network.NewClientRetryWrapper(s.eventSource, &network.ExpCoolDown{})
 
@@ -303,14 +302,15 @@ func (s *Service) Close() {
 
 	// Cleanup pending removes.
 	for _, s := range s.pendingSubscriptions {
-		if s.op == pendingOpRemove {
-			if !processed.Contains(s.sub) {
+		if !processed.Contains(s.sub) {
+			processed.Add(s.sub)
+
+			if s.op == pendingOpRemove {
+				s.sub.close()
+			} else {
+				s.sub.cancel()
 				s.sub.close()
 			}
-		} else {
-			s.sub.cancel()
-			s.sub.close()
-			processed.Add(s.sub)
 		}
 	}
 

internal/user/user.go

@@ -589,6 +589,8 @@ func (user *User) Logout(ctx context.Context, withAPI bool) error {
 		return fmt.Errorf("failed to remove user from imap server: %w", err)
 	}
 
+	user.tasks.CancelAndWait()
+
 	// Stop Services
 	user.serviceGroup.CancelAndWait()
 
@@ -598,8 +600,6 @@ func (user *User) Logout(ctx context.Context, withAPI bool) error {
 	// Close imap service.
 	user.imapService.Close()
 
-	user.tasks.CancelAndWait()
-
 	if withAPI {
 		user.log.Debug("Logging out from API")
 
@@ -621,6 +621,9 @@ func (user *User) Logout(ctx context.Context, withAPI bool) error {
 func (user *User) Close() {
 	user.log.Info("Closing user")
 
+	// Stop any ongoing background tasks.
+	user.tasks.CancelAndWait()
+
 	// Stop Services
 	user.serviceGroup.CancelAndWait()
 
@@ -630,9 +633,6 @@ func (user *User) Close() {
 	// Close imap service.
 	user.imapService.Close()
 
-	// Stop any ongoing background tasks.
-	user.tasks.CancelAndWait()
-
 	// Close the user's API client.
 	user.client.Close()
 

pkg/files/removal.go

@@ -72,11 +72,12 @@ func remove(dir string, except ...string) error {
 
 	sort.Sort(sort.Reverse(sort.StringSlice(toRemove)))
 
+	var multiErr error
 	for _, target := range toRemove {
 		if err := os.RemoveAll(target); err != nil {
-			return err
+			multiErr = multierror.Append(multiErr, err)
 		}
 	}
 
-	return nil
+	return multiErr
 }

tests/features/imap/message/copy.feature

@@ -85,3 +85,18 @@ Feature: IMAP copy messages
       | from              | to                   | subject | unread |
       | john.doe@mail.com | [user:user]@[domain] | foo     | false  |
 
+  Scenario: Move message to trash then copy to folder does not delete message
+    When IMAP client "1" moves the message with subject "foo" from "INBOX" to "Trash"
+    And it succeeds
+    Then IMAP client "1" eventually sees the following messages in "Trash":
+      | from              | to                   | subject | unread |
+      | john.doe@mail.com | [user:user]@[domain] | foo     | false  |
+    When IMAP client "1" copies the message with subject "foo" from "Trash" to "Folders/mbox"
+    And it succeeds
+    When IMAP client "1" marks the message with subject "foo" as deleted
+    Then it succeeds
+    When IMAP client "1" expunges
+    Then it succeeds
+    Then IMAP client "1" eventually sees the following messages in "Folders/mbox":
+      | from              | to                   | subject | unread |
+      | john.doe@mail.com | [user:user]@[domain] | foo     | false  |

tests/features/imap/message/delete_from_trash.feature

@@ -7,7 +7,7 @@ Feature: IMAP remove messages from Trash
       | label | label  |
     Then it succeeds
 
-  Scenario Outline: Message in Trash and some other label is permanently deleted
+  Scenario Outline: Message in Trash and some other label is not permanently deleted
     Given the address "[user:user]@[domain]" of account "[user:user]" has the following messages in "Trash":
       | from              | to                   | subject | body  |
       | john.doe@mail.com | [user:user]@[domain] | foo     | hello |
@@ -27,8 +27,8 @@ Feature: IMAP remove messages from Trash
     When IMAP client "1" expunges
     Then it succeeds
     And IMAP client "1" eventually sees 1 messages in "Trash"
-    And IMAP client "1" eventually sees 1 messages in "All Mail"
-    And IMAP client "1" eventually sees 0 messages in "Labels/label"
+    And IMAP client "1" eventually sees 2 messages in "All Mail"
+    And IMAP client "1" eventually sees 1 messages in "Labels/label"
 
   Scenario Outline: Message in Trash only is permanently deleted
     Given the address "[user:user]@[domain]" of account "[user:user]" has the following messages in "Trash":