Compare commits

...

9 Commits

Author SHA1 Message Date
b7ef6e1486 chore: Umshiang Bridge 3.5.1 changelog. 2023-09-27 13:18:23 +02:00
0d03f84711 fix(GODT-2963): Use multi error to report file removal errors
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.
2023-09-27 12:34:07 +02:00
949666724d chore: Umshiang Bridge 3.5.1 changelog. 2023-09-27 10:54:50 +02:00
bbe19bf960 fix(GODT-2956): Restore old deletion rules
When unlabeling a message from trash we have to check if this message is
present in another folder before perma-deleting.
2023-09-26 14:06:31 +02:00
bfe25e3a46 fix(GODT-2951): Negative WaitGroup Counter
Do not defer call to `wg.Done()` in `job.onJobFinished`. If there is an
error it will also call `wg.Done()`.
2023-09-26 13:58:46 +02:00
236c958703 fix(GODT-2590): Fix send on closed channel
Ensure periodic user tasks are terminated before the other user
services. The panic triggered due to the fact that the telemetry service
was shutdown before this periodic task.
2023-09-26 13:58:18 +02:00
e6b312b437 fix(GODT-2949): Fix close of close channel in event service
This issue is triggered due to the `Service.Close()` call after the
go-routine for the event service exists. It is possible that during this
period a recently added subscriber with `pendingOpAdd` gets cancelled
and closed.

However, the subscriber later also enqueues a `pendingOpRemove` which
gets processed again with a call in `user.eventService.Close()` leading
to the double close panic.

This patch simply removes the `s.Close()` from the service, and leaves
the cleanup to called externally from user.Close() or user.Logout().
2023-09-26 13:58:07 +02:00
45d2e9ea63 chore: update changelog. 2023-09-13 10:25:47 +02:00
86e8a566c7 chore: Umshiang Bridge 3.5.0 changelog. 2023-09-12 07:45:08 +02:00
9 changed files with 116 additions and 22 deletions

View File

@ -3,9 +3,21 @@
Changelog [format](http://keepachangelog.com/en/1.0.0/) 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 ## Umshiang Bridge 3.5.0
### Added ### Added
* GODT-2734: Add testing steps to modify account settings.
* GODT-2746: Integration tests for reporting a problem.
* GODT-2891: Allow message create & delete during sync. * GODT-2891: Allow message create & delete during sync.
* GODT-2848: Decouple IMAP service from Event Loop. * GODT-2848: Decouple IMAP service from Event Loop.
* Add trace profiling option. * Add trace profiling option.
@ -19,6 +31,8 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/)
* GODT-2803: Bridge Database access. * GODT-2803: Bridge Database access.
### Changed ### Changed
* GODT-2909: Remove Timeout on event publish.
* GODT-2913: Reduce the number of configuration failure detected.
* GODT-2828: Increase sync progress report frequency. * GODT-2828: Increase sync progress report frequency.
* Test: Fix TestBridge_SyncWithOnGoingEvents. * Test: Fix TestBridge_SyncWithOnGoingEvents.
* GODT-2871: Is telemetry enabled as service. * GODT-2871: Is telemetry enabled as service.
@ -71,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-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. * GODT-2778: Fix login screen being disabled after an 'already logged in' error.
* Fix typos found by codespell. * Fix typos found by codespell.
* GODT-2577: Answered flag should only be applied to replied messages.
## Trift Bridge 3.4.1 ## Trift Bridge 3.4.1

View File

@ -11,7 +11,7 @@ ROOT_DIR:=$(shell dirname $(realpath $(firstword $(MAKEFILE_LIST))))
.PHONY: build build-gui build-nogui build-launcher versioner hasher .PHONY: build build-gui build-nogui build-launcher versioner hasher
# Keep version hardcoded so app build works also without Git repository. # 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_VERSION:=${BRIDGE_APP_VERSION}
APP_FULL_NAME:=Proton Mail Bridge APP_FULL_NAME:=Proton Mail Bridge
APP_VENDOR:=Proton AG APP_VENDOR:=Proton AG

View File

@ -37,6 +37,7 @@ import (
"github.com/ProtonMail/proton-bridge/v3/pkg/message" "github.com/ProtonMail/proton-bridge/v3/pkg/message"
"github.com/ProtonMail/proton-bridge/v3/pkg/message/parser" "github.com/ProtonMail/proton-bridge/v3/pkg/message/parser"
"github.com/bradenaw/juniper/stream" "github.com/bradenaw/juniper/stream"
"github.com/bradenaw/juniper/xslices"
"github.com/sirupsen/logrus" "github.com/sirupsen/logrus"
"golang.org/x/exp/slices" "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 mboxID == proton.TrashLabel || mboxID == proton.DraftsLabel {
if err := s.client.DeleteMessage(ctx, msgIDs...); err != nil { const ChunkSize = 150
return err 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
}
} }
} }

View File

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

View File

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

View File

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

View File

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

View File

@ -85,3 +85,18 @@ Feature: IMAP copy messages
| from | to | subject | unread | | from | to | subject | unread |
| john.doe@mail.com | [user:user]@[domain] | foo | false | | 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 |

View File

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