[8.0][IMP][mail_tracking] Speed installation time and discard concurrent events (#82)

[IMP] mail_tracking: Speed installation time, discard concurrent events and other fixes

mail_tracking/__init__.py

@@ -5,4 +5,4 @@
 
 from . import models
 from . import controllers
-from .hooks import post_init_hook
+from .hooks import pre_init_hook

mail_tracking/__openerp__.py

@@ -5,7 +5,7 @@
 {
     "name": "Email tracking",
     "summary": "Email tracking system for all mails sent",
-    "version": "8.0.2.0.0",
+    "version": "8.0.2.0.1",
     "category": "Social Network",
     "website": "http://www.tecnativa.com",
     "author": "Tecnativa, "
@@ -28,5 +28,5 @@
     "qweb": [
         "static/src/xml/mail_tracking.xml",
     ],
-    "post_init_hook": "post_init_hook",
+    "pre_init_hook": "pre_init_hook",
 }

mail_tracking/hooks.py

@@ -3,22 +3,34 @@
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 
 import logging
-from openerp import api, SUPERUSER_ID
+from psycopg2.extensions import AsIs
 
 _logger = logging.getLogger(__name__)
 
 
-def post_init_hook(cr, registry):
-    with api.Environment.manage():
-        env = api.Environment(cr, SUPERUSER_ID, {})
-        # Recalculate all partner tracking_email_ids
-        partners = env['res.partner'].search([
-            ('email', '!=', False),
-        ])
-        emails = partners.mapped('email')
-        _logger.info(
-            "Recalculating 'tracking_email_ids' in 'res.partner' "
-            "model for %d email addresses", len(emails))
-        for email in emails:
-            env['mail.tracking.email'].tracking_ids_recalculate(
-                'res.partner', 'email', 'tracking_email_ids', email)
+def column_exists(cr, table, column):
+    cr.execute("""
+        SELECT column_name
+        FROM information_schema.columns
+        WHERE table_name = %s AND column_name = %s""", (table, column))
+    return bool(cr.fetchall())
+
+
+def column_add_with_value(cr, table, column, field_type, value):
+    if not column_exists(cr, table, column):
+        cr.execute("""
+            ALTER TABLE %s
+            ADD COLUMN %s %s""", (AsIs(table), AsIs(column), AsIs(field_type)))
+        cr.execute("""
+            UPDATE %s SET %s = %s""", (AsIs(table), AsIs(column), value))
+
+
+def pre_init_hook(cr):
+    _logger.info("Creating res.partner.tracking_emails_count column "
+                 "with value 0")
+    column_add_with_value(
+        cr, "res_partner", "tracking_emails_count", "integer", 0)
+    _logger.info("Creating res.partner.email_score column "
+                 "with value 50.0")
+    column_add_with_value(
+        cr, "res_partner", "email_score", "double precision", 50.0)

mail_tracking/models/mail_message.py

@@ -46,7 +46,7 @@ class MailMessage(models.Model):
                     tracking_email = self.env['mail.tracking.email'].search([
                         ('mail_message_id', '=', mail_message_id),
                         ('partner_id', '=', partner_id),
-                    ])
+                    ], limit=1)
                     status = self._partner_tracking_status_get(tracking_email)
                     partner_trackings[str(partner_id)] = (
                         status, tracking_email.id)

mail_tracking/models/mail_tracking_email.py

@@ -13,6 +13,9 @@ import openerp.addons.decimal_precision as dp
 
 _logger = logging.getLogger(__name__)
 
+EVENT_OPEN_DELTA = 10  # seconds
+EVENT_CLICK_DELTA = 5  # seconds
+
 
 class MailTrackingEmail(models.Model):
     _name = "mail.tracking.email"
@@ -104,7 +107,7 @@ class MailTrackingEmail(models.Model):
                     obj.write({
                         tracking_field: [(5, False, False)]
                     })
-        return True
+        return objects
 
     @api.model
     def _tracking_ids_to_write(self, email):
@@ -256,13 +259,41 @@ class MailTrackingEmail(models.Model):
             _logger.info('Unknown event type: %s' % event_type)
         return False
 
+    def _concurrent_events(self, event_type, metadata):
+        m_event = self.env['mail.tracking.event']
+        self.ensure_one()
+        concurrent_event_ids = False
+        if event_type in {'open', 'click'}:
+            ts = metadata.get('timestamp', time.time())
+            delta = EVENT_OPEN_DELTA if event_type == 'open' \
+                else EVENT_CLICK_DELTA
+            domain = [
+                ('timestamp', '>=', ts - delta),
+                ('timestamp', '<=', ts + delta),
+                ('tracking_email_id', '=', self.id),
+                ('event_type', '=', event_type),
+            ]
+            if event_type == 'click':
+                domain.append(('url', '=', metadata.get('url', False)))
+            concurrent_event_ids = m_event.search(domain)
+        return concurrent_event_ids
+
     @api.multi
     def event_create(self, event_type, metadata):
         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()
+            else:
+                _logger.debug("Concurrent event '%s' discarded", event_type)
         return event_ids
 
     @api.model

mail_tracking/models/res_partner.py

@@ -15,13 +15,18 @@ class ResPartner(models.Model):
         string="Tracking emails count", store=True, readonly=True,
         compute="_compute_tracking_emails_count")
     email_score = fields.Float(
-        string="Email score",
-        compute="_compute_email_score", store=True, readonly=True)
+        string="Email score", readonly=True, default=50.0)
 
-    @api.one
-    @api.depends('tracking_email_ids.state')
-    def _compute_email_score(self):
-        self.email_score = self.tracking_email_ids.email_score()
+    @api.multi
+    def email_score_calculate(self):
+        # This is not a compute method because is causing a inter-block
+        # in mail_tracking_email PostgreSQL table
+        # We suspect that tracking_email write to state field block that
+        # table and then inside write ORM try to read from DB
+        # tracking_email_ids because it's not in cache.
+        # PostgreSQL blocks read because we have not committed yet the write
+        for partner in self:
+            partner.email_score = partner.tracking_email_ids.email_score()
 
     @api.one
     @api.depends('tracking_email_ids')

mail_tracking/tests/test_mail_tracking.py

@@ -4,9 +4,9 @@
 
 import mock
 import base64
+import time
 from openerp.tests.common import TransactionCase
-from openerp.addons.mail_tracking.controllers.main import \
-    MailTrackingController, BLANK
+from ..controllers.main import MailTrackingController, BLANK
 
 mock_request = 'openerp.http.request'
 mock_send_email = ('openerp.addons.base.ir.ir_mail_server.'
@@ -114,6 +114,77 @@ class TestMailTracking(TransactionCase):
             res = controller.mail_tracking_open(db, tracking.id)
             self.assertEqual(image, res.response[0])
 
+    def test_concurrent_open(self):
+        mail, tracking = self.mail_send()
+        ts = time.time()
+        metadata = {
+            'ip': '127.0.0.1',
+            'user_agent': 'Odoo Test/1.0',
+            'os_family': 'linux',
+            'ua_family': 'odoo',
+            'timestamp': ts,
+        }
+        # First open event
+        tracking.event_create('open', metadata)
+        opens = tracking.tracking_event_ids.filtered(
+            lambda r: r.event_type == 'open'
+        )
+        self.assertEqual(len(opens), 1)
+        # Concurrent open event
+        metadata['timestamp'] = ts + 2
+        tracking.event_create('open', metadata)
+        opens = tracking.tracking_event_ids.filtered(
+            lambda r: r.event_type == 'open'
+        )
+        self.assertEqual(len(opens), 1)
+        # Second open event
+        metadata['timestamp'] = ts + 350
+        tracking.event_create('open', metadata)
+        opens = tracking.tracking_event_ids.filtered(
+            lambda r: r.event_type == 'open'
+        )
+        self.assertEqual(len(opens), 2)
+
+    def test_concurrent_click(self):
+        mail, tracking = self.mail_send()
+        ts = time.time()
+        metadata = {
+            'ip': '127.0.0.1',
+            'user_agent': 'Odoo Test/1.0',
+            'os_family': 'linux',
+            'ua_family': 'odoo',
+            'timestamp': ts,
+            'url': 'https://www.example.com/route/1',
+        }
+        # First click event (URL 1)
+        tracking.event_create('click', metadata)
+        opens = tracking.tracking_event_ids.filtered(
+            lambda r: r.event_type == 'click'
+        )
+        self.assertEqual(len(opens), 1)
+        # Concurrent click event (URL 1)
+        metadata['timestamp'] = ts + 2
+        tracking.event_create('click', metadata)
+        opens = tracking.tracking_event_ids.filtered(
+            lambda r: r.event_type == 'click'
+        )
+        self.assertEqual(len(opens), 1)
+        # Second click event (URL 1)
+        metadata['timestamp'] = ts + 350
+        tracking.event_create('click', metadata)
+        opens = tracking.tracking_event_ids.filtered(
+            lambda r: r.event_type == 'click'
+        )
+        self.assertEqual(len(opens), 2)
+        # Concurrent click event (URL 2)
+        metadata['timestamp'] = ts + 2
+        metadata['url'] = 'https://www.example.com/route/2'
+        tracking.event_create('click', metadata)
+        opens = tracking.tracking_event_ids.filtered(
+            lambda r: r.event_type == 'click'
+        )
+        self.assertEqual(len(opens), 3)
+
     def test_smtp_error(self):
         with mock.patch(mock_send_email) as mock_func:
             mock_func.side_effect = Warning('Test error')

mail_tracking_mailgun/models/mail_tracking_email.py

@@ -42,15 +42,11 @@ class MailTrackingEmail(models.Model):
             'dropped': 'reject',
         }
 
-    @property
-    def _mailgun_supported_event_types(self):
-        return self._mailgun_event_type_mapping.keys()
-
     def _mailgun_event_type_verify(self, event):
         event = event or {}
         mailgun_event_type = event.get('event')
-        if mailgun_event_type not in self._mailgun_supported_event_types:
-            _logger.info("Mailgun: event type '%s' not supported",
+        if mailgun_event_type not in self._mailgun_event_type_mapping:
+            _logger.error("Mailgun: event type '%s' not supported",
                           mailgun_event_type)
             return False
         # OK, event type is valid
@@ -66,12 +62,12 @@ class MailTrackingEmail(models.Model):
         event = event or {}
         api_key = self.env['ir.config_parameter'].get_param('mailgun.apikey')
         if not api_key:
-            _logger.info("No Mailgun api key configured. "
+            _logger.warning("No Mailgun api key configured. "
                             "Please add 'mailgun.apikey' to System parameters "
-                         "to enable Mailgun authentication webhoook requests. "
-                         "More info at: "
-                         "https://documentation.mailgun.com/user_manual.html"
-                         "#webhooks")
+                            "to enable Mailgun authentication webhoook "
+                            "requests. More info at: "
+                            "https://documentation.mailgun.com/"
+                            "user_manual.html#webhooks")
         else:
             timestamp = event.get('timestamp')
             token = event.get('token')
@@ -89,7 +85,7 @@ class MailTrackingEmail(models.Model):
         odoo_db = event.get('odoo_db')
         current_db = self.env.cr.dbname
         if odoo_db != current_db:
-            _logger.info("Mailgun: Database '%s' is not the current database",
+            _logger.error("Mailgun: Database '%s' is not the current database",
                           odoo_db)
             return False
         # OK, DB is current
@@ -124,7 +120,7 @@ class MailTrackingEmail(models.Model):
                 metadata[k] = event[v]
         # Special field mapping
         metadata.update({
-            'mobile': event.get('device-type') in ('mobile', 'tablet'),
+            'mobile': event.get('device-type') in {'mobile', 'tablet'},
             'user_country_id': self._country_search(
                 event.get('country', False)),
         })

mail_tracking_mass_mailing/__init__.py

@@ -3,4 +3,4 @@
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 
 from . import models
-from .hooks import post_init_hook
+from .hooks import pre_init_hook

mail_tracking_mass_mailing/__openerp__.py

@@ -5,7 +5,7 @@
 {
     "name": "Mail tracking for mass mailing",
     "summary": "Improve mass mailing email tracking",
-    "version": "8.0.1.0.0",
+    "version": "8.0.1.0.1",
     "category": "Social Network",
     "website": "http://www.tecnativa.com",
     "author": "Tecnativa, "
@@ -24,5 +24,5 @@
         "views/mail_mass_mailing_view.xml",
         "views/mail_mass_mailing_contact_view.xml",
     ],
-    "post_init_hook": "post_init_hook",
+    "pre_init_hook": "pre_init_hook",
 }

mail_tracking_mass_mailing/hooks.py

@@ -3,26 +3,18 @@
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 
 import logging
-from openerp import api, SUPERUSER_ID
+try:
+    from openerp.addons.mail_tracking.hooks import column_add_with_value
+except ImportError:
+    column_add_with_value = False
 
 _logger = logging.getLogger(__name__)
 
 
-def post_init_hook(cr, registry):
-    with api.Environment.manage():
-        env = api.Environment(cr, SUPERUSER_ID, {})
-        # Recalculate all mass_mailing contacts tracking_email_ids
-        contacts = env['mail.mass_mailing.contact'].search([
-            ('email', '!=', False),
-        ])
-        emails = contacts.mapped('email')
-        _logger.info(
-            "Recalculating 'tracking_email_ids' in "
-            "'mail.mass_mailing.contact' model for %d email addresses",
-            len(emails))
-        for n, email in enumerate(emails):
-            env['mail.tracking.email'].tracking_ids_recalculate(
-                'mail.mass_mailing.contact', 'email', 'tracking_email_ids',
-                email)
-            if n % 500 == 0:  # pragma: no cover
-                _logger.info("   Recalculated %d of %d", n, len(emails))
+def pre_init_hook(cr):
+    if column_add_with_value:
+        _logger.info("Creating mail.mass_mailing.contact.email_score column "
+                     "with value 50.0")
+        column_add_with_value(
+            cr, 'mail_mass_mailing_contact', 'email_score', 'double precision',
+            50.0)

mail_tracking_mass_mailing/models/mail_mail_statistics.py

@@ -14,5 +14,3 @@ class MailMailStatistics(models.Model):
     tracking_event_ids = fields.One2many(
         string="Tracking events", comodel_name='mail.tracking.event',
         related='mail_tracking_id.tracking_event_ids', readonly=True)
-    tracking_state = fields.Selection(
-        string="State", related='mail_tracking_id.state', store=True)

mail_tracking_mass_mailing/models/mail_mass_mailing_contact.py

@@ -12,13 +12,12 @@ class MailMassMailingContact(models.Model):
         string="Tracking emails", comodel_name="mail.tracking.email",
         readonly=True)
     email_score = fields.Float(
-        string="Email score",
-        compute="_compute_email_score", store=True, readonly=True)
+        string="Email score", readonly=True, default=50.0)
 
-    @api.one
-    @api.depends('tracking_email_ids', 'tracking_email_ids.state')
-    def _compute_email_score(self):
-        self.email_score = self.tracking_email_ids.email_score()
+    @api.multi
+    def email_score_calculate(self):
+        for contact in self:
+            contact.email_score = contact.tracking_email_ids.email_score()
 
     @api.multi
     def write(self, vals):

mail_tracking_mass_mailing/models/mail_tracking_email.py

@@ -39,3 +39,14 @@ class MailTrackingEmail(models.Model):
             'mail.mass_mailing.contact', 'email', 'tracking_email_ids',
             tracking.recipient_address, new_tracking=tracking)
         return tracking
+
+    @api.multi
+    def event_create(self, event_type, metadata):
+        res = super(MailTrackingEmail, self).event_create(event_type, metadata)
+        for tracking_email in self:
+            contacts = self.tracking_ids_recalculate(
+                'mail.mass_mailing.contact', 'email', 'tracking_email_ids',
+                tracking_email.recipient_address)
+            if contacts:
+                contacts.email_score_calculate()
+        return res

mail_tracking_mass_mailing/models/mail_tracking_event.py

@@ -15,3 +15,35 @@ class MailTrackingEvent(models.Model):
         mail_mail_stats = self.sudo().env['mail.mail.statistics']
         mail_mail_stats.set_opened(mail_mail_ids=[tracking_email.mail_id_int])
         return res
+
+    def _tracking_set_bounce(self, tracking_email, metadata):
+        mail_mail_stats = self.sudo().env['mail.mail.statistics']
+        mail_mail_stats.set_bounced(mail_mail_ids=[tracking_email.mail_id_int])
+
+    @api.model
+    def process_hard_bounce(self, tracking_email, metadata):
+        res = super(MailTrackingEvent, self).process_hard_bounce(
+            tracking_email, metadata)
+        self._tracking_set_bounce(tracking_email, metadata)
+        return res
+
+    @api.model
+    def process_soft_bounce(self, tracking_email, metadata):
+        res = super(MailTrackingEvent, self).process_soft_bounce(
+            tracking_email, metadata)
+        self._tracking_set_bounce(tracking_email, metadata)
+        return res
+
+    @api.model
+    def process_reject(self, tracking_email, metadata):
+        res = super(MailTrackingEvent, self).process_reject(
+            tracking_email, metadata)
+        self._tracking_set_bounce(tracking_email, metadata)
+        return res
+
+    @api.model
+    def process_spam(self, tracking_email, metadata):
+        res = super(MailTrackingEvent, self).process_spam(
+            tracking_email, metadata)
+        self._tracking_set_bounce(tracking_email, metadata)
+        return res

mail_tracking_mass_mailing/tests/test_mass_mailing.py

@@ -72,7 +72,34 @@ class TestMassMailing(TransactionCase):
             }
             tracking_email.event_create('open', metadata)
             self.assertTrue(stat.opened)
-            self.assertEqual(stat.tracking_state, 'opened')
+
+    def _tracking_email_bounce(self, event_type, state):
+        self.mailing.send_mail()
+        for stat in self.mailing.statistics_ids:
+            if stat.mail_mail_id:
+                stat.mail_mail_id.send()
+            tracking_email = self.env['mail.tracking.email'].search([
+                ('mail_id_int', '=', stat.mail_mail_id_int),
+            ])
+            # And now mark the email as bounce
+            metadata = {
+                'bounce_type': '499',
+                'bounce_description': 'Unable to connect to MX servers',
+            }
+            tracking_email.event_create(event_type, metadata)
+            self.assertTrue(stat.bounced)
+
+    def test_tracking_email_hard_bounce(self):
+            self._tracking_email_bounce('hard_bounce', 'bounced')
+
+    def test_tracking_email_soft_bounce(self):
+            self._tracking_email_bounce('soft_bounce', 'soft-bounced')
+
+    def test_tracking_email_reject(self):
+            self._tracking_email_bounce('reject', 'rejected')
+
+    def test_tracking_email_spam(self):
+            self._tracking_email_bounce('spam', 'spam')
 
     def test_contact_tracking_emails(self):
         self.mailing.send_mail()

mail_tracking_mass_mailing/views/mail_mail_statistics_view.xml

@@ -9,9 +9,6 @@
     <field name="model">mail.mail.statistics</field>
     <field name="inherit_id" ref="mass_mailing.view_mail_mail_statistics_form"/>
     <field name="arch" type="xml">
-        <field name="res_id" position="after">
-            <field name="tracking_state"/>
-        </field>
         <xpath expr="//form" position="inside">
             <group>
                 <field name="mail_tracking_id" />
@@ -34,16 +31,5 @@
     </field>
 </record>
 
-<record model="ir.ui.view" id="view_mail_mail_statistics_tree">
-    <field name="name">Add partner and state columns</field>
-    <field name="model">mail.mail.statistics</field>
-    <field name="inherit_id" ref="mass_mailing.view_mail_mail_statistics_tree"/>
-    <field name="arch" type="xml">
-        <field name="sent" position="before">
-            <field name="tracking_state"/>
-        </field>
-    </field>
-</record>
-
 </data>
 </openerp>