[FIX] fetchmail_attach_from_folder. Refactor for more testing.

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')

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 = []

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(

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')

fetchmail_attach_from_folder/models/fetchmail_server.py

@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 # Copyright - 2013-2018 Therp BV <https://therp.nl>.
 # 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()
+            try:
+                # New connection for retrieving folders
+                connection = this.connect()
                 for folder in this.folder_ids.filtered('active'):
-                folder.retrieve_imap_folder()
+                    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

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,94 +91,99 @@ 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:
+        self.ensure_one()
+        server = self.server_id
         _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)
+            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':
-                    _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)
+            raise UserError(_(
+                "Could not search mailbox %s on %s") %
+                (self.path, server.name))
         _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()
+            '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:
+
+    @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):
         """Attach mail to match_object."""

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']

fetchmail_attach_from_folder/wizard/attach_mail_manually.py

@@ -19,32 +19,15 @@ 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'] = []
+        folder = self.env.context.get('folder')
         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
+        criteria = 'FLAGGED' if folder.flag_nonmatching else 'UNDELETED'
+        msgids = folder.get_msgids(connection, criteria)
         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)
+            mail_message, message_org = folder.fetch_msg(connection, msgid)
             defaults['mail_ids'].append((0, 0, {
                 'msgid': msgid,
                 'subject': mail_message.get('subject', ''),
@@ -55,28 +38,20 @@ class AttachMailManually(models.TransientModel):
 
     @api.multi
     def attach_mails(self):
-        thread_model = self.env['mail.thread']
-        for this in self:
-            folder = this.folder_id
+        self.ensure_one()
+        folder = self.folder_id
         server = folder.server_id
         connection = server.connect()
         connection.select(folder.path)
-            for mail in this.mail_ids:
+        for mail in self.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)
+            msgid = mail.msgid
+            mail_message, message_org = folder.fetch_msg(connection, msgid)
             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')
+            folder.update_msg(
+                connection, msgid, matched=True,
+                flagged=folder.flag_nonmatching)
         connection.close()
         return {'type': 'ir.actions.act_window_close'}
 
@@ -88,9 +63,7 @@ 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]):
+        folder = self.env.context.get('folder')
         tree['fields']['object_id']['selection'] = [
             (folder.model_id.model, folder.model_id.name)]
         return result