GODT-1984: Handle permanent message deletion

Only delete messages when unlabeled from trash/spam if they only exists
in All Mail and (spam or trash).

This patch also ports delete_from_trash.feature and use status rather
than fetch to count messages in a mailboxes.
This commit is contained in:
Leander Beernaert
2022-10-27 14:56:26 +02:00
committed by James Houlahan
parent df818bc2b8
commit 8f420d728c
7 changed files with 141 additions and 16 deletions

4
go.mod
View File

@ -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.13.1-0.20221026080908-3f1806709bdd
github.com/ProtonMail/gluon v0.13.1-0.20221028093845-b3dcaa4500d4
github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a
github.com/ProtonMail/go-rfc5322 v0.11.0
github.com/ProtonMail/gopenpgp/v2 v2.4.10
@ -39,7 +39,7 @@ require (
github.com/stretchr/testify v1.8.0
github.com/urfave/cli/v2 v2.20.3
github.com/vmihailenco/msgpack/v5 v5.3.5
gitlab.protontech.ch/go/liteapi v0.36.2
gitlab.protontech.ch/go/liteapi v0.37.0
go.uber.org/goleak v1.2.0
golang.org/x/exp v0.0.0-20221023144134-a1e5550cf13e
golang.org/x/net v0.1.0

8
go.sum
View File

@ -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.13.1-0.20221026080908-3f1806709bdd h1:2R9kbvUVmGSHK4b0kGLyxruX9Ea0EO23km2xwYheWrY=
github.com/ProtonMail/gluon v0.13.1-0.20221026080908-3f1806709bdd/go.mod h1:XW/gcr4jErc5bX5yMqkUq3U+AucC2QZHJ5L231k3Nw4=
github.com/ProtonMail/gluon v0.13.1-0.20221028093845-b3dcaa4500d4 h1:/HiAwJI7sgwkDmLKmRSmBHLrMVgwjgjMG64fyM/kVfk=
github.com/ProtonMail/gluon v0.13.1-0.20221028093845-b3dcaa4500d4/go.mod h1:XW/gcr4jErc5bX5yMqkUq3U+AucC2QZHJ5L231k3Nw4=
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=
@ -403,8 +403,8 @@ github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673/go.mod h1:N3UwUGtsr
github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
github.com/zclconf/go-cty v1.11.0 h1:726SxLdi2SDnjY+BStqB9J1hNp4+2WlzyXLuimibIe0=
github.com/zclconf/go-cty v1.11.0/go.mod h1:s9IfD1LK5ccNMSWCVFCE2rJfHiZgi7JijgeWIMfhLvA=
gitlab.protontech.ch/go/liteapi v0.36.2 h1:Vu7zKIwJNQ46X7ou0n8wXDS9uoRTAhyyVnuH5tM2vVY=
gitlab.protontech.ch/go/liteapi v0.36.2/go.mod h1:IM7ADWjgIL2hXopzx0WNamizEuMgM2QZl7QH12FNflk=
gitlab.protontech.ch/go/liteapi v0.37.0 h1:HLK+jixbAhLL/h8ySx4dl7JT5sb2zUAkOzcBIF4IqSw=
gitlab.protontech.ch/go/liteapi v0.37.0/go.mod h1:IM7ADWjgIL2hXopzx0WNamizEuMgM2QZl7QH12FNflk=
go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU=
go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU=
go.opencensus.io v0.22.0/go.mod h1:+kGneAE2xo2IficOXnaByMWTGM9T73dGwxeWcUqIpI8=

View File

@ -289,7 +289,9 @@ func (user *User) handleMessageEvents(ctx context.Context, messageEvents []litea
}
case liteapi.EventDelete:
return ErrNotImplemented
if err := user.handleDeleteMessageEvent(ctx, event); err != nil {
return fmt.Errorf("failed to handle delete message event: %w", err)
}
}
}
@ -331,6 +333,20 @@ func (user *User) handleUpdateMessageEvent(_ context.Context, event liteapi.Mess
}, user.updateChLock)
}
func (user *User) handleDeleteMessageEvent(_ context.Context, event liteapi.MessageEvent) error { //nolint:unparam
return safe.RLockRet(func() error {
for _, updateCh := range user.updateCh {
update := imap.NewMessagesDeleted(
imap.MessageID(event.ID),
)
updateCh.Enqueue(update)
}
return nil
}, user.updateChLock)
}
func getMailboxName(label liteapi.Label) []string {
var name []string

View File

@ -311,7 +311,34 @@ func (conn *imapConnector) AddMessagesToMailbox(ctx context.Context, messageIDs
// RemoveMessagesFromMailbox unlabels the given messages with the given label ID.
func (conn *imapConnector) RemoveMessagesFromMailbox(ctx context.Context, messageIDs []imap.MessageID, mailboxID imap.MailboxID) error {
return conn.client.UnlabelMessages(ctx, mapTo[imap.MessageID, string](messageIDs), string(mailboxID))
if err := conn.client.UnlabelMessages(ctx, mapTo[imap.MessageID, string](messageIDs), string(mailboxID)); err != nil {
return err
}
if mailboxID == liteapi.SpamLabel || mailboxID == liteapi.TrashLabel {
// check if messages are only in Trash and AllMail before they are permanently deleted.
var messagesToDelete []string
// GODT-1993 - Update to more efficient method.
for _, messageID := range messageIDs {
m, err := conn.client.GetMessage(ctx, string(messageID))
if err != nil {
return fmt.Errorf("failed to get message info")
}
if len(m.LabelIDs) == 1 && m.LabelIDs[0] == liteapi.AllMailLabel {
messagesToDelete = append(messagesToDelete, m.ID)
}
}
if len(messagesToDelete) == 0 {
return nil
}
return conn.client.DeleteMessage(ctx, messagesToDelete...)
}
return nil
}
// MoveMessages removes the given messages from one label and adds them to the other label.

View File

@ -165,6 +165,7 @@ func TestFeatures(testingT *testing.T) {
ctx.Step(`^IMAP client "([^"]*)" sees (\d+) messages in "([^"]*)"$`, s.imapClientSeesMessagesInMailbox)
ctx.Step(`^IMAP client "([^"]*)" eventually sees (\d+) messages in "([^"]*)"$`, s.imapClientEventuallySeesMessagesInMailbox)
ctx.Step(`^IMAP client "([^"]*)" marks message (\d+) as deleted$`, s.imapClientMarksMessageAsDeleted)
ctx.Step(`^IMAP client "([^"]*)" marks the message with subject "([^"]*)" as deleted$`, s.imapClientMarksTheMessageWithSubjectAsDeleted)
ctx.Step(`^IMAP client "([^"]*)" marks message (\d+) as not deleted$`, s.imapClientMarksMessageAsNotDeleted)
ctx.Step(`^IMAP client "([^"]*)" marks all messages as deleted$`, s.imapClientMarksAllMessagesAsDeleted)
ctx.Step(`^IMAP client "([^"]*)" sees that message (\d+) has the flag "([^"]*)"$`, s.imapClientSeesThatMessageHasTheFlag)

View File

@ -0,0 +1,59 @@
Feature: IMAP remove messages from Trash
Background:
Given there exists an account with username "user@pm.me" and password "password"
And the account "user@pm.me" has the following custom mailboxes:
| name | type |
| mbox | folder |
| label | label |
Scenario Outline: Message in Trash or Spam and some other label is not permanently deleted
And the address "user@pm.me" of account "user@pm.me" has the following messages in "<mailbox>":
| from | to | subject | body |
| john.doe@mail.com | user@pm.me | foo | hello |
| jane.doe@mail.com | name@pm.me | bar | world |
And bridge starts
And the user logs in with username "user@pm.me" and password "password"
And user "user@pm.me" finishes syncing
And user "user@pm.me" connects and authenticates IMAP client "1"
And IMAP client "1" selects "<mailbox>"
When IMAP client "1" copies the message with subject "foo" from "<mailbox>" to "Labels/label"
Then it succeeds
When IMAP client "1" marks the message with subject "foo" as deleted
Then it succeeds
And IMAP client "1" sees 2 messages in "<mailbox>"
And IMAP client "1" sees 2 messages in "All Mail"
And IMAP client "1" sees 1 messages in "Labels/label"
When IMAP client "1" expunges
Then it succeeds
And IMAP client "1" sees 1 messages in "<mailbox>"
And IMAP client "1" sees 2 messages in "All Mail"
And IMAP client "1" sees 1 messages in "Labels/label"
Examples:
| mailbox |
| Spam |
| Trash |
Scenario Outline: Message in Trash or Spam only is permanently deleted
And the address "user@pm.me" of account "user@pm.me" has the following messages in "<mailbox>":
| from | to | subject | body |
| john.doe@mail.com | user@pm.me | foo | hello |
| jane.doe@mail.com | name@pm.me | bar | world |
And bridge starts
And the user logs in with username "user@pm.me" and password "password"
And user "user@pm.me" finishes syncing
And user "user@pm.me" connects and authenticates IMAP client "1"
And IMAP client "1" selects "<mailbox>"
When IMAP client "1" marks the message with subject "foo" as deleted
Then it succeeds
And IMAP client "1" sees 2 messages in "<mailbox>"
And IMAP client "1" sees 2 messages in "All Mail"
When IMAP client "1" expunges
Then it succeeds
And IMAP client "1" sees 1 messages in "<mailbox>"
And IMAP client "1" eventually sees 1 messages in "All Mail"
Examples:
| mailbox |
| Spam |
| Trash |

View File

@ -284,13 +284,13 @@ func (s *scenario) imapClientEventuallySeesTheFollowingMessagesInMailbox(clientI
func (s *scenario) imapClientSeesMessagesInMailbox(clientID string, count int, mailbox string) error {
_, client := s.t.getIMAPClient(clientID)
fetch, err := clientFetch(client, mailbox)
status, err := client.Status(mailbox, []imap.StatusItem{imap.StatusMessages})
if err != nil {
return err
}
if len(fetch) != count {
return fmt.Errorf("expected mailbox %v to be empty, got %v", mailbox, fetch)
if int(status.Messages) != count {
return fmt.Errorf("expected mailbox %v to have %v items, got %v", mailbox, count, status.Messages)
}
return nil
@ -305,18 +305,32 @@ func (s *scenario) imapClientEventuallySeesMessagesInMailbox(clientID string, co
func (s *scenario) imapClientMarksMessageAsDeleted(clientID string, seq int) error {
_, client := s.t.getIMAPClient(clientID)
_, err := clientStore(client, seq, seq, imap.FormatFlagsOp(imap.AddFlags, true), imap.DeletedFlag)
if _, err := clientStore(client, seq, seq, false, imap.FormatFlagsOp(imap.AddFlags, true), imap.DeletedFlag); err != nil {
return err
}
return nil
}
func (s *scenario) imapClientMarksTheMessageWithSubjectAsDeleted(clientID, subject string) error {
_, client := s.t.getIMAPClient(clientID)
uid, err := clientGetUIDBySubject(client, client.Mailbox().Name, subject)
if err != nil {
return err
}
if _, err := clientStore(client, int(uid), int(uid), true, imap.FormatFlagsOp(imap.AddFlags, true), imap.DeletedFlag); err != nil {
return err
}
return nil
}
func (s *scenario) imapClientMarksMessageAsNotDeleted(clientID string, seq int) error {
_, client := s.t.getIMAPClient(clientID)
_, err := clientStore(client, seq, seq, imap.FormatFlagsOp(imap.RemoveFlags, true), imap.DeletedFlag)
_, err := clientStore(client, seq, seq, false, imap.FormatFlagsOp(imap.RemoveFlags, true), imap.DeletedFlag)
if err != nil {
return err
}
@ -327,7 +341,7 @@ func (s *scenario) imapClientMarksMessageAsNotDeleted(clientID string, seq int)
func (s *scenario) imapClientMarksAllMessagesAsDeleted(clientID string) error {
_, client := s.t.getIMAPClient(clientID)
_, err := clientStore(client, 1, int(client.Mailbox().Messages), imap.FormatFlagsOp(imap.AddFlags, true), imap.DeletedFlag)
_, err := clientStore(client, 1, int(client.Mailbox().Messages), false, imap.FormatFlagsOp(imap.AddFlags, true), imap.DeletedFlag)
if err != nil {
return err
}
@ -488,11 +502,19 @@ func clientCopy(client *client.Client, from, to string, uid ...uint32) error {
return client.UidCopy(seqset, to)
}
func clientStore(client *client.Client, from, to int, item imap.StoreItem, flags ...string) ([]*imap.Message, error) { //nolint:unparam
func clientStore(client *client.Client, from, to int, isUID bool, item imap.StoreItem, flags ...string) ([]*imap.Message, error) { //nolint:unparam
resCh := make(chan *imap.Message)
go func() {
if err := client.Store(
var storeFunc func(seqset *imap.SeqSet, item imap.StoreItem, value interface{}, ch chan *imap.Message) error
if isUID {
storeFunc = client.UidStore
} else {
storeFunc = client.Store
}
if err := storeFunc(
&imap.SeqSet{Set: []imap.Seq{{Start: uint32(from), Stop: uint32(to)}}},
item,
xslices.Map(flags, func(flag string) interface{} { return flag }),