From 00a782ef62b92adc0fce6685cf49ac6e6e6e276f Mon Sep 17 00:00:00 2001 From: Xavier Michelon Date: Tue, 7 Feb 2023 10:58:06 +0100 Subject: [PATCH] feat(GODT-1264): adds 'Hidden if empty' options for mailboxes. --- connector/connector.go | 4 +-- connector/dummy.go | 41 ++++++++++++------------ imap/mailbox.go | 8 +++++ internal/backend/state_connector_impl.go | 4 +-- internal/state/connector.go | 4 +-- internal/state/state.go | 19 +++++++++-- tests/list_test.go | 28 +++++++++++++--- tests/lsub_test.go | 8 ++--- tests/session_test.go | 2 +- 9 files changed, 80 insertions(+), 38 deletions(-) diff --git a/connector/connector.go b/connector/connector.go index a54136e0..57969a4d 100644 --- a/connector/connector.go +++ b/connector/connector.go @@ -19,8 +19,8 @@ type Connector interface { // Note: this can get called from different go routines. GetMessageLiteral(ctx context.Context, id imap.MessageID) ([]byte, error) - // IsMailboxVisible can be used to hide mailboxes from connected clients. - IsMailboxVisible(ctx context.Context, mboxID imap.MailboxID) bool + // GetMailboxVisibility can be used to retrieve the visibility of mailboxes for connected clients. + GetMailboxVisibility(ctx context.Context, mboxID imap.MailboxID) imap.MailboxVisibility // UpdateMailboxName sets the name of the mailbox with the given ID. UpdateMailboxName(ctx context.Context, mboxID imap.MailboxID, newName []string) error diff --git a/connector/dummy.go b/connector/dummy.go index 0bd61751..f7c037b4 100644 --- a/connector/dummy.go +++ b/connector/dummy.go @@ -53,8 +53,8 @@ type Dummy struct { queue []imap.Update queueLock sync.Mutex - // hiddenMailboxes holds mailboxes that are hidden from the user. - hiddenMailboxes map[imap.MailboxID]struct{} + // hiddenMailboxes holds the visibility status of the mailboxes. Mailboxes not listed are considered visible. + mailboxVisibilities map[imap.MailboxID]imap.MailboxVisibility allowMessageCreateWithUnknownMailboxID bool @@ -63,16 +63,16 @@ type Dummy struct { func NewDummy(usernames []string, password []byte, period time.Duration, flags, permFlags, attrs imap.FlagSet) *Dummy { conn := &Dummy{ - state: newDummyState(flags, permFlags, attrs), - usernames: usernames, - password: password, - flags: flags, - permFlags: permFlags, - attrs: attrs, - updateCh: make(chan imap.Update, constants.ChannelBufferCount), - updateQuitCh: make(chan struct{}), - ticker: ticker.New(period), - hiddenMailboxes: make(map[imap.MailboxID]struct{}), + state: newDummyState(flags, permFlags, attrs), + usernames: usernames, + password: password, + flags: flags, + permFlags: permFlags, + attrs: attrs, + updateCh: make(chan imap.Update, constants.ChannelBufferCount), + updateQuitCh: make(chan struct{}), + ticker: ticker.New(period), + mailboxVisibilities: make(map[imap.MailboxID]imap.MailboxVisibility), } go func() { @@ -316,18 +316,17 @@ func (conn *Dummy) ClearUpdates() { conn.popUpdates() } -func (conn *Dummy) IsMailboxVisible(_ context.Context, id imap.MailboxID) bool { - _, ok := conn.hiddenMailboxes[id] +func (conn *Dummy) GetMailboxVisibility(_ context.Context, id imap.MailboxID) imap.MailboxVisibility { + visibility, ok := conn.mailboxVisibilities[id] + if !ok { + return imap.Visible + } - return !ok + return visibility } -func (conn *Dummy) SetMailboxVisible(id imap.MailboxID, visible bool) { - if !visible { - conn.hiddenMailboxes[id] = struct{}{} - } else { - delete(conn.hiddenMailboxes, id) - } +func (conn *Dummy) SetMailboxVisibility(id imap.MailboxID, visibility imap.MailboxVisibility) { + conn.mailboxVisibilities[id] = visibility } func (conn *Dummy) pushUpdate(update imap.Update) { diff --git a/imap/mailbox.go b/imap/mailbox.go index 620b4a66..a5d46ee8 100644 --- a/imap/mailbox.go +++ b/imap/mailbox.go @@ -9,3 +9,11 @@ type Mailbox struct { } const Inbox = "INBOX" + +type MailboxVisibility int + +const ( + Hidden MailboxVisibility = iota + Visible + HiddenIfEmpty +) diff --git a/internal/backend/state_connector_impl.go b/internal/backend/state_connector_impl.go index a3b3f2f9..0b583641 100644 --- a/internal/backend/state_connector_impl.go +++ b/internal/backend/state_connector_impl.go @@ -124,8 +124,8 @@ func (sc *stateConnectorImpl) SetMessagesFlagged(ctx context.Context, messageIDs return sc.connector.MarkMessagesFlagged(ctx, messageIDs, flagged) } -func (sc *stateConnectorImpl) IsMailboxVisible(ctx context.Context, id imap.MailboxID) bool { - return sc.connector.IsMailboxVisible(ctx, id) +func (sc *stateConnectorImpl) GetMailboxVisibility(ctx context.Context, id imap.MailboxID) imap.MailboxVisibility { + return sc.connector.GetMailboxVisibility(ctx, id) } func (sc *stateConnectorImpl) getMetadataValue(key string) any { diff --git a/internal/state/connector.go b/internal/state/connector.go index 06d6a17e..7f2b4e7d 100644 --- a/internal/state/connector.go +++ b/internal/state/connector.go @@ -73,6 +73,6 @@ type Connector interface { // SetMessagesFlagged marks the message with the given ID as seen or unseen. SetMessagesFlagged(ctx context.Context, messageIDs []imap.MessageID, flagged bool) error - // IsMailboxVisible checks whether a mailbox is visible to a client. - IsMailboxVisible(ctx context.Context, id imap.MailboxID) bool + // GetMailboxVisibility retrieves the visibility status of a mailbox for a client. + GetMailboxVisibility(ctx context.Context, id imap.MailboxID) imap.MailboxVisibility } diff --git a/internal/state/state.go b/internal/state/state.go index 4d07f350..e01c7ce1 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -97,7 +97,22 @@ func (state *State) List(ctx context.Context, ref, pattern string, subscribed bo return false } - return state.user.GetRemote().IsMailboxVisible(ctx, mailbox.RemoteID) + switch visibility := state.user.GetRemote().GetMailboxVisibility(ctx, mailbox.RemoteID); visibility { + case imap.Hidden: + return false + case imap.Visible: + return true + case imap.HiddenIfEmpty: + count, err := db.GetMailboxMessageCount(ctx, client, mailbox.ID) + if err != nil { + logrus.WithError(err).Error("Failed to get recovery mailbox message count, assuming not empty") + return true + } + return count > 0 + default: + logrus.Errorf("Unknown IMAP Mailbox visibility %v", visibility) + return true + } }) var deletedSubscriptions map[imap.MailboxID]*ent.DeletedSubscription @@ -126,7 +141,7 @@ func (state *State) List(ctx context.Context, ref, pattern string, subscribed bo if subscribed { // Insert any remaining mailboxes that have been deleted but are still subscribed. for _, s := range deletedSubscriptions { - if !state.user.GetRemote().IsMailboxVisible(ctx, s.RemoteID) { + if state.user.GetRemote().GetMailboxVisibility(ctx, s.RemoteID) != imap.Visible { continue } diff --git a/tests/list_test.go b/tests/list_test.go index 2b60dee8..d926e09d 100644 --- a/tests/list_test.go +++ b/tests/list_test.go @@ -2,6 +2,7 @@ package tests import ( "testing" + "time" "github.com/ProtonMail/gluon/imap" ) @@ -282,13 +283,18 @@ func TestListHiddenMailbox(t *testing.T) { m3 := s.mailboxCreatedWithAttributes("user", []string{"Spam"}, imap.NewFlagSet()) s.mailboxCreatedWithAttributes("user", []string{"Kos"}, imap.NewFlagSet()) m4 := s.mailboxCreatedWithAttributes("user", []string{"Vsechny zpravy"}, imap.NewFlagSet()) + m5 := s.mailboxCreatedWithAttributes("user", []string{"HiddenIfEmpty1"}, imap.NewFlagSet()) + m6 := s.mailboxCreatedWithAttributes("user", []string{"HiddenIfEmpty2"}, imap.NewFlagSet()) + msg1 := s.messageCreatedWithMailboxes("user", []imap.MailboxID{m5}, []byte("To: no-reply@pm.me"), time.Now()) { connector := s.conns[s.userIDs["user"]] - connector.SetMailboxVisible(m1, false) - connector.SetMailboxVisible(m2, false) - connector.SetMailboxVisible(m3, false) - connector.SetMailboxVisible(m4, false) + connector.SetMailboxVisibility(m1, imap.Hidden) + connector.SetMailboxVisibility(m2, imap.Hidden) + connector.SetMailboxVisibility(m3, imap.Hidden) + connector.SetMailboxVisibility(m4, imap.Hidden) + connector.SetMailboxVisibility(m5, imap.HiddenIfEmpty) + connector.SetMailboxVisibility(m6, imap.HiddenIfEmpty) } c.C(`a list "" "*"`) @@ -297,6 +303,20 @@ func TestListHiddenMailbox(t *testing.T) { `* LIST (\Unmarked) "." "Odeslane"`, `* LIST (\Unmarked) "." "Archiv"`, `* LIST (\Unmarked) "." "Kos"`, + `* LIST (\Marked) "." "HiddenIfEmpty1"`, + ) + c.OK(`a`) + + s.messageRemoved("user", msg1, m5) + s.messageAdded("user", msg1, m6) + + c.C(`a list "" "*"`) + c.S( + `* LIST (\Unmarked) "." "INBOX"`, + `* LIST (\Unmarked) "." "Odeslane"`, + `* LIST (\Unmarked) "." "Archiv"`, + `* LIST (\Unmarked) "." "Kos"`, + `* LIST (\Marked) "." "HiddenIfEmpty2"`, ) c.OK(`a`) }) diff --git a/tests/lsub_test.go b/tests/lsub_test.go index bd3b68be..7b44ddb2 100644 --- a/tests/lsub_test.go +++ b/tests/lsub_test.go @@ -143,10 +143,10 @@ func TestLsubWithHiddenMailbox(t *testing.T) { { connector := s.conns[s.userIDs["user"]] - connector.SetMailboxVisible(m1, false) - connector.SetMailboxVisible(m2, false) - connector.SetMailboxVisible(m3, false) - connector.SetMailboxVisible(m4, false) + connector.SetMailboxVisibility(m1, imap.Hidden) + connector.SetMailboxVisibility(m2, imap.Hidden) + connector.SetMailboxVisibility(m3, imap.Hidden) + connector.SetMailboxVisibility(m4, imap.HiddenIfEmpty) } { diff --git a/tests/session_test.go b/tests/session_test.go index b189e36d..47d1b69e 100644 --- a/tests/session_test.go +++ b/tests/session_test.go @@ -31,7 +31,7 @@ type Connector interface { MailboxCreated(imap.Mailbox) error MailboxDeleted(imap.MailboxID) error - SetMailboxVisible(imap.MailboxID, bool) + SetMailboxVisibility(imap.MailboxID, imap.MailboxVisibility) SetAllowMessageCreateWithUnknownMailboxID(value bool)