Implement deleted flag GODT-461

Changelog.md

@@ -25,9 +25,11 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/)
 
 ### Added
 * GODT-633 Persistent anonymous API cookies for better load balancing and abuse detection. 
+* GODT-461 Add support for `\Deleted` flag.
 
 ### Changed
 * GODT-462 Pausing event loop while FETCHing to prevent EXPUNGE
+* Wait for unilateral response to be delivered
 * GODT-409 Set flags have to replace all flags.
 * GODT-531 Better way to add trusted certificate in macOS.
 * Bumped golangci-lint to v1.29.0

go.mod

@@ -73,7 +73,7 @@ require (
 
 replace (
 	github.com/docker/docker-credential-helpers => github.com/ProtonMail/docker-credential-helpers v1.1.0
-	github.com/emersion/go-imap => github.com/jameshoulahan/go-imap v0.0.0-20200728140727-d57327f48843
+	github.com/emersion/go-imap => github.com/ProtonMail/go-imap v0.0.0-20200828124548-d04b0dc1f399
 	github.com/emersion/go-smtp => github.com/ProtonMail/go-smtp v0.0.0-20181206232543-8261df20d309
 	github.com/jameskeane/bcrypt => github.com/ProtonMail/bcrypt v0.0.0-20170924085257-7509ea014998
 	golang.org/x/crypto => github.com/ProtonMail/crypto v0.0.0-20200416114516-1fa7f403fb9c

go.sum

@@ -13,6 +13,8 @@ github.com/ProtonMail/go-apple-mobileconfig v0.0.0-20160701194735-7ea9927a11f6 h
 github.com/ProtonMail/go-apple-mobileconfig v0.0.0-20160701194735-7ea9927a11f6/go.mod h1:EtDfBMIDWmVe4viZCuBTEfe3OIIo0ghbpOaAZVO+hVg=
 github.com/ProtonMail/go-autostart v0.0.0-20181114175602-c5272053443a h1:fXK2KsfnkBV9Nh+9SKzHchYjuE9s0vI20JG1mbtEAcc=
 github.com/ProtonMail/go-autostart v0.0.0-20181114175602-c5272053443a/go.mod h1:oTGdE7/DlWIr23G0IKW3OXK9wZ5Hw1GGiaJFccTvZi4=
+github.com/ProtonMail/go-imap v0.0.0-20200828124548-d04b0dc1f399 h1:wBo/Xgb/Dn2loU47D+PJaOoIZ67i3AqYp51gLn8YE5U=
+github.com/ProtonMail/go-imap v0.0.0-20200828124548-d04b0dc1f399/go.mod h1:yKASt+C3ZiDAiCSssxg9caIckWF/JG7ZQTO7GAmvicU=
 github.com/ProtonMail/go-imap-id v0.0.0-20190926060100-f94a56b9ecde h1:5koQozTDELymYOyFbQ/VSubexAEXzDR8qGM5mO8GRdw=
 github.com/ProtonMail/go-imap-id v0.0.0-20190926060100-f94a56b9ecde/go.mod h1:795VPXcRUIQ9JyMNHP4el582VokQfippgjkQP3Gk0r0=
 github.com/ProtonMail/go-mime v0.0.0-20190923161245-9b5a4261663a h1:W6RrgN/sTxg1msqzFFb+G80MFmpjMw61IU+slm+wln4=

internal/imap/mailbox.go

@@ -173,9 +173,8 @@ func (im *imapMailbox) Check() error {
 
 // Expunge permanently removes all messages that have the \Deleted flag set
 // from the currently selected mailbox.
-// Our messages do not have \Deleted flag, nothing to do here.
 func (im *imapMailbox) Expunge() error {
-	return nil
+	return im.storeMailbox.RemoveDeleted()
 }
 
 func (im *imapMailbox) ListQuotas() ([]string, error) {

internal/imap/mailbox_message.go

@@ -220,6 +220,9 @@ func (im *imapMailbox) getMessage(storeMessage storeMessageProvider, items []ima
 			}
 		case imap.FetchFlags:
 			msg.Flags = message.GetFlags(m)
+			if storeMessage.IsMarkedDeleted() {
+				msg.Flags = append(msg.Flags, imap.DeletedFlag)
+			}
 		case imap.FetchInternalDate:
 			msg.InternalDate = time.Unix(m.Time, 0)
 		case imap.FetchRFC822Size:
@@ -237,26 +240,30 @@ func (im *imapMailbox) getMessage(storeMessage storeMessageProvider, items []ima
 				return nil, err
 			}
 		default:
-			s := item
+			if err = im.getLiteralForSection(item, msg, storeMessage); err != nil {
-
-			var section *imap.BodySectionName
-			if section, err = imap.ParseBodySectionName(s); err != nil {
-				err = nil // Ignore error
-				break
-			}
-
-			var literal imap.Literal
-			if literal, err = im.getMessageBodySection(storeMessage, section); err != nil {
 				return
 			}
-
-			msg.Body[section] = literal
 		}
 	}
 
 	return msg, err
 }
 
+func (im *imapMailbox) getLiteralForSection(itemSection imap.FetchItem, msg *imap.Message, storeMessage storeMessageProvider) error {
+	section, err := imap.ParseBodySectionName(itemSection)
+	if err != nil { // Ignore error
+		return nil
+	}
+
+	var literal imap.Literal
+	if literal, err = im.getMessageBodySection(storeMessage, section); err != nil {
+		return err
+	}
+
+	msg.Body[section] = literal
+	return nil
+}
+
 func (im *imapMailbox) getBodyStructure(storeMessage storeMessageProvider) (
 	structure *message.BodyStructure,
 	bodyReader *bytes.Reader, err error,

internal/imap/mailbox_messages.go

@@ -97,7 +97,11 @@ func (im *imapMailbox) setFlags(messageIDs, flags []string) error {
 	}
 
 	if deleted {
-		if err := im.storeMailbox.DeleteMessages(messageIDs); err != nil {
+		if err := im.storeMailbox.MarkMessagesDeleted(messageIDs); err != nil {
+			return err
+		}
+	} else {
+		if err := im.storeMailbox.MarkMessagesUndeleted(messageIDs); err != nil {
 			return err
 		}
 	}
@@ -145,11 +149,15 @@ func (im *imapMailbox) addOrRemoveFlags(operation imap.FlagsOp, messageIDs, flag
 				}
 			}
 		case imap.DeletedFlag:
-			if operation == imap.RemoveFlags {
+			switch operation {
-				break // Nothing to do, no message has the \Deleted flag.
+			case imap.AddFlags:
-			}
+				if err := im.storeMailbox.MarkMessagesDeleted(messageIDs); err != nil {
-			if err := im.storeMailbox.DeleteMessages(messageIDs); err != nil {
+					return err
-				return err
+				}
+			case imap.RemoveFlags:
+				if err := im.storeMailbox.MarkMessagesUndeleted(messageIDs); err != nil {
+					return err
+				}
 			}
 		case imap.AnsweredFlag, imap.DraftFlag, imap.RecentFlag:
 			// Not supported.
@@ -349,6 +357,9 @@ func (im *imapMailbox) SearchMessages(isUID bool, criteria *imap.SearchCriteria)
 		if !m.Has(pmapi.FlagOpened) {
 			messageFlagsMap[imap.RecentFlag] = true
 		}
+		if storeMessage.IsMarkedDeleted() {
+			messageFlagsMap[imap.DeletedFlag] = true
+		}
 
 		flagMatch := true
 		for _, flag := range criteria.WithFlags {

internal/imap/store.go

@@ -83,8 +83,10 @@ type storeMailboxProvider interface {
 	MarkMessagesUnread(apiID []string) error
 	MarkMessagesStarred(apiID []string) error
 	MarkMessagesUnstarred(apiID []string) error
+	MarkMessagesDeleted(apiID []string) error
+	MarkMessagesUndeleted(apiID []string) error
 	ImportMessage(msg *pmapi.Message, body []byte, labelIDs []string) error
-	DeleteMessages(apiID []string) error
+	RemoveDeleted() error
 }
 
 type storeMessageProvider interface {
@@ -92,6 +94,7 @@ type storeMessageProvider interface {
 	UID() (uint32, error)
 	SequenceNumber() (uint32, error)
 	Message() *pmapi.Message
+	IsMarkedDeleted() bool
 
 	SetSize(int64) error
 	SetContentTypeAndHeader(string, mail.Header) error

internal/imap/uidplus/extension.go

@@ -25,6 +25,7 @@
 package uidplus
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/emersion/go-imap"
@@ -120,11 +121,51 @@ func (os *OrderedSeq) String() string {
 // If not implemented it would cause harmless IMAP error.
 //
 // This overrides the standard EXPUNGE functionality.
-type UIDExpunge struct{}
+type UIDExpunge struct {
+	expunge *server.Expunge
+	seqset  *imap.SeqSet
+}
 
-func (e *UIDExpunge) Parse(fields []interface{}) error { log.Traceln("parse", fields); return nil }
+func newUIDExpunge() *UIDExpunge {
-func (e *UIDExpunge) Handle(conn server.Conn) error    { log.Traceln("handle"); return nil }
+	return &UIDExpunge{expunge: &server.Expunge{}}
-func (e *UIDExpunge) UidHandle(conn server.Conn) error { log.Traceln("uid handle"); return nil } //nolint[golint]
+}
+
+func (e *UIDExpunge) Parse(fields []interface{}) error {
+	if len(fields) < 1 { // asuming no UID
+		return e.expunge.Parse(fields)
+	}
+
+	var err error
+	if seqset, ok := fields[0].(string); !ok {
+		return errors.New("sequence set must be an atom")
+	} else if e.seqset, err = imap.ParseSeqSet(seqset); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func (e *UIDExpunge) Handle(conn server.Conn) error {
+	log.Traceln("handle")
+	return e.expunge.Handle(conn)
+}
+func (e *UIDExpunge) UidHandle(conn server.Conn) error { //nolint[golint]
+	log.Traceln("uid handle")
+	// RFC4315#section-2.1
+	// The UID EXPUNGE command permanently removes all messages that both
+	// have the \Deleted flag set and have a UID that is included in the
+	// specified sequence set from the currently selected mailbox. If a
+	// message either does not have the \Deleted flag set or has a UID
+	// that is not included in the specified sequence set, it is not
+	// affected.
+	//
+	// NOTE missing implementation: It will probably need mailbox interface
+	// change: ExpungeUIDs(seqSet) not sure how to combine with original
+	// e.expunge.Handle().
+	//
+	// Current implementation deletes all marked as deleted.
+	return e.expunge.Handle(conn)
+}
 
 type extension struct{}
 
@@ -143,7 +184,7 @@ func (ext *extension) Capabilities(c server.Conn) []string {
 func (ext *extension) Command(name string) server.HandlerFactory {
 	if name == "EXPUNGE" {
 		return func() server.Handler {
-			return &UIDExpunge{}
+			return newUIDExpunge()
 		}
 	}
 

internal/store/change.go

@@ -48,18 +48,26 @@ func (store *Store) imapNotice(address, notice string) {
 	store.imapSendUpdate(update)
 }
 
-func (store *Store) imapUpdateMessage(address, mailboxName string, uid, sequenceNumber uint32, msg *pmapi.Message) {
+func (store *Store) imapUpdateMessage(
+	address, mailboxName string,
+	uid, sequenceNumber uint32,
+	msg *pmapi.Message, hasDeletedFlag bool,
+) {
 	store.log.WithFields(logrus.Fields{
 		"address": address,
 		"mailbox": mailboxName,
 		"seqNum":  sequenceNumber,
 		"uid":     uid,
 		"flags":   message.GetFlags(msg),
+		"deleted": hasDeletedFlag,
 	}).Trace("IDLE update")
 	update := new(imapBackend.MessageUpdate)
 	update.Update = imapBackend.NewUpdate(address, mailboxName)
 	update.Message = imap.NewMessage(sequenceNumber, []imap.FetchItem{imap.FetchFlags, imap.FetchUid})
 	update.Message.Flags = message.GetFlags(msg)
+	if hasDeletedFlag {
+		update.Message.Flags = append(update.Message.Flags, imap.DeletedFlag)
+	}
 	update.Message.Uid = uid
 	store.imapSendUpdate(update)
 }
@@ -114,10 +122,13 @@ func (store *Store) imapSendUpdate(update imapBackend.Update) {
 		return
 	}
 
+	done := update.Done()
+	go func() { store.imapUpdates <- update }()
+
 	select {
+	case <-done:
 	case <-time.After(1 * time.Second):
 		store.log.Error("Could not send IMAP update (timeout)")
 		return
-	case store.imapUpdates <- update:
 	}
 }

internal/store/mailbox.go

@@ -238,6 +238,17 @@ func (storeMailbox *Mailbox) txGetAPIIDsBucket(tx *bolt.Tx) *bolt.Bucket {
 	return storeMailbox.txGetBucket(tx).Bucket(apiIDsBucket)
 }
 
+// txGetDeletedIDsBucket returns the bucket with messagesID marked as deleted
+func (storeMailbox *Mailbox) txGetDeletedIDsBucket(tx *bolt.Tx) *bolt.Bucket {
+	// There should be no error since it _...returns an error if the bucket
+	// name is blank, or if the bucket name is too long._
+	bucket, err := storeMailbox.txGetBucket(tx).CreateBucketIfNotExists(deletedIDsBucket)
+	if err != nil || bucket == nil {
+		storeMailbox.log.WithError(err).Error("Cannot create or get bucket with deleted IDs.")
+	}
+	return bucket
+}
+
 // txGetBucket returns the bucket of mailbox containing mapping buckets.
 func (storeMailbox *Mailbox) txGetBucket(tx *bolt.Tx) *bolt.Bucket {
 	return tx.Bucket(mailboxesBucket).Bucket(storeMailbox.getBucketName())

internal/store/mailbox_ids.go

@@ -129,7 +129,11 @@ func (storeMailbox *Mailbox) getUID(apiID string) (uid uint32, err error) {
 }
 
 func (storeMailbox *Mailbox) txGetUID(tx *bolt.Tx, apiID string) (uint32, error) {
-	b := storeMailbox.txGetAPIIDsBucket(tx)
+	return storeMailbox.txGetUIDFromBucket(storeMailbox.txGetAPIIDsBucket(tx), apiID)
+}
+
+// txGetUIDFromBucket expects pointer to API bucket.
+func (storeMailbox *Mailbox) txGetUIDFromBucket(b *bolt.Bucket, apiID string) (uint32, error) {
 	v := b.Get([]byte(apiID))
 	if v == nil {
 		return 0, ErrNoSuchAPIID
@@ -137,6 +141,19 @@ func (storeMailbox *Mailbox) txGetUID(tx *bolt.Tx, apiID string) (uint32, error)
 	return btoi(v), nil
 }
 
+// getUID returns IMAP UID in this mailbox for message ID.
+func (storeMailbox *Mailbox) getDeletedAPIIDs() (apiIDs []string, err error) {
+	err = storeMailbox.db().Update(func(tx *bolt.Tx) error {
+		b := storeMailbox.txGetDeletedIDsBucket(tx)
+		c := b.Cursor()
+		for k, _ := c.First(); k != nil; k, _ = c.Next() {
+			apiIDs = append(apiIDs, string(k))
+		}
+		return nil
+	})
+	return
+}
+
 // getSequenceNumber returns IMAP sequence number in the mailbox for the message with the given API ID `apiID`.
 func (storeMailbox *Mailbox) getSequenceNumber(apiID string) (seqNum uint32, err error) {
 	err = storeMailbox.db().View(func(tx *bolt.Tx) error {

internal/store/mailbox_message.go

@@ -24,6 +24,8 @@ import (
 	bolt "go.etcd.io/bbolt"
 )
 
+// ErrAllMailOpNotAllowed is error user when user tries to do unsupported
+// operation on All Mail folder
 var ErrAllMailOpNotAllowed = errors.New("operation not allowed for 'All Mail' folder")
 
 // GetMessage returns the `pmapi.Message` struct wrapped in `StoreMessage`
@@ -96,11 +98,8 @@ func (storeMailbox *Mailbox) LabelMessages(apiIDs []string) error {
 // It has to be propagated to all the same messages in all mailboxes.
 // The propagation is processed by the event loop.
 func (storeMailbox *Mailbox) UnlabelMessages(apiIDs []string) error {
-	log.WithFields(logrus.Fields{
+	storeMailbox.log.WithField("messages", apiIDs).
-		"messages": apiIDs,
+		Trace("Unlabeling messages")
-		"label":    storeMailbox.labelID,
-		"mailbox":  storeMailbox.Name,
-	}).Trace("Unlabeling messages")
 	if storeMailbox.labelID == pmapi.AllMailLabel {
 		return ErrAllMailOpNotAllowed
 	}
@@ -173,54 +172,57 @@ func (storeMailbox *Mailbox) MarkMessagesUnstarred(apiIDs []string) error {
 	return storeMailbox.client().UnlabelMessages(apiIDs, pmapi.StarredLabel)
 }
 
-// DeleteMessages deletes messages.
+// MarkMessagesDeleted adds local flag \Deleted. This is not propagated to API
-// If the mailbox is All Mail or All Sent, it does nothing.
+// until RemoveDeleted is called
-// If the mailbox is Trash or Spam and message is not in any other mailbox, messages is deleted.
+func (storeMailbox *Mailbox) MarkMessagesDeleted(apiIDs []string) error {
-// In all other cases the message is only removed from the mailbox.
-func (storeMailbox *Mailbox) DeleteMessages(apiIDs []string) error {
 	log.WithFields(logrus.Fields{
 		"messages": apiIDs,
 		"label":    storeMailbox.labelID,
 		"mailbox":  storeMailbox.Name,
-	}).Trace("Deleting messages")
+	}).Trace("Marking messages as deleted")
+	return storeMailbox.store.db.Update(func(tx *bolt.Tx) error {
+		return storeMailbox.txMarkMessagesAsDeleted(tx, apiIDs, true)
+	})
+}
+
+// MarkMessagesUndeleted removes local flag \Deleted. This is not propagated to
+// API.
+func (storeMailbox *Mailbox) MarkMessagesUndeleted(apiIDs []string) error {
+	log.WithFields(logrus.Fields{
+		"messages": apiIDs,
+		"label":    storeMailbox.labelID,
+		"mailbox":  storeMailbox.Name,
+	}).Trace("Marking messages as undeleted")
+	return storeMailbox.store.db.Update(func(tx *bolt.Tx) error {
+		return storeMailbox.txMarkMessagesAsDeleted(tx, apiIDs, false)
+	})
+}
+
+// RemoveDeleted sends request to API to remove message from mailbox.
+// If the mailbox is All Mail or All Sent, it does nothing.
+// If the mailbox is Trash or Spam and message is not in any other mailbox, messages is deleted.
+// In all other cases the message is only removed from the mailbox.
+func (storeMailbox *Mailbox) RemoveDeleted() error {
+	storeMailbox.log.Trace("Deleting messages")
+
+	apiIDs, err := storeMailbox.getDeletedAPIIDs()
+	if err != nil {
+		return err
+	}
+
+	if len(apiIDs) == 0 {
+		storeMailbox.log.Debug("List to expunge is empty")
+		return nil
+	}
+
 	defer storeMailbox.pollNow()
 
 	switch storeMailbox.labelID {
 	case pmapi.AllMailLabel, pmapi.AllSentLabel:
 		break
 	case pmapi.TrashLabel, pmapi.SpamLabel:
-		messageIDsToDelete := []string{}
+		if err := storeMailbox.deleteFromTrashOrSpam(apiIDs); err != nil {
-		messageIDsToUnlabel := []string{}
+			return err
-		for _, apiID := range apiIDs {
-			msg, err := storeMailbox.store.getMessageFromDB(apiID)
-			if err != nil {
-				return err
-			}
-
-			otherLabels := false
-			// If the message has any custom label, we don't want to delete it, only remove trash/spam label.
-			for _, label := range msg.LabelIDs {
-				if label != pmapi.SpamLabel && label != pmapi.TrashLabel && label != pmapi.AllMailLabel && label != pmapi.AllSentLabel && label != pmapi.DraftLabel && label != pmapi.AllDraftsLabel {
-					otherLabels = true
-					break
-				}
-			}
-
-			if otherLabels {
-				messageIDsToUnlabel = append(messageIDsToUnlabel, apiID)
-			} else {
-				messageIDsToDelete = append(messageIDsToDelete, apiID)
-			}
-		}
-		if len(messageIDsToUnlabel) > 0 {
-			if err := storeMailbox.client().UnlabelMessages(messageIDsToUnlabel, storeMailbox.labelID); err != nil {
-				log.WithError(err).Warning("Cannot unlabel before deleting")
-			}
-		}
-		if len(messageIDsToDelete) > 0 {
-			if err := storeMailbox.client().DeleteMessages(messageIDsToDelete); err != nil {
-				return err
-			}
 		}
 	case pmapi.DraftLabel:
 		if err := storeMailbox.client().DeleteMessages(apiIDs); err != nil {
@@ -234,6 +236,50 @@ func (storeMailbox *Mailbox) DeleteMessages(apiIDs []string) error {
 	return nil
 }
 
+// deleteFromTrashOrSpam will remove messages from API forever. If messages
+// still has some custom label the message will not be deleted. Instead it will
+// be removed from Trash or Spam.
+func (storeMailbox *Mailbox) deleteFromTrashOrSpam(apiIDs []string) error {
+	l := storeMailbox.log.WithField("messages", apiIDs)
+	l.Trace("Deleting messages from trash")
+
+	messageIDsToDelete := []string{}
+	messageIDsToUnlabel := []string{}
+	for _, apiID := range apiIDs {
+		msg, err := storeMailbox.store.getMessageFromDB(apiID)
+		if err != nil {
+			return err
+		}
+
+		otherLabels := false
+		// If the message has any custom label, we don't want to delete it, only remove trash/spam label.
+		for _, label := range msg.LabelIDs {
+			if label != pmapi.SpamLabel && label != pmapi.TrashLabel && label != pmapi.AllMailLabel && label != pmapi.AllSentLabel && label != pmapi.DraftLabel && label != pmapi.AllDraftsLabel {
+				otherLabels = true
+				break
+			}
+		}
+
+		if otherLabels {
+			messageIDsToUnlabel = append(messageIDsToUnlabel, apiID)
+		} else {
+			messageIDsToDelete = append(messageIDsToDelete, apiID)
+		}
+	}
+	if len(messageIDsToUnlabel) > 0 {
+		if err := storeMailbox.client().UnlabelMessages(messageIDsToUnlabel, storeMailbox.labelID); err != nil {
+			l.WithError(err).Warning("Cannot unlabel before deleting")
+		}
+	}
+	if len(messageIDsToDelete) > 0 {
+		if err := storeMailbox.client().DeleteMessages(messageIDsToDelete); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func (storeMailbox *Mailbox) txSkipAndRemoveFromMailbox(tx *bolt.Tx, msg *pmapi.Message) (skipAndRemove bool) {
 	defer func() {
 		if skipAndRemove {
@@ -273,7 +319,7 @@ func (storeMailbox *Mailbox) txCreateOrUpdateMessages(tx *bolt.Tx, msgs []*pmapi
 
 	// Buckets are not initialized right away because it's a heavy operation.
 	// The best option is to get the same bucket only once and only when needed.
-	var apiBucket, imapBucket *bolt.Bucket
+	var apiBucket, imapBucket, deletedBucket *bolt.Bucket
 	for _, msg := range msgs {
 		if storeMailbox.txSkipAndRemoveFromMailbox(tx, msg) {
 			continue
@@ -292,12 +338,15 @@ func (storeMailbox *Mailbox) txCreateOrUpdateMessages(tx *bolt.Tx, msgs []*pmapi
 			}
 		} else {
 			uidb := apiBucket.Get([]byte(msg.ID))
-
 			if uidb != nil {
 				if imapBucket == nil {
 					imapBucket = storeMailbox.txGetIMAPIDsBucket(tx)
 				}
 				seqNum, seqErr := storeMailbox.txGetSequenceNumberOfUID(imapBucket, uidb)
+				if deletedBucket == nil {
+					deletedBucket = storeMailbox.txGetDeletedIDsBucket(tx)
+				}
+				isMarkedAsDeleted := deletedBucket.Get([]byte(msg.ID)) != nil
 				if seqErr == nil {
 					storeMailbox.store.imapUpdateMessage(
 						storeMailbox.storeAddress.address,
@@ -305,6 +354,7 @@ func (storeMailbox *Mailbox) txCreateOrUpdateMessages(tx *bolt.Tx, msgs []*pmapi
 						btoi(uidb),
 						seqNum,
 						msg,
+						isMarkedAsDeleted,
 					)
 				}
 				continue
@@ -338,6 +388,7 @@ func (storeMailbox *Mailbox) txCreateOrUpdateMessages(tx *bolt.Tx, msgs []*pmapi
 			uid,
 			seqNum,
 			msg,
+			false, // new message is never marked as deleted
 		)
 		shouldSendMailboxUpdate = true
 	}
@@ -362,6 +413,7 @@ func (storeMailbox *Mailbox) txDeleteMessage(tx *bolt.Tx, apiID string) error {
 	}
 
 	imapBucket := storeMailbox.txGetIMAPIDsBucket(tx)
+	deletedBucket := storeMailbox.txGetDeletedIDsBucket(tx)
 
 	seqNum, seqNumErr := storeMailbox.txGetSequenceNumberOfUID(imapBucket, uidb)
 	if seqNumErr != nil {
@@ -376,6 +428,10 @@ func (storeMailbox *Mailbox) txDeleteMessage(tx *bolt.Tx, apiID string) error {
 		return errors.Wrap(err, "cannot delete from API bucket")
 	}
 
+	if err := deletedBucket.Delete(apiIDb); err != nil {
+		return errors.Wrap(err, "cannot delete from mark-as-deleted bucket")
+	}
+
 	if seqNumErr == nil {
 		storeMailbox.store.imapDeleteMessage(
 			storeMailbox.storeAddress.address,
@@ -404,3 +460,50 @@ func (storeMailbox *Mailbox) txMailboxStatusUpdate(tx *bolt.Tx) error {
 	)
 	return nil
 }
+
+func (storeMailbox *Mailbox) txMarkMessagesAsDeleted(tx *bolt.Tx, apiIDs []string, markAsDeleted bool) error {
+	// Load all buckets before looping over apiIDs
+	metaBucket := tx.Bucket(metadataBucket)
+	apiBucket := storeMailbox.txGetAPIIDsBucket(tx)
+	uidBucket := storeMailbox.txGetIMAPIDsBucket(tx)
+	deletedBucket := storeMailbox.txGetDeletedIDsBucket(tx)
+	for _, apiID := range apiIDs {
+		if markAsDeleted {
+			if err := deletedBucket.Put([]byte(apiID), []byte{1}); err != nil {
+				return err
+			}
+		} else {
+			if err := deletedBucket.Delete([]byte(apiID)); err != nil {
+				return err
+			}
+		}
+
+		msg, err := storeMailbox.store.txGetMessageFromBucket(metaBucket, apiID)
+		if err != nil {
+			return err
+		}
+
+		uid, err := storeMailbox.txGetUIDFromBucket(apiBucket, apiID)
+		if err != nil {
+			return err
+		}
+
+		seqNum, err := storeMailbox.txGetSequenceNumberOfUID(uidBucket, itob(uid))
+		if err != nil {
+			return err
+		}
+
+		// In order to send flags in format
+		// S: * 2 FETCH (FLAGS (\Deleted \Seen))
+		storeMailbox.store.imapUpdateMessage(
+			storeMailbox.storeAddress.address,
+			storeMailbox.labelName,
+			uid,
+			seqNum,
+			msg,
+			markAsDeleted,
+		)
+	}
+
+	return nil
+}

internal/store/message.go

@@ -62,6 +62,21 @@ func (message *Message) Message() *pmapi.Message {
 	return message.msg
 }
 
+// IsMarkedDeleted returns true if message is marked as deleted for specific
+// mailbox
+func (message *Message) IsMarkedDeleted() bool {
+	isMarkedAsDeleted := false
+	err := message.storeMailbox.db().Update(func(tx *bolt.Tx) error {
+		isMarkedAsDeleted = message.storeMailbox.txGetDeletedIDsBucket(tx).Get([]byte(message.msg.ID)) != nil
+		return nil
+	})
+	if err != nil {
+		message.storeMailbox.log.WithError(err).Error("Not able to retrieve deleted mark, assuming false.")
+		return false
+	}
+	return isMarkedAsDeleted
+}
+
 // SetSize updates the information about size of decrypted message which can be
 // used for IMAP. This should not trigger any IMAP update.
 // NOTE: The size from the server corresponds to pure body bytes. Hence it

internal/store/store.go

@@ -70,6 +70,8 @@ var (
 	//       * {imapUID} -> string messageID
 	//     * api_ids
 	//       * {messageID} -> uint32 imapUID
+	//     * deleted_ids (can be missing or have no keys)
+	//       * {messageID} -> true
 	metadataBucket    = []byte("metadata")          //nolint[gochecknoglobals]
 	countsBucket      = []byte("counts")            //nolint[gochecknoglobals]
 	addressInfoBucket = []byte("address_info")      //nolint[gochecknoglobals]
@@ -78,6 +80,7 @@ var (
 	mailboxesBucket   = []byte("mailboxes")         //nolint[gochecknoglobals]
 	imapIDsBucket     = []byte("imap_ids")          //nolint[gochecknoglobals]
 	apiIDsBucket      = []byte("api_ids")           //nolint[gochecknoglobals]
+	deletedIDsBucket  = []byte("deleted_ids")       //nolint[gochecknoglobals]
 	mboxVersionBucket = []byte("mailboxes_version") //nolint[gochecknoglobals]
 
 	// ErrNoSuchAPIID when mailbox does not have API ID.

internal/store/store_test_exports.go

@@ -106,11 +106,13 @@ func txDumpMailsFactory(tb assert.TestingT) func(tx *bolt.Tx) error {
 		err := mailboxes.ForEach(func(mboxName, mboxData []byte) error {
 			fmt.Println("mbox:", string(mboxName))
 			b := mailboxes.Bucket(mboxName).Bucket(imapIDsBucket)
+			deletedMailboxes := mailboxes.Bucket(mboxName).Bucket(deletedIDsBucket)
 			c := b.Cursor()
 			i := 0
 			for imapID, apiID := c.First(); imapID != nil; imapID, apiID = c.Next() {
 				i++
-				fmt.Println("  ", i, "imap", btoi(imapID), "api", string(apiID))
+				isDeleted := deletedMailboxes != nil && deletedMailboxes.Get(apiID) != nil
+				fmt.Println("  ", i, "imap", btoi(imapID), "api", string(apiID), "isDeleted", isDeleted)
 				data := metadata.Get(apiID)
 				if !assert.NotNil(tb, data) {
 					continue

internal/store/user_message.go

@@ -143,8 +143,10 @@ func (store *Store) getMessageFromDB(apiID string) (msg *pmapi.Message, err erro
 }
 
 func (store *Store) txGetMessage(tx *bolt.Tx, apiID string) (*pmapi.Message, error) {
-	b := tx.Bucket(metadataBucket)
+	return store.txGetMessageFromBucket(tx.Bucket(metadataBucket), apiID)
+}
 
+func (store *Store) txGetMessageFromBucket(b *bolt.Bucket, apiID string) (*pmapi.Message, error) {
 	msgb := b.Get([]byte(apiID))
 	if msgb == nil {
 		return nil, ErrNoSuchAPIID

test/context/pmapi_controller.go

@@ -34,6 +34,7 @@ type PMAPIController interface {
 	AddUserMessage(username string, message *pmapi.Message) error
 	GetMessageID(username, messageIndex string) string
 	GetMessages(username, labelID string) ([]*pmapi.Message, error)
+	GetLastMessageID(username string) string
 	ReorderAddresses(user *pmapi.User, addressIDs []string) error
 	PrintCalls()
 	WasCalled(method, path string, expectedRequest []byte) bool

test/fakeapi/controller_control.go

@@ -172,4 +172,8 @@ func (ctl *Controller) GetMessages(username, labelID string) ([]*pmapi.Message,
 		}
 	}
 	return messages, nil
+
+func (ctl *Controller) GetLastMessageID(username string) string {
+	msgs := ctl.messagesByUsername[username]
+	return msgs[len(msgs)-1].ID
 }

test/fakeapi/messages.go

@@ -277,7 +277,8 @@ func (api *FakePMAPI) deleteMessages(method method, path string, request interfa
 	newMessages := []*pmapi.Message{}
 	for _, message := range api.messages {
 		if shouldBeDeleted(message) {
-			if hasItem(message.LabelIDs, pmapi.TrashLabel) {
+			if hasItem(message.LabelIDs, pmapi.TrashLabel) ||
+				hasItem(message.LabelIDs, pmapi.SpamLabel) {
 				api.addEventMessage(pmapi.EventDelete, message)
 				continue
 			}

test/features/bridge/imap/message/delete.feature

@@ -11,7 +11,8 @@ Feature: IMAP remove messages from mailbox
     When IMAP client marks message "2" as deleted
     Then IMAP response is "OK"
     And mailbox "<mailbox>" for "user" has 10 messages
-    And message "2" in "INBOX" for "user" is marked as deleted
+    And message "9" in "<mailbox>" for "user" is marked as deleted
+    And IMAP response contains "\* 2 FETCH[ (]*FLAGS \([^)]*\\Deleted"
     When IMAP client sends expunge
     Then IMAP response is "OK"
     And IMAP response contains "* 2 EXPUNGE"
@@ -77,7 +78,7 @@ Feature: IMAP remove messages from mailbox
     When IMAP client marks message "2" as deleted
     Then IMAP response is "OK"
     And mailbox "INBOX" for "user" has 10 messages
-    And message "2" in "INBOX" for "user" is marked as deleted
+    And message "9" in "INBOX" for "user" is marked as deleted
     When IMAP client sends command "<leave>"
     Then IMAP response is "OK"
     And mailbox "INBOX" for "user" has <n> messages

test/features/bridge/imap/message/update.feature

@@ -57,23 +57,27 @@ Feature: IMAP update messages
     And message "1" in "Spam" for "user" is marked as unstarred
 
   Scenario: Mark message as deleted
-    When IMAP client marks message "2" as deleted
+    # Mark message as Starred so we can check that mark as Deleted is not
+    # tempering with Starred flag
+    When IMAP client marks message "1" as starred
+    Then IMAP response is "OK"
+    When IMAP client marks message "1" as deleted
     Then IMAP response is "OK"
     And message "2" in "INBOX" for "user" is marked as read
     And message "2" in "INBOX" for "user" is marked as starred
     And message "2" in "INBOX" for "user" is marked as deleted
 
   Scenario: Mark message as undeleted
-    When IMAP client marks message "2" as undeleted
+    When IMAP client marks message "1" as undeleted
     Then IMAP response is "OK"
     And message "2" in "INBOX" for "user" is marked as read
     And message "2" in "INBOX" for "user" is marked as starred
     And message "2" in "INBOX" for "user" is marked as undeleted
 
   Scenario: Mark message as deleted only
-    When IMAP client marks message "2" with "\Deleted"
+    When IMAP client marks message "1" with "\Deleted"
     Then IMAP response is "OK"
     And message "2" in "INBOX" for "user" is marked as unread
     And message "2" in "INBOX" for "user" is marked as unstarred
-    And message "2" in "INBOX" for "user" is marked as undeleted
+    And message "2" in "INBOX" for "user" is marked as deleted
 

test/features/imap/message/delete-trash.feature

@@ -4,14 +4,15 @@ Feature: IMAP remove messages from Trash
     And there is "user" with mailbox "Folders/mbox"
     And there is "user" with mailbox "Labels/label"
 
-  Scenario Outline: Delete messages from Trash/Spam removes all labels first
+  Scenario Outline: Delete messages from Trash/Spam does not remove from All Mail
     Given there are messages in mailbox "<mailbox>" for "user"
       | from              | to         | subject | body  |
       | john.doe@mail.com | user@pm.me | foo     | hello |
       | jane.doe@mail.com | name@pm.me | bar     | world |
     And there is IMAP client logged in as "user"
     And there is IMAP client selected in "<mailbox>"
-    And IMAP client copies messages "2" to "Labels/label"
+    When IMAP client copies messages "2" to "Labels/label"
+    Then IMAP response is "OK"
     When IMAP client marks message "2" as deleted
     Then IMAP response is "OK"
     And mailbox "<mailbox>" for "user" has 2 messages
@@ -19,9 +20,9 @@ Feature: IMAP remove messages from Trash
     And mailbox "Labels/label" for "user" has 1 messages
     When IMAP client sends expunge
     Then IMAP response is "OK"
-    And mailbox "<mailbox>" for "user" has 2 messages
+    And mailbox "<mailbox>" for "user" has 1 messages
     And mailbox "All Mail" for "user" has 2 messages
-    And mailbox "Labels/label" for "user" has 0 messages
+    And mailbox "Labels/label" for "user" has 1 messages
 
     Examples:
       | mailbox |
@@ -29,7 +30,7 @@ Feature: IMAP remove messages from Trash
       | Trash   |
 
 
-  Scenario Outline: Delete messages from Trash/Spamm deletes from All Mail
+  Scenario Outline: Delete messages from Trash/Spamm removes from All Mail
     Given there are messages in mailbox "<mailbox>" for "user"
       | from              | to         | subject | body  |
       | john.doe@mail.com | user@pm.me | foo     | hello |

test/liveapi/messages.go

@@ -162,4 +162,8 @@ func (ctl *Controller) GetMessages(username, labelID string) ([]*pmapi.Message,
 	}
 
 	return messages, nil
+
+func (ctl *Controller) GetLastMessageID(username string) string {
+	ids := ctl.messageIDsByUsername[username]
+	return ids[len(ids)-1]
 }

test/store_checks_test.go

@@ -23,7 +23,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/ProtonMail/proton-bridge/pkg/pmapi"
+	"github.com/ProtonMail/proton-bridge/internal/store"
 	"github.com/ProtonMail/proton-bridge/test/accounts"
 	"github.com/cucumber/godog"
 	"github.com/cucumber/godog/gherkin"
@@ -128,13 +128,13 @@ func mailboxForAddressOfUserHasMessages(mailboxName, bddAddressID, bddUserID str
 	if err != nil {
 		return internalError(err, "getting API IDs from sequence range")
 	}
-	allMessages := []*pmapi.Message{}
+	allMessages := []*store.Message{}
 	for _, apiID := range apiIDs {
 		message, err := mailbox.GetMessage(apiID)
 		if err != nil {
 			return internalError(err, "getting message by ID")
 		}
-		allMessages = append(allMessages, message.Message())
+		allMessages = append(allMessages, message)
 	}
 
 	head := messages.Rows[0].Cells
@@ -168,9 +168,10 @@ func mailboxForAddressOfUserHasMessages(mailboxName, bddAddressID, bddUserID str
 	return nil
 }
 
-func messagesContainsMessageRow(account *accounts.TestAccount, allMessages []*pmapi.Message, head []*gherkin.TableCell, row *gherkin.TableRow) (bool, error) { //nolint[funlen]
+func messagesContainsMessageRow(account *accounts.TestAccount, allMessages []*store.Message, head []*gherkin.TableCell, row *gherkin.TableRow) (bool, error) { //nolint[funlen]
 	found := false
-	for _, message := range allMessages {
+	for _, storeMessage := range allMessages {
+		message := storeMessage.Message()
 		matches := true
 		for n, cell := range row.Cells {
 			switch head[n].Value {
@@ -220,8 +221,8 @@ func messagesContainsMessageRow(account *accounts.TestAccount, allMessages []*pm
 					matches = false
 				}
 			case "deleted":
-				// TODO
+				expectedDeleted := cell.Value == "true"
-				matches = false
+				matches = storeMessage.IsMarkedDeleted() == expectedDeleted
 			default:
 				return false, fmt.Errorf("unexpected column name: %s", head[n].Value)
 			}
@@ -247,56 +248,60 @@ func areAddressesSame(first, second string) bool {
 }
 
 func messagesInMailboxForUserIsMarkedAsRead(messageIDs, mailboxName, bddUserID string) error {
-	return checkMessages(bddUserID, mailboxName, messageIDs, func(message *pmapi.Message) error {
+	return checkMessages(bddUserID, mailboxName, messageIDs, func(message *store.Message) error {
-		if message.Unread == 0 {
+		if message.Message().Unread == 0 {
 			return nil
 		}
-		return fmt.Errorf("message %s \"%s\" is expected to be read but is not", message.ID, message.Subject)
+		return fmt.Errorf("message %s \"%s\" is expected to be read but is not", message.ID(), message.Message().Subject)
 	})
 }
 
 func messagesInMailboxForUserIsMarkedAsUnread(messageIDs, mailboxName, bddUserID string) error {
-	return checkMessages(bddUserID, mailboxName, messageIDs, func(message *pmapi.Message) error {
+	return checkMessages(bddUserID, mailboxName, messageIDs, func(message *store.Message) error {
-		if message.Unread == 1 {
+		if message.Message().Unread == 1 {
 			return nil
 		}
-		return fmt.Errorf("message %s \"%s\" is expected to not be read but is", message.ID, message.Subject)
+		return fmt.Errorf("message %s \"%s\" is expected to not be read but is", message.ID(), message.Message().Subject)
 	})
 }
 
 func messagesInMailboxForUserIsMarkedAsStarred(messageIDs, mailboxName, bddUserID string) error {
-	return checkMessages(bddUserID, mailboxName, messageIDs, func(message *pmapi.Message) error {
+	return checkMessages(bddUserID, mailboxName, messageIDs, func(message *store.Message) error {
-		if hasItem(message.LabelIDs, "10") {
+		if hasItem(message.Message().LabelIDs, "10") {
 			return nil
 		}
-		return fmt.Errorf("message %s \"%s\" is expected to be starred but is not", message.ID, message.Subject)
+		return fmt.Errorf("message %s \"%s\" is expected to be starred but is not", message.ID(), message.Message().Subject)
 	})
 }
 
 func messagesInMailboxForUserIsMarkedAsUnstarred(messageIDs, mailboxName, bddUserID string) error {
-	return checkMessages(bddUserID, mailboxName, messageIDs, func(message *pmapi.Message) error {
+	return checkMessages(bddUserID, mailboxName, messageIDs, func(message *store.Message) error {
-		if !hasItem(message.LabelIDs, "10") {
+		if !hasItem(message.Message().LabelIDs, "10") {
 			return nil
 		}
-		return fmt.Errorf("message %s \"%s\" is expected to not be starred but is", message.ID, message.Subject)
+		return fmt.Errorf("message %s \"%s\" is expected to not be starred but is", message.ID(), message.Message().Subject)
 	})
 }
 
 func messagesInMailboxForUserIsMarkedAsDeleted(messageIDs, mailboxName, bddUserID string) error {
-	return checkMessages(bddUserID, mailboxName, messageIDs, func(message *pmapi.Message) error {
+	return checkMessages(bddUserID, mailboxName, messageIDs, func(message *store.Message) error {
-		// TODO
+		if message.IsMarkedDeleted() {
-		return fmt.Errorf("TODO message %s \"%s\" is expected to be deleted but is not", message.ID, message.Subject)
+			return nil
+		}
+		return fmt.Errorf("message %s \"%s\" is expected to be deleted but is not", message.ID(), message.Message().Subject)
 	})
 }
 
 func messagesInMailboxForUserIsMarkedAsUndeleted(messageIDs, mailboxName, bddUserID string) error {
-	return checkMessages(bddUserID, mailboxName, messageIDs, func(message *pmapi.Message) error {
+	return checkMessages(bddUserID, mailboxName, messageIDs, func(message *store.Message) error {
-		// TODO
+		if !message.IsMarkedDeleted() {
-		return fmt.Errorf("TODO message %s \"%s\" is expected to not be deleted but is", message.ID, message.Subject)
+			return nil
+		}
+		return fmt.Errorf("message %s \"%s\" is expected to not be deleted but is", message.ID(), message.Message().Subject)
 	})
 }
 
-func checkMessages(bddUserID, mailboxName, messageIDs string, callback func(*pmapi.Message) error) error {
+func checkMessages(bddUserID, mailboxName, messageIDs string, callback func(*store.Message) error) error {
 	account := ctx.GetTestAccount(bddUserID)
 	if account == nil {
 		return godog.ErrPending
@@ -313,9 +318,9 @@ func checkMessages(bddUserID, mailboxName, messageIDs string, callback func(*pma
 	return nil
 }
 
-func getMessages(username, addressID, mailboxName, messageIDs string) ([]*pmapi.Message, error) {
+func getMessages(username, addressID, mailboxName, messageIDs string) ([]*store.Message, error) {
-	msgs := []*pmapi.Message{}
+	msgs := []*store.Message{}
-	var msg *pmapi.Message
+	var msg *store.Message
 	var err error
 	iterateOverSeqSet(messageIDs, func(messageID string) {
 		messageID = ctx.GetPMAPIController().GetMessageID(username, messageID)
@@ -327,16 +332,12 @@ func getMessages(username, addressID, mailboxName, messageIDs string) ([]*pmapi.
 	return msgs, err
 }
 
-func getMessage(username, addressID, mailboxName, messageID string) (*pmapi.Message, error) {
+func getMessage(username, addressID, mailboxName, messageID string) (*store.Message, error) {
 	mailbox, err := ctx.GetStoreMailbox(username, addressID, mailboxName)
 	if err != nil {
 		return nil, err
 	}
-	message, err := mailbox.GetMessage(messageID)
+	return mailbox.GetMessage(messageID)
-	if err != nil {
-		return nil, err
-	}
-	return message.Message(), nil
 }
 
 func hasItem(items []string, value string) bool {

test/store_setup_test.go

@@ -79,14 +79,16 @@ func thereAreMessagesInMailboxesForAddressOfUser(mailboxNames, bddAddressID, bdd
 	if account == nil {
 		return godog.ErrPending
 	}
-	head := messages.Rows[0].Cells
 
 	labelIDs, err := ctx.GetPMAPIController().GetLabelIDs(account.Username(), strings.Split(mailboxNames, ","))
 	if err != nil {
 		return internalError(err, "getting labels %s for %s", mailboxNames, account.Username())
 	}
 
-	for _, row := range messages.Rows {
+	var markMessageIDsDeleted []string
+
+	head := messages.Rows[0].Cells
+	for _, row := range messages.Rows[1:] {
 		message := &pmapi.Message{
 			MIMEType:  "text/plain",
 			LabelIDs:  labelIDs,
@@ -97,6 +99,8 @@ func thereAreMessagesInMailboxesForAddressOfUser(mailboxNames, bddAddressID, bdd
 			message.Flags |= pmapi.FlagSent
 		}
 
+		hasDeletedFlag := false
+
 		for n, cell := range row.Cells {
 			switch head[n].Value {
 			case "from":
@@ -134,11 +138,7 @@ func thereAreMessagesInMailboxesForAddressOfUser(mailboxNames, bddAddressID, bdd
 				}
 				message.Time = date.Unix()
 			case "deleted":
-				if cell.Value == "true" {
+				hasDeletedFlag = cell.Value == "true"
-					/* TODO
-					   Remember that this message should be marked as deleted
-					*/
-				}
 			default:
 				return fmt.Errorf("unexpected column name: %s", head[n].Value)
 			}
@@ -146,13 +146,28 @@ func thereAreMessagesInMailboxesForAddressOfUser(mailboxNames, bddAddressID, bdd
 		if err := ctx.GetPMAPIController().AddUserMessage(account.Username(), message); err != nil {
 			return internalError(err, "adding message")
 		}
+
+		if hasDeletedFlag {
+			lastMessageID := ctx.GetPMAPIController().GetLastMessageID(account.Username())
+			markMessageIDsDeleted = append(markMessageIDsDeleted, lastMessageID)
+		}
 	}
 
-	/* TODO
+	if err := internalError(ctx.WaitForSync(account.Username()), "waiting for sync"); err != nil {
-	storeMailbox.MarkMessageAsDeleted(msgID)
+		return err
-	*/
+	}
 
-	return internalError(ctx.WaitForSync(account.Username()), "waiting for sync")
+	for _, mailboxName := range strings.Split(mailboxNames, ",") {
+		storeMailbox, err := ctx.GetStoreMailbox(account.Username(), account.AddressID(), mailboxName)
+		if err != nil {
+			return err
+		}
+		if err := storeMailbox.MarkMessagesDeleted(markMessageIDsDeleted); err != nil {
+			return err
+		}
+	}
+
+	return nil
 }
 
 func thereAreSomeMessagesInMailboxesForUser(numberOfMessages int, mailboxNames, bddUserID string) error {