[8.0][IMP] Performance issues in mail_tracking addons

mail_tracking/__openerp__.py

@@ -5,7 +5,7 @@
 {
     "name": "Email tracking",
     "summary": "Email tracking system for all mails sent",
-    "version": "8.0.2.0.2",
+    "version": "8.0.3.0.0",
     "category": "Social Network",
     "website": "http://www.tecnativa.com",
     "author": "Tecnativa, "

mail_tracking/controllers/main.py

@@ -11,17 +11,40 @@
 BLANK = 'R0lGODlhAQABAIAAANvf7wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw=='
 
 
-def _env_get(db):
+def _env_get(db, callback, tracking_id, event_type, **kw):
+    res = 'NOT FOUND'
     reg = False
-    try:
-        reg = registry(db)
-    except OperationalError:
-        _logger.warning("Selected BD '%s' not found", db)
-    except:  # pragma: no cover
-        _logger.warning("Selected BD '%s' connection error", db)
-    if reg:
-        return api.Environment(reg.cursor(), SUPERUSER_ID, {})
-    return False
+    current = http.request.db and db == http.request.db
+    env = current and http.request.env
+    if not env:
+        try:
+            reg = registry(db)
+        except OperationalError:
+            _logger.warning("Selected BD '%s' not found", db)
+        except:  # pragma: no cover
+            _logger.warning("Selected BD '%s' connection error", db)
+        if reg:
+            _logger.info("Creating a new environment for database '%s'", db)
+            env = api.Environment(reg.cursor(), SUPERUSER_ID, {})
+    else:
+        # make sudo when reusing environment
+        env = env(user=SUPERUSER_ID)
+    if env:
+        # This try-except-finally is equivalent to a with statement
+        # We assure that new created cursor si commit/rollback/close
+        # in any case
+        try:
+            res = callback(env, tracking_id, event_type, **kw)
+            if not current:
+                env.cr.commit()
+        except Exception as e:
+            if not current:
+                env.cr.rollback()
+            raise e
+        finally:
+            if not current:
+                env.cr.close()
+    return res
 
 
 class MailTrackingController(http.Controller):
@@ -35,49 +58,37 @@ def _request_metadata(self):
             'ua_family': request.user_agent.browser or False,
         }
 
+    def _tracking_open(self, env, tracking_id, event_type, **kw):
+        tracking_email = env['mail.tracking.email'].search([
+            ('id', '=', tracking_id),
+        ])
+        if tracking_email:
+            metadata = self._request_metadata()
+            tracking_email.event_create('open', metadata)
+        else:
+            _logger.warning(
+                "MailTracking email '%s' not found", tracking_id)
+
+    def _tracking_event(self, env, tracking_id, event_type, **kw):
+        metadata = self._request_metadata()
+        return env['mail.tracking.email'].event_process(
+            http.request, kw, metadata, event_type=event_type)
+
     @http.route('/mail/tracking/all/<string:db>',
                 type='http', auth='none')
     def mail_tracking_all(self, db, **kw):
-        env = _env_get(db)
-        if not env:
-            return 'NOT FOUND'
-        metadata = self._request_metadata()
-        response = env['mail.tracking.email'].event_process(
-            http.request, kw, metadata)
-        env.cr.commit()
-        env.cr.close()
-        return response
+        return _env_get(db, self._tracking_event, None, None, **kw)
 
     @http.route('/mail/tracking/event/<string:db>/<string:event_type>',
                 type='http', auth='none')
     def mail_tracking_event(self, db, event_type, **kw):
-        env = _env_get(db)
-        if not env:
-            return 'NOT FOUND'
-        metadata = self._request_metadata()
-        response = env['mail.tracking.email'].event_process(
-            http.request, kw, metadata, event_type=event_type)
-        env.cr.commit()
-        env.cr.close()
-        return response
+        return _env_get(db, self._tracking_event, None, event_type, **kw)
 
     @http.route('/mail/tracking/open/<string:db>'
                 '/<int:tracking_email_id>/blank.gif',
                 type='http', auth='none')
     def mail_tracking_open(self, db, tracking_email_id, **kw):
-        env = _env_get(db)
-        if env:
-            tracking_email = env['mail.tracking.email'].search([
-                ('id', '=', tracking_email_id),
-            ])
-            if tracking_email:
-                metadata = self._request_metadata()
-                tracking_email.event_create('open', metadata)
-            else:
-                _logger.warning(
-                    "MailTracking email '%s' not found", tracking_email_id)
-            env.cr.commit()
-            env.cr.close()
+        _env_get(db, self._tracking_open, tracking_email_id, None, **kw)
 
         # Always return GIF blank image
         response = werkzeug.wrappers.Response()

mail_tracking/models/mail_tracking_email.py

@@ -3,6 +3,7 @@
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 
 import logging
+import threading
 import urlparse
 import time
 import re
@@ -23,14 +24,19 @@ class MailTrackingEmail(models.Model):
     _rec_name = 'display_name'
     _description = 'MailTracking email'
 
+    # This table is going to growth fast and to infinite, so we index:
+    # - name: Search in tree view
+    # - time: default order fields
+    # - recipient_address: Used for email_store calculation (non-store)
+    # - state: Search and group_by in tree view
     name = fields.Char(string="Subject", readonly=True, index=True)
     display_name = fields.Char(
         string="Display name", readonly=True, store=True,
         compute="_compute_display_name")
     timestamp = fields.Float(
         string='UTC timestamp', readonly=True,
         digits=dp.get_precision('MailTracking Timestamp'))
-    time = fields.Datetime(string="Time", readonly=True)
+    time = fields.Datetime(string="Time", readonly=True, index=True)
     date = fields.Date(
         string="Date", readonly=True, compute="_compute_date", store=True)
     mail_message_id = fields.Many2one(
@@ -42,7 +48,7 @@ class MailTrackingEmail(models.Model):
     recipient = fields.Char(string='Recipient email', readonly=True)
     recipient_address = fields.Char(
         string='Recipient email address', readonly=True, store=True,
-        compute='_compute_recipient_address')
+        compute='_compute_recipient_address', index=True)
     sender = fields.Char(string='Sender email', readonly=True)
     state = fields.Selection([
         ('error', 'Error'),
@@ -87,70 +93,49 @@ class MailTrackingEmail(models.Model):
         string="Tracking events", comodel_name='mail.tracking.event',
         inverse_name='tracking_email_id', readonly=True)
 
-    @api.model
-    def tracking_ids_recalculate(self, model, email_field, tracking_field,
-                                 email, new_tracking=None):
-        objects = self.env[model].search([
-            (email_field, '=ilike', email),
-        ])
-        for obj in objects:
-            trackings = obj[tracking_field]
-            if new_tracking:
-                trackings |= new_tracking
-            trackings = trackings._email_score_tracking_filter()
-            if set(obj[tracking_field].ids) != set(trackings.ids):
-                if trackings:
-                    obj.write({
-                        tracking_field: [(6, False, trackings.ids)]
-                    })
-                else:
-                    obj.write({
-                        tracking_field: [(5, False, False)]
-                    })
-        return objects
+    def _email_score_tracking_filter(self, domain, order='time desc',
+                                     limit=10):
+        """Default tracking search. Ready to be inherited."""
+        return self.search(domain, limit=limit, order=order)
 
     @api.model
-    def _tracking_ids_to_write(self, email):
-        trackings = self.env['mail.tracking.email'].search([
-            ('recipient_address', '=ilike', email)
-        ])
-        trackings = trackings._email_score_tracking_filter()
-        if trackings:
-            return [(6, False, trackings.ids)]
-        else:
-            return [(5, False, False)]
-
-    @api.multi
-    def _email_score_tracking_filter(self):
-        """Default email score filter for tracking emails"""
-        # Consider only last 10 tracking emails
-        return self.sorted(key=lambda r: r.time, reverse=True)[:10]
+    def email_is_bounced(self, email):
+        return len(self._email_score_tracking_filter([
+            ('recipient_address', '=ilike', email),
+            ('state', 'in', ('error', 'rejected', 'spam', 'bounced')),
+        ])) > 0
 
     @api.model
     def email_score_from_email(self, email):
-        trackings = self.env['mail.tracking.email'].search([
+        return self._email_score_tracking_filter([
             ('recipient_address', '=ilike', email)
-        ])
-        return trackings.email_score()
+        ]).email_score()
 
     @api.multi
     def email_score(self):
-        """Default email score algorimth"""
+        """Default email score algorimth. Ready to be inherited
+
+        Must return a value beetwen 0.0 and 100.0
+        - Bad reputation: Value between 0 and 50.0
+        - Unknown reputation: Value 50.0
+        - Good reputation: Value between 50.0 and 100.0
+        """
         score = 50.0
-        trackings = self._email_score_tracking_filter()
-        for tracking in trackings:
+        for tracking in self:
             if tracking.state in ('error',):
                 score -= 50.0
             elif tracking.state in ('rejected', 'spam', 'bounced'):
                 score -= 25.0
             elif tracking.state in ('soft-bounced', 'unsub'):
                 score -= 10.0
             elif tracking.state in ('delivered',):
-                score += 5.0
+                score += 1.0
             elif tracking.state in ('opened',):
-                score += 10.0
+                score += 5.0
         if score > 100.0:
             score = 100.0
+        if score < 0.0:
+            score = 0.0
         return score
 
     @api.multi
@@ -167,7 +152,7 @@ def _compute_recipient_address(self):
     @api.depends('name', 'recipient')
     def _compute_display_name(self):
         for email in self:
-            parts = [email.name]
+            parts = [email.name or '']
             if email.recipient:
                 parts.append(email.recipient)
             email.display_name = ' - '.join(parts)
@@ -179,14 +164,6 @@ def _compute_date(self):
             email.date = fields.Date.to_string(
                 fields.Date.from_string(email.time))
 
-    @api.model
-    def create(self, vals):
-        tracking = super(MailTrackingEmail, self).create(vals)
-        self.tracking_ids_recalculate(
-            'res.partner', 'email', 'tracking_email_ids',
-            tracking.recipient_address, new_tracking=tracking)
-        return tracking
-
     def _get_mail_tracking_img(self):
         base_url = self.env['ir.config_parameter'].get_param('web.base.url')
         path_url = (
@@ -202,6 +179,13 @@ def _get_mail_tracking_img(self):
                 'tracking_email_id': self.id,
             })
 
+    @api.multi
+    def _partners_email_bounced_set(self, reason):
+        for tracking_email in self:
+            self.env['res.partner'].search([
+                ('email', '=ilike', tracking_email.recipient_address)
+            ]).email_bounced_set(tracking_email, reason)
+
     @api.multi
     def smtp_error(self, mail_server, smtp_server, exception):
         self.sudo().write({
@@ -210,6 +194,7 @@ def smtp_error(self, mail_server, smtp_server, exception):
             'error_description': tools.ustr(exception),
             'state': 'error',
         })
+        self.sudo()._partners_email_bounced_set('error')
         return True
 
     @api.multi
@@ -228,7 +213,7 @@ def _message_partners_check(self, message, message_id):
         partners = mail_message.notified_partner_ids | mail_message.partner_ids
         if (self.partner_id and self.partner_id not in partners):
             # If mail_message haven't tracking partner, then
-            # add it in order to see his trackking status in chatter
+            # add it in order to see his tracking status in chatter
             if mail_message.subtype_id:
                 mail_message.sudo().write({
                     'notified_partner_ids': [(4, self.partner_id.id)],
@@ -287,20 +272,21 @@ def _concurrent_events(self, event_type, metadata):
 
     @api.multi
     def event_create(self, event_type, metadata):
+        testing = getattr(threading.currentThread(), 'testing', False)
         event_ids = self.env['mail.tracking.event']
         for tracking_email in self:
             other_ids = tracking_email._concurrent_events(event_type, metadata)
             if not other_ids:
                 vals = tracking_email._event_prepare(event_type, metadata)
                 if vals:
                     event_ids += event_ids.sudo().create(vals)
-                partners = self.tracking_ids_recalculate(
-                    'res.partner', 'email', 'tracking_email_ids',
-                    tracking_email.recipient_address)
-                if partners:
-                    partners.email_score_calculate()
+                    # Commit to DB to release exclusive lock
+                    if not testing:
+                        self.env.cr.commit()  # pragma: no cover
             else:
                 _logger.debug("Concurrent event '%s' discarded", event_type)
+        if event_type in {'hard_bounce', 'spam', 'reject'}:
+            self.sudo()._partners_email_bounced_set(event_type)
         return event_ids
 
     @api.model

mail_tracking/models/mail_tracking_event.py

@@ -23,7 +23,7 @@ class MailTrackingEvent(models.Model):
     date = fields.Date(
         string="Date", readonly=True, compute="_compute_date", store=True)
     tracking_email_id = fields.Many2one(
-        string='Message', readonly=True,
+        string='Message', readonly=True, required=True, ondelete='cascade',
         comodel_name='mail.tracking.email')
     event_type = fields.Selection(string='Event type', selection=[
         ('sent', 'Sent'),