From 5a9d09ff6a2f7b38ce083350cba7b9f4019b8232 Mon Sep 17 00:00:00 2001 From: Ronald Portier Date: Mon, 26 Mar 2018 23:18:33 +0200 Subject: [PATCH] [FIX] fetchmail_attach_from_folder. Refactor for more testing. --- .../match_algorithm/base.py | 4 +- .../match_algorithm/email_domain.py | 5 +- .../match_algorithm/email_exact.py | 3 +- .../match_algorithm/odoo_standard.py | 4 +- .../models/fetchmail_server.py | 20 ++- .../models/fetchmail_server_folder.py | 134 +++++++++--------- .../tests/test_match_algorithms.py | 20 ++- .../wizard/attach_mail_manually.py | 91 +++++------- 8 files changed, 141 insertions(+), 140 deletions(-) diff --git a/fetchmail_attach_from_folder/match_algorithm/base.py b/fetchmail_attach_from_folder/match_algorithm/base.py index d40fc10be..0b823d0e4 100644 --- a/fetchmail_attach_from_folder/match_algorithm/base.py +++ b/fetchmail_attach_from_folder/match_algorithm/base.py @@ -12,7 +12,7 @@ class Base(object): # Fields on fetchmail_server folder readonly for this algorithm readonly_fields = [] - def search_matches(self, folder, mail_message, mail_message_org): + def search_matches(self, folder, mail_message): """Returns recordset found for model with mail_message.""" return [] @@ -21,5 +21,3 @@ class Base(object): mail_message, mail_message_org, msgid): """Do whatever it takes to handle a match""" folder.attach_mail(match_object, mail_message) - if folder.delete_matching: - connection.store(msgid, '+FLAGS', '\\DELETED') diff --git a/fetchmail_attach_from_folder/match_algorithm/email_domain.py b/fetchmail_attach_from_folder/match_algorithm/email_domain.py index 6740ae724..4e8d15274 100644 --- a/fetchmail_attach_from_folder/match_algorithm/email_domain.py +++ b/fetchmail_attach_from_folder/match_algorithm/email_domain.py @@ -11,10 +11,9 @@ class EmailDomain(EmailExact): """ name = 'Domain of email address' - def search_matches(self, folder, mail_message, mail_message_org): + def search_matches(self, folder, mail_message): """Returns recordset of matching objects.""" - matches = super(EmailDomain, self).search_matches( - folder, mail_message, mail_message_org) + matches = super(EmailDomain, self).search_matches(folder, mail_message) if not matches: object_model = folder.env[folder.model_id.model] domains = [] diff --git a/fetchmail_attach_from_folder/match_algorithm/email_exact.py b/fetchmail_attach_from_folder/match_algorithm/email_exact.py index 20fa85db4..8ee08b271 100644 --- a/fetchmail_attach_from_folder/match_algorithm/email_exact.py +++ b/fetchmail_attach_from_folder/match_algorithm/email_exact.py @@ -9,7 +9,6 @@ from .base import Base class EmailExact(Base): """Search for exactly the mailadress as noted in the email""" - name = 'Exact mailadress' required_fields = ['model_field', 'mail_field'] @@ -32,7 +31,7 @@ class EmailExact(Base): safe_eval(folder.domain or '[]')) return search_domain - def search_matches(self, folder, mail_message, mail_message_org): + def search_matches(self, folder, mail_message): """Returns recordset of matching objects.""" object_model = folder.env[folder.model_id.model] search_domain = self._get_mailaddress_search_domain( diff --git a/fetchmail_attach_from_folder/match_algorithm/odoo_standard.py b/fetchmail_attach_from_folder/match_algorithm/odoo_standard.py index a8a6983eb..0ae48592f 100644 --- a/fetchmail_attach_from_folder/match_algorithm/odoo_standard.py +++ b/fetchmail_attach_from_folder/match_algorithm/odoo_standard.py @@ -17,7 +17,7 @@ class OdooStandard(Base): 'flag_nonmatching', ] - def search_matches(self, folder, mail_message, mail_message_org): + def search_matches(self, folder, mail_message): """Always match. Duplicates will be fished out by message_id""" return [True] @@ -29,5 +29,3 @@ class OdooStandard(Base): folder.model_id.model, mail_message_org, save_original=folder.server_id.original, strip_attachments=(not folder.server_id.attach)) - if folder.delete_matching: - connection.store(msgid, '+FLAGS', '\\DELETED') diff --git a/fetchmail_attach_from_folder/models/fetchmail_server.py b/fetchmail_attach_from_folder/models/fetchmail_server.py index cb07cdc61..7f29187e5 100644 --- a/fetchmail_attach_from_folder/models/fetchmail_server.py +++ b/fetchmail_attach_from_folder/models/fetchmail_server.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # Copyright - 2013-2018 Therp BV . # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). +import logging import simplejson from lxml import etree @@ -9,6 +10,9 @@ from odoo.tools.safe_eval import safe_eval from odoo.tools.misc import UnquoteEvalContext +_logger = logging.getLogger(__name__) + + class FetchmailServer(models.Model): _inherit = 'fetchmail.server' @@ -38,8 +42,20 @@ class FetchmailServer(models.Model): for this in self: if not this.folders_only: super(FetchmailServer, this).fetch_mail() - for folder in this.folder_ids.filtered('active'): - folder.retrieve_imap_folder() + try: + # New connection for retrieving folders + connection = this.connect() + for folder in this.folder_ids.filtered('active'): + folder.retrieve_imap_folder(connection) + connection.close() + except Exception: + _logger.error(_( + "General failure when trying to connect to" + " %s server %s."), + this.type, this.name, exc_info=True) + finally: + if connection: + connection.logout() return True @api.multi diff --git a/fetchmail_attach_from_folder/models/fetchmail_server_folder.py b/fetchmail_attach_from_folder/models/fetchmail_server_folder.py index 1c1ee7378..a238077ec 100644 --- a/fetchmail_attach_from_folder/models/fetchmail_server_folder.py +++ b/fetchmail_attach_from_folder/models/fetchmail_server_folder.py @@ -5,6 +5,7 @@ import base64 import logging from odoo import _, api, models, fields +from odoo.exceptions import UserError from .. import match_algorithm @@ -90,93 +91,98 @@ class FetchmailServerFolder(models.Model): @api.multi def button_attach_mail_manually(self): + self.ensure_one() return { 'type': 'ir.actions.act_window', 'res_model': 'fetchmail.attach.mail.manually', 'target': 'new', - 'context': dict(self.env.context, default_folder_id=self.id), + 'context': dict(self.env.context, folder=self), 'view_type': 'form', 'view_mode': 'form'} @api.multi - def get_msgids(self, connection): + def get_msgids(self, connection, criteria): """Return imap ids of messages to process""" - return connection.search(None, 'UNDELETED') - - @api.multi - def retrieve_imap_folder(self): - """Retrieve all mails for one IMAP folder. - - For each folder on each server we create a separate connection. - """ - for this in self: - server = this.server_id - try: - _logger.info( - 'start checking for emails in folder %s on server %s', - this.path, server.name) - imap_server = server.connect() - if imap_server.select(this.path)[0] != 'OK': - _logger.error( - 'Could not open mailbox %s on %s', - this.path, server.name) - continue - result, msgids = this.get_msgids(imap_server) - if result != 'OK': - _logger.error( - 'Could not search mailbox %s on %s', - this.path, server.name) - continue - match_algorithm = this.get_algorithm() - for msgid in msgids[0].split(): - this.apply_matching( - imap_server, msgid, match_algorithm) - _logger.info( - 'finished checking for emails in %s server %s', - this.path, server.name) - except Exception: - _logger.info(_( - "General failure when trying to fetch mail from" - " %s server %s."), - server.type, server.name, exc_info=True) - finally: - if imap_server: - imap_server.close() - imap_server.logout() + self.ensure_one() + server = self.server_id + _logger.info( + 'start checking for emails in folder %s on server %s', + self.path, server.name) + if connection.select(self.path)[0] != 'OK': + raise UserError(_( + "Could not open mailbox %s on %s") % + (self.path, server.name)) + result, msgids = connection.search(None, criteria) + if result != 'OK': + raise UserError(_( + "Could not search mailbox %s on %s") % + (self.path, server.name)) + _logger.info( + 'finished checking for emails in %s on server %s', + self.path, server.name) + return msgids @api.multi - def apply_matching(self, connection, msgid, match_algorithm): - """Return ids of objects matched""" + def fetch_msg(self, connection, msgid): + """Select a single message from a folder.""" self.ensure_one() + server = self.server_id result, msgdata = connection.fetch(msgid, '(RFC822)') if result != 'OK': - _logger.error( - 'Could not fetch %s in %s on %s', - msgid, self.path, self.server_id.server) - return + raise UserError(_( + "Could not fetch %s in %s on %s") % + (msgid, self.path, server.server)) + message_org = msgdata[0][1] # rfc822 message source mail_message = self.env['mail.thread'].message_parse( - msgdata[0][1], save_original=self.server_id.original) - if self.env['mail.message'].search( - [('message_id', '=', mail_message['message_id'])]): - # Ignore mails that have been handled already - return - matches = match_algorithm.search_matches( - self, mail_message, msgdata[0][1]) - if matches and (len(matches) == 1 or self.match_first): + message_org, save_original=server.original) + return (mail_message, message_org) + + @api.multi + def retrieve_imap_folder(self, connection): + """Retrieve all mails for one IMAP folder.""" + self.ensure_one() + msgids = self.get_msgids(connection, 'UNDELETED') + match_algorithm = self.get_algorithm() + for msgid in msgids[0].split(): try: self.env.cr.execute('savepoint apply_matching') - match_algorithm.handle_match( - connection, - matches[0], self, mail_message, - msgdata[0][1], msgid) + self.apply_matching(connection, msgid, match_algorithm) 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 %s from %s", msgid, self.server_id.name) - elif self.flag_nonmatching: - connection.store(msgid, '+FLAGS', '\\FLAGGED') + + @api.multi + def update_msg(self, connection, msgid, 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') + elif flagged and self.flag_nonmatching: + connection.store(msgid, '-FLAGS', '\\FLAGGED') + else: + if self.flag_nonmatching: + connection.store(msgid, '+FLAGS', '\\FLAGGED') + + @api.multi + def apply_matching(self, connection, msgid, match_algorithm): + """Return ids of objects matched""" + self.ensure_one() + mail_message, message_org = self.fetch_msg(connection, msgid) + if self.env['mail.message'].search( + [('message_id', '=', mail_message['message_id'])]): + # Ignore mails that have been handled already + return + matches = match_algorithm.search_matches(self, mail_message) + matched = matches and (len(matches) == 1 or self.match_first) + if matched: + match_algorithm.handle_match( + connection, + matches[0], self, mail_message, + message_org, msgid) + self.update_msg(connection, msgid, matched=matched) @api.multi def attach_mail(self, match_object, mail_message): diff --git a/fetchmail_attach_from_folder/tests/test_match_algorithms.py b/fetchmail_attach_from_folder/tests/test_match_algorithms.py index fa2e820f3..28cbfbf8d 100644 --- a/fetchmail_attach_from_folder/tests/test_match_algorithms.py +++ b/fetchmail_attach_from_folder/tests/test_match_algorithms.py @@ -30,6 +30,10 @@ MSG_BODY = [ class MockConnection(): + def select(self, path): + """Mock selecting a folder.""" + return ('OK', ) + def store(self, msgid, msg_item, value): """Mock store command.""" return 'OK' @@ -38,6 +42,10 @@ class MockConnection(): """Return RFC822 formatted message.""" return ('OK', MSG_BODY) + def search(self, charset, criteria): + """Return some msgid's.""" + return ('OK', ['123 456']) + class TestMatchAlgorithms(TransactionCase): @@ -56,8 +64,7 @@ class TestMatchAlgorithms(TransactionCase): self, match_algorithm, expected_xmlid, folder, mail_message, mail_message_org=None): matcher = match_algorithm() - matches = matcher.search_matches( - folder, mail_message, mail_message_org) + matches = matcher.search_matches(folder, mail_message) self.assertEqual(len(matches), 1) self.assertEqual( matches[0], self.env.ref(expected_xmlid)) @@ -108,8 +115,7 @@ class TestMatchAlgorithms(TransactionCase): folder = self._get_base_folder() folder.match_algorithm = 'OdooStandard' matcher = odoo_standard.OdooStandard() - matches = matcher.search_matches( - folder, None, mail_message_org) + matches = matcher.search_matches(folder, None) self.assertEqual(len(matches), 1) matcher.handle_match( None, matches[0], folder, None, mail_message_org, None) @@ -126,6 +132,12 @@ class TestMatchAlgorithms(TransactionCase): matcher = email_exact.EmailExact() folder.apply_matching(connection, msgid, matcher) + def test_retrieve_imap_folder_domain(self): + folder = self._get_base_folder() + folder.match_algorithm = 'EmailDomain' + connection = MockConnection() + folder.retrieve_imap_folder(connection) + def test_field_view_get(self): """For the moment just check execution withouth errors.""" server_model = self.env['fetchmail.server'] diff --git a/fetchmail_attach_from_folder/wizard/attach_mail_manually.py b/fetchmail_attach_from_folder/wizard/attach_mail_manually.py index 23b16ebcd..e3100896d 100644 --- a/fetchmail_attach_from_folder/wizard/attach_mail_manually.py +++ b/fetchmail_attach_from_folder/wizard/attach_mail_manually.py @@ -19,65 +19,40 @@ class AttachMailManually(models.TransientModel): @api.model def default_get(self, fields_list): - folder_model = self.env['fetchmail.server.folder'] - thread_model = self.env['mail.thread'] defaults = super(AttachMailManually, self).default_get(fields_list) - default_folder_id = self.env.context.get('default_folder_id') - for folder in folder_model.browse([default_folder_id]): - defaults['mail_ids'] = [] - connection = folder.server_id.connect() - connection.select(folder.path) - result, msgids = connection.search( - None, - 'FLAGGED' if folder.flag_nonmatching else 'UNDELETED') - if result != 'OK': - _logger.error( - 'Could not search mailbox %s on %s', - folder.path, folder.server_id.name) - continue - for msgid in msgids[0].split(): - result, msgdata = connection.fetch(msgid, '(RFC822)') - if result != 'OK': - _logger.error( - 'Could not fetch %s in %s on %s', - msgid, folder.path, folder.server_id.name) - continue - mail_message = thread_model.message_parse( - msgdata[0][1], - save_original=folder.server_id.original) - defaults['mail_ids'].append((0, 0, { - 'msgid': msgid, - 'subject': mail_message.get('subject', ''), - 'date': mail_message.get('date', ''), - 'object_id': '%s,-1' % folder.model_id.model})) - connection.close() + defaults['mail_ids'] = [] + folder = self.env.context.get('folder') + connection = folder.server_id.connect() + connection.select(folder.path) + criteria = 'FLAGGED' if folder.flag_nonmatching else 'UNDELETED' + msgids = folder.get_msgids(connection, criteria) + for msgid in msgids[0].split(): + mail_message, message_org = folder.fetch_msg(connection, msgid) + defaults['mail_ids'].append((0, 0, { + 'msgid': msgid, + 'subject': mail_message.get('subject', ''), + 'date': mail_message.get('date', ''), + 'object_id': '%s,-1' % folder.model_id.model})) + connection.close() return defaults @api.multi def attach_mails(self): - thread_model = self.env['mail.thread'] - for this in self: - folder = this.folder_id - server = folder.server_id - connection = server.connect() - connection.select(folder.path) - for mail in this.mail_ids: - if not mail.object_id: - continue - result, msgdata = connection.fetch(mail.msgid, '(RFC822)') - if result != 'OK': - _logger.error( - 'Could not fetch %s in %s on %s', - mail.msgid, folder.path, server) - continue - mail_message = thread_model.message_parse( - msgdata[0][1], save_original=server.original) - folder.attach_mail(mail.object_id, mail_message) - if folder.delete_matching: - connection.store(mail.msgid, '+FLAGS', '\\DELETED') - elif folder.flag_nonmatching: - connection.store(mail.msgid, '-FLAGS', '\\FLAGGED') - connection.close() + self.ensure_one() + folder = self.folder_id + server = folder.server_id + connection = server.connect() + connection.select(folder.path) + for mail in self.mail_ids: + if not mail.object_id: + continue + msgid = mail.msgid + mail_message, message_org = folder.fetch_msg(connection, msgid) + folder.attach_mail(mail.object_id, mail_message) + folder.update_msg( + connection, msgid, matched=True, + flagged=folder.flag_nonmatching) + connection.close() return {'type': 'ir.actions.act_window_close'} @api.model @@ -88,11 +63,9 @@ class AttachMailManually(models.TransientModel): view_id=view_id, view_type=view_type, toolbar=toolbar, submenu=submenu) tree = result['fields']['mail_ids']['views']['tree'] - folder_model = self.env['fetchmail.server.folder'] - default_folder_id = self.env.context.get('default_folder_id') - for folder in folder_model.browse([default_folder_id]): - tree['fields']['object_id']['selection'] = [ - (folder.model_id.model, folder.model_id.name)] + folder = self.env.context.get('folder') + tree['fields']['object_id']['selection'] = [ + (folder.model_id.model, folder.model_id.name)] return result