[IMP] mail : apply mail.address.mixin on blacklist

Apply the mail.address.mixin on blacklist mixin. So that every model
inheriting the blacklist.mixin will also inherit the address.mixin.

That simplify also the comparaison to - and the searches on - the
blacklist.

Task ID 1896677

6 files changed, 40 insertions, 56 deletions

diff --git a/addons/mail/models/mail_blacklist.py b/addons/mail/models/mail_blacklist.py
index 10df7379fa0..d9f7b95a0a0 100644
--- a/addons/mail/models/mail_blacklist.py
+++ b/addons/mail/models/mail_blacklist.py
@@ -30,7 +30,7 @@ class MailBlackList(models.Model):
         new_values = []
         all_emails = []
         for value in values:
-            email = self._sanitize_email(value.get('email'))
+            email = self._normalize_email(value.get('email'))
             if not email:
                 raise UserError(_('Invalid email address %r') % value['email'])
             if email in all_emails:
@@ -44,8 +44,7 @@ class MailBlackList(models.Model):
         emails = [v['email'] for v in new_values]
         self._cr.execute(sql, (emails,))
         bl_entries = dict(self._cr.fetchall())
-        to_create = [v for v in new_values
-                     if v['email'] not in bl_entries]
+        to_create = [v for v in new_values if v['email'] not in bl_entries]
 
         # TODO DBE Fixme : reorder ids according to incoming ids.
         results = super(MailBlackList, self).create(to_create)
@@ -54,7 +53,7 @@ class MailBlackList(models.Model):
     @api.multi
     def write(self, values):
         if 'email' in values:
-            values['email'] = self._sanitize_email(values['email'])
+            values['email'] = self._normalize_email(values['email'])
         return super(MailBlackList, self).write(values)
 
     def _search(self, args, offset=0, limit=None, order=None, count=False, access_rights_uid=None):
@@ -64,9 +63,9 @@ class MailBlackList(models.Model):
             new_args = []
             for arg in args:
                 if isinstance(arg, (list, tuple)) and arg[0] == 'email' and isinstance(arg[2], tools.pycompat.text_type):
-                    sanitized = self.env['mail.blacklist']._sanitize_email(arg[2])
-                    if sanitized:
-                        new_args.append([arg[0], arg[1], sanitized])
+                    normalized = self._normalize_email(arg[2])
+                    if normalized:
+                        new_args.append([arg[0], arg[1], normalized])
                     else:
                         new_args.append(arg)
                 else:
@@ -76,8 +75,8 @@ class MailBlackList(models.Model):
         return super(MailBlackList, self)._search(new_args, offset=offset, limit=limit, order=order, count=count, access_rights_uid=access_rights_uid)
 
     def _add(self, email):
-        sanitized = self._sanitize_email(email)
-        record = self.env["mail.blacklist"].with_context(active_test=False).search([('email', '=', sanitized)])
+        normalized = self._normalize_email(email)
+        record = self.env["mail.blacklist"].with_context(active_test=False).search([('email', '=', normalized)])
         if len(record) > 0:
             record.write({'active': True})
         else:
@@ -85,16 +84,16 @@ class MailBlackList(models.Model):
         return record
 
     def _remove(self, email):
-        sanitized = self._sanitize_email(email)
-        record = self.env["mail.blacklist"].with_context(active_test=False).search([('email', '=', sanitized)])
+        normalized = self._normalize_email(email)
+        record = self.env["mail.blacklist"].with_context(active_test=False).search([('email', '=', normalized)])
         if len(record) > 0:
             record.write({'active': False})
         else:
             record = record.create({'email': email, 'active': False})
         return record
 
-    def _sanitize_email(self, email):
-        """ Sanitize and standardize blacklist entries: all emails should be
+    def _normalize_email(self, email):
+        """ Sanitize and standardize email address entries: all emails should be
         only real email extracted from strings (A <a@a> -> a@a)  and should be
         lower case. """
         emails = tools.email_split(email)
@@ -105,12 +104,12 @@ class MailBlackList(models.Model):
 
 class MailBlackListMixin(models.AbstractModel):
     """ Mixin that is inherited by all model with opt out.
-        USAGE : the field '_primary_email' must be overridden in the model that inherit the mixin
-        and must contain the email field of the model.
+        This mixin is inheriting of another mixin, mail.address.mixin which defines the _primary_email variable
+        and the email_normalized field that are mandatory to use the blacklist mixin.
         """
     _name = 'mail.blacklist.mixin'
     _description = 'Mail Blacklist mixin'
-    _primary_email = ['email']
+    _inherit = ['mail.address.mixin']
 
     # Note : is_blacklisted sould only be used for display. As the compute is not depending on the blacklist,
     # once read, it won't be re-computed again if the blacklist is modified in the same request.
@@ -118,15 +117,6 @@ class MailBlackListMixin(models.AbstractModel):
         store=False, search="_search_is_blacklisted", groups="base.group_user",
         help="If the email address is on the blacklist, the contact won't receive mass mailing anymore, from any list")
 
-    def _assert_primary_email(self):
-        if not hasattr(self, "_primary_email") or \
-                not isinstance(self._primary_email, (list, tuple)) or \
-                not len(self._primary_email) == 1:
-            raise UserError(_('Invalid primary email field on model %s') % self._name)
-        field_name = self._primary_email[0]
-        if field_name not in self._fields or self._fields[field_name].type != 'char':
-            raise UserError(_('Invalid primary email field on model %s') % self._name)
-
     @api.model
     def _search_is_blacklisted(self, operator, value):
         # Assumes operator is '=' or '!=' and value is True or False
@@ -137,35 +127,32 @@ class MailBlackListMixin(models.AbstractModel):
             else:
                 raise NotImplementedError()
 
-        [email_field] = self._primary_email
         if value:
             query = """
                 SELECT m.id
                     FROM mail_blacklist bl
                     JOIN %s m
-                    ON (LOWER(substring(m.%s, '([^ ,;<@]+@[^> ,;]+)')) = bl.email AND bl.active)
+                    ON m.email_normalized = bl.email AND bl.active
             """
         else:
             query = """
                 SELECT m.id
                     FROM %s m
                     LEFT JOIN mail_blacklist bl
-                    ON (LOWER(substring(m.%s, '([^ ,;<@]+@[^> ,;]+)')) = bl.email AND bl.active)
+                    ON m.email_normalized = bl.email AND bl.active
                     WHERE bl.id IS NULL
             """
-        self._cr.execute(query % (self._table, email_field))
+        self._cr.execute(query % self._table)
         res = self._cr.fetchall()
         if not res:
             return [(0, '=', 1)]
         return [('id', 'in', [r[0] for r in res])]
 
-    @api.depends(lambda self: self._primary_email)
+    @api.depends('email_normalized')
     def _compute_is_blacklisted(self):
-        self._assert_primary_email()
-        [email_field] = self._primary_email
         # TODO : Should remove the sudo as compute_sudo defined on methods.
         # But if user doesn't have access to mail.blacklist, doen't work without sudo().
-        sanitized = [self.env['mail.blacklist']._sanitize_email(email) for email in self.mapped(email_field)]
-        blacklist = set(self.env['mail.blacklist'].sudo().search([('email', 'in', sanitized)]).mapped('email'))
+        blacklist = set(self.env['mail.blacklist'].sudo().search([
+            ('email', 'in', self.mapped('email_normalized'))]).mapped('email'))
         for record in self:
-            record.is_blacklisted = self.env['mail.blacklist']._sanitize_email(record[email_field]) in blacklist
+            record.is_blacklisted = record.email_normalized in blacklist
diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py
index 5422a115fdf..b9a5845896c 100644
--- a/addons/mail/wizard/mail_compose_message.py
+++ b/addons/mail/wizard/mail_compose_message.py
@@ -280,17 +280,14 @@ class MailComposer(models.TransientModel):
             reply_to_value = self.env['mail.thread']._notify_get_reply_to_on_records(default=self.email_from, records=records)
 
         blacklisted_rec_ids = []
-        if mass_mail_mode and hasattr(self.env[self.model], "_primary_email"):
+        if mass_mail_mode and issubclass(type(self.env[self.model]), self.pool['mail.blacklist.mixin']):
             BL_sudo = self.env['mail.blacklist'].sudo()
             blacklist = set(BL_sudo.search([]).mapped('email'))
             if blacklist:
-                [email_field] = self.env[self.model]._primary_email
-                targets = self.env[self.model].browse(res_ids).read([email_field])
+                targets = self.env[self.model].browse(res_ids).read(['email_normalized'])
                 # First extract email from recipient before comparing with blacklist
-                for target in targets:
-                    sanitized_email = self.env['mail.blacklist']._sanitize_email(target.get(email_field))
-                    if sanitized_email and sanitized_email in blacklist:
-                        blacklisted_rec_ids.append(target['id'])
+                blacklisted_rec_ids.extend([target['id'] for target in targets
+                                            if target['email_normalized'] and target['email_normalized'] in blacklist])
 
         for res_id in res_ids:
             # static wizard (mail.message) values
diff --git a/addons/mass_mailing/controllers/main.py b/addons/mass_mailing/controllers/main.py
index 7d43038a3a6..66d17709b46 100644
--- a/addons/mass_mailing/controllers/main.py
+++ b/addons/mass_mailing/controllers/main.py
@@ -39,7 +39,7 @@ class MassMailController(http.Controller):
                 # Unsubscribe directly + Let the user choose his subscriptions
                 mailing.update_opt_out(email, mailing.contact_list_ids.ids, True)
 
-                contacts = request.env['mail.mass_mailing.contact'].sudo().search([('email', '=', email)])
+                contacts = request.env['mail.mass_mailing.contact'].sudo().search([('email_normalized', '=ilike', email)])
                 subscription_list_ids = contacts.mapped('subscription_list_ids')
                 # In many user are found : if user is opt_out on the list with contact_id 1 but not with contact_id 2,
                 # assume that the user is not opt_out on both
@@ -152,8 +152,7 @@ class MassMailController(http.Controller):
             if not self._valid_unsubscribe_token(mailing_id, res_id, email, token):
                 return 'unauthorized'
             model = request.env[mailing.mailing_model_real]
-            [email_field] = model._primary_email
-            records = model.sudo().search([(email_field, '=ilike', email)])
+            records = model.sudo().search([('email_normalized', '=ilike', email)])
             for record in records:
                 record.sudo().message_post(body=_("Feedback from %s: %s" % (email, feedback)))
             return bool(records)
diff --git a/addons/mass_mailing/models/mass_mailing.py b/addons/mass_mailing/models/mass_mailing.py
index 48d5c6704d9..33c202a396b 100644
--- a/addons/mass_mailing/models/mass_mailing.py
+++ b/addons/mass_mailing/models/mass_mailing.py
@@ -25,7 +25,6 @@ MASS_MAILING_BUSINESS_MODELS = [
     'mail.mass_mailing.list',
     'mail.mass_mailing.contact'
 ]
-EMAIL_PATTERN = '([^ ,;<@]+@[^> ,;]+)'
 
 
 class MassMailingTag(models.Model):
@@ -115,16 +114,16 @@ class MassMailingList(models.Model):
             from
                 mail_mass_mailing_contact_list_rel r
                 left join mail_mass_mailing_contact c on (r.contact_id=c.id)
-                left join mail_blacklist bl on (LOWER(substring(c.email, %s)) = bl.email and bl.active)
+                left join mail_blacklist bl on c.email_normalized = bl.email and bl.active
             where
                 list_id in %s
-                COALESCE(r.opt_out,FALSE) = FALSE
-                AND c.email IS NOT NULL
+                AND COALESCE(r.opt_out,FALSE) = FALSE
+                AND c.email_normalized IS NOT NULL
                 AND bl.id IS NULL
             group by
                 list_id
-        ''')
-        data = dict(self.env.cr.fetchall(), [EMAIL_PATTERN, tuple(self.ids)])
+        ''', (tuple(self.ids), ))
+        data = dict(self.env.cr.fetchall())
         for mailing_list in self:
             mailing_list.contact_nbr = data.get(mailing_list.id, 0)
 
@@ -176,7 +175,7 @@ class MassMailingList(models.Model):
                     mail_mass_mailing_list mailing_list
                 WHERE contact.id=contact_list_rel.contact_id
                 AND COALESCE(contact_list_rel.opt_out,FALSE) = FALSE
-                AND LOWER(substring(contact.email, %s)) NOT IN (select email from mail_blacklist where active = TRUE)
+                AND contact.email_normalized NOT IN (select email from mail_blacklist where active = TRUE)
                 AND mailing_list.id=contact_list_rel.list_id
                 AND mailing_list.id IN %s
                 AND NOT EXISTS
@@ -190,7 +189,7 @@ class MassMailingList(models.Model):
                     AND contact_list_rel2.list_id = %s
                     )
                 ) st
-            WHERE st.rn = 1;""", (self.id, EMAIL_PATTERN, tuple(src_lists.ids), self.id))
+            WHERE st.rn = 1;""", (self.id, tuple(src_lists.ids), self.id))
         self.invalidate_cache()
         if archive:
             (src_lists - self).write({'active': False})
@@ -231,7 +230,8 @@ class MassMailingContact(models.Model):
     @api.depends('email')
     def _compute_is_email_valid(self):
         for record in self:
-            record.is_email_valid = re.match(EMAIL_PATTERN, record.email)
+            normalized = self._normalize_email(record.email)
+            record.is_email_valid = normalized if not normalized else True
 
     @api.model
     def _search_opt_out(self, operator, value):
diff --git a/addons/test_mass_mailing/models/mass_mail_test.py b/addons/test_mass_mailing/models/mass_mail_test.py
index 87e72497399..ac9d3eeb65a 100644
--- a/addons/test_mass_mailing/models/mass_mail_test.py
+++ b/addons/test_mass_mailing/models/mass_mail_test.py
@@ -9,7 +9,8 @@ class MassMailTest(models.Model):
     mailing features and base performances. """
     _description = 'Simple Mass Mailing Model'
     _name = 'mass.mail.test'
-    _inherit = ['mail.thread']
+    _inherit = ['mail.thread', 'mail.address.mixin']
+    _primary_email = ['email_from']
 
     name = fields.Char()
     email_from = fields.Char()
diff --git a/addons/test_mass_mailing/tests/test_performance.py b/addons/test_mass_mailing/tests/test_performance.py
index a25c7164202..8fc27fe8cfb 100644
--- a/addons/test_mass_mailing/tests/test_performance.py
+++ b/addons/test_mass_mailing/tests/test_performance.py
@@ -49,7 +49,7 @@ class TestMassMailPerformance(TestMassMailPerformanceBase):
             'mailing_domain': [('id', 'in', self.mm_recs.ids)],
         })
 
-        with self.assertQueryCount(__system__=2432, marketing=3088):
+        with self.assertQueryCount(__system__=2433, marketing=3089):
             mailing.send_mail()
 
         self.assertEqual(mailing.sent, 50)