From 0fda77e7772624091da1c56fca196e6def1b47ff Mon Sep 17 00:00:00 2001 From: Emanuel Cino <ecino@compassion.ch> Date: Wed, 12 Sep 2018 10:47:43 +0000 Subject: [PATCH] [FIX] base_geolocalize allow to use Openstreetmap instead of Google - Change API provider to prevent invalid API calls - Move geo_find methods into base.geocoder object to ease inheritance - Add system parameter to allow use of another provider fixes #x26924 fixes #x6929 --- addons/base_geolocalize/README.md | 22 +++ addons/base_geolocalize/__manifest__.py | 2 +- addons/base_geolocalize/models/__init__.py | 1 + .../base_geolocalize/models/base_geocoder.py | 136 ++++++++++++++++++ addons/base_geolocalize/models/res_partner.py | 82 +++-------- addons/base_geolocalize/tests/__init__.py | 3 + .../tests/test_geolocalize.py | 27 ++++ .../models/crm_lead.py | 19 ++- .../tests/test_partner_assign.py | 9 +- 9 files changed, 223 insertions(+), 78 deletions(-) create mode 100644 addons/base_geolocalize/README.md create mode 100644 addons/base_geolocalize/models/base_geocoder.py create mode 100644 addons/base_geolocalize/tests/__init__.py create mode 100644 addons/base_geolocalize/tests/test_geolocalize.py diff --git a/addons/base_geolocalize/README.md b/addons/base_geolocalize/README.md new file mode 100644 index 000000000000..7909b1a42852 --- /dev/null +++ b/addons/base_geolocalize/README.md @@ -0,0 +1,22 @@ +Partner geolocalize +=================== + +Contacts geolocation API to convert partner addresses into GPS coordinates. + +Configure +--------- +You can add a system parameter to change the default provider of the geolocation API service. + +* `base_geolocalize.provider = <service>` + +A method `_call_<service>` should be implemented in object `base.geocoder` that accepts an address string as parameter and return (latitude, longitude) tuple for this to work. +If no parameter is set, Openstreetmap will be used by default. + +An optional method `_geo_query_address_<service>` which takes address fields as parameters can be defined to encode the query string for the provider. + +Google Places +------------- +You can use Google Maps API if you have a valid apikey. In that case you should add the following system parameters: + +* `base_geolocalize.provider = google` +* `google.api_key_geocode = <your_api_key>` diff --git a/addons/base_geolocalize/__manifest__.py b/addons/base_geolocalize/__manifest__.py index c7d279cb43ee..d2c4ab38ad16 100644 --- a/addons/base_geolocalize/__manifest__.py +++ b/addons/base_geolocalize/__manifest__.py @@ -2,7 +2,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. { 'name': 'Partners Geolocation', - 'version': '2.0', + 'version': '2.1', 'category': 'Sales', 'description': """ Partners Geolocation diff --git a/addons/base_geolocalize/models/__init__.py b/addons/base_geolocalize/models/__init__.py index 3f2b7ae35b3a..bd7b8ee21e51 100644 --- a/addons/base_geolocalize/models/__init__.py +++ b/addons/base_geolocalize/models/__init__.py @@ -1,3 +1,4 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +from . import base_geocoder from . import res_partner diff --git a/addons/base_geolocalize/models/base_geocoder.py b/addons/base_geolocalize/models/base_geocoder.py new file mode 100644 index 000000000000..87751cd400d4 --- /dev/null +++ b/addons/base_geolocalize/models/base_geocoder.py @@ -0,0 +1,136 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. +import json +import urllib2 +import logging + +from odoo import api, models, tools, _ +from odoo.exceptions import UserError + + +_logger = logging.getLogger(__name__) + + +class GeoCoder(models.AbstractModel): + """ + Abstract class used to call Geolocalization API and convert addresses + into GPS coordinates. + """ + _name = "base.geocoder" + + @api.model + def geo_query_address(self, street=None, zip=None, city=None, state=None, country=None): + """ Converts address fields into a valid string for querying + geolocation APIs. + :param street: street address + :param zip: zip code + :param city: city + :param state: state + :param country: country + :return: formatted string + """ + provider = self.env['ir.config_parameter'].get_param('base_geolocalize.provider', 'openstreetmap') + if hasattr(self, '_geo_query_address_' + provider): + # Makes the transformation defined for provider + service = getattr(self, '_geo_query_address_' + provider) + return service(street, zip, city, state, country) + else: + # By default, join the non-empty parameters + return tools.ustr(', '.join(filter(None, [ + street, + ("%s %s" % (zip or '', city or '')).strip(), + state, + country]))) + + @api.model + def geo_find(self, addr): + """Use a location provider API to convert an address string into a latitude, longitude tuple. + Here we use Openstreetmap Nominatim by default. + :param addr: Address string passed to API + :return: (latitude, longitude) or None if not found + """ + provider = self.env['ir.config_parameter'].get_param( + 'base_geolocalize.provider', 'openstreetmap') + try: + service = getattr(self, '_call_' + provider) + result = service(addr) + except AttributeError: + raise UserError(_( + 'Provider %s is not implemented for geolocation service.' + ) % provider) + except UserError: + raise + except: + _logger.debug('Geolocalize call failed', exc_info=True) + result = None + return result + + @api.model + def _call_openstreetmap(self, addr): + """ + Use Openstreemap Nominatim service to retrieve location + :return: (latitude, longitude) or None if not found + """ + if not addr: + _logger.info('invalid address given') + return None + url = 'https://nominatim.openstreetmap.org/search?format=json&q=' + url += urllib2.quote(addr.encode('utf8')) + try: + _logger.info('openstreetmap nominatim service called') + result = json.load(urllib2.urlopen(url)) + except Exception as e: + self._raise_internet_access_error(e) + geo = result[0] + return float(geo['lat']), float(geo['lon']) + + @api.model + def _call_google(self, addr): + """ Use google maps API. It won't work without a valid API key. + :return: (latitude, longitude) or None if not found + """ + apikey = self.env['ir.config_parameter'].sudo().get_param( + 'google.api_key_geocode') + if not apikey: + raise UserError(_( + "API key for GeoCoding (Places) required.\n" + "Save this key in System Parameters with key: google.api_key_geocode, value: <your api key> Visit https://developers.google.com/maps/documentation/geocoding/get-api-key for more information." + )) + url = 'https://maps.googleapis.com/maps/api/geocode/json?key=%s&sensor=false&address=' % apikey + url += urllib2.quote(addr.encode('utf8')) + try: + result = json.load(urllib2.urlopen(url)) + except Exception as e: + self._raise_internet_access_error(e) + + try: + if result['status'] != 'OK': + _logger.debug('Invalid Gmaps call: %s - %s', + result['status'], result.get('error_message', '')) + return None + geo = result['results'][0]['geometry']['location'] + return float(geo['lat']), float(geo['lng']) + except (KeyError, ValueError): + _logger.debug('Unexpected Gmaps API answer %s', result.get('error_message', '')) + return None + + @api.model + def _geo_query_address_google(self, street=None, zip=None, city=None, + state=None, country=None): + # This may be useful if using GMaps API. + # put country qualifier in front, otherwise GMap gives wrong + # results, e.g. 'Congo, Democratic Republic of the' => + # 'Democratic Republic of the Congo' + if country and ',' in country and ( + country.endswith(' of') or country.endswith(' of the')): + country = '{1} {0}'.format(*country.split(',', 1)) + return tools.ustr(', '.join(filter(None, [ + street, + ("%s %s" % (zip or '', city or '')).strip(), + state, + country]))) + + def _raise_internet_access_error(self, error): + raise UserError(_( + 'Cannot contact geolocation servers. Please make sure that your Internet connection is up and running (%s).' + ) % error) diff --git a/addons/base_geolocalize/models/res_partner.py b/addons/base_geolocalize/models/res_partner.py index 3594e1cb61b0..bc1185d1b199 100644 --- a/addons/base_geolocalize/models/res_partner.py +++ b/addons/base_geolocalize/models/res_partner.py @@ -1,53 +1,4 @@ -# -*- coding: utf-8 -*- -# Part of Odoo. See LICENSE file for full copyright and licensing details. -import json -import logging - -import requests - -from odoo import api, fields, models, tools, _ -from odoo.exceptions import UserError - -_logger = logging.getLogger(__name__) - - -def geo_find(addr, apikey=False): - if not addr: - return None - - if not apikey: - raise UserError(_('''API key for GeoCoding (Places) required.\n - Save this key in System Parameters with key: google.api_key_geocode, value: <your api key> - Visit https://developers.google.com/maps/documentation/geocoding/get-api-key for more information. - ''')) - - url = "https://maps.googleapis.com/maps/api/geocode/json" - try: - result = requests.get(url, params={'sensor': 'false', 'address': addr, 'key': apikey}).json() - except Exception as e: - raise UserError(_('Cannot contact geolocation servers. Please make sure that your Internet connection is up and running (%s).') % e) - - if result['status'] != 'OK': - if result.get('error_message'): - _logger.error(result['error_message']) - return None - - try: - geo = result['results'][0]['geometry']['location'] - return float(geo['lat']), float(geo['lng']) - except (KeyError, ValueError): - return None - - -def geo_query_address(street=None, zip=None, city=None, state=None, country=None): - if country and ',' in country and (country.endswith(' of') or country.endswith(' of the')): - # put country qualifier in front, otherwise GMap gives wrong results, - # e.g. 'Congo, Democratic Republic of the' => 'Democratic Republic of the Congo' - country = '{1} {0}'.format(*country.split(',', 1)) - return tools.ustr(', '.join( - field for field in [street, ("%s %s" % (zip or '', city or '')).strip(), state, country] - if field - )) +from odoo import api, fields, models class ResPartner(models.Model): @@ -57,26 +8,25 @@ class ResPartner(models.Model): partner_longitude = fields.Float(string='Geo Longitude', digits=(16, 5)) date_localization = fields.Date(string='Geolocation Date') - @classmethod - def _geo_localize(cls, apikey, street='', zip='', city='', state='', country=''): - search = geo_query_address(street=street, zip=zip, city=city, state=state, country=country) - result = geo_find(search, apikey) - if result is None: - search = geo_query_address(city=city, state=state, country=country) - result = geo_find(search, apikey) - return result - @api.multi def geo_localize(self): # We need country names in English below - apikey = self.env['ir.config_parameter'].sudo().get_param('google.api_key_geocode') + geo_obj = self.env['base.geocoder'] for partner in self.with_context(lang='en_US'): - result = partner._geo_localize(apikey, - partner.street, - partner.zip, - partner.city, - partner.state_id.name, - partner.country_id.name) + result = geo_obj.geo_find(geo_obj.geo_query_address( + street=partner.street, + zip=partner.zip, + city=partner.city, + state=partner.state_id.name, + country=partner.country_id.name + )) + if result is None: + result = geo_obj.geo_find(geo_obj.geo_query_address( + city=partner.city, + state=partner.state_id.name, + country=partner.country_id.name + )) + if result: partner.write({ 'partner_latitude': result[0], diff --git a/addons/base_geolocalize/tests/__init__.py b/addons/base_geolocalize/tests/__init__.py new file mode 100644 index 000000000000..44d3f6100bc8 --- /dev/null +++ b/addons/base_geolocalize/tests/__init__.py @@ -0,0 +1,3 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. +from . import test_geolocalize diff --git a/addons/base_geolocalize/tests/test_geolocalize.py b/addons/base_geolocalize/tests/test_geolocalize.py new file mode 100644 index 000000000000..3bc7cb547053 --- /dev/null +++ b/addons/base_geolocalize/tests/test_geolocalize.py @@ -0,0 +1,27 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. +from odoo.tests import TransactionCase +from odoo.exceptions import UserError + + +class TestGeoLocalize(TransactionCase): + + def test_default_openstreetmap(self): + """ Test that openstreetmap localize service works. """ + test_partner = self.env.ref('base.res_partner_2') + test_partner.geo_localize() + self.assertTrue(test_partner.partner_longitude) + self.assertTrue(test_partner.partner_latitude) + self.assertTrue(test_partner.date_localization) + + def test_google_without_api_key(self): + """ Without providing API key to google maps, + the service doesn't work.""" + test_partner = self.env.ref('base.res_partner_address_4') + self.env['ir.config_parameter'].set_param('base_geolocalize.provider', + 'google') + with self.assertRaises(UserError): + test_partner.geo_localize() + self.assertFalse(test_partner.partner_longitude) + self.assertFalse(test_partner.partner_latitude) + self.assertFalse(test_partner.date_localization) diff --git a/addons/website_crm_partner_assign/models/crm_lead.py b/addons/website_crm_partner_assign/models/crm_lead.py index 215b96e94573..49cbc6fdb275 100644 --- a/addons/website_crm_partner_assign/models/crm_lead.py +++ b/addons/website_crm_partner_assign/models/crm_lead.py @@ -81,15 +81,26 @@ class CrmLead(models.Model): 'partner_longitude': longitude }) return True + geo_obj = self.env['base.geocoder'] # Don't pass context to browse()! We need country name in english below for lead in self: if lead.partner_latitude and lead.partner_longitude: continue if lead.country_id: - apikey = self.env['ir.config_parameter'].sudo().get_param('google.api_key_geocode') - result = self.env['res.partner']._geo_localize(apikey, - lead.street, lead.zip, lead.city, - lead.state_id.name, lead.country_id.name) + result = geo_obj.geo_find(geo_obj.geo_query_address( + street=lead.street, + zip=lead.zip, + city=lead.city, + state=lead.state_id.name, + country=lead.country_id.name)) + + if result is None: + result = geo_obj.geo_find(geo_obj.geo_query_address( + city=lead.city, + state=lead.state_id.name, + country=lead.country_id.name + )) + if result: lead.write({ 'partner_latitude': result[0], diff --git a/addons/website_crm_partner_assign/tests/test_partner_assign.py b/addons/website_crm_partner_assign/tests/test_partner_assign.py index 1945e6bd2e44..52ddbe427e1d 100644 --- a/addons/website_crm_partner_assign/tests/test_partner_assign.py +++ b/addons/website_crm_partner_assign/tests/test_partner_assign.py @@ -16,18 +16,13 @@ class TestPartnerAssign(TransactionCase): def setUp(self): super(TestPartnerAssign, self).setUp() - def geo_find(addr, apikey): + def geo_find(addr): return { 'Wavre, Belgium': (50.7158956, 4.6128075), 'Cannon Hill Park, B46 3AG Birmingham, United Kingdom': (52.45216, -1.898578), }.get(addr) - patcher = patch('odoo.addons.base_geolocalize.models.res_partner.geo_find', wraps=geo_find) - patcher.start() - self.addCleanup(patcher.stop) - - patcher = patch('odoo.addons.website_crm_partner_assign.models.crm_lead.geo_find', - wraps=geo_find) + patcher = patch('odoo.addons.base_geolocalize.models.base_geocoder.GeoCoder.geo_find', wraps=geo_find) patcher.start() self.addCleanup(patcher.stop) -- GitLab