Send unilateral responses before sending OK

Makefile

@@ -209,7 +209,7 @@ coverage: test
 mocks:
 	mockgen --package mocks github.com/ProtonMail/proton-bridge/internal/users Configer,PanicHandler,ClientManager,CredentialsStorer,StoreMaker > internal/users/mocks/mocks.go
 	mockgen --package mocks github.com/ProtonMail/proton-bridge/internal/transfer PanicHandler,ClientManager,IMAPClientProvider > internal/transfer/mocks/mocks.go
-	mockgen --package mocks github.com/ProtonMail/proton-bridge/internal/store PanicHandler,ClientManager,BridgeUser > internal/store/mocks/mocks.go
+	mockgen --package mocks github.com/ProtonMail/proton-bridge/internal/store PanicHandler,ClientManager,BridgeUser,ChangeNotifier > internal/store/mocks/mocks.go
 	mockgen --package mocks github.com/ProtonMail/proton-bridge/pkg/listener Listener > internal/store/mocks/utils_mocks.go
 	mockgen --package mocks github.com/ProtonMail/proton-bridge/pkg/pmapi Client > pkg/pmapi/mocks/mocks.go
 

internal/imap/backend.go

@@ -46,6 +46,9 @@ type imapBackend struct {
 	imapCache     map[string]map[string]string
 	imapCachePath string
 	imapCacheLock *sync.RWMutex
+
+	updatesBlocking       map[string]bool
+	updatesBlockingLocker sync.Locker
 }
 
 // NewIMAPBackend returns struct implementing go-imap/backend interface.
@@ -58,10 +61,6 @@ func NewIMAPBackend(
 	bridgeWrap := newBridgeWrap(bridge)
 	backend := newIMAPBackend(panicHandler, cfg, bridgeWrap, eventListener)
 
-	// We want idle updates coming from bridge's updates channel (which in turn come
-	// from the bridge users' stores) to be sent to the imap backend's update channel.
-	backend.updates = bridge.GetIMAPUpdatesChannel()
-
 	go backend.monitorDisconnectedUsers()
 
 	return backend
@@ -84,6 +83,9 @@ func newIMAPBackend(
 
 		imapCachePath: cfg.GetIMAPCachePath(),
 		imapCacheLock: &sync.RWMutex{},
+
+		updatesBlocking:       map[string]bool{},
+		updatesBlockingLocker: &sync.Mutex{},
 	}
 }
 
@@ -169,7 +171,9 @@ func (ib *imapBackend) Login(_ *imap.ConnInfo, username, password string) (goIMA
 	// The update channel should be nil until we try to login to IMAP for the first time
 	// so that it doesn't make bridge slow for users who are only using bridge for SMTP
 	// (otherwise the store will be locked for 1 sec per email during synchronization).
-	imapUser.user.SetIMAPIdleUpdateChannel()
+	if store := imapUser.user.GetStore(); store != nil {
+		store.SetChangeNotifier(ib)
+	}
 
 	return imapUser, nil
 }

internal/imap/backend_updates.go

@@ -0,0 +1,169 @@
+// Copyright (c) 2020 Proton Technologies AG
+//
+// This file is part of ProtonMail Bridge.
+//
+// ProtonMail Bridge is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// ProtonMail Bridge is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with ProtonMail Bridge.  If not, see <https://www.gnu.org/licenses/>.
+
+package imap
+
+import (
+	"strings"
+	"time"
+
+	"github.com/ProtonMail/proton-bridge/internal/store"
+	"github.com/ProtonMail/proton-bridge/pkg/message"
+	"github.com/ProtonMail/proton-bridge/pkg/pmapi"
+	imap "github.com/emersion/go-imap"
+	goIMAPBackend "github.com/emersion/go-imap/backend"
+	"github.com/sirupsen/logrus"
+)
+
+type operation string
+
+const (
+	operationUpdateMessage operation = "store"
+	operationDeleteMessage operation = "expunge"
+)
+
+func (ib *imapBackend) setUpdatesBeBlocking(address, mailboxName string, op operation) {
+	ib.changeUpdatesBlocking(address, mailboxName, op, true)
+}
+
+func (ib *imapBackend) unsetUpdatesBeBlocking(address, mailboxName string, op operation) {
+	ib.changeUpdatesBlocking(address, mailboxName, op, false)
+}
+
+func (ib *imapBackend) changeUpdatesBlocking(address, mailboxName string, op operation, block bool) {
+	ib.updatesBlockingLocker.Lock()
+	defer ib.updatesBlockingLocker.Unlock()
+
+	key := strings.ToLower(address + "_" + mailboxName + "_" + string(op))
+	if block {
+		ib.updatesBlocking[key] = true
+	} else {
+		delete(ib.updatesBlocking, key)
+	}
+}
+
+func (ib *imapBackend) isBlocking(address, mailboxName string, op operation) bool {
+	key := strings.ToLower(address + "_" + mailboxName + "_" + string(op))
+	return ib.updatesBlocking[key]
+}
+
+func (ib *imapBackend) Notice(address, notice string) {
+	update := new(goIMAPBackend.StatusUpdate)
+	update.Update = goIMAPBackend.NewUpdate(address, "")
+	update.StatusResp = &imap.StatusResp{
+		Type: imap.StatusRespOk,
+		Code: imap.CodeAlert,
+		Info: notice,
+	}
+	ib.sendIMAPUpdate(update, false)
+}
+
+func (ib *imapBackend) UpdateMessage(
+	address, mailboxName string,
+	uid, sequenceNumber uint32,
+	msg *pmapi.Message, hasDeletedFlag bool,
+) {
+	log.WithFields(logrus.Fields{
+		"address": address,
+		"mailbox": mailboxName,
+		"seqNum":  sequenceNumber,
+		"uid":     uid,
+		"flags":   message.GetFlags(msg),
+		"deleted": hasDeletedFlag,
+	}).Trace("IDLE update")
+	update := new(goIMAPBackend.MessageUpdate)
+	update.Update = goIMAPBackend.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
+	ib.sendIMAPUpdate(update, ib.isBlocking(address, mailboxName, operationUpdateMessage))
+}
+
+func (ib *imapBackend) DeleteMessage(address, mailboxName string, sequenceNumber uint32) {
+	log.WithFields(logrus.Fields{
+		"address": address,
+		"mailbox": mailboxName,
+		"seqNum":  sequenceNumber,
+	}).Trace("IDLE delete")
+	update := new(goIMAPBackend.ExpungeUpdate)
+	update.Update = goIMAPBackend.NewUpdate(address, mailboxName)
+	update.SeqNum = sequenceNumber
+	ib.sendIMAPUpdate(update, ib.isBlocking(address, mailboxName, operationDeleteMessage))
+}
+
+func (ib *imapBackend) MailboxCreated(address, mailboxName string) {
+	log.WithFields(logrus.Fields{
+		"address": address,
+		"mailbox": mailboxName,
+	}).Trace("IDLE mailbox info")
+	update := new(goIMAPBackend.MailboxInfoUpdate)
+	update.Update = goIMAPBackend.NewUpdate(address, "")
+	update.MailboxInfo = &imap.MailboxInfo{
+		Attributes: []string{imap.NoInferiorsAttr},
+		Delimiter:  store.PathDelimiter,
+		Name:       mailboxName,
+	}
+	ib.sendIMAPUpdate(update, false)
+}
+
+func (ib *imapBackend) MailboxStatus(address, mailboxName string, total, unread, unreadSeqNum uint32) {
+	log.WithFields(logrus.Fields{
+		"address":      address,
+		"mailbox":      mailboxName,
+		"total":        total,
+		"unread":       unread,
+		"unreadSeqNum": unreadSeqNum,
+	}).Trace("IDLE status")
+	update := new(goIMAPBackend.MailboxUpdate)
+	update.Update = goIMAPBackend.NewUpdate(address, mailboxName)
+	update.MailboxStatus = imap.NewMailboxStatus(mailboxName, []imap.StatusItem{imap.StatusMessages, imap.StatusUnseen})
+	update.MailboxStatus.Messages = total
+	update.MailboxStatus.Unseen = unread
+	update.MailboxStatus.UnseenSeqNum = unreadSeqNum
+	ib.sendIMAPUpdate(update, false)
+}
+
+func (ib *imapBackend) sendIMAPUpdate(update goIMAPBackend.Update, block bool) {
+	if ib.updates == nil {
+		log.Trace("IMAP IDLE unavailable")
+		return
+	}
+
+	done := update.Done()
+	go func() {
+		select {
+		case <-time.After(1 * time.Second):
+			log.Warn("IMAP update could not be sent (timeout)")
+			return
+		case ib.updates <- update:
+		}
+	}()
+
+	if !block {
+		return
+	}
+
+	select {
+	case <-done:
+	case <-time.After(1 * time.Second):
+		log.Warn("IMAP update could not be delivered (timeout).")
+		return
+	}
+}

internal/imap/bridge.go

@@ -40,7 +40,6 @@ type bridgeUser interface {
 	IsCombinedAddressMode() bool
 	GetAddressID(address string) (string, error)
 	GetPrimaryAddress() string
-	SetIMAPIdleUpdateChannel()
 	UpdateUser() error
 	Logout() error
 	CloseConnection(address string)

internal/imap/mailbox.go

@@ -177,6 +177,9 @@ func (im *imapMailbox) Check() error {
 // Expunge permanently removes all messages that have the \Deleted flag set
 // from the currently selected mailbox.
 func (im *imapMailbox) Expunge() error {
+	im.user.backend.setUpdatesBeBlocking(im.user.currentAddressLowercase, im.name, operationDeleteMessage)
+	defer im.user.backend.unsetUpdatesBeBlocking(im.user.currentAddressLowercase, im.name, operationDeleteMessage)
+
 	return im.storeMailbox.RemoveDeleted()
 }
 

internal/imap/mailbox_messages.go

@@ -46,6 +46,9 @@ func (im *imapMailbox) UpdateMessagesFlags(uid bool, seqSet *imap.SeqSet, operat
 	// Called from go-imap in goroutines - we need to handle panics for each function.
 	defer im.panicHandler.HandlePanic()
 
+	im.user.backend.setUpdatesBeBlocking(im.user.currentAddressLowercase, im.name, operationUpdateMessage)
+	defer im.user.backend.unsetUpdatesBeBlocking(im.user.currentAddressLowercase, im.name, operationUpdateMessage)
+
 	messageIDs, err := im.apiIDsFromSeqSet(uid, seqSet)
 	if err != nil || len(messageIDs) == 0 {
 		return err

internal/imap/store.go

@@ -43,6 +43,8 @@ type storeUserProvider interface {
 		parentID string) (*pmapi.Message, []*pmapi.Attachment, error)
 
 	PauseEventLoop(bool)
+
+	SetChangeNotifier(store.ChangeNotifier)
 }
 
 type storeAddressProvider interface {

internal/store/address_mailbox.go

@@ -78,7 +78,7 @@ func (storeAddress *Address) createOrUpdateMailboxEvent(label *pmapi.Label) erro
 			return err
 		}
 		storeAddress.mailboxes[label.ID] = mailbox
-		mailbox.store.imapMailboxCreated(storeAddress.address, mailbox.labelName)
+		mailbox.store.notifyMailboxCreated(storeAddress.address, mailbox.labelName)
 	} else {
 		mailbox.labelName = prefix + label.Path
 		mailbox.color = label.Color

internal/store/change.go

@@ -18,119 +18,56 @@
 package store
 
 import (
-	"time"
-
-	"github.com/ProtonMail/proton-bridge/pkg/message"
 	"github.com/ProtonMail/proton-bridge/pkg/pmapi"
-	imap "github.com/emersion/go-imap"
-	imapBackend "github.com/emersion/go-imap/backend"
-	"github.com/sirupsen/logrus"
 )
 
-// SetIMAPUpdateChannel sets the channel on which imap update messages will be sent. This should be the channel
+type ChangeNotifier interface {
-// on which the imap backend listens for imap updates.
+	Notice(address, notice string)
-func (store *Store) SetIMAPUpdateChannel(updates chan imapBackend.Update) {
+	UpdateMessage(
-	store.log.Debug("Listening for IMAP updates")
+		address, mailboxName string,
-
+		uid, sequenceNumber uint32,
-	if store.imapUpdates = updates; store.imapUpdates == nil {
+		msg *pmapi.Message, hasDeletedFlag bool)
-		store.log.Error("The IMAP Updates channel is nil")
+	DeleteMessage(address, mailboxName string, sequenceNumber uint32)
-	}
+	MailboxCreated(address, mailboxName string)
+	MailboxStatus(address, mailboxName string, total, unread, unreadSeqNum uint32)
 }
 
-func (store *Store) imapNotice(address, notice string) *imapBackend.StatusUpdate {
+// SetChangeNotifier sets notifier to be called once mailbox or message changes.
-	update := new(imapBackend.StatusUpdate)
+func (store *Store) SetChangeNotifier(notifier ChangeNotifier) {
-	update.Update = imapBackend.NewUpdate(address, "")
+	store.notifier = notifier
-	update.StatusResp = &imap.StatusResp{
-		Type: imap.StatusRespOk,
-		Code: imap.CodeAlert,
-		Info: notice,
-	}
-	store.imapSendUpdate(update)
-	return update
 }
 
-func (store *Store) imapUpdateMessage(
+func (store *Store) notifyNotice(address, notice string) {
-	address, mailboxName string,
+	if store.notifier == nil {
-	uid, sequenceNumber uint32,
-	msg *pmapi.Message, hasDeletedFlag bool,
-) *imapBackend.MessageUpdate {
-	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)
-	return update
-}
-
-func (store *Store) imapDeleteMessage(address, mailboxName string, sequenceNumber uint32) *imapBackend.ExpungeUpdate {
-	store.log.WithFields(logrus.Fields{
-		"address": address,
-		"mailbox": mailboxName,
-		"seqNum":  sequenceNumber,
-	}).Trace("IDLE delete")
-	update := new(imapBackend.ExpungeUpdate)
-	update.Update = imapBackend.NewUpdate(address, mailboxName)
-	update.SeqNum = sequenceNumber
-	store.imapSendUpdate(update)
-	return update
-}
-
-func (store *Store) imapMailboxCreated(address, mailboxName string) *imapBackend.MailboxInfoUpdate {
-	store.log.WithFields(logrus.Fields{
-		"address": address,
-		"mailbox": mailboxName,
-	}).Trace("IDLE mailbox info")
-	update := new(imapBackend.MailboxInfoUpdate)
-	update.Update = imapBackend.NewUpdate(address, "")
-	update.MailboxInfo = &imap.MailboxInfo{
-		Attributes: []string{imap.NoInferiorsAttr},
-		Delimiter:  PathDelimiter,
-		Name:       mailboxName,
-	}
-	store.imapSendUpdate(update)
-	return update
-}
-
-func (store *Store) imapMailboxStatus(address, mailboxName string, total, unread, unreadSeqNum uint) *imapBackend.MailboxUpdate {
-	store.log.WithFields(logrus.Fields{
-		"address":      address,
-		"mailbox":      mailboxName,
-		"total":        total,
-		"unread":       unread,
-		"unreadSeqNum": unreadSeqNum,
-	}).Trace("IDLE status")
-	update := new(imapBackend.MailboxUpdate)
-	update.Update = imapBackend.NewUpdate(address, mailboxName)
-	update.MailboxStatus = imap.NewMailboxStatus(mailboxName, []imap.StatusItem{imap.StatusMessages, imap.StatusUnseen})
-	update.MailboxStatus.Messages = uint32(total)
-	update.MailboxStatus.Unseen = uint32(unread)
-	update.MailboxStatus.UnseenSeqNum = uint32(unreadSeqNum)
-	store.imapSendUpdate(update)
-	return update
-}
-
-func (store *Store) imapSendUpdate(update imapBackend.Update) {
-	if store.imapUpdates == nil {
-		store.log.Trace("IMAP IDLE unavailable")
 		return
 	}
-
+	store.notifier.Notice(address, notice)
-	select {
+}
-	case <-time.After(1 * time.Second):
+
-		store.log.Warn("IMAP update could not be sent (timeout)")
+func (store *Store) notifyUpdateMessage(address, mailboxName string, uid, sequenceNumber uint32, msg *pmapi.Message, hasDeletedFlag bool) {
-		return
+	if store.notifier == nil {
-	case store.imapUpdates <- update:
+		return
-	}
+	}
+	store.notifier.UpdateMessage(address, mailboxName, uid, sequenceNumber, msg, hasDeletedFlag)
+}
+
+func (store *Store) notifyDeleteMessage(address, mailboxName string, sequenceNumber uint32) {
+	if store.notifier == nil {
+		return
+	}
+	store.notifier.DeleteMessage(address, mailboxName, sequenceNumber)
+}
+
+func (store *Store) notifyMailboxCreated(address, mailboxName string) {
+	if store.notifier == nil {
+		return
+	}
+	store.notifier.MailboxCreated(address, mailboxName)
+}
+
+func (store *Store) notifyMailboxStatus(address, mailboxName string, total, unread, unreadSeqNum uint) {
+	if store.notifier == nil {
+		return
+	}
+	store.notifier.MailboxStatus(address, mailboxName, uint32(total), uint32(unread), uint32(unreadSeqNum))
 }

internal/store/change_test.go

@@ -21,52 +21,43 @@ import (
 	"testing"
 
 	"github.com/ProtonMail/proton-bridge/pkg/pmapi"
-	imapBackend "github.com/emersion/go-imap/backend"
+	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/require"
 )
 
-func TestCreateOrUpdateMessageIMAPUpdates(t *testing.T) {
+func TestNotifyChangeCreateOrUpdateMessage(t *testing.T) {
 	m, clear := initMocks(t)
 	defer clear()
 
-	updates := make(chan imapBackend.Update)
+	m.changeNotifier.EXPECT().MailboxStatus(addr1, "All Mail", uint32(1), uint32(0), uint32(0))
+	m.changeNotifier.EXPECT().MailboxStatus(addr1, "All Mail", uint32(2), uint32(0), uint32(0))
+	m.changeNotifier.EXPECT().UpdateMessage(addr1, "All Mail", uint32(1), uint32(1), gomock.Any(), false)
+	m.changeNotifier.EXPECT().UpdateMessage(addr1, "All Mail", uint32(2), uint32(2), gomock.Any(), false)
 
 	m.newStoreNoEvents(true)
-	m.store.SetIMAPUpdateChannel(updates)
+	m.store.SetChangeNotifier(m.changeNotifier)
-
-	go checkIMAPUpdates(t, updates, []func(interface{}) bool{
-		checkMessageUpdate(addr1, "All Mail", 1, 1),
-		checkMessageUpdate(addr1, "All Mail", 2, 2),
-	})
 
 	insertMessage(t, m, "msg1", "Test message 1", addrID1, 0, []string{pmapi.AllMailLabel})
 	insertMessage(t, m, "msg2", "Test message 2", addrID1, 0, []string{pmapi.AllMailLabel})
-
-	close(updates)
 }
 
-func TestCreateOrUpdateMessageIMAPUpdatesBulkUpdate(t *testing.T) {
+func TestNotifyChangeCreateOrUpdateMessages(t *testing.T) {
 	m, clear := initMocks(t)
 	defer clear()
 
-	updates := make(chan imapBackend.Update)
+	m.changeNotifier.EXPECT().MailboxStatus(addr1, "All Mail", uint32(2), uint32(0), uint32(0))
+	m.changeNotifier.EXPECT().UpdateMessage(addr1, "All Mail", uint32(1), uint32(1), gomock.Any(), false)
+	m.changeNotifier.EXPECT().UpdateMessage(addr1, "All Mail", uint32(2), uint32(2), gomock.Any(), false)
 
 	m.newStoreNoEvents(true)
-	m.store.SetIMAPUpdateChannel(updates)
+	m.store.SetChangeNotifier(m.changeNotifier)
-
-	go checkIMAPUpdates(t, updates, []func(interface{}) bool{
-		checkMessageUpdate(addr1, "All Mail", 1, 1),
-		checkMessageUpdate(addr1, "All Mail", 2, 2),
-	})
 
 	msg1 := getTestMessage("msg1", "Test message 1", addrID1, 0, []string{pmapi.AllMailLabel})
 	msg2 := getTestMessage("msg2", "Test message 2", addrID1, 0, []string{pmapi.AllMailLabel})
 	require.Nil(t, m.store.createOrUpdateMessagesEvent([]*pmapi.Message{msg1, msg2}))
-
-	close(updates)
 }
 
-func TestDeleteMessageIMAPUpdate(t *testing.T) {
+func TestNotifyChangeDeleteMessage(t *testing.T) {
 	m, clear := initMocks(t)
 	defer clear()
 
@@ -75,55 +66,10 @@ func TestDeleteMessageIMAPUpdate(t *testing.T) {
 	insertMessage(t, m, "msg1", "Test message 1", addrID1, 0, []string{pmapi.AllMailLabel})
 	insertMessage(t, m, "msg2", "Test message 2", addrID1, 0, []string{pmapi.AllMailLabel})
 
-	updates := make(chan imapBackend.Update)
+	m.changeNotifier.EXPECT().DeleteMessage(addr1, "All Mail", uint32(2))
-	m.store.SetIMAPUpdateChannel(updates)
+	m.changeNotifier.EXPECT().DeleteMessage(addr1, "All Mail", uint32(1))
-	go checkIMAPUpdates(t, updates, []func(interface{}) bool{
-		checkMessageDelete(addr1, "All Mail", 2),
-		checkMessageDelete(addr1, "All Mail", 1),
-	})
 
+	m.store.SetChangeNotifier(m.changeNotifier)
 	require.Nil(t, m.store.deleteMessageEvent("msg2"))
 	require.Nil(t, m.store.deleteMessageEvent("msg1"))
-	close(updates)
-}
-
-func checkIMAPUpdates(t *testing.T, updates chan imapBackend.Update, checkFunctions []func(interface{}) bool) {
-	idx := 0
-	for update := range updates {
-		if idx >= len(checkFunctions) {
-			continue
-		}
-		if !checkFunctions[idx](update) {
-			continue
-		}
-		idx++
-	}
-	require.True(t, idx == len(checkFunctions), "Less updates than expected: %+v of %+v", idx, len(checkFunctions))
-}
-
-func checkMessageUpdate(username, mailbox string, seqNum, uid int) func(interface{}) bool { //nolint[unparam]
-	return func(update interface{}) bool {
-		switch u := update.(type) {
-		case *imapBackend.MessageUpdate:
-			return (u.Update.Username() == username &&
-				u.Update.Mailbox() == mailbox &&
-				u.Message.SeqNum == uint32(seqNum) &&
-				u.Message.Uid == uint32(uid))
-		default:
-			return false
-		}
-	}
-}
-
-func checkMessageDelete(username, mailbox string, seqNum int) func(interface{}) bool { //nolint[unparam]
-	return func(update interface{}) bool {
-		switch u := update.(type) {
-		case *imapBackend.ExpungeUpdate:
-			return (u.Update.Username() == username &&
-				u.Update.Mailbox() == mailbox &&
-				u.SeqNum == uint32(seqNum))
-		default:
-			return false
-		}
-	}
 }

internal/store/event_loop.go

@@ -571,7 +571,7 @@ func (loop *eventLoop) processNotices(l *logrus.Entry, notices []string) {
 	for _, notice := range notices {
 		l.Infof("Notice: %q", notice)
 		for _, address := range loop.user.GetStoreAddresses() {
-			loop.store.imapNotice(address, notice)
+			loop.store.notifyNotice(address, notice)
 		}
 	}
 }

internal/store/mailbox_message.go

@@ -18,8 +18,6 @@
 package store
 
 import (
-	"time"
-
 	"github.com/ProtonMail/proton-bridge/pkg/pmapi"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -356,7 +354,7 @@ func (storeMailbox *Mailbox) txCreateOrUpdateMessages(tx *bolt.Tx, msgs []*pmapi
 				}
 				isMarkedAsDeleted := deletedBucket.Get([]byte(msg.ID)) != nil
 				if seqErr == nil {
-					storeMailbox.store.imapUpdateMessage(
+					storeMailbox.store.notifyUpdateMessage(
 						storeMailbox.storeAddress.address,
 						storeMailbox.labelName,
 						btoi(uidb),
@@ -390,7 +388,7 @@ func (storeMailbox *Mailbox) txCreateOrUpdateMessages(tx *bolt.Tx, msgs []*pmapi
 		if err != nil {
 			return errors.Wrap(err, "cannot get sequence number from UID")
 		}
-		storeMailbox.store.imapUpdateMessage(
+		storeMailbox.store.notifyUpdateMessage(
 			storeMailbox.storeAddress.address,
 			storeMailbox.labelName,
 			uid,
@@ -441,7 +439,7 @@ func (storeMailbox *Mailbox) txDeleteMessage(tx *bolt.Tx, apiID string) error {
 	}
 
 	if seqNumErr == nil {
-		storeMailbox.store.imapDeleteMessage(
+		storeMailbox.store.notifyDeleteMessage(
 			storeMailbox.storeAddress.address,
 			storeMailbox.labelName,
 			seqNum,
@@ -459,7 +457,7 @@ func (storeMailbox *Mailbox) txMailboxStatusUpdate(tx *bolt.Tx) error {
 	if err != nil {
 		return errors.Wrap(err, "cannot get counts for mailbox status update")
 	}
-	storeMailbox.store.imapMailboxStatus(
+	storeMailbox.store.notifyMailboxStatus(
 		storeMailbox.storeAddress.address,
 		storeMailbox.labelName,
 		total,
@@ -503,7 +501,7 @@ func (storeMailbox *Mailbox) txMarkMessagesAsDeleted(tx *bolt.Tx, apiIDs []strin
 
 		// In order to send flags in format
 		// S: * 2 FETCH (FLAGS (\Deleted \Seen))
-		update := storeMailbox.store.imapUpdateMessage(
+		storeMailbox.store.notifyUpdateMessage(
 			storeMailbox.storeAddress.address,
 			storeMailbox.labelName,
 			uid,
@@ -511,14 +509,6 @@ func (storeMailbox *Mailbox) txMarkMessagesAsDeleted(tx *bolt.Tx, apiIDs []strin
 			msg,
 			markAsDeleted,
 		)
-
-		// txMarkMessagesAsDeleted is called only during processing request
-		// from IMAP call (i.e., not from event loop) and in such cases we
-		// have to wait to propagate update back before closing the response.
-		select {
-		case <-time.After(1 * time.Second):
-		case <-update.Done():
-		}
 	}
 
 	return nil

internal/store/mocks/mocks.go

@@ -1,14 +1,13 @@
 // Code generated by MockGen. DO NOT EDIT.
-// Source: github.com/ProtonMail/proton-bridge/internal/store (interfaces: PanicHandler,ClientManager,BridgeUser)
+// Source: github.com/ProtonMail/proton-bridge/internal/store (interfaces: PanicHandler,ClientManager,BridgeUser,ChangeNotifier)
 
 // Package mocks is a generated GoMock package.
 package mocks
 
 import (
-	reflect "reflect"
-
 	pmapi "github.com/ProtonMail/proton-bridge/pkg/pmapi"
 	gomock "github.com/golang/mock/gomock"
+	reflect "reflect"
 )
 
 // MockPanicHandler is a mock of PanicHandler interface
@@ -242,3 +241,86 @@ func (mr *MockBridgeUserMockRecorder) UpdateUser() *gomock.Call {
 	mr.mock.ctrl.T.Helper()
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateUser", reflect.TypeOf((*MockBridgeUser)(nil).UpdateUser))
 }
+
+// MockChangeNotifier is a mock of ChangeNotifier interface
+type MockChangeNotifier struct {
+	ctrl     *gomock.Controller
+	recorder *MockChangeNotifierMockRecorder
+}
+
+// MockChangeNotifierMockRecorder is the mock recorder for MockChangeNotifier
+type MockChangeNotifierMockRecorder struct {
+	mock *MockChangeNotifier
+}
+
+// NewMockChangeNotifier creates a new mock instance
+func NewMockChangeNotifier(ctrl *gomock.Controller) *MockChangeNotifier {
+	mock := &MockChangeNotifier{ctrl: ctrl}
+	mock.recorder = &MockChangeNotifierMockRecorder{mock}
+	return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use
+func (m *MockChangeNotifier) EXPECT() *MockChangeNotifierMockRecorder {
+	return m.recorder
+}
+
+// DeleteMessage mocks base method
+func (m *MockChangeNotifier) DeleteMessage(arg0, arg1 string, arg2 uint32) {
+	m.ctrl.T.Helper()
+	m.ctrl.Call(m, "DeleteMessage", arg0, arg1, arg2)
+}
+
+// DeleteMessage indicates an expected call of DeleteMessage
+func (mr *MockChangeNotifierMockRecorder) DeleteMessage(arg0, arg1, arg2 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteMessage", reflect.TypeOf((*MockChangeNotifier)(nil).DeleteMessage), arg0, arg1, arg2)
+}
+
+// MailboxCreated mocks base method
+func (m *MockChangeNotifier) MailboxCreated(arg0, arg1 string) {
+	m.ctrl.T.Helper()
+	m.ctrl.Call(m, "MailboxCreated", arg0, arg1)
+}
+
+// MailboxCreated indicates an expected call of MailboxCreated
+func (mr *MockChangeNotifierMockRecorder) MailboxCreated(arg0, arg1 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MailboxCreated", reflect.TypeOf((*MockChangeNotifier)(nil).MailboxCreated), arg0, arg1)
+}
+
+// MailboxStatus mocks base method
+func (m *MockChangeNotifier) MailboxStatus(arg0, arg1 string, arg2, arg3, arg4 uint32) {
+	m.ctrl.T.Helper()
+	m.ctrl.Call(m, "MailboxStatus", arg0, arg1, arg2, arg3, arg4)
+}
+
+// MailboxStatus indicates an expected call of MailboxStatus
+func (mr *MockChangeNotifierMockRecorder) MailboxStatus(arg0, arg1, arg2, arg3, arg4 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MailboxStatus", reflect.TypeOf((*MockChangeNotifier)(nil).MailboxStatus), arg0, arg1, arg2, arg3, arg4)
+}
+
+// Notice mocks base method
+func (m *MockChangeNotifier) Notice(arg0, arg1 string) {
+	m.ctrl.T.Helper()
+	m.ctrl.Call(m, "Notice", arg0, arg1)
+}
+
+// Notice indicates an expected call of Notice
+func (mr *MockChangeNotifierMockRecorder) Notice(arg0, arg1 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Notice", reflect.TypeOf((*MockChangeNotifier)(nil).Notice), arg0, arg1)
+}
+
+// UpdateMessage mocks base method
+func (m *MockChangeNotifier) UpdateMessage(arg0, arg1 string, arg2, arg3 uint32, arg4 *pmapi.Message, arg5 bool) {
+	m.ctrl.T.Helper()
+	m.ctrl.Call(m, "UpdateMessage", arg0, arg1, arg2, arg3, arg4, arg5)
+}
+
+// UpdateMessage indicates an expected call of UpdateMessage
+func (mr *MockChangeNotifierMockRecorder) UpdateMessage(arg0, arg1, arg2, arg3, arg4, arg5 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateMessage", reflect.TypeOf((*MockChangeNotifier)(nil).UpdateMessage), arg0, arg1, arg2, arg3, arg4, arg5)
+}

internal/store/store.go

@@ -26,7 +26,6 @@ import (
 
 	"github.com/ProtonMail/proton-bridge/pkg/listener"
 	"github.com/ProtonMail/proton-bridge/pkg/pmapi"
-	imapBackend "github.com/emersion/go-imap/backend"
 	"github.com/hashicorp/go-multierror"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -100,12 +99,12 @@ type Store struct {
 
 	log *logrus.Entry
 
-	cache       *Cache
+	cache     *Cache
-	filePath    string
+	filePath  string
-	db          *bolt.DB
+	db        *bolt.DB
-	lock        *sync.RWMutex
+	lock      *sync.RWMutex
-	addresses   map[string]*Address
+	addresses map[string]*Address
-	imapUpdates chan imapBackend.Update
+	notifier  ChangeNotifier
 
 	isSyncRunning bool
 	syncCooldown  cooldown

internal/store/store_test.go

@@ -44,13 +44,14 @@ const (
 type mocksForStore struct {
 	tb testing.TB
 
-	ctrl          *gomock.Controller
+	ctrl           *gomock.Controller
-	events        *storemocks.MockListener
+	events         *storemocks.MockListener
-	user          *storemocks.MockBridgeUser
+	user           *storemocks.MockBridgeUser
-	client        *pmapimocks.MockClient
+	client         *pmapimocks.MockClient
-	clientManager *storemocks.MockClientManager
+	clientManager  *storemocks.MockClientManager
-	panicHandler  *storemocks.MockPanicHandler
+	panicHandler   *storemocks.MockPanicHandler
-	store         *Store
+	changeNotifier *storemocks.MockChangeNotifier
+	store          *Store
 
 	tmpDir string
 	cache  *Cache
@@ -59,13 +60,14 @@ type mocksForStore struct {
 func initMocks(tb testing.TB) (*mocksForStore, func()) {
 	ctrl := gomock.NewController(tb)
 	mocks := &mocksForStore{
-		tb:            tb,
+		tb:             tb,
-		ctrl:          ctrl,
+		ctrl:           ctrl,
-		events:        storemocks.NewMockListener(ctrl),
+		events:         storemocks.NewMockListener(ctrl),
-		user:          storemocks.NewMockBridgeUser(ctrl),
+		user:           storemocks.NewMockBridgeUser(ctrl),
-		client:        pmapimocks.NewMockClient(ctrl),
+		client:         pmapimocks.NewMockClient(ctrl),
-		clientManager: storemocks.NewMockClientManager(ctrl),
+		clientManager:  storemocks.NewMockClientManager(ctrl),
-		panicHandler:  storemocks.NewMockPanicHandler(ctrl),
+		panicHandler:   storemocks.NewMockPanicHandler(ctrl),
+		changeNotifier: storemocks.NewMockChangeNotifier(ctrl),
 	}
 
 	// Called during clean-up.

internal/users/user.go

@@ -27,7 +27,6 @@ import (
 	"github.com/ProtonMail/proton-bridge/internal/users/credentials"
 	"github.com/ProtonMail/proton-bridge/pkg/listener"
 	"github.com/ProtonMail/proton-bridge/pkg/pmapi"
-	imapBackend "github.com/emersion/go-imap/backend"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -43,8 +42,6 @@ type User struct {
 	clientManager ClientManager
 	credStorer    CredentialsStorer
 
-	imapUpdatesChannel chan imapBackend.Update
-
 	storeFactory StoreMaker
 	store        *store.Store
 
@@ -95,7 +92,7 @@ func (u *User) client() pmapi.Client {
 // have the apitoken and password), authorising the user against the api, loading the user store (creating a new one
 // if necessary), and setting the imap idle updates channel (used to send imap idle updates to the imap backend if
 // something in the store changed).
-func (u *User) init(idleUpdates chan imapBackend.Update) (err error) {
+func (u *User) init() (err error) {
 	u.log.Info("Initialising user")
 
 	// Reload the user's credentials (if they log out and back in we need the new
@@ -134,20 +131,9 @@ func (u *User) init(idleUpdates chan imapBackend.Update) (err error) {
 	}
 	u.store = store
 
-	// Save the imap updates channel here so it can be set later when imap connects.
-	u.imapUpdatesChannel = idleUpdates
-
 	return err
 }
 
-func (u *User) SetIMAPIdleUpdateChannel() {
-	if u.store == nil {
-		return
-	}
-
-	u.store.SetIMAPUpdateChannel(u.imapUpdatesChannel)
-}
-
 // authorizeIfNecessary checks whether user is logged in and is connected to api auth channel.
 // If user is not already connected to the api auth channel (for example there was no internet during start),
 // it tries to connect it.
@@ -539,7 +525,7 @@ func (u *User) CloseAllConnections() {
 	}
 
 	if u.store != nil {
-		u.store.SetIMAPUpdateChannel(nil)
+		u.store.SetChangeNotifier(nil)
 	}
 }
 

internal/users/user_credentials_test.go

@@ -221,7 +221,7 @@ func TestCheckBridgeLoginLoggedOut(t *testing.T) {
 		m.pmapiClient.EXPECT().Addresses().Return(nil),
 	)
 
-	err = user.init(nil)
+	err = user.init()
 	assert.Error(t, err)
 
 	defer cleanUpUserData(user)

internal/users/user_new_test.go

@@ -139,7 +139,7 @@ func checkNewUserHasCredentials(creds *credentials.Credentials, m mocks) {
 	user, _ := newUser(m.PanicHandler, "user", m.eventListener, m.credentialsStore, m.clientManager, m.storeMaker)
 	defer cleanUpUserData(user)
 
-	_ = user.init(nil)
+	_ = user.init()
 
 	waitForEvents()
 

internal/users/user_test.go

@@ -34,7 +34,7 @@ func testNewUser(m mocks) *User {
 	user, err := newUser(m.PanicHandler, "user", m.eventListener, m.credentialsStore, m.clientManager, m.storeMaker)
 	assert.NoError(m.t, err)
 
-	err = user.init(nil)
+	err = user.init()
 	assert.NoError(m.t, err)
 
 	mockAuthUpdate(user, "reftok", m)
@@ -51,7 +51,7 @@ func testNewUserForLogout(m mocks) *User {
 	user, err := newUser(m.PanicHandler, "user", m.eventListener, m.credentialsStore, m.clientManager, m.storeMaker)
 	assert.NoError(m.t, err)
 
-	err = user.init(nil)
+	err = user.init()
 	assert.NoError(m.t, err)
 
 	return user

internal/users/users.go

@@ -26,7 +26,6 @@ import (
 	"github.com/ProtonMail/proton-bridge/internal/metrics"
 	"github.com/ProtonMail/proton-bridge/pkg/listener"
 	"github.com/ProtonMail/proton-bridge/pkg/pmapi"
-	imapBackend "github.com/emersion/go-imap/backend"
 	"github.com/hashicorp/go-multierror"
 	"github.com/pkg/errors"
 	logrus "github.com/sirupsen/logrus"
@@ -58,11 +57,6 @@ type Users struct {
 	// as is, without requesting server again.
 	useOnlyActiveAddresses bool
 
-	// idleUpdates is a channel which the imap backend listens to and which it
-	// uses to send idle updates to the mail client (eg thunderbird).
-	// The user stores should send idle updates on this channel.
-	idleUpdates chan imapBackend.Update
-
 	lock sync.RWMutex
 
 	// stopAll can be closed to stop all goroutines from looping (watchAppOutdated, watchAPIAuths, heartbeat etc).
@@ -88,7 +82,6 @@ func New(
 		credStorer:             credStorer,
 		storeFactory:           storeFactory,
 		useOnlyActiveAddresses: useOnlyActiveAddresses,
-		idleUpdates:            make(chan imapBackend.Update),
 		lock:                   sync.RWMutex{},
 		stopAll:                make(chan struct{}),
 	}
@@ -132,7 +125,7 @@ func (u *Users) loadUsersFromCredentialsStore() (err error) {
 
 		u.users = append(u.users, user)
 
-		if initUserErr := user.init(u.idleUpdates); initUserErr != nil {
+		if initUserErr := user.init(); initUserErr != nil {
 			l.WithField("user", userID).WithError(initUserErr).Warn("Could not initialise user")
 		}
 	}
@@ -285,7 +278,7 @@ func (u *Users) connectExistingUser(user *User, auth *pmapi.Auth, hashedPassphra
 		return errors.Wrap(err, "failed to update token of user in credentials store")
 	}
 
-	if err = user.init(u.idleUpdates); err != nil {
+	if err = user.init(); err != nil {
 		return errors.Wrap(err, "failed to initialise user")
 	}
 
@@ -326,7 +319,7 @@ func (u *Users) addNewUser(apiUser *pmapi.User, auth *pmapi.Auth, hashedPassphra
 	// The user needs to be part of the users list in order for it to receive an auth during initialisation.
 	u.users = append(u.users, user)
 
-	if err = user.init(u.idleUpdates); err != nil {
+	if err = user.init(); err != nil {
 		u.users = u.users[:len(u.users)-1]
 		return errors.Wrap(err, "failed to initialise user")
 	}
@@ -464,15 +457,6 @@ func (u *Users) SendMetric(m metrics.Metric) {
 	}).Debug("Metric successfully sent")
 }
 
-// GetIMAPUpdatesChannel sets the channel on which idle events should be sent.
-func (u *Users) GetIMAPUpdatesChannel() chan imapBackend.Update {
-	if u.idleUpdates == nil {
-		log.Warn("IMAP updates channel is nil")
-	}
-
-	return u.idleUpdates
-}
-
 // AllowProxy instructs the app to use DoH to access an API proxy if necessary.
 // It also needs to work before the app is initialised (because we may need to use the proxy at startup).
 func (u *Users) AllowProxy() {

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

@@ -4,7 +4,6 @@ Feature: IMAP remove messages from mailbox
     And there is "user" with mailbox "Folders/mbox"
     And there is "user" with mailbox "Labels/label"
 
-  @ignore
   Scenario Outline: Mark message as deleted and EXPUNGE
     Given there are 10 messages in mailbox "<mailbox>" for "user"
     And there is IMAP client logged in as "user"
@@ -27,7 +26,6 @@ Feature: IMAP remove messages from mailbox
       | Spam         |
       | Trash        |
 
-  @ignore
   Scenario Outline: Mark all messages as deleted and EXPUNGE
     Given there are 5 messages in mailbox "<mailbox>" for "user"
     And there is IMAP client logged in as "user"
@@ -51,7 +49,6 @@ Feature: IMAP remove messages from mailbox
       | Spam         |
       | Trash        |
 
-  @ignore
   Scenario Outline: Mark messages as undeleted and EXPUNGE
     Given there are 5 messages in mailbox "<mailbox>" for "user"
     And there is IMAP client logged in as "user"
@@ -74,7 +71,6 @@ Feature: IMAP remove messages from mailbox
       | Spam         |
       | Trash        |
 
-  @ignore
   Scenario Outline: Mark message as deleted and leave mailbox
     Given there are 10 messages in mailbox "INBOX" for "user"
     And there is IMAP client logged in as "user"
@@ -97,7 +93,6 @@ Feature: IMAP remove messages from mailbox
       | LOGOUT        | 9  |
       | UNSELECT      | 10 |
 
-  @ignore
   Scenario: Not possible to delete from All Mail
     Given there are 1 messages in mailbox "INBOX" for "user"
     And there is IMAP client logged in as "user"

unreleased.md

@@ -9,6 +9,7 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/)
 ### Changed
 * GODT-893 Bump go-rfc5322 dependency to v0.2.1 to properly detect syntax errors during parsing.
 * GODT-892 Swap type and value from sentry exception and cut panic handlers from the traceback.
+* GODT-854 EXPUNGE and FETCH unilateral responses are returned before OK EXPUNGE or OK STORE, respectively.
 
 ### Removed
 * GODT-651 Build creates proper binary names.