From 48cf89b1a6fe02adaaee292e2ed1896ec404a9e9 Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Fri, 11 Nov 2022 11:12:29 +0100 Subject: [PATCH] Other: Use cached labels for label sync rather than refetch --- internal/user/events.go | 4 +-- internal/user/sync.go | 55 ++++++++++++++------------------------ internal/user/user_test.go | 38 +++++++++++++------------- 3 files changed, 42 insertions(+), 55 deletions(-) diff --git a/internal/user/events.go b/internal/user/events.go index 201871b7..b1da6fb6 100644 --- a/internal/user/events.go +++ b/internal/user/events.go @@ -145,13 +145,13 @@ func (user *User) handleCreateAddressEvent(ctx context.Context, event liteapi.Ad // Perform the sync in an RLock. return safe.RLockRet(func() error { if user.vault.AddressMode() == vault.SplitMode { - if err := syncLabels(ctx, user.client, user.updateCh[event.Address.ID]); err != nil { + if err := syncLabels(ctx, user.apiLabels, user.updateCh[event.Address.ID]); err != nil { return fmt.Errorf("failed to sync labels to new address: %w", err) } } return nil - }, user.apiAddrsLock, user.updateChLock) + }, user.apiAddrsLock, user.apiLabelsLock, user.updateChLock) } func (user *User) handleUpdateAddressEvent(_ context.Context, event liteapi.AddressEvent) error { //nolint:unparam diff --git a/internal/user/sync.go b/internal/user/sync.go index a5cd50cb..f9810818 100644 --- a/internal/user/sync.go +++ b/internal/user/sync.go @@ -80,7 +80,7 @@ func (user *User) sync(ctx context.Context) error { if !user.vault.SyncStatus().HasLabels { user.log.Info("Syncing labels") - if err := syncLabels(ctx, user.client, xslices.Unique(maps.Values(user.updateCh))...); err != nil { + if err := syncLabels(ctx, user.apiLabels, xslices.Unique(maps.Values(user.updateCh))...); err != nil { return fmt.Errorf("failed to sync labels: %w", err) } @@ -121,50 +121,35 @@ func (user *User) sync(ctx context.Context) error { return nil }) - }, user.apiUserLock, user.apiAddrsLock, user.updateChLock) + }, user.apiUserLock, user.apiAddrsLock, user.apiLabelsLock, user.updateChLock) } -func syncLabels(ctx context.Context, client *liteapi.Client, updateCh ...*queue.QueuedChannel[imap.Update]) error { - // Sync the system folders. - system, err := client.GetLabels(ctx, liteapi.LabelTypeSystem) - if err != nil { - return fmt.Errorf("failed to get system labels: %w", err) - } - - for _, label := range xslices.Filter(system, func(label liteapi.Label) bool { return wantLabelID(label.ID) }) { - for _, updateCh := range updateCh { - updateCh.Enqueue(newSystemMailboxCreatedUpdate(imap.MailboxID(label.ID), label.Name)) - } - } - - // Create Folders/Labels mailboxes with a random ID and with the \Noselect attribute. +// nolint:exhaustive +func syncLabels(ctx context.Context, apiLabels map[string]liteapi.Label, updateCh ...*queue.QueuedChannel[imap.Update]) error { + // Create placeholder Folders/Labels mailboxes with a random ID and with the \Noselect attribute. for _, prefix := range []string{folderPrefix, labelPrefix} { for _, updateCh := range updateCh { updateCh.Enqueue(newPlaceHolderMailboxCreatedUpdate(prefix)) } } - // Sync the API folders. - folders, err := client.GetLabels(ctx, liteapi.LabelTypeFolder) - if err != nil { - return fmt.Errorf("failed to get folders: %w", err) - } + // Sync the user's labels. + for labelID, label := range apiLabels { + switch label.Type { + case liteapi.LabelTypeSystem: + if wantLabelID(labelID) { + for _, updateCh := range updateCh { + updateCh.Enqueue(newSystemMailboxCreatedUpdate(imap.MailboxID(label.ID), label.Name)) + } + } - for _, folder := range folders { - for _, updateCh := range updateCh { - updateCh.Enqueue(newMailboxCreatedUpdate(imap.MailboxID(folder.ID), getMailboxName(folder))) - } - } + case liteapi.LabelTypeFolder, liteapi.LabelTypeLabel: + for _, updateCh := range updateCh { + updateCh.Enqueue(newMailboxCreatedUpdate(imap.MailboxID(labelID), getMailboxName(label))) + } - // Sync the API labels. - labels, err := client.GetLabels(ctx, liteapi.LabelTypeLabel) - if err != nil { - return fmt.Errorf("failed to get labels: %w", err) - } - - for _, label := range labels { - for _, updateCh := range updateCh { - updateCh.Enqueue(newMailboxCreatedUpdate(imap.MailboxID(label.ID), getMailboxName(label))) + default: + return fmt.Errorf("unknown label type: %d", label.Type) } } diff --git a/internal/user/user_test.go b/internal/user/user_test.go index 5c7df345..9f561411 100644 --- a/internal/user/user_test.go +++ b/internal/user/user_test.go @@ -47,7 +47,7 @@ func TestMain(m *testing.M) { func TestUser_Info(t *testing.T) { withAPI(t, context.Background(), func(ctx context.Context, s *server.Server, m *liteapi.Manager) { - withAccount(t, s, "username", "password", []string{"email@pm.me", "alias@pm.me"}, func(userID string, addrIDs []string) { + withAccount(t, s, "username", "password", []string{"email@pm.me", "alias@pm.me"}, func(userID string, _ []string) { withUser(t, ctx, s, m, "username", "password", func(user *User) { // User's ID should be correct. require.Equal(t, userID, user.ID()) @@ -70,11 +70,8 @@ func TestUser_Info(t *testing.T) { func TestUser_Sync(t *testing.T) { withAPI(t, context.Background(), func(ctx context.Context, s *server.Server, m *liteapi.Manager) { - withAccount(t, s, "username", "password", []string{"email@pm.me"}, func(userID string, addrIDs []string) { + withAccount(t, s, "username", "password", []string{"email@pm.me"}, func(string, []string) { withUser(t, ctx, s, m, "username", "password", func(user *User) { - // Process the IMAP updates as if we were gluon. - handleUpdates(t, user) - // User starts a sync at startup. require.IsType(t, events.SyncStarted{}, <-user.GetEventCh()) @@ -90,11 +87,8 @@ func TestUser_Sync(t *testing.T) { func TestUser_AddressMode(t *testing.T) { withAPI(t, context.Background(), func(ctx context.Context, s *server.Server, m *liteapi.Manager) { - withAccount(t, s, "username", "password", []string{"email@pm.me", "alias@pm.me"}, func(userID string, addrIDs []string) { + withAccount(t, s, "username", "password", []string{"email@pm.me", "alias@pm.me"}, func(string, []string) { withUser(t, ctx, s, m, "username", "password", func(user *User) { - // Process the IMAP updates as if we were gluon. - handleUpdates(t, user) - // User finishes syncing at startup. require.IsType(t, events.SyncStarted{}, <-user.GetEventCh()) require.IsType(t, events.SyncProgress{}, <-user.GetEventCh()) @@ -106,8 +100,18 @@ func TestUser_AddressMode(t *testing.T) { // User should be able to switch to split mode. require.NoError(t, user.SetAddressMode(ctx, vault.SplitMode)) - // Process the IMAP updates as if we were gluon. - handleUpdates(t, user) + // Create a new set of IMAP connectors (after switching to split mode). + imapConn, err := user.NewIMAPConnectors() + require.NoError(t, err) + + // Process updates from the new set of IMAP connectors. + for _, imapConn := range imapConn { + go func(imapConn connector.Connector) { + for update := range imapConn.GetUpdates() { + update.Done() + } + }(imapConn) + } // User finishes syncing after switching to split mode. require.IsType(t, events.SyncStarted{}, <-user.GetEventCh()) @@ -120,7 +124,7 @@ func TestUser_AddressMode(t *testing.T) { func TestUser_Deauth(t *testing.T) { withAPI(t, context.Background(), func(ctx context.Context, s *server.Server, m *liteapi.Manager) { - withAccount(t, s, "username", "password", []string{"email@pm.me"}, func(userID string, addrIDs []string) { + withAccount(t, s, "username", "password", []string{"email@pm.me"}, func(string, []string) { withUser(t, ctx, s, m, "username", "password", func(user *User) { eventCh := user.GetEventCh() @@ -128,7 +132,7 @@ func TestUser_Deauth(t *testing.T) { require.NoError(t, s.RevokeUser(user.ID())) // The user should eventually be logged out. - require.Eventually(t, func() bool { _, ok := (<-eventCh).(events.UserDeauth); return ok }, 5*time.Second, 100*time.Millisecond) + require.Eventually(t, func() bool { _, ok := (<-eventCh).(events.UserDeauth); return ok }, 500*time.Second, 100*time.Millisecond) }) }) }) @@ -186,12 +190,8 @@ func withUser(tb testing.TB, ctx context.Context, _ *server.Server, m *liteapi.M require.NoError(tb, err) defer user.Close() - fn(user) -} - -func handleUpdates(t *testing.T, user *User) { imapConn, err := user.NewIMAPConnectors() - require.NoError(t, err) + require.NoError(tb, err) for _, imapConn := range imapConn { go func(imapConn connector.Connector) { @@ -200,4 +200,6 @@ func handleUpdates(t *testing.T, user *User) { } }(imapConn) } + + fn(user) }