[FIX] base, base_address_extended: optimisation of partners import

If bulk-importing clients with a parent_id set, there's a ton of extra / post-processing work which is not batched and "de-optimises" the batching added to import. High-level comparison of performing a "Test Import" on a synthetic test file of 10000 partners (all with a parent_id set to a pre-existing record) went from ~30mn to ~8mn on my machine. Fixes #31549 closes odoo/odoo#31876 Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
2019-04-04 09:28:12 +02:00
parent 27ea18a21b 6c2b54ff9c
commit 251021796c
3 changed files with 123 additions and 22 deletions
@@ -33,11 +33,6 @@ class Partner(models.Model):
    street_number2 = fields.Char('Door', compute='_split_street', help="Door Number",
                                 inverse='_set_street', store=True)

-    @api.model
-    def _address_fields(self):
-        """Returns the list of address fields that are synced from the parent."""
-        return super(Partner, self)._address_fields() + ['street_name', 'street_number', 'street_number2']
-
    def get_street_fields(self):
        """Returns the fields that can be used in a street format.
        Overwrite this function if you want to add your own fields."""
@@ -144,7 +139,7 @@ class Partner(models.Model):

    def write(self, vals):
        res = super(Partner, self).write(vals)
-        if 'country_id' in vals:
+        if 'country_id' in vals and 'street' not in vals:
            self._set_street()
        return res

@@ -2,6 +2,7 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.

 import base64
+import collections
 import datetime
 import hashlib
 import pytz
@@ -274,8 +275,32 @@ class Partner(models.Model):

    @api.depends('is_company', 'parent_id.commercial_partner_id')
    def _compute_commercial_partner(self):
+        self.env.cr.execute("""
+        WITH RECURSIVE cpid(id, parent_id, commercial_partner_id, final) AS (
+            SELECT
+                id, parent_id, id,
+                (coalesce(is_company, false) OR parent_id IS NULL) as final
+            FROM res_partner
+            WHERE id = ANY(%s)
+        UNION
+            SELECT
+                cpid.id, p.parent_id, p.id,
+                (coalesce(is_company, false) OR p.parent_id IS NULL) as final
+            FROM res_partner p
+            JOIN cpid ON (cpid.parent_id = p.id)
+            WHERE NOT cpid.final
+        )
+        SELECT cpid.id, cpid.commercial_partner_id
+        FROM cpid
+        WHERE final AND id = ANY(%s);
+        """, [self.ids, self.ids])
+
+        d = dict(self.env.cr.fetchall())
        for partner in self:
-            if partner.is_company or not partner.parent_id:
+            fetched = d.get(partner.id)
+            if fetched is not None:
+                partner.commercial_partner_id = fetched
+            elif partner.is_company or not partner.parent_id:
                partner.commercial_partner_id = partner
            else:
                partner.commercial_partner_id = partner.parent_id.commercial_partner_id
@@ -469,21 +494,25 @@ class Partner(models.Model):
                self.update_address(onchange_vals)

        # 2. To DOWNSTREAM: sync children
-        if self.child_ids:
-            # 2a. Commercial Fields: sync if commercial entity
-            if self.commercial_partner_id == self:
-                commercial_fields = self._commercial_fields()
-                if any(field in values for field in commercial_fields):
-                    self._commercial_sync_to_children()
-            for child in self.child_ids.filtered(lambda c: not c.is_company):
-                if child.commercial_partner_id != self.commercial_partner_id :
-                    self._commercial_sync_to_children()
-                    break
-            # 2b. Address fields: sync if address changed
-            address_fields = self._address_fields()
-            if any(field in values for field in address_fields):
-                contacts = self.child_ids.filtered(lambda c: c.type == 'contact')
-                contacts.update_address(values)
+        self._children_sync(values)
+
+    def _children_sync(self, values):
+        if not self.child_ids:
+            return
+        # 2a. Commercial Fields: sync if commercial entity
+        if self.commercial_partner_id == self:
+            commercial_fields = self._commercial_fields()
+            if any(field in values for field in commercial_fields):
+                self._commercial_sync_to_children()
+        for child in self.child_ids.filtered(lambda c: not c.is_company):
+            if child.commercial_partner_id != self.commercial_partner_id:
+                self._commercial_sync_to_children()
+                break
+        # 2b. Address fields: sync if address changed
+        address_fields = self._address_fields()
+        if any(field in values for field in address_fields):
+            contacts = self.child_ids.filtered(lambda c: c.type == 'contact')
+            contacts.update_address(values)

    @api.multi
    def _handle_first_contact_creation(self):
@@ -556,11 +585,53 @@ class Partner(models.Model):
                vals['image'] = self._get_default_image(vals.get('type'), vals.get('is_company'), vals.get('parent_id'))
            tools.image_resize_images(vals, sizes={'image': (1024, None)})
        partners = super(Partner, self).create(vals_list)
+
+        if self.env.context.get('_partners_skip_fields_sync'):
+            return partners
+
        for partner, vals in pycompat.izip(partners, vals_list):
            partner._fields_sync(vals)
            partner._handle_first_contact_creation()
        return partners

+    def _load_records_create(self, vals_list):
+        partners = super(Partner, self.with_context(_partners_skip_fields_sync=True))._load_records_create(vals_list)
+
+        # batch up first part of _fields_sync
+        # group partners by commercial_partner_id (if not self) and parent_id (if type == contact)
+        groups = collections.defaultdict(list)
+        for partner, vals in pycompat.izip(partners, vals_list):
+            cp_id = None
+            if vals.get('parent_id') and partner.commercial_partner_id != partner:
+                cp_id = partner.commercial_partner_id.id
+
+            add_id = None
+            if partner.parent_id and partner.type == 'contact':
+                add_id = partner.parent_id.id
+            groups[(cp_id, add_id)].append(partner.id)
+
+        for (cp_id, add_id), children in groups.items():
+            # values from parents (commercial, regular) written to their common children
+            to_write = {} 
+            # commercial fields from commercial partner
+            if cp_id:
+                to_write = self.browse(cp_id)._update_fields_values(self._commercial_fields())
+            # address fields from parent
+            if add_id:
+                parent = self.browse(add_id)
+                for f in self._address_fields():
+                    v = parent[f]
+                    if v:
+                        to_write[f] = v.id if isinstance(v, models.BaseModel) else v
+            if to_write:
+                self.browse(children).write(to_write)
+
+        # do the second half of _fields_sync the "normal" way
+        for partner, vals in pycompat.izip(partners, vals_list):
+            partner._children_sync(vals)
+            partner._handle_first_contact_creation()
+        return partners
+
    @api.multi
    def create_company(self):
        self.ensure_one()
@@ -252,6 +252,41 @@ class TestBase(TransactionCase):
        self.assertEqual(leaf111.address_get([]),
                        {'contact': branch11.id}, 'Invalid address resolution, branch11 should now be contact')

+    def test_commercial_partner_nullcompany(self):
+        """ The commercial partner is the first/nearest ancestor-or-self which
+        is a company or doesn't have a parent
+        """
+        P = self.env['res.partner']
+        p0 = P.create({'name': '0', 'email': '0'})
+        self.assertEqual(p0.commercial_partner_id, p0, "partner without a parent is their own commercial partner")
+
+        p1 = P.create({'name': '1', 'email': '1', 'parent_id': p0.id})
+        self.assertEqual(p1.commercial_partner_id, p0, "partner's parent is their commercial partner")
+        p12 = P.create({'name': '12', 'email': '12', 'parent_id': p1.id})
+        self.assertEqual(p12.commercial_partner_id, p0, "partner's GP is their commercial partner")
+
+        p2 = P.create({'name': '2', 'email': '2', 'parent_id': p0.id, 'is_company': True})
+        self.assertEqual(p2.commercial_partner_id, p2, "partner flagged as company is their own commercial partner")
+        p21 = P.create({'name': '21', 'email': '21', 'parent_id': p2.id})
+        self.assertEqual(p21.commercial_partner_id, p2, "commercial partner is closest ancestor with themselves as commercial partner")
+
+        p3 = P.create({'name': '3', 'email': '3', 'is_company': True})
+        self.assertEqual(p3.commercial_partner_id, p3, "being both parent-less and company should be the same as either")
+
+        notcompanies = p0 | p1 | p12 | p21
+        self.env.cr.execute('update res_partner set is_company=null where id = any(%s)', [notcompanies.ids])
+        for parent in notcompanies:
+            p = P.create({
+                'name': parent.name + '_sub',
+                'email': parent.email + '_sub',
+                'parent_id': parent.id,
+            })
+            self.assertEqual(
+                p.commercial_partner_id,
+                parent.commercial_partner_id,
+                "check that is_company=null is properly handled when looking for ancestor"
+            )
+
    def test_50_res_partner_commercial_sync(self):
        res_partner = self.env['res.partner']
        p0 = res_partner.create({'name': 'Sigurd Sunknife',