[FIX+IMP] base_location: Re-import process

* Performance was very poor due to several reasons:

  * City name translatable.
  * Lack on indexes when deleting obsolete records.
  * Some prefetching.
* City was not being searched also by state, having the chance of returning
  incorrect matches if same city name on different states.
* Previous code was removing all previous records of zip entries!
* Remove possible obsolete FK keys after migrating from v11.

base_location/__manifest__.py

@@ -1,10 +1,10 @@
 # Copyright 2016 Nicolas Bessi, Camptocamp SA
-# Copyright 2018-2019 Tecnativa - Pedro M. Baeza
+# Copyright 2018-2020 Tecnativa - Pedro M. Baeza
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
 
 {
     'name': 'Location management (aka Better ZIP)',
-    'version': '12.0.1.0.3',
+    'version': '12.0.1.1.0',
     'depends': [
         'base_address_city',
         'contacts',

base_location/migrations/12.0.1.0.0/pre-migration.py

@@ -1,4 +1,4 @@
-# Copyright 2018 Tecnativa - Pedro M. Baeza
+# Copyright 2018-2020 Tecnativa - Pedro M. Baeza
 # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html).
 
 from openupgradelib import openupgrade
@@ -13,3 +13,4 @@ def migrate(env, version):
             ]
         }
     )
+    openupgrade.remove_tables_fks(env.cr, ['res_better_zip'])

base_location/models/res_city_zip.py

@@ -18,7 +18,9 @@ class ResCityZip(models.Model):
         'res.city',
         'City',
         required=True,
-        auto_join=True
+        auto_join=True,
+        ondelete="cascade",
+        index=True,
     )
     display_name = fields.Char(compute='_compute_new_display_name',
                                store=True, index=True)

base_location/models/res_partner.py

@@ -9,7 +9,8 @@ from odoo.exceptions import ValidationError
 class ResPartner(models.Model):
     _inherit = 'res.partner'
 
-    zip_id = fields.Many2one('res.city.zip', 'ZIP Location')
+    zip_id = fields.Many2one('res.city.zip', 'ZIP Location', index=True)
+    city_id = fields.Many2one(index=True)  # add index for performance
 
     @api.onchange('city_id')
     def _onchange_city_id(self):

base_location_geonames_import/__manifest__.py

@@ -1,14 +1,14 @@
 # Copyright 2014-2016 Akretion (Alexis de Lattre
 #                     <alexis.delattre@akretion.com>)
 # Copyright 2014 Lorenzo Battistini <lorenzo.battistini@agilebg.com>
-# Copyright 2016 Pedro M. Baeza <pedro.baeza@tecnativa.com>
+# Copyright 2016-2020 Tecnativa - Pedro M. Baeza
 # Copyright 2017 Eficent Business and IT Consulting Services, S.L.
 #                <contact@eficent.com>
 # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html).
 
 {
     'name': 'Base Location Geonames Import',
-    'version': '12.0.1.0.1',
+    'version': '12.0.1.0.2',
     'category': 'Partner Management',
     'license': 'AGPL-3',
     'summary': 'Import zip entries from Geonames',

base_location_geonames_import/wizard/geonames_import.py

@@ -4,7 +4,7 @@
 # Copyright 2017 Eficent Business and IT Consulting Services, S.L.
 #                <contact@eficent.com>
 # Copyright 2018 Aitor Bouzas <aitor.bouzas@adaptivecity.com>
-# Copyright 2016-2019 Tecnativa - Pedro M. Baeza
+# Copyright 2016-2020 Tecnativa - Pedro M. Baeza
 # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html).
 
 from odoo import _, api, fields, models
@@ -56,15 +56,10 @@ class CityZipGeonamesImport(models.TransientModel):
         return res
 
     @api.model
-    def _domain_search_res_city(self, row, country):
-        return [('name', '=', self.transform_city_name(row[2], country)),
-                ('country_id', '=', country.id)]
-
-    @api.model
-    def _domain_search_city_zip(self, row, res_city):
+    def _domain_search_city_zip(self, row, city_id=False):
         domain = [('name', '=', row[1])]
-        if res_city:
-            domain += [('city_id', '=', res_city.id)]
+        if city_id:
+            domain += [('city_id', '=', city_id)]
         return domain
 
     @api.model
@@ -76,16 +71,21 @@ class CityZipGeonamesImport(models.TransientModel):
         )
 
     @api.model
-    def select_city(self, row, country):
-        res_city_model = self.env['res.city']
-        return res_city_model.search(self._domain_search_res_city(
-            row, country), limit=1)
+    def select_city(self, row, country, state_id):
+        # This has to be done by SQL for performance reasons avoiding
+        # left join with ir_translation on the translatable field "name"
+        self.env.cr.execute(
+            "SELECT id, name FROM res_city "
+            "WHERE name = %s AND country_id = %s AND state_id = %s LIMIT 1",
+            (self.transform_city_name(row[2], country), country.id, state_id))
+        row = self.env.cr.fetchone()
+        return (row[0], row[1]) if row else (False, False)
 
     @api.model
-    def select_zip(self, row, country):
-        city = self.select_city(row, country)
+    def select_zip(self, row, country, state_id):
+        city_id, _ = self.select_city(row, country, state_id)
         return self.env['res.city.zip'].search(self._domain_search_city_zip(
-            row, city))
+            row, city_id))
 
     @api.model
     def prepare_state(self, row, country):
@@ -169,16 +169,19 @@ class CityZipGeonamesImport(models.TransientModel):
             if max_import and i == max_import:
                 break
             state_id = state_dict[row[self.code_row_index or 4]]
-            city = self.select_city(
-                row, self.country_id) if search_cities else False
-            if not city:
+            city_id, city_name = self.select_city(
+                row, self.country_id, state_id) if search_cities else False
+            if not city_id:
                 city_vals = self.prepare_city(
                     row, self.country_id, state_id)
                 if city_vals not in city_vals_list:
                     city_vals_list.append(city_vals)
             else:
-                city_dict[(city.name, state_id)] = city.id
-        created_cities = self.env['res.city'].create(city_vals_list)
+                city_dict[(city_name, state_id)] = city_id
+        ctx = dict(self.env.context)
+        ctx.pop('lang', None)  # make sure no translation is added
+        created_cities = self.env['res.city'].with_context(
+            ctx).create(city_vals_list)
         for i, vals in enumerate(city_vals_list):
             city_dict[(vals['name'], vals['state_id'])] = created_cities[i].id
         return city_dict
@@ -193,26 +196,23 @@ class CityZipGeonamesImport(models.TransientModel):
         state_model = self.env['res.country.state']
         zip_model = self.env['res.city.zip']
         res_city_model = self.env['res.city']
-
         # Store current record list
-        current_zips = zip_model.search(
-            [('city_id.country_id', '=', self.country_id.id)])
-        search_zips = True and len(current_zips) > 0 or False
-        current_cities = res_city_model.search(
-            [('country_id', '=', self.country_id.id)])
-        search_cities = True and len(current_cities) > 0 or False
+        old_zips = set(zip_model.search(
+            [('city_id.country_id', '=', self.country_id.id)]).ids)
+        search_zips = len(old_zips) > 0
+        old_cities = set(res_city_model.search(
+            [('country_id', '=', self.country_id.id)]).ids)
+        search_cities = len(old_cities) > 0
         current_states = state_model.search(
             [('country_id', '=', self.country_id.id)])
-        search_states = True and len(current_states) > 0 or False
-
+        search_states = len(current_states) > 0
         max_import = self.env.context.get('max_import', 0)
         logger.info('Starting to create the cities and/or city zip entries')
-
+        # Pre-create states and cities
         state_dict = self._create_states(parsed_csv,
                                          search_states, max_import)
         city_dict = self._create_cities(parsed_csv,
                                         search_cities, max_import, state_dict)
-
         # Zips
         zip_vals_list = []
         for i, row in enumerate(parsed_csv):
@@ -220,10 +220,10 @@ class CityZipGeonamesImport(models.TransientModel):
                 break
             # Don't search if there aren't any records
             zip_code = False
+            state_id = state_dict[row[self.code_row_index or 4]]
             if search_zips:
-                zip_code = self.select_zip(row, self.country_id)
+                zip_code = self.select_zip(row, self.country_id, state_id)
             if not zip_code:
-                state_id = state_dict[row[self.code_row_index or 4]]
                 city_id = city_dict[(
                     self.transform_city_name(row[2], self.country_id),
                     state_id,
@@ -231,28 +231,21 @@ class CityZipGeonamesImport(models.TransientModel):
                 zip_vals = self.prepare_zip(row, city_id)
                 if zip_vals not in zip_vals_list:
                     zip_vals_list.append(zip_vals)
-
-        delete_zips = self.env['res.city.zip'].create(zip_vals_list)
-        current_zips -= delete_zips
-
+            else:
+                old_zips.remove(zip_code.id)
+        self.env['res.city.zip'].create(zip_vals_list)
         if not max_import:
-            current_zips.unlink()
+            if old_zips:
+                logger.info('removing city zip entries')
+                self.env['res.city.zip'].browse(list(old_zips)).unlink()
                 logger.info('%d city zip entries deleted for country %s' %
-                        (len(current_zips), self.country_id.name))
-
-            # Since we wrapped the entire cities
-            # creation in a function we need
-            # to perform a search with city_dict in
-            # order to know which are the new ones so
-            # we can delete the old ones
-            created_cities = res_city_model.search(
-                [('country_id', '=', self.country_id.id),
-                 ('id', 'in', list(city_dict.values()))]
-            )
-            current_cities -= created_cities
-            current_cities.unlink()
+                            (len(old_zips), self.country_id.name))
+            old_cities -= set(city_dict.values())
+            if old_cities:
+                logger.info('removing city entries')
+                self.env['res.city'].browse(list(old_cities)).unlink()
                 logger.info('%d res.city entries deleted for country %s' %
-                        (len(current_cities), self.country_id.name))
+                            (len(old_cities), self.country_id.name))
         logger.info(
             'The wizard to create cities and/or city zip entries from '
             'geonames has been successfully completed.')