From 1711442878276eee8192107957d517f5e74f4345 Mon Sep 17 00:00:00 2001 From: Michal Horejsek Date: Thu, 23 Jul 2020 14:20:20 +0200 Subject: [PATCH] Fix setting flags --- Changelog.md | 5 ++ internal/imap/mailbox_messages.go | 63 +++++++++++++++++++++-- test/features/imap/message/delete.feature | 10 ++++ test/features/imap/message/move.feature | 2 + test/features/imap/message/search.feature | 2 +- test/features/imap/message/update.feature | 22 ++++++++ test/imap_actions_messages_test.go | 12 +++++ test/mocks/imap.go | 4 ++ 8 files changed, 115 insertions(+), 5 deletions(-) diff --git a/Changelog.md b/Changelog.md index 3288722f..e089bb0b 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,11 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/) +## Unreleased + +### Changed +* GODT-409 Set flags have to replace all flags. + ## [v1.3.x] Emma (beta 2020-07-XXX) ### Added diff --git a/internal/imap/mailbox_messages.go b/internal/imap/mailbox_messages.go index e832bf36..132f2cf2 100644 --- a/internal/imap/mailbox_messages.go +++ b/internal/imap/mailbox_messages.go @@ -51,18 +51,73 @@ func (im *imapMailbox) UpdateMessagesFlags(uid bool, seqSet *imap.SeqSet, operat return err } + if operation == imap.SetFlags { + return im.setFlags(messageIDs, flags) + } + return im.addOrRemoveFlags(operation, messageIDs, flags) +} + +func (im *imapMailbox) setFlags(messageIDs, flags []string) error { + seen := false + flagged := false + deleted := false + spam := false + + for _, f := range flags { + switch f { + case imap.SeenFlag: + seen = true + case imap.FlaggedFlag: + flagged = true + case imap.DeletedFlag: + deleted = true + case message.AppleMailJunkFlag, message.ThunderbirdJunkFlag: + spam = true + } + } + + if seen { + _ = im.storeMailbox.MarkMessagesRead(messageIDs) + } else { + _ = im.storeMailbox.MarkMessagesUnread(messageIDs) + } + + if flagged { + _ = im.storeMailbox.MarkMessagesStarred(messageIDs) + } else { + _ = im.storeMailbox.MarkMessagesUnstarred(messageIDs) + } + + if deleted { + _ = im.storeMailbox.DeleteMessages(messageIDs) + } + + spamMailbox, err := im.storeAddress.GetMailbox("Spam") + if err != nil { + return err + } + if spam { + _ = spamMailbox.LabelMessages(messageIDs) + } else { + _ = spamMailbox.UnlabelMessages(messageIDs) + } + + return nil +} + +func (im *imapMailbox) addOrRemoveFlags(operation imap.FlagsOp, messageIDs, flags []string) error { for _, f := range flags { switch f { case imap.SeenFlag: switch operation { - case imap.SetFlags, imap.AddFlags: + case imap.AddFlags: _ = im.storeMailbox.MarkMessagesRead(messageIDs) case imap.RemoveFlags: _ = im.storeMailbox.MarkMessagesUnread(messageIDs) } case imap.FlaggedFlag: switch operation { - case imap.SetFlags, imap.AddFlags: + case imap.AddFlags: _ = im.storeMailbox.MarkMessagesStarred(messageIDs) case imap.RemoveFlags: _ = im.storeMailbox.MarkMessagesUnstarred(messageIDs) @@ -75,7 +130,7 @@ func (im *imapMailbox) UpdateMessagesFlags(uid bool, seqSet *imap.SeqSet, operat case imap.AnsweredFlag, imap.DraftFlag, imap.RecentFlag: // Not supported. case message.AppleMailJunkFlag, message.ThunderbirdJunkFlag: - storeMailbox, err := im.storeAddress.GetMailbox(pmapi.SpamLabel) + storeMailbox, err := im.storeAddress.GetMailbox("Spam") if err != nil { return err } @@ -84,7 +139,7 @@ func (im *imapMailbox) UpdateMessagesFlags(uid bool, seqSet *imap.SeqSet, operat switch operation { // No label removal is necessary because Spam and Inbox are both exclusive labels so the backend // will automatically take care of label removal. - case imap.SetFlags, imap.AddFlags: + case imap.AddFlags: _ = storeMailbox.LabelMessages(messageIDs) case imap.RemoveFlags: _ = storeMailbox.UnlabelMessages(messageIDs) diff --git a/test/features/imap/message/delete.feature b/test/features/imap/message/delete.feature index 308694f5..0a589612 100644 --- a/test/features/imap/message/delete.feature +++ b/test/features/imap/message/delete.feature @@ -33,3 +33,13 @@ Feature: IMAP delete messages | Folders/mbox | | Labels/label | | Trash | + + Scenario: Delete message by setting flags + Given there are 1 messages in mailbox "INBOX" for "user" + And there is IMAP client logged in as "user" + And there is IMAP client selected in "INBOX" + When IMAP client marks message "1" with "\Deleted" + Then IMAP response is "OK" + And mailbox "INBOX" for "user" has 0 messages + # Unread because we set flags without \Seen. + And message "1" in "Trash" for "user" is marked as unread diff --git a/test/features/imap/message/move.feature b/test/features/imap/message/move.feature index 01a05ba2..b58c1654 100644 --- a/test/features/imap/message/move.feature +++ b/test/features/imap/message/move.feature @@ -2,6 +2,8 @@ Feature: IMAP move messages Background: Given there is connected user "user" And there is "user" with mailbox "Folders/mbox" + # Messages are inserted in opposite way to keep increasing ID. + # Sequence numbers are then opposite than listed above. And there are messages in mailbox "INBOX" for "user" | from | to | subject | body | | john.doe@mail.com | user@pm.me | foo | hello | diff --git a/test/features/imap/message/search.feature b/test/features/imap/message/search.feature index 95dd9298..f926f38c 100644 --- a/test/features/imap/message/search.feature +++ b/test/features/imap/message/search.feature @@ -1,7 +1,7 @@ Feature: IMAP search messages Background: Given there is connected user "user" - # Messages are inserted in opposite way to keep increasing UID. + # Messages are inserted in opposite way to keep increasing ID. # Sequence numbers are then opposite than listed above. Given there are messages in mailbox "INBOX" for "user" | from | to | cc | subject | read | starred | body | diff --git a/test/features/imap/message/update.feature b/test/features/imap/message/update.feature index 0864829a..e6e2e068 100644 --- a/test/features/imap/message/update.feature +++ b/test/features/imap/message/update.feature @@ -1,6 +1,8 @@ Feature: IMAP update messages 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 "INBOX" for "user" | from | to | subject | body | read | starred | | john.doe@mail.com | user@pm.me | foo | hello | false | false | @@ -33,3 +35,23 @@ Feature: IMAP update messages 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 unstarred + + Scenario: Mark message as read and starred + When IMAP client marks message "2" with "\Seen \Flagged" + Then IMAP response is "OK" + And message "1" in "INBOX" for "user" is marked as read + And message "1" in "INBOX" for "user" is marked as starred + + Scenario: Mark message as read only + When IMAP client marks message "1" with "\Seen" + Then IMAP response is "OK" + And message "2" in "INBOX" for "user" is marked as read + # Unstarred because we set flags without \Starred. + And message "2" in "INBOX" for "user" is marked as unstarred + + Scenario: Mark message as spam only + When IMAP client marks message "1" with "Junk" + Then IMAP response is "OK" + # Unread and unstarred because we set flags without \Seen and \Starred. + And message "1" in "Spam" for "user" is marked as unread + And message "1" in "Spam" for "user" is marked as unstarred diff --git a/test/imap_actions_messages_test.go b/test/imap_actions_messages_test.go index 195323b6..0f3159de 100644 --- a/test/imap_actions_messages_test.go +++ b/test/imap_actions_messages_test.go @@ -38,6 +38,8 @@ func IMAPActionsMessagesFeatureContext(s *godog.Suite) { s.Step(`^IMAP client creates message "([^"]*)" from "([^"]*)" to "([^"]*)" with body "([^"]*)" in "([^"]*)"$`, imapClientCreatesMessageFromToWithBody) s.Step(`^IMAP client creates message "([^"]*)" from "([^"]*)" to address "([^"]*)" of "([^"]*)" with body "([^"]*)" in "([^"]*)"$`, imapClientCreatesMessageFromToAddressOfUserWithBody) s.Step(`^IMAP client creates message "([^"]*)" from address "([^"]*)" of "([^"]*)" to "([^"]*)" with body "([^"]*)" in "([^"]*)"$`, imapClientCreatesMessageFromAddressOfUserToWithBody) + s.Step(`^IMAP client marks message "([^"]*)" with "([^"]*)"$`, imapClientMarksMessageWithFlags) + s.Step(`^IMAP client "([^"]*)" marks message "([^"]*)" with "([^"]*)"$`, imapClientNamedMarksMessageWithFlags) s.Step(`^IMAP client marks message "([^"]*)" as read$`, imapClientMarksMessageAsRead) s.Step(`^IMAP client "([^"]*)" marks message "([^"]*)" as read$`, imapClientNamedMarksMessageAsRead) s.Step(`^IMAP client marks message "([^"]*)" as unread$`, imapClientMarksMessageAsUnread) @@ -138,6 +140,16 @@ func imapClientCreatesMessageFromAddressOfUserToWithBody(subject, bddAddressID, return imapClientCreatesMessageFromToWithBody(subject, account.Address(), to, body, mailboxName) } +func imapClientMarksMessageWithFlags(messageRange, flags string) error { + return imapClientNamedMarksMessageWithFlags("imap", messageRange, flags) +} + +func imapClientNamedMarksMessageWithFlags(imapClient, messageRange, flags string) error { + res := ctx.GetIMAPClient(imapClient).SetFlags(messageRange, flags) + ctx.SetIMAPLastResponse(imapClient, res) + return nil +} + func imapClientMarksMessageAsRead(messageRange string) error { return imapClientNamedMarksMessageAsRead("imap", messageRange) } diff --git a/test/mocks/imap.go b/test/mocks/imap.go index 0341b053..fb43f295 100644 --- a/test/mocks/imap.go +++ b/test/mocks/imap.go @@ -207,6 +207,10 @@ func (c *IMAPClient) MarkAsUnstarred(ids string) *IMAPResponse { return c.RemoveFlags(ids, "\\Flagged") } +func (c *IMAPClient) SetFlags(ids, flags string) *IMAPResponse { + return c.changeFlags(ids, flags, "") +} + func (c *IMAPClient) AddFlags(ids, flags string) *IMAPResponse { return c.changeFlags(ids, flags, "+") }