Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(GODT-1264): adds 'Hidden if empty' options for mailboxes. #273

Merged
merged 1 commit into from
Feb 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions connector/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
41 changes: 20 additions & 21 deletions connector/dummy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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() {
Expand Down Expand Up @@ -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) {
Expand Down
8 changes: 8 additions & 0 deletions imap/mailbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,11 @@ type Mailbox struct {
}

const Inbox = "INBOX"

type MailboxVisibility int

const (
Hidden MailboxVisibility = iota
Visible
HiddenIfEmpty
)
4 changes: 2 additions & 2 deletions internal/backend/state_connector_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions internal/state/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
19 changes: 17 additions & 2 deletions internal/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
28 changes: 24 additions & 4 deletions tests/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package tests

import (
"testing"
"time"

"github.com/ProtonMail/gluon/imap"
)
Expand Down Expand Up @@ -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 "" "*"`)
Expand All @@ -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`)
})
Expand Down
8 changes: 4 additions & 4 deletions tests/lsub_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

{
Expand Down
2 changes: 1 addition & 1 deletion tests/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down