Merge pull request #441 from CartoDB/development

Release python library 0.16.3

NEWS.md

@@ -1,3 +1,11 @@
+January 31th, 2018
+==================
+* Version `0.16.3` of the python library
+    * Fix for Mapbox geocoder to handle empty requests and empty responses
+    * Remove raising an exception when non parameters are passed to the HERE geocoder
+    * Fix for HERE geocoder with non empty requests
+    * Added more coverage to the google geocoder credentials parse logic
+
 January 29th, 2018
 ==================
 * Version `0.30.1` of server side

README.md

@@ -296,3 +296,6 @@ ALTER ROLE "<USER_ROLE>" SET search_path="$user", public, cartodb, cdb_dataservi
 #### Option 2 (from builder)
 
 See [the **Configuring Dataservices** documentation](http://cartodb.readthedocs.io/en/latest/operations/configure_data_services.html)
+
+### Rate limits
+See [docs](https://github.com/CartoDB/dataservices-api/blob/master/doc/rate_limits.md)

server/lib/python/cartodb_services/cartodb_services/here/geocoder.py

@@ -67,10 +67,10 @@ class HereMapsGeocoder(Traceable):
     def geocode(self, **kwargs):
         params = {}
         for key, value in kwargs.iteritems():
-            if value:
+            if value and value.strip():
                 params[key] = value
         if not params:
-            raise NoGeocodingParams()
+            return []
         return self._execute_geocode(params)
 
     def _execute_geocode(self, params):

server/lib/python/cartodb_services/cartodb_services/mapbox/geocoder.py

@@ -39,7 +39,7 @@ class MapboxGeocoder(Traceable):
     def _parse_geocoder_response(self, response):
         json_response = json.loads(response)
 
-        if json_response:
+        if json_response and json_response[ENTRY_FEATURES]:
             feature = json_response[ENTRY_FEATURES][0]
 
             return self._extract_lng_lat_from_feature(feature)
@@ -60,11 +60,14 @@ class MapboxGeocoder(Traceable):
     @qps_retry(qps=10)
     def geocode(self, searchtext, city=None, state_province=None,
                 country=None):
-        address = [searchtext]
+        if searchtext and searchtext.strip():
-        if city:
+            address = [searchtext]
-            address.append(city)
+            if city:
-        if state_province:
+                address.append(city)
-            address.append(state_province)
+            if state_province:
+                address.append(state_province)
+        else:
+            return []
 
         country = [country] if country else None
 

server/lib/python/cartodb_services/setup.py

@@ -10,7 +10,7 @@ from setuptools import setup, find_packages
 setup(
     name='cartodb_services',
 
-    version='0.16.2',
+    version='0.16.3',
 
     description='CartoDB Services API Python Library',
 

server/lib/python/cartodb_services/test/test_google_client_factory.py

@@ -18,10 +18,10 @@ class GoogleMapsClientFactoryTestCase(unittest.TestCase):
         GoogleMapsClientFactory.clients = {}
 
     def test_consecutive_calls_with_same_params_return_same_client(self):
-        id = 'any_id'
+        client_id = 'any_id'
         key = base64.b64encode('any_key')
-        client1 = GoogleMapsClientFactory.get(id, key)
+        client1 = GoogleMapsClientFactory.get(client_id, key)
-        client2 = GoogleMapsClientFactory.get(id, key)
+        client2 = GoogleMapsClientFactory.get(client_id, key)
         self.assertEqual(client1, client2)
 
     def test_consecutive_calls_with_different_key_return_different_clients(self):
@@ -29,11 +29,11 @@ class GoogleMapsClientFactoryTestCase(unittest.TestCase):
         This requirement is important for security reasons as well as not to
         cache a wrong key accidentally.
         """
-        id = 'any_id'
+        client_id = 'any_id'
         key1 = base64.b64encode('any_key')
         key2 = base64.b64encode('another_key')
-        client1 = GoogleMapsClientFactory.get(id, key1)
+        client1 = GoogleMapsClientFactory.get(client_id, key1)
-        client2 = GoogleMapsClientFactory.get(id, key2)
+        client2 = GoogleMapsClientFactory.get(client_id, key2)
         self.assertNotEqual(client1, client2)
 
     def test_consecutive_calls_with_different_ids_return_different_clients(self):
@@ -59,3 +59,11 @@ class GoogleMapsClientFactoryTestCase(unittest.TestCase):
     def test_credentials_with_underscores_can_be_valid(self):
         client = GoogleMapsClientFactory.get('yet_another_dummy_client_id', 'Ola_k_ase___')
         self.assertIsInstance(client, googlemaps.Client)
+
+    def test_invalid_credentials(self):
+        with self.assertRaises(InvalidGoogleCredentials):
+            GoogleMapsClientFactory.get('dummy_client_id', 'lalala')
+
+    def test_credentials_with_channel(self):
+        client = GoogleMapsClientFactory.get('yet_another_dummy_client_id', 'Ola_k_ase___', 'channel')
+        self.assertIsInstance(client, googlemaps.Client)

server/lib/python/cartodb_services/test/test_googlegeocoder.py

@@ -118,3 +118,18 @@ class GoogleGeocoderTestCase(unittest.TestCase):
                 searchtext='Calle Eloy Gonzalo 27',
                 city='Madrid',
                 country='España')
+
+    def test_client_data_extraction(self, req_mock):
+        client_id, channel = self.geocoder.parse_client_id('dummy_client_id')
+        self.assertEqual(client_id, 'dummy_client_id')
+        self.assertEqual(channel, None)
+
+    def test_client_data_extraction_with_client_parameter(self, req_mock):
+        client_id, channel = self.geocoder.parse_client_id('client=gme-test')
+        self.assertEqual(client_id, 'gme-test')
+        self.assertEqual(channel, None)
+
+    def test_client_data_extraction_with_client_and_channel_parameter(self, req_mock):
+        client_id, channel = self.geocoder.parse_client_id('client=gme-test&channel=testchannel')
+        self.assertEqual(client_id, 'gme-test')
+        self.assertEqual(channel, 'testchannel')

server/lib/python/cartodb_services/test/test_heremapsgeocoder.py

@@ -128,8 +128,14 @@ class HereMapsGeocoderTestCase(unittest.TestCase):
     def test_geocode_address_with_no_params(self, req_mock):
         req_mock.register_uri('GET', HereMapsGeocoder.PRODUCTION_GEOCODE_JSON_URL,
                        text=self.GOOD_RESPONSE)
-        with self.assertRaises(NoGeocodingParams):
+        result = self.geocoder.geocode()
-            self.geocoder.geocode()
+        self.assertEqual(result, [])
+
+    def test_geocode_address_with_non_empty_string_params(self, req_mock):
+        req_mock.register_uri('GET', HereMapsGeocoder.PRODUCTION_GEOCODE_JSON_URL,
+                       text=self.GOOD_RESPONSE)
+        result = self.geocoder.geocode(searchtext=" ", city=None, state=" ", country=" ")
+        self.assertEqual(result, [])
 
     def test_geocode_address_empty_response(self, req_mock):
         req_mock.register_uri('GET', HereMapsGeocoder.PRODUCTION_GEOCODE_JSON_URL,

server/lib/python/cartodb_services/test/test_mapboxgeocoder.py

@@ -34,3 +34,18 @@ class MapboxGeocoderTestCase(unittest.TestCase):
         place = self.geocoder.geocode(searchtext='New York', country='us')
 
         assert place
+
+    def test_empty_request(self):
+        place = self.geocoder.geocode(searchtext='', country=None, city=None, state_province=None)
+
+        assert place == []
+
+    def test_empty_search_text_request(self):
+        place = self.geocoder.geocode(searchtext='     ', country='us', city=None, state_province="")
+
+        assert place == []
+
+    def test_unknown_place_request(self):
+        place = self.geocoder.geocode(searchtext='[unknown]', country='ch', state_province=None, city=None)
+
+        assert place == []