Set flags by FLAGS (not using +/-FLAGS) do not change spam state

Changelog.md

@@ -4,6 +4,13 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/)
 
 ## Unreleased
 
+## [Bridge 1.4.3] Forth
+
+### Changed
+* Reverted sending IMAP updates to be not blocking again.
+### Fixed
+* GODT-783 Settings flags by FLAGS (not using +/-FLAGS) do not change spam state.
+
 ## [Bridge 1.4.2] Forth
 
 ### Changed

internal/imap/mailbox_messages.go

@@ -57,6 +57,10 @@ func (im *imapMailbox) UpdateMessagesFlags(uid bool, seqSet *imap.SeqSet, operat
 	return im.addOrRemoveFlags(operation, messageIDs, flags)
 }
 
+// setFlags is used for FLAGS command (not +FLAGS or -FLAGS), which means
+// to set flags passed as an argument and unset the rest. For example,
+// if message is not read, is flagged and is not deleted, call FLAGS \Seen
+// should flag message as read, unflagged and keep undeleted.
 func (im *imapMailbox) setFlags(messageIDs, flags []string) error { //nolint
 	seen := false
 	flagged := false
@@ -106,16 +110,17 @@ func (im *imapMailbox) setFlags(messageIDs, flags []string) error { //nolint
 		}
 	}
 
-	spamMailbox, err := im.storeAddress.GetMailbox("Spam")
+	// Spam should not be taken into action here as Outlook is using FLAGS
-	if err != nil {
+	// without preserving junk flag. Probably it's because junk is not standard
-		return err
+	// in the rfc3501 and thus Outlook expects calling FLAGS \Seen will not
-	}
+	// change the state of junk or other non-standard flags.
+	// Still, its safe to label as spam once any client sends the request.
 	if spam {
-		if err := spamMailbox.LabelMessages(messageIDs); err != nil {
+		spamMailbox, err := im.storeAddress.GetMailbox("Spam")
+		if err != nil {
 			return err
 		}
-	} else {
+		if err := spamMailbox.LabelMessages(messageIDs); err != nil {
-		if err := spamMailbox.UnlabelMessages(messageIDs); err != nil {
 			return err
 		}
 	}

internal/store/change.go

@@ -122,22 +122,10 @@ func (store *Store) imapSendUpdate(update imapBackend.Update) {
 		return
 	}
 
-	done := update.Done()
-	go func() {
-		// This timeout is to not keep running many blocked goroutines.
-		// In case nothing listens to this channel, this thread should stop.
-		select {
-		case store.imapUpdates <- update:
-		case <-time.After(1 * time.Second):
-			store.log.Warn("IMAP update could not be sent (timeout).")
-		}
-	}()
-
-	// This timeout is to not block IMAP backend by wait for IMAP client.
 	select {
-	case <-done:
 	case <-time.After(1 * time.Second):
-		store.log.Warn("IMAP update could not be delivered (timeout).")
+		store.log.Warn("IMAP update could not be sent (timeout)")
 		return
+	case store.imapUpdates <- update:
 	}
 }

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

@@ -0,0 +1,21 @@
+Feature: IMAP update messages in Spam folder
+  Background:
+    Given there is connected user "user"
+    # Messages are inserted in opposite way to keep increasing ID.
+    # Sequence numbers are then opposite than listed above.
+    And there are messages in mailbox "Spam" for "user"
+      | from              | to         | subject | body  | read  | starred | deleted |
+      | john.doe@mail.com | user@pm.me | foo     | hello | false | false   | false   |
+      | jane.doe@mail.com | name@pm.me | bar     | world | true  | true    | false   |
+    And there is IMAP client logged in as "user"
+    And there is IMAP client selected in "Spam"
+
+  Scenario: Mark message as read only
+    When IMAP client marks message "2" with "\Seen"
+    Then IMAP response is "OK"
+    And message "1" in "Spam" for "user" is marked as read
+    And message "1" in "Spam" for "user" is marked as unstarred
+    And API mailbox "Spam" for "user" has messages
+      | from              | to         | subject |
+      | john.doe@mail.com | user@pm.me | foo     |
+      | jane.doe@mail.com | name@pm.me | bar     |