partner_geolocalize_usability/README.rst

@@ -21,7 +21,25 @@ It also modify the partner form vieww for a better understanding of the geolocat
 Configuration
 =============
 
-No configuration needed.
+The OpenStreetMap Nominatim geocoding service requires a genuine, identifying
+``User-Agent`` header with a valid contact address (see the `Nominatim usage
+policy <https://operations.osmfoundation.org/policies/nominatim/>`_). The
+default Odoo ``User-Agent`` is shared by many instances and gets rate-limited or
+blocked (HTTP 429 "Too many requests").
+
+To fix this, set the system parameter (Settings > Technical > System
+Parameters):
+
+- **Key**: ``partner_geolocalize_usability.user_agent``
+- **Value**: a User-Agent identifying your deployment and a valid contact,
+  e.g. ``acme-crm/1.0 (contact: gis@acme.example)``
+
+The module ships this parameter with a placeholder value; you MUST replace it
+with your own, otherwise Nominatim will reject the requests.
+
+Requests to Nominatim are additionally throttled to one request per second to
+comply with the usage policy, so mass geolocation of many partners is slow by
+design (roughly one partner per second).
 
 Known issues / Roadmap
 ======================

partner_geolocalize_usability/__manifest__.py

@@ -17,10 +17,11 @@
     ],
     "qweb": [],
     "external_dependencies": {
-        "python": [],
+        "python": ["requests"],
     },
     # always loaded
     "data": [
+        "data/ir_config_parameter.xml",
         "views/res_partner.xml",
     ],
     # only loaded in demonstration mode

partner_geolocalize_usability/data/ir_config_parameter.xml

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<odoo>
+  <!--
+    User-Agent sent to the OpenStreetMap Nominatim geocoding service.
+
+    Nominatim's usage policy requires a genuine User-Agent identifying the
+    application and a valid contact address. The value below is a PLACEHOLDER:
+    each deployment MUST replace it with its own service name and contact,
+    e.g. "acme-crm/1.0 (contact: gis@acme.example)". Leaving the placeholder
+    will result in HTTP 429 (Too many requests) from Nominatim.
+  -->
+  <record id="param_nominatim_user_agent" model="ir.config_parameter">
+    <field name="key">partner_geolocalize_usability.user_agent</field>
+    <field name="value">SET-ME partner_geolocalize_usability (contact: you@example.invalid)</field>
+  </record>
+</odoo>

partner_geolocalize_usability/models/__init__.py

@@ -1,2 +1,3 @@
 
+from . import base_geocoder
 from . import res_partner

partner_geolocalize_usability/models/base_geocoder.py

@@ -0,0 +1,79 @@
+import logging
+import time
+
+import requests
+
+from odoo import api, models
+
+_logger = logging.getLogger(__name__)
+
+# System parameter holding the User-Agent sent to Nominatim.
+# Nominatim's usage policy requires a genuine, identifying User-Agent with a
+# valid contact. The stock Odoo User-Agent is shared by thousands of instances
+# and is rate-limited/blocked (HTTP 429), so we let each deployment set its own.
+USER_AGENT_PARAM = "partner_geolocalize_usability.user_agent"
+
+# Placeholder shown until the deployment sets a real value. It intentionally
+# does NOT identify any real service so an unconfigured instance fails loudly
+# rather than silently reusing a banned generic agent.
+USER_AGENT_PLACEHOLDER = (
+    "SET-ME partner_geolocalize_usability (contact: you@example.invalid)"
+)
+
+# Nominatim's public API allows at most 1 request per second. We wait slightly
+# more than a second between calls to stay safely under the limit.
+NOMINATIM_MIN_INTERVAL = 1.1
+
+
+class GeoCoder(models.AbstractModel):
+    _inherit = "base.geocoder"
+
+    def _get_nominatim_user_agent(self):
+        """Return the configured User-Agent for Nominatim requests.
+
+        Falls back to an obvious placeholder so a misconfigured instance is easy
+        to spot in logs (and so we never silently reuse Odoo's banned agent).
+        """
+        return (
+            self.env["ir.config_parameter"]
+            .sudo()
+            .get_param(USER_AGENT_PARAM, USER_AGENT_PLACEHOLDER)
+        )
+
+    @api.model
+    def _call_openstreetmap(self, addr, **kw):
+        """Query Nominatim with a configurable User-Agent and rate limiting.
+
+        Overrides the stock implementation to fix two problems that cause
+        HTTP 429 ("Too many requests"):
+
+        * the shared/generic Odoo User-Agent is blocked by Nominatim -> use a
+          deployment-specific one from a system parameter;
+        * bursts of requests exceed the 1 req/s policy -> throttle between calls.
+        """
+        if not addr:
+            _logger.info("invalid address given")
+            return None
+        url = "https://nominatim.openstreetmap.org/search"
+        try:
+            headers = {"User-Agent": self._get_nominatim_user_agent()}
+            # Respect Nominatim's 1 req/s policy. Sleeping before the call keeps
+            # the throttle effective even when callers loop over many records.
+            time.sleep(NOMINATIM_MIN_INTERVAL)
+            response = requests.get(
+                url, headers=headers, params={"format": "json", "q": addr}
+            )
+            _logger.info("openstreetmap nominatim service called")
+            if response.status_code != 200:
+                _logger.warning(
+                    "Request to openstreetmap failed.\nCode: %s\nContent: %s",
+                    response.status_code,
+                    response.content,
+                )
+            result = response.json()
+        except Exception as e:
+            self._raise_query_error(e)
+        if not result:
+            return None
+        geo = result[0]
+        return float(geo["lat"]), float(geo["lon"])

partner_geolocalize_usability/tests/__init__.py

@@ -0,0 +1 @@
+from . import test_base_geocoder

partner_geolocalize_usability/tests/test_base_geocoder.py

@@ -0,0 +1,92 @@
+from unittest.mock import MagicMock, patch
+
+from odoo.tests.common import TransactionCase
+
+from ..models.base_geocoder import (
+    NOMINATIM_MIN_INTERVAL,
+    USER_AGENT_PARAM,
+    USER_AGENT_PLACEHOLDER,
+)
+
+_MODULE = "odoo.addons.partner_geolocalize_usability.models.base_geocoder"
+
+
+def _fake_response(payload, status_code=200):
+    response = MagicMock()
+    response.status_code = status_code
+    response.json.return_value = payload
+    return response
+
+
+class TestBaseGeocoder(TransactionCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.geocoder = cls.env["base.geocoder"]
+        cls.Param = cls.env["ir.config_parameter"].sudo()
+
+    def test_uses_configured_user_agent(self):
+        # A deployment-specific User-Agent must be sent to Nominatim.
+        self.Param.set_param(USER_AGENT_PARAM, "acme/1.0 (contact: x@acme.test)")
+
+        with patch(f"{_MODULE}.time.sleep"), patch(
+            f"{_MODULE}.requests.get",
+            return_value=_fake_response([{"lat": "1.5", "lon": "2.5"}]),
+        ) as mocked_get:
+            result = self.geocoder._call_openstreetmap("some address")
+
+        self.assertEqual(result, (1.5, 2.5))
+        _, kwargs = mocked_get.call_args
+        self.assertEqual(
+            kwargs["headers"]["User-Agent"], "acme/1.0 (contact: x@acme.test)"
+        )
+
+    def test_falls_back_to_placeholder_user_agent(self):
+        # Without configuration, the obvious placeholder is used (never Odoo's
+        # banned generic agent).
+        self.Param.set_param(USER_AGENT_PARAM, False)
+
+        with patch(f"{_MODULE}.time.sleep"), patch(
+            f"{_MODULE}.requests.get",
+            return_value=_fake_response([{"lat": "1.0", "lon": "2.0"}]),
+        ) as mocked_get:
+            self.geocoder._call_openstreetmap("addr")
+
+        _, kwargs = mocked_get.call_args
+        self.assertEqual(
+            kwargs["headers"]["User-Agent"], USER_AGENT_PLACEHOLDER
+        )
+
+    def test_throttles_before_request(self):
+        # The 1 req/s policy is enforced by sleeping before each call.
+        self.Param.set_param(USER_AGENT_PARAM, "acme/1.0 (contact: x@acme.test)")
+
+        with patch(f"{_MODULE}.time.sleep") as mocked_sleep, patch(
+            f"{_MODULE}.requests.get",
+            return_value=_fake_response([{"lat": "1.0", "lon": "2.0"}]),
+        ):
+            self.geocoder._call_openstreetmap("addr")
+
+        mocked_sleep.assert_called_once_with(NOMINATIM_MIN_INTERVAL)
+
+    def test_empty_result_returns_none(self):
+        # An unknown address (empty Nominatim result) must not raise IndexError.
+        self.Param.set_param(USER_AGENT_PARAM, "acme/1.0 (contact: x@acme.test)")
+
+        with patch(f"{_MODULE}.time.sleep"), patch(
+            f"{_MODULE}.requests.get", return_value=_fake_response([])
+        ):
+            result = self.geocoder._call_openstreetmap("nowhere at all")
+
+        self.assertIsNone(result)
+
+    def test_blank_address_returns_none_without_request(self):
+        # Empty address short-circuits: no HTTP call, no throttle.
+        with patch(f"{_MODULE}.time.sleep") as mocked_sleep, patch(
+            f"{_MODULE}.requests.get"
+        ) as mocked_get:
+            result = self.geocoder._call_openstreetmap("")
+
+        self.assertIsNone(result)
+        mocked_get.assert_not_called()
+        mocked_sleep.assert_not_called()