From 0823d393ed7c972e79fa49e5aa690045c0ec386c Mon Sep 17 00:00:00 2001 From: Xavier Michelon Date: Thu, 9 Feb 2023 16:25:21 +0100 Subject: [PATCH] feat(GODT-1264): creation and visibility of the 'Scheduled' system label. feat(GODT-1264): typo in error message feat(GODT-1264): fix split mode broken by previous commit. --- go.mod | 2 +- go.sum | 4 +-- internal/bridge/bridge_test.go | 2 +- internal/bridge/sync_test.go | 45 ++++++++++++++++++++++++++++++++++ internal/user/imap.go | 17 ++++++++++--- internal/user/sync.go | 32 +++++++++++++++++++++++- internal/user/user.go | 7 +++++- 7 files changed, 100 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index d46e2155..28168d93 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.18 require ( github.com/0xAX/notificator v0.0.0-20220220101646-ee9b8921e557 github.com/Masterminds/semver/v3 v3.1.1 - github.com/ProtonMail/gluon v0.14.2-0.20230206162331-cf36d870802b + github.com/ProtonMail/gluon v0.14.2-0.20230207142445-9f98ae47a031 github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a github.com/ProtonMail/go-proton-api v0.4.0 github.com/ProtonMail/go-rfc5322 v0.11.0 diff --git a/go.sum b/go.sum index df711095..bf8875d9 100644 --- a/go.sum +++ b/go.sum @@ -28,8 +28,8 @@ github.com/ProtonMail/bcrypt v0.0.0-20211005172633-e235017c1baf h1:yc9daCCYUefEs github.com/ProtonMail/bcrypt v0.0.0-20211005172633-e235017c1baf/go.mod h1:o0ESU9p83twszAU8LBeJKFAAMX14tISa0yk4Oo5TOqo= github.com/ProtonMail/docker-credential-helpers v1.1.0 h1:+kvUIpwWcbtP3WFv5sSvkFn/XLzSqPOB5AAthuk9xPk= github.com/ProtonMail/docker-credential-helpers v1.1.0/go.mod h1:mK0aBveCxhnQ756AmaTfXMZDeULvheYVhF/MWMErN5g= -github.com/ProtonMail/gluon v0.14.2-0.20230206162331-cf36d870802b h1:v/XwH5Em8gFSpJQErhSCt0XAsIxojFxgrVcfPUEWH7I= -github.com/ProtonMail/gluon v0.14.2-0.20230206162331-cf36d870802b/go.mod h1:HYHr7hG7LPWI1S50M8NfHRb1kYi5B+Yu4/N/H+y+JUY= +github.com/ProtonMail/gluon v0.14.2-0.20230207142445-9f98ae47a031 h1:QxHAyLmGqBtxTybgDwXwqvcA8auTcTGo8ivcEh2IIr8= +github.com/ProtonMail/gluon v0.14.2-0.20230207142445-9f98ae47a031/go.mod h1:HYHr7hG7LPWI1S50M8NfHRb1kYi5B+Yu4/N/H+y+JUY= github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a h1:D+aZah+k14Gn6kmL7eKxoo/4Dr/lK3ChBcwce2+SQP4= github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a/go.mod h1:oTGdE7/DlWIr23G0IKW3OXK9wZ5Hw1GGiaJFccTvZi4= github.com/ProtonMail/go-crypto v0.0.0-20210428141323-04723f9f07d7/go.mod h1:z4/9nQmJSSwwds7ejkxaJwO37dru3geImFUdJlaLzQo= diff --git a/internal/bridge/bridge_test.go b/internal/bridge/bridge_test.go index ec4f5b83..992d241f 100644 --- a/internal/bridge/bridge_test.go +++ b/internal/bridge/bridge_test.go @@ -770,7 +770,7 @@ func withBridge( }) } -func waitForEvent[T any](t *testing.T, eventCh <-chan events.Event, wantEvent T) { +func waitForEvent[T any](t *testing.T, eventCh <-chan events.Event, _ T) { t.Helper() for event := range eventCh { diff --git a/internal/bridge/sync_test.go b/internal/bridge/sync_test.go index 329a42c5..db9854ad 100644 --- a/internal/bridge/sync_test.go +++ b/internal/bridge/sync_test.go @@ -24,6 +24,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "sync/atomic" "testing" "time" @@ -43,6 +44,8 @@ import ( "github.com/stretchr/testify/require" ) +const scheduled = "Scheduled" + func TestBridge_Sync(t *testing.T) { numMsg := 1 << 8 @@ -468,3 +471,45 @@ func countBytesRead(ctl *proton.NetCtl, fn func()) uint64 { return read } + +func TestBridge_ScheduledLabel(t *testing.T) { + withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, storeKey []byte) { + hasScheduledSystemLabel := func(imapClient *client.Client) bool { + return xslices.Any(clientList(imapClient), func(mailboxInfo *imap.MailboxInfo) bool { return mailboxInfo.Name == scheduled }) + } + + _, _, err := s.CreateUser(username, password) + require.NoError(t, err) + + withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(b *bridge.Bridge, _ *bridge.Mocks) { + // Perform initial sync + syncCh, done := chToType[events.Event, events.SyncFinished](b.GetEvents(events.SyncFinished{})) + defer done() + userID, err := b.LoginFull(ctx, username, password, nil, nil) + require.NoError(t, err) + require.Equal(t, userID, (<-syncCh).UserID) + + // connect an IMAP client + info, err := b.GetUserInfo(userID) + require.NoError(t, err) + imapClient, err := client.Dial(fmt.Sprintf("%v:%v", constants.Host, b.GetIMAPPort())) + require.NoError(t, err) + require.NoError(t, imapClient.Login(info.Addresses[0], string(info.BridgePass))) + defer func() { _ = imapClient.Logout() }() + + // Scheduled mailbox is empty. It's not listed. + require.False(t, hasScheduledSystemLabel(imapClient)) + + // Add a message to the Schedule mailbox. It's now listed. + require.NoError(t, imapClient.Append(scheduled, []string{}, time.Now(), strings.NewReader("To: no_reply@pm.me"))) + require.True(t, hasScheduledSystemLabel(imapClient)) + + // delete message from Scheduled. The mailbox is now empty and not listed anymore + _, err = imapClient.Select(scheduled, false) + require.NoError(t, err) + require.NoError(t, clientStore(imapClient, 1, 1, true, imap.FormatFlagsOp(imap.AddFlags, true), imap.DeletedFlag)) + require.NoError(t, imapClient.Expunge(nil)) + require.False(t, hasScheduledSystemLabel(imapClient)) + }) + }) +} diff --git a/internal/user/imap.go b/internal/user/imap.go index 84fc8943..e144f3da 100644 --- a/internal/user/imap.go +++ b/internal/user/imap.go @@ -505,9 +505,20 @@ func (conn *imapConnector) GetUpdates() <-chan imap.Update { }, conn.updateChLock) } -// IsMailboxVisible returns whether this mailbox should be visible over IMAP. -func (conn *imapConnector) IsMailboxVisible(_ context.Context, mailboxID imap.MailboxID) bool { - return atomic.LoadUint32(&conn.showAllMail) != 0 || mailboxID != proton.AllMailLabel +// GetMailboxVisibility returns the visibility of a mailbox over IMAP. +func (conn *imapConnector) GetMailboxVisibility(_ context.Context, mailboxID imap.MailboxID) imap.MailboxVisibility { + switch mailboxID { + case proton.AllMailLabel: + if atomic.LoadUint32(&conn.showAllMail) != 0 { + return imap.Visible + } + return imap.Hidden + + case proton.AllScheduledLabel: + return imap.HiddenIfEmpty + default: + return imap.Visible + } } // Close the connector will no longer be used and all resources should be closed/released. diff --git a/internal/user/sync.go b/internal/user/sync.go index f584c662..a46c81f4 100644 --- a/internal/user/sync.go +++ b/internal/user/sync.go @@ -43,7 +43,31 @@ import ( "golang.org/x/exp/slices" ) -// doSync begins syncing the users data. +// syncSystemLabels ensures that system labels are all known to gluon. +func (user *User) syncSystemLabels(ctx context.Context) error { + return safe.RLockRet(func() error { + var updates []imap.Update + + for _, label := range xslices.Filter(maps.Values(user.apiLabels), func(label proton.Label) bool { return label.Type == proton.LabelTypeSystem }) { + if !wantLabel(label) { + continue + } + + for _, updateCh := range xslices.Unique(maps.Values(user.updateCh)) { + update := newSystemMailboxCreatedUpdate(imap.MailboxID(label.ID), label.Name) + updateCh.Enqueue(update) + updates = append(updates, update) + } + } + if err := waitOnIMAPUpdates(ctx, updates); err != nil { + return fmt.Errorf("could not sync system labels: %w", err) + } + + return nil + }, user.apiUserLock, user.apiAddrsLock, user.apiLabelsLock, user.updateChLock) +} + +// doSync begins syncing the user's data. // It first ensures the latest event ID is known; if not, it fetches it. // It sends a SyncStarted event and then either SyncFinished or SyncFailed // depending on whether the sync was successful. @@ -658,6 +682,9 @@ func newSystemMailboxCreatedUpdate(labelID imap.MailboxID, labelName string) *im case proton.StarredLabel: attrs = attrs.Add(imap.AttrFlagged) + + case proton.AllScheduledLabel: + labelName = "Scheduled" // API actual name is "All Scheduled" } return imap.NewMailboxCreated(imap.Mailbox{ @@ -720,6 +747,9 @@ func wantLabel(label proton.Label) bool { case proton.StarredLabel: return true + case proton.AllScheduledLabel: + return true + default: return false } diff --git a/internal/user/user.go b/internal/user/user.go index a5cffba6..ae85d7a9 100644 --- a/internal/user/user.go +++ b/internal/user/user.go @@ -190,7 +190,12 @@ func New( // Sync the user. user.syncAbort.Do(ctx, func(ctx context.Context) { if user.vault.SyncStatus().IsComplete() { - user.log.Info("Sync already complete, skipping") + user.log.Info("Sync already complete, only system label will be updated") + if err := user.syncSystemLabels(ctx); err != nil { + user.log.WithError(err).Error("Failed to update system labels") + return + } + user.log.Info("System label update complete, starting API event stream") return }