From d420cefc9b36a5a563fa94fb35e32a16b064796c Mon Sep 17 00:00:00 2001 From: Antonio Espinosa Date: Fri, 9 Sep 2016 13:29:58 +0200 Subject: [PATCH] [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 | 2 +- mail_tracking/__openerp__.py | 4 +- mail_tracking/hooks.py | 42 +++++++---- mail_tracking/models/mail_message.py | 2 +- mail_tracking/models/mail_tracking_email.py | 39 +++++++++- mail_tracking/models/res_partner.py | 17 +++-- mail_tracking/tests/test_mail_tracking.py | 75 ++++++++++++++++++- .../models/mail_tracking_email.py | 28 +++---- mail_tracking_mass_mailing/__init__.py | 2 +- mail_tracking_mass_mailing/__openerp__.py | 4 +- mail_tracking_mass_mailing/hooks.py | 30 +++----- .../models/mail_mail_statistics.py | 2 - .../models/mail_mass_mailing_contact.py | 11 ++- .../models/mail_tracking_email.py | 11 +++ .../models/mail_tracking_event.py | 32 ++++++++ .../tests/test_mass_mailing.py | 29 ++++++- .../views/mail_mail_statistics_view.xml | 14 ---- 17 files changed, 252 insertions(+), 92 deletions(-) diff --git a/mail_tracking/__init__.py b/mail_tracking/__init__.py index 1c66d89ec..e32deff2c 100644 --- a/mail_tracking/__init__.py +++ b/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 diff --git a/mail_tracking/__openerp__.py b/mail_tracking/__openerp__.py index 10cb3dcff..fe23e6aff 100644 --- a/mail_tracking/__openerp__.py +++ b/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", } diff --git a/mail_tracking/hooks.py b/mail_tracking/hooks.py index 2b6b20b16..8d3dc43db 100644 --- a/mail_tracking/hooks.py +++ b/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) diff --git a/mail_tracking/models/mail_message.py b/mail_tracking/models/mail_message.py index d5d381eee..682ef619c 100644 --- a/mail_tracking/models/mail_message.py +++ b/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) diff --git a/mail_tracking/models/mail_tracking_email.py b/mail_tracking/models/mail_tracking_email.py index d0452add0..6b2944be6 100644 --- a/mail_tracking/models/mail_tracking_email.py +++ b/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: - vals = tracking_email._event_prepare(event_type, metadata) - if vals: - event_ids += event_ids.sudo().create(vals) + 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 diff --git a/mail_tracking/models/res_partner.py b/mail_tracking/models/res_partner.py index d4ae19de6..c4eb294ae 100644 --- a/mail_tracking/models/res_partner.py +++ b/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') diff --git a/mail_tracking/tests/test_mail_tracking.py b/mail_tracking/tests/test_mail_tracking.py index 816a6bef7..a6eacc601 100644 --- a/mail_tracking/tests/test_mail_tracking.py +++ b/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') diff --git a/mail_tracking_mailgun/models/mail_tracking_email.py b/mail_tracking_mailgun/models/mail_tracking_email.py index ebd7d6581..f7008ed72 100644 --- a/mail_tracking_mailgun/models/mail_tracking_email.py +++ b/mail_tracking_mailgun/models/mail_tracking_email.py @@ -42,16 +42,12 @@ 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", - mailgun_event_type) + 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 return True @@ -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. " - "Please add 'mailgun.apikey' to System parameters " - "to enable Mailgun authentication webhoook requests. " - "More info at: " - "https://documentation.mailgun.com/user_manual.html" - "#webhooks") + _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") else: timestamp = event.get('timestamp') token = event.get('token') @@ -89,8 +85,8 @@ 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", - odoo_db) + _logger.error("Mailgun: Database '%s' is not the current database", + odoo_db) return False # OK, DB is current return True @@ -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)), }) diff --git a/mail_tracking_mass_mailing/__init__.py b/mail_tracking_mass_mailing/__init__.py index e5e39cc1d..ba08c9ff4 100644 --- a/mail_tracking_mass_mailing/__init__.py +++ b/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 diff --git a/mail_tracking_mass_mailing/__openerp__.py b/mail_tracking_mass_mailing/__openerp__.py index 13a903425..0f01f6e0e 100644 --- a/mail_tracking_mass_mailing/__openerp__.py +++ b/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", } diff --git a/mail_tracking_mass_mailing/hooks.py b/mail_tracking_mass_mailing/hooks.py index 4ffb47a9b..b3601a298 100644 --- a/mail_tracking_mass_mailing/hooks.py +++ b/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) diff --git a/mail_tracking_mass_mailing/models/mail_mail_statistics.py b/mail_tracking_mass_mailing/models/mail_mail_statistics.py index 2dd26aba9..dbcfe9bd2 100644 --- a/mail_tracking_mass_mailing/models/mail_mail_statistics.py +++ b/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) diff --git a/mail_tracking_mass_mailing/models/mail_mass_mailing_contact.py b/mail_tracking_mass_mailing/models/mail_mass_mailing_contact.py index 8fc6c561d..bfc179145 100644 --- a/mail_tracking_mass_mailing/models/mail_mass_mailing_contact.py +++ b/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): diff --git a/mail_tracking_mass_mailing/models/mail_tracking_email.py b/mail_tracking_mass_mailing/models/mail_tracking_email.py index e8b0de6ff..571fe7601 100644 --- a/mail_tracking_mass_mailing/models/mail_tracking_email.py +++ b/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 diff --git a/mail_tracking_mass_mailing/models/mail_tracking_event.py b/mail_tracking_mass_mailing/models/mail_tracking_event.py index c38fe903e..97e091194 100644 --- a/mail_tracking_mass_mailing/models/mail_tracking_event.py +++ b/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 diff --git a/mail_tracking_mass_mailing/tests/test_mass_mailing.py b/mail_tracking_mass_mailing/tests/test_mass_mailing.py index 9c18bb6ef..b4c1f89fb 100644 --- a/mail_tracking_mass_mailing/tests/test_mass_mailing.py +++ b/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() diff --git a/mail_tracking_mass_mailing/views/mail_mail_statistics_view.xml b/mail_tracking_mass_mailing/views/mail_mail_statistics_view.xml index c424a9560..d67ddd219 100644 --- a/mail_tracking_mass_mailing/views/mail_mail_statistics_view.xml +++ b/mail_tracking_mass_mailing/views/mail_mail_statistics_view.xml @@ -9,9 +9,6 @@ mail.mail.statistics - - - @@ -34,16 +31,5 @@ - - Add partner and state columns - mail.mail.statistics - - - - - - - -