Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[16.0][FIX] fetch...folder: use message uid's not sequence #3017

Open
wants to merge 2 commits into
base: 16.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 37 additions & 27 deletions fetchmail_attach_from_folder/models/fetchmail_server_folder.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,22 +180,26 @@
def retrieve_imap_folder(self, connection):
"""Retrieve all mails for one IMAP folder."""
self.ensure_one()
msgids = self.get_msgids(connection, self.get_criteria())
for msgid in msgids[0].split():
message_uids = self.get_message_uids(connection, self.get_criteria())
for message_uid in message_uids[0].split():
# We will accept exceptions for single messages
try:
self.env.cr.execute("savepoint apply_matching")
self.apply_matching(connection, msgid)
self.apply_matching(connection, message_uid)
self.env.cr.execute("release savepoint apply_matching")
except Exception:
self.env.cr.execute("rollback to savepoint apply_matching")
_logger.exception(
"Failed to fetch mail %(msgid)s from server %(server)s",
{"msgid": msgid, "server": self.server_id.name},
"Failed to fetch mail %(message_uid)s from server %(server)s",
{"message_uid": message_uid, "server": self.server_id.name},
)

def get_msgids(self, connection, criteria):
def get_message_uids(self, connection, criteria):
"""Return imap ids of messages to process"""
# Note that these message_uids are linked to the folder on
# the imap server. They have nothing at all todo with the
# message_id field that is part of the message itself, and the
# two should not be confused.
self.ensure_one()
server = self.server_id
_logger.info(
Expand All @@ -207,7 +211,8 @@
_("Could not open folder %(folder)s on server %(server)s")
% {"folder": self.path, "server": server.name}
)
result, msgids = connection.search(None, criteria)
charset = None # Generally we do not need to set a charset.
result, message_uids = connection.uid("search", charset, criteria)
if result != "OK":
raise UserError(
_("Could not search folder %(folder)s on server %(server)s")
Expand All @@ -217,14 +222,14 @@
"finished checking for emails in folder %(folder)s on server %(server)s",
{"folder": self.path, "server": server.name},
)
return msgids
return message_uids

def apply_matching(self, connection, msgid):
def apply_matching(self, connection, message_uid):
"""Return id of object matched (which will be the thread_id)."""
self.ensure_one()
thread_id = None
thread_model = self.env["mail.thread"]
message_org = self.fetch_msg(connection, msgid)
message_org = self.fetch_msg(connection, message_uid)
if self.match_algorithm == "odoo_standard":
thread_id = thread_model.message_process(
self.model_id.model,
Expand All @@ -242,9 +247,9 @@
matched = True if thread_id else False
if matched:
self.run_server_action(thread_id)
self.update_msg(connection, msgid, matched=matched)
if self.archive_path:
self._archive_msg(connection, msgid)
self.update_msg(connection, message_uid, matched=matched)
if self.archive_path and not self.delete_matching:
self._archive_msg(connection, message_uid)
return thread_id # Can be None if no match found.

def run_server_action(self, matched_object_ids):
Expand All @@ -263,34 +268,39 @@
}
).run()

def fetch_msg(self, connection, msgid):
def fetch_msg(self, connection, message_uid):
"""Select a single message from a folder."""
self.ensure_one()
result, msgdata = connection.fetch(msgid, "(RFC822)")
result, msgdata = connection.uid("fetch", message_uid, "(RFC822)")
if result != "OK":
raise UserError(
_("Could not fetch %(msgid)s in folder %(folder)s on server %(server)s")
% {"msgid": msgid, "folder": self.path, "server": self.server_id.name}
_(
"Could not fetch %(message_uid)s in folder %(folder)s on server %(server)s"
)
% {
"message_uid": message_uid,
"folder": self.path,
"server": self.server_id.name,
}
)
message_org = msgdata[0][1] # rfc822 message source
return message_org

def update_msg(self, connection, msgid, matched=True, flagged=False):
def update_msg(self, connection, message_uid, matched=True, flagged=False):
"""Update msg in imap folder depending on match and settings."""
if matched:
if self.delete_matching:
connection.store(msgid, "+FLAGS", "\\DELETED")
connection.uid("store", message_uid, "+FLAGS", "\\DELETED")

Check warning on line 293 in fetchmail_attach_from_folder/models/fetchmail_server_folder.py

View check run for this annotation

Codecov / codecov/patch

fetchmail_attach_from_folder/models/fetchmail_server_folder.py#L293

Added line #L293 was not covered by tests
elif flagged and self.flag_nonmatching:
connection.store(msgid, "-FLAGS", "\\FLAGGED")
connection.uid("store", message_uid, "-FLAGS", "\\FLAGGED")

Check warning on line 295 in fetchmail_attach_from_folder/models/fetchmail_server_folder.py

View check run for this annotation

Codecov / codecov/patch

fetchmail_attach_from_folder/models/fetchmail_server_folder.py#L295

Added line #L295 was not covered by tests
else:
if self.flag_nonmatching:
connection.store(msgid, "+FLAGS", "\\FLAGGED")
connection.uid("store", message_uid, "+FLAGS", "\\FLAGGED")

def _archive_msg(self, connection, msgid):
def _archive_msg(self, connection, message_uid):
"""Archive message. Folder should already have been created."""
self.ensure_one()
connection.copy(msgid, self.archive_path)
connection.store(msgid, "+FLAGS", "\\Deleted")
connection.uid("copy", message_uid, self.archive_path)
connection.uid("store", message_uid, "+FLAGS", "\\Deleted")
connection.expunge()

@api.model
Expand Down Expand Up @@ -331,10 +341,10 @@
matches = matcher.search_matches(self, message_dict)
if not matches:
_logger.info(
"No match found for message %(subject)s with msgid %(msgid)s",
"No match found for message %(subject)s with message_uid %(message_uid)s",
{
"subject": message_dict.get("subject", "no subject"),
"msgid": message_dict.get("message_id", "no msgid"),
"message_uid": message_dict.get("message_id", "no message_uid"),
},
)
return None
Expand Down
30 changes: 25 additions & 5 deletions fetchmail_attach_from_folder/tests/test_match_algorithms.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Copyright - 2015-2018 Therp BV <https://acme.com>.
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
# pylint: disable=method-required-super
from odoo.tests.common import TransactionCase

from ..match_algorithm import email_domain
Expand Down Expand Up @@ -36,18 +37,31 @@
"""Mock selecting a folder."""
return ("OK",)

def store(self, msgid, msg_item, value):
def create(self, path):
"""Mock creating a folder."""
return ("OK",)

Check warning on line 42 in fetchmail_attach_from_folder/tests/test_match_algorithms.py

View check run for this annotation

Codecov / codecov/patch

fetchmail_attach_from_folder/tests/test_match_algorithms.py#L42

Added line #L42 was not covered by tests

def store(self, message_uid, msg_item, value):
"""Mock store command."""
return "OK"

def fetch(self, msgid, parts):
def copy(self, message_uid, folder_path):
"""Mock copy command."""
return "OK"

def fetch(self, message_uid, parts):
"""Return RFC822 formatted message."""
return ("OK", MSG_BODY)

def search(self, charset, criteria):
"""Return some msgid's."""
"""Return some message uid's."""
return ("OK", ["123 456"])

def uid(self, command, *args):
"""Return from the appropiate mocked method."""
method = getattr(self, command)
return method(*args)


class TestMatchAlgorithms(TransactionCase):
@classmethod
Expand Down Expand Up @@ -155,15 +169,21 @@
folder = self.folder
folder.match_algorithm = "email_exact"
connection = MockConnection()
msgid = "<[email protected]>"
folder.apply_matching(connection, msgid)
message_uid = "<[email protected]>"
folder.apply_matching(connection, message_uid)

def test_retrieve_imap_folder_domain(self):
folder = self.folder
folder.match_algorithm = "email_domain"
connection = MockConnection()
folder.retrieve_imap_folder(connection)

def test_archive_messages(self):
folder = self.folder
folder.archive_path = "archived_messages"
connection = MockConnection()
folder.retrieve_imap_folder(connection)

def test_non_action(self):
connection = MockConnection()
self.folder.action_id = False
Expand Down
Loading