From f001e026a2c64a786e2271f47cba4d68b5f9e816 Mon Sep 17 00:00:00 2001 From: Thibault Francois Date: Fri, 29 Jan 2021 14:39:42 +0000 Subject: [PATCH] [IMP] mail: improvement perf of find_or_create Behavior before the commit -------------------------- find_or_create define in mail module called super if no partner is found based on the email_normalized super call find_or_create define in base that make a search again on the email before the creation This cost two search every time a new partner should be created and the search on email is less efficient than the search on email_normalized after the commit ---------------- Only the search on email_normalized is done before the creation drawback: if a module that does not depends on mail module override find_or_create the code will not be triggered anymore the module should depends on mail module closes odoo/odoo#65565 X-original-commit: 49d5b3813e57f2cd91d582de9b9bb534f05d2dc3 Signed-off-by: Thibault Delavallee (tde) --- addons/mail/models/res_partner.py | 9 ++- addons/mail/tests/test_res_partner.py | 89 +++++++++++++++++++++++---- 2 files changed, 85 insertions(+), 13 deletions(-) diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py index f139e483d58..cdacd081db5 100644 --- a/addons/mail/models/res_partner.py +++ b/addons/mail/models/res_partner.py @@ -60,7 +60,14 @@ class Partner(models.Model): if partners: return partners - return super(Partner, self).find_or_create(email, assert_valid_email=assert_valid_email) + # We don't want to call `super()` to avoid searching twice on the email + # Especially when the search `email =ilike` cannot be as efficient as + # a search on email_normalized with a btree index + # If you want to override `find_or_create()` your module should depend on `mail` + create_values = {self._rec_name: parsed_name or parsed_email} + if parsed_email: # otherwise keep default_email in context + create_values['email'] = parsed_email + return self.create(create_values) def mail_partner_format(self): self.ensure_one() diff --git a/addons/mail/tests/test_res_partner.py b/addons/mail/tests/test_res_partner.py index de635930512..d2dffc92d2d 100644 --- a/addons/mail/tests/test_res_partner.py +++ b/addons/mail/tests/test_res_partner.py @@ -3,25 +3,90 @@ from odoo.tests.common import TransactionCase +# samples use effective TLDs from the Mozilla public suffix +# list at http://publicsuffix.org +SAMPLES = [ + ('"Raoul Grosbedon" ', 'Raoul Grosbedon', 'raoul@chirurgiens-dentistes.fr'), + ('ryu+giga-Sushi@aizubange.fukushima.jp', '', 'ryu+giga-Sushi@aizubange.fukushima.jp'), + ('Raoul chirurgiens-dentistes.fr', 'Raoul chirurgiens-dentistes.fr', ''), + (" Raoul O'hara ", "Raoul O'hara", '!@historicalsociety.museum'), + ('Raoul Grosbedon ', 'Raoul Grosbedon', 'raoul@CHIRURGIENS-dentistes.fr'), + ('Raoul megaraoul@chirurgiens-dentistes.fr', 'Raoul', 'megaraoul@chirurgiens-dentistes.fr'), + ('"Patrick Da Beast Poilvache" ', 'Patrick Poilvache', 'patrick@example.com'), + ('Patrick Caché ', 'Patrick Poilvache', 'patrick@example.com'), + ('Patrick Caché <2patrick@EXAMPLE.COM>', 'Patrick Caché', '2patrick@example.com'), + +] class TestPartner(TransactionCase): + def _check_find_or_create(self, test_string, expected_name, expected_email, expected_email_normalized=False, check_partner=False, should_create=False): + expected_email_normalized = expected_email_normalized or expected_email + partner = self.env['res.partner'].find_or_create(test_string) + if should_create and check_partner: + self.assertTrue(partner.id > check_partner.id, 'find_or_create failed - should have found existing') + elif check_partner: + self.assertEqual(partner, check_partner, 'find_or_create failed - should have found existing') + self.assertEqual(partner.name, expected_name) + self.assertEqual(partner.email or '', expected_email) + self.assertEqual(partner.email_normalized or '', expected_email_normalized) + return partner + def test_res_partner_find_or_create(self): Partner = self.env['res.partner'] + partner = Partner.browse(Partner.name_create(SAMPLES[0][0])[0]) + self._check_find_or_create( + SAMPLES[0][0], SAMPLES[0][1], SAMPLES[0][2], + check_partner=partner, should_create=False + ) + + partner_2 = Partner.browse(Partner.name_create('sarah.john@connor.com')[0]) + found_2 = self._check_find_or_create( + 'john@connor.com', 'john@connor.com', 'john@connor.com', + check_partner=partner_2, should_create=True + ) + + new = self._check_find_or_create( + SAMPLES[1][0], SAMPLES[1][2].lower(), SAMPLES[1][2].lower(), + check_partner=found_2, should_create=True + ) + + new2 = self._check_find_or_create( + SAMPLES[2][0], SAMPLES[2][1], SAMPLES[2][2], + check_partner=new, should_create=True + ) + + self._check_find_or_create( + SAMPLES[3][0], SAMPLES[3][1], SAMPLES[3][2], + check_partner=new2, should_create=True + ) + + new4 = self._check_find_or_create( + SAMPLES[4][0], SAMPLES[0][1], SAMPLES[0][2], + check_partner=partner, should_create=False + ) + + self._check_find_or_create( + SAMPLES[5][0], SAMPLES[5][1], SAMPLES[5][2], + check_partner=new4, should_create=True + ) + existing = Partner.create({ - 'name': 'Patrick Poilvache', - 'email': '"Patrick Da Beast Poilvache" ', + 'name': SAMPLES[6][1], + 'email': SAMPLES[6][0], }) - self.assertEqual(existing.name, 'Patrick Poilvache') - self.assertEqual(existing.email, '"Patrick Da Beast Poilvache" ') - self.assertEqual(existing.email_normalized, 'patrick@example.com') + self.assertEqual(existing.name, SAMPLES[6][1]) + self.assertEqual(existing.email, SAMPLES[6][0]) + self.assertEqual(existing.email_normalized, SAMPLES[6][2]) - new = Partner.find_or_create('Patrick Caché ') - self.assertEqual(new, existing) + new6 = self._check_find_or_create( + SAMPLES[7][0], SAMPLES[6][1], SAMPLES[6][0], + expected_email_normalized=SAMPLES[6][2], + check_partner=existing, should_create=False + ) - new2 = Partner.find_or_create('Patrick Caché <2patrick@EXAMPLE.COM>') - self.assertTrue(new2.id > new.id) - self.assertEqual(new2.name, 'Patrick Caché') - self.assertEqual(new2.email, '2patrick@example.com') - self.assertEqual(new2.email_normalized, '2patrick@example.com') + self._check_find_or_create( + SAMPLES[8][0], SAMPLES[8][1], SAMPLES[8][2], + check_partner=new6, should_create=True + )