Merge pull request #418 from geopy/v2/callback-style

geopy 2.0: Convert `_call_geocoder` to callback style

geopy/geocoders/algolia.py

@@ -1,4 +1,5 @@
 import collections.abc
+from functools import partial
 from urllib.parse import urlencode
 
 from geopy.geocoders.base import DEFAULT_SENTINEL, Geocoder
@@ -201,11 +202,8 @@ def geocode(
             headers['X-Algolia-API-Key'] = self.api_key
 
         logger.debug('%s.geocode: %s', self.__class__.__name__, url)
-        return self._parse_json(
-            self._call_geocoder(url, headers=headers, timeout=timeout),
-            exactly_one,
-            language=language,
-        )
+        callback = partial(self._parse_json, exactly_one=exactly_one, language=language)
+        return self._call_geocoder(url, callback, headers=headers, timeout=timeout)
 
     def reverse(
             self,
@@ -264,11 +262,8 @@ def reverse(
             headers['X-Algolia-API-Key'] = self.api_key
 
         logger.debug("%s.reverse: %s", self.__class__.__name__, url)
-        return self._parse_json(
-            self._call_geocoder(url, headers=headers, timeout=timeout),
-            exactly_one,
-            language=language,
-        )
+        callback = partial(self._parse_json, exactly_one=exactly_one, language=language)
+        return self._call_geocoder(url, callback, headers=headers, timeout=timeout)
 
     def _parse_feature(self, feature, language):
         # Parse each resource.

geopy/geocoders/arcgis.py

@@ -1,4 +1,5 @@
 import json
+from functools import partial
 from time import time
 from urllib.parse import urlencode
 
@@ -7,7 +8,7 @@
     GeocoderAuthenticationFailure,
     GeocoderServiceError,
 )
-from geopy.geocoders.base import DEFAULT_SENTINEL, Geocoder
+from geopy.geocoders.base import DEFAULT_SENTINEL, Geocoder, _synchronized
 from geopy.location import Location
 from geopy.util import logger
 
@@ -24,7 +25,6 @@ class ArcGIS(Geocoder):
     """
 
     _TOKEN_EXPIRED = 498
-    _MAX_RETRIES = 3
 
     auth_path = '/sharing/generateToken'
     geocode_path = '/arcgis/rest/services/World/GeocodeServer/findAddressCandidates'
@@ -110,8 +110,6 @@ def __init__(
                 raise ConfigurationError(
                     "Authenticated mode requires scheme of 'https'"
                 )
-            self._base_call_geocoder = self._call_geocoder
-            self._call_geocoder = self._authenticated_call_geocoder
 
         self.username = username
         self.password = password
@@ -121,10 +119,7 @@ def __init__(
             '%s://%s%s' % (self.scheme, self.auth_domain, self.auth_path)
         )
 
-        self.token = None
         self.token_lifetime = token_lifetime * 60  # store in seconds
-        self.token_expiry = None
-        self.retry = 1
 
         self.domain = domain.strip('/')
         self.api = (
@@ -134,15 +129,9 @@ def __init__(
             '%s://%s%s' % (self.scheme, self.domain, self.reverse_path)
         )
 
-    def _authenticated_call_geocoder(self, url, timeout=DEFAULT_SENTINEL):
-        """
-        Wrap self._call_geocoder, handling tokens.
-        """
-        if self.token is None or int(time()) > self.token_expiry:
-            self._refresh_authentication_token()
-        url = "&".join((url, urlencode({"token": self.token})))
-        headers = {"Referer": self.referer}
-        return self._base_call_geocoder(url, timeout=timeout, headers=headers)
+        # Mutable state
+        self.token = None
+        self.token_expiry = None
 
     def geocode(self, query, *, exactly_one=True, timeout=DEFAULT_SENTINEL,
                 out_fields=None):
@@ -180,16 +169,11 @@ def geocode(self, query, *, exactly_one=True, timeout=DEFAULT_SENTINEL,
                 params['outFields'] = ",".join(out_fields)
         url = "?".join((self.api, urlencode(params)))
         logger.debug("%s.geocode: %s", self.__class__.__name__, url)
-        response = self._call_geocoder(url, timeout=timeout)
+        callback = partial(self._parse_geocode, exactly_one=exactly_one)
+        return self._authenticated_call_geocoder(url, callback, timeout=timeout)
 
-        # Handle any errors; recursing in the case of an expired token.
+    def _parse_geocode(self, response, exactly_one):
         if 'error' in response:
-            if response['error']['code'] == self._TOKEN_EXPIRED:
-                self.retry += 1
-                self._refresh_authentication_token()
-                return self.geocode(
-                    query, exactly_one=exactly_one, timeout=timeout
-                )
             raise GeocoderServiceError(str(response['error']))
 
         # Success; convert from the ArcGIS JSON format.
@@ -239,16 +223,13 @@ def reverse(self, query, *, exactly_one=True, timeout=DEFAULT_SENTINEL,
             params['distance'] = distance
         url = "?".join((self.reverse_api, urlencode(params)))
         logger.debug("%s.reverse: %s", self.__class__.__name__, url)
-        response = self._call_geocoder(url, timeout=timeout)
+        callback = partial(self._parse_reverse, exactly_one=exactly_one)
+        return self._authenticated_call_geocoder(url, callback, timeout=timeout)
+
+    def _parse_reverse(self, response, exactly_one):
         if not len(response):
             return None
         if 'error' in response:
-            if response['error']['code'] == self._TOKEN_EXPIRED:
-                self.retry += 1
-                self._refresh_authentication_token()
-                return self.reverse(query, exactly_one=exactly_one,
-                                    timeout=timeout, distance=distance,
-                                    wkid=wkid)
             # https://developers.arcgis.com/rest/geocode/api-reference/geocoding-service-output.htm
             if response['error']['code'] == 400:
                 # 'details': ['Unable to find address for the specified location.']}
@@ -272,14 +253,50 @@ def reverse(self, query, *, exactly_one=True, timeout=DEFAULT_SENTINEL,
         else:
             return [location]
 
-    def _refresh_authentication_token(self):
-        """
-        POST to ArcGIS requesting a new token.
-        """
-        if self.retry == self._MAX_RETRIES:
-            raise GeocoderAuthenticationFailure(
-                'Too many retries for auth: %s' % self.retry
+    def _authenticated_call_geocoder(
+        self, url, parse_callback, *, timeout=DEFAULT_SENTINEL
+    ):
+        if not self.username:
+            return self._call_geocoder(url, parse_callback, timeout=timeout)
+
+        def query_callback():
+            call_url = "&".join((url, urlencode({"token": self.token})))
+            headers = {"Referer": self.referer}
+            return self._call_geocoder(
+                call_url,
+                partial(maybe_reauthenticate_callback, from_token=self.token),
+                timeout=timeout,
+                headers=headers,
             )
+
+        def maybe_reauthenticate_callback(response, *, from_token):
+            if "error" in response:
+                if response["error"]["code"] == self._TOKEN_EXPIRED:
+                    return self._refresh_authentication_token(
+                        query_retry_callback, timeout=timeout, from_token=from_token
+                    )
+            return parse_callback(response)
+
+        def query_retry_callback():
+            call_url = "&".join((url, urlencode({"token": self.token})))
+            headers = {"Referer": self.referer}
+            return self._call_geocoder(
+                call_url, parse_callback, timeout=timeout, headers=headers
+            )
+
+        if self.token is None or int(time()) > self.token_expiry:
+            return self._refresh_authentication_token(
+                query_callback, timeout=timeout, from_token=self.token
+            )
+        else:
+            return query_callback()
+
+    @_synchronized
+    def _refresh_authentication_token(self, callback_success, *, timeout, from_token):
+        if from_token != self.token:
+            # Token has already been updated by a concurrent call.
+            return callback_success()
+
         token_request_arguments = {
             'username': self.username,
             'password': self.password,
@@ -292,13 +309,15 @@ def _refresh_authentication_token(self):
             "%s._refresh_authentication_token: %s",
             self.__class__.__name__, url
         )
-        self.token_expiry = int(time()) + self.token_lifetime
-        response = self._base_call_geocoder(url)
-        if 'token' not in response:
-            raise GeocoderAuthenticationFailure(
-                'Missing token in auth request.'
-                'Request URL: %s; response JSON: %s' %
-                (url, json.dumps(response))
-            )
-        self.retry = 0
-        self.token = response['token']
+
+        def cb(response):
+            if "token" not in response:
+                raise GeocoderAuthenticationFailure(
+                    "Missing token in auth request."
+                    "Request URL: %s; response JSON: %s" % (url, json.dumps(response))
+                )
+            self.token = response["token"]
+            self.token_expiry = int(time()) + self.token_lifetime
+            return callback_success()
+
+        return self._call_geocoder(url, cb, timeout=timeout)

geopy/geocoders/baidu.py

@@ -1,4 +1,5 @@
 import hashlib
+from functools import partial
 from urllib.parse import quote_plus, urlencode
 
 from geopy.exc import (
@@ -125,9 +126,8 @@ def geocode(
         url = self._construct_url(self.api, self.api_path, params)
 
         logger.debug("%s.geocode: %s", self.__class__.__name__, url)
-        return self._parse_json(
-            self._call_geocoder(url, timeout=timeout), exactly_one=exactly_one
-        )
+        callback = partial(self._parse_json, exactly_one=exactly_one)
+        return self._call_geocoder(url, callback, timeout=timeout)
 
     def reverse(self, query, *, exactly_one=True, timeout=DEFAULT_SENTINEL):
         """
@@ -159,9 +159,8 @@ def reverse(self, query, *, exactly_one=True, timeout=DEFAULT_SENTINEL):
         url = self._construct_url(self.reverse_api, self.reverse_path, params)
 
         logger.debug("%s.reverse: %s", self.__class__.__name__, url)
-        return self._parse_reverse_json(
-            self._call_geocoder(url, timeout=timeout), exactly_one=exactly_one
-        )
+        callback = partial(self._parse_reverse_json, exactly_one=exactly_one)
+        return self._call_geocoder(url, callback, timeout=timeout)
 
     def _parse_reverse_json(self, page, exactly_one=True):
         """

geopy/geocoders/banfrance.py

@@ -1,3 +1,4 @@
+from functools import partial
 from urllib.parse import urlencode
 
 from geopy.geocoders.base import DEFAULT_SENTINEL, Geocoder
@@ -113,9 +114,8 @@ def geocode(
         url = "?".join((self.geocode_api, urlencode(params)))
 
         logger.debug("%s.geocode: %s", self.__class__.__name__, url)
-        return self._parse_json(
-            self._call_geocoder(url, timeout=timeout), exactly_one
-        )
+        callback = partial(self._parse_json, exactly_one=exactly_one)
+        return self._call_geocoder(url, callback, timeout=timeout)
 
     def reverse(
             self,
@@ -157,9 +157,8 @@ def reverse(
 
         url = "?".join((self.reverse_api, urlencode(params)))
         logger.debug("%s.reverse: %s", self.__class__.__name__, url)
-        return self._parse_json(
-            self._call_geocoder(url, timeout=timeout), exactly_one
-        )
+        callback = partial(self._parse_json, exactly_one=exactly_one)
+        return self._call_geocoder(url, callback, timeout=timeout)
 
     def _parse_feature(self, feature):
         # Parse each resource.

geopy/geocoders/base.py

@@ -1,3 +1,6 @@
+import functools
+import threading
+
 from geopy.adapters import AdapterHTTPError, RequestsAdapter, URLLibAdapter
 from geopy.exc import (
     ConfigurationError,
@@ -284,6 +287,7 @@ def _geocoder_exception_handler(self, error):
     def _call_geocoder(
             self,
             url,
+            callback,
             *,
             timeout=DEFAULT_SENTINEL,
             is_json=True,
@@ -302,9 +306,10 @@ def _call_geocoder(
 
         try:
             if is_json:
-                return self.adapter.get_json(url, timeout=timeout, headers=req_headers)
+                result = self.adapter.get_json(url, timeout=timeout, headers=req_headers)
             else:
-                return self.adapter.get_text(url, timeout=timeout, headers=req_headers)
+                result = self.adapter.get_text(url, timeout=timeout, headers=req_headers)
+            return callback(result)
         except AdapterHTTPError as error:
             if error.text:
                 logger.info(
@@ -325,3 +330,19 @@ def _call_geocoder(
 
     # def reverse(self, query, *, exactly_one=True, timeout=DEFAULT_SENTINEL):
     #     raise NotImplementedError()
+
+
+def _synchronized(func):
+    """A decorator for geocoder methods which makes the method always run
+    under a lock. The lock is reentrant.
+
+    This decorator transparently handles sync and async working modes.
+    """
+    lock = threading.RLock()
+
+    @functools.wraps(func)
+    def f(self, *args, **kwargs):
+        with lock:
+            return func(self, *args, **kwargs)
+
+    return f

geopy/geocoders/bing.py

@@ -1,4 +1,5 @@
 import collections.abc
+from functools import partial
 from urllib.parse import quote, urlencode
 
 from geopy.exc import (
@@ -156,10 +157,8 @@ def geocode(
 
         url = "?".join((self.geocode_api, urlencode(params)))
         logger.debug("%s.geocode: %s", self.__class__.__name__, url)
-        return self._parse_json(
-            self._call_geocoder(url, timeout=timeout),
-            exactly_one
-        )
+        callback = partial(self._parse_json, exactly_one=exactly_one)
+        return self._call_geocoder(url, callback, timeout=timeout)
 
     def reverse(
             self,
@@ -208,10 +207,8 @@ def reverse(
                         urlencode(params)))
 
         logger.debug("%s.reverse: %s", self.__class__.__name__, url)
-        return self._parse_json(
-            self._call_geocoder(url, timeout=timeout),
-            exactly_one
-        )
+        callback = partial(self._parse_json, exactly_one=exactly_one)
+        return self._call_geocoder(url, callback, timeout=timeout)
 
     def _parse_json(self, doc, exactly_one=True):
         """

geopy/geocoders/databc.py

@@ -1,3 +1,4 @@
+from functools import partial
 from urllib.parse import urlencode
 
 from geopy.exc import GeocoderQueryError
@@ -119,8 +120,10 @@ def geocode(
 
         url = "?".join((self.api, urlencode(params)))
         logger.debug("%s.geocode: %s", self.__class__.__name__, url)
-        response = self._call_geocoder(url, timeout=timeout)
+        callback = partial(self._parse_json, exactly_one=exactly_one)
+        return self._call_geocoder(url, callback, timeout=timeout)
 
+    def _parse_json(self, response, exactly_one):
         # Success; convert from GeoJSON
         if not len(response['features']):
             return None