Merge pull request #441 from CartoDB/development

Release python library 0.16.3

NEWS.md

@@ -1,3 +1,23 @@
+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
+    * Fix for Mapbox geocoding function due to the iso3166 library doesn't support UTF-8 names for the countries
+* Version `0.16.2` of the python library
+    * Fix for Mapbox cycling profile
+
+January 18th, 2018
+==================
+* Version `0.16.1` of the python library
+    * Fixed encoding problem with Mapbox geocoding (using the Mapbox Python library)
+
 January 17th, 2018
 ==================
 * Version `0.16.0` of the python library

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/extension/cdb_dataservices_server--0.30.0--0.30.1.sql

@@ -0,0 +1,47 @@
+--DO NOT MODIFY THIS FILE, IT IS GENERATED AUTOMATICALLY FROM SOURCES
+-- Complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "ALTER EXTENSION cdb_dataservices_server UPDATE TO '0.30.1'" to load this file. \quit
+
+-- HERE goes your code to upgrade/downgrade
+CREATE OR REPLACE FUNCTION cdb_dataservices_server._cdb_mapbox_geocode_street_point(username TEXT, orgname TEXT, searchtext TEXT, city TEXT DEFAULT NULL, state_province TEXT DEFAULT NULL, country TEXT DEFAULT NULL)
+RETURNS Geometry AS $$
+  from iso3166 import countries
+  from cartodb_services.tools import ServiceManager
+  from cartodb_services.mapbox import MapboxGeocoder
+  from cartodb_services.tools.country import country_to_iso3
+  from cartodb_services.refactor.service.mapbox_geocoder_config import MapboxGeocoderConfigBuilder
+
+  import cartodb_services
+  cartodb_services.init(plpy, GD)
+
+  service_manager = ServiceManager('geocoder', MapboxGeocoderConfigBuilder, username, orgname, GD)
+  service_manager.assert_within_limits()
+
+  try:
+    geocoder = MapboxGeocoder(service_manager.config.mapbox_api_key, service_manager.logger, service_manager.config.service_params)
+
+    country_iso3166 = None
+    if country:
+      country_iso3 = country_to_iso3(country)
+      if country_iso3:
+        country_iso3166 = countries.get(country_iso3).alpha2.lower()
+
+    coordinates = geocoder.geocode(searchtext=searchtext, city=city,
+                                   state_province=state_province,
+                                   country=country_iso3166)
+    if coordinates:
+      service_manager.quota_service.increment_success_service_use()
+      plan = plpy.prepare("SELECT ST_SetSRID(ST_MakePoint($1, $2), 4326); ", ["double precision", "double precision"])
+      point = plpy.execute(plan, [coordinates[0], coordinates[1]], 1)[0]
+      return point['st_setsrid']
+    else:
+      service_manager.quota_service.increment_empty_service_use()
+      return None
+  except BaseException as e:
+    import sys
+    service_manager.quota_service.increment_failed_service_use()
+    service_manager.logger.error('Error trying to geocode street point using mapbox', sys.exc_info(), data={"username": username, "orgname": orgname})
+    raise Exception('Error trying to geocode street point using mapbox')
+  finally:
+    service_manager.quota_service.increment_total_service_use()
+$$ LANGUAGE plpythonu STABLE PARALLEL RESTRICTED;

server/extension/cdb_dataservices_server--0.30.1--0.30.0.sql

@@ -0,0 +1,44 @@
+--DO NOT MODIFY THIS FILE, IT IS GENERATED AUTOMATICALLY FROM SOURCES
+-- Complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "ALTER EXTENSION cdb_dataservices_server UPDATE TO '0.30.0'" to load this file. \quit
+
+-- HERE goes your code to upgrade/downgrade
+CREATE OR REPLACE FUNCTION cdb_dataservices_server._cdb_mapbox_geocode_street_point(username TEXT, orgname TEXT, searchtext TEXT, city TEXT DEFAULT NULL, state_province TEXT DEFAULT NULL, country TEXT DEFAULT NULL)
+RETURNS Geometry AS $$
+  from iso3166 import countries
+  from cartodb_services.tools import ServiceManager
+  from cartodb_services.mapbox import MapboxGeocoder
+  from cartodb_services.refactor.service.mapbox_geocoder_config import MapboxGeocoderConfigBuilder
+
+  import cartodb_services
+  cartodb_services.init(plpy, GD)
+
+  service_manager = ServiceManager('geocoder', MapboxGeocoderConfigBuilder, username, orgname, GD)
+  service_manager.assert_within_limits()
+
+  try:
+    geocoder = MapboxGeocoder(service_manager.config.mapbox_api_key, service_manager.logger, service_manager.config.service_params)
+
+    country_iso3166 = None
+    if country:
+      country_iso3166 = countries.get(country).alpha2.lower()
+
+    coordinates = geocoder.geocode(searchtext=searchtext, city=city,
+                                    state_province=state_province,
+                                    country=country_iso3166)
+    if coordinates:
+      service_manager.quota_service.increment_success_service_use()
+      plan = plpy.prepare("SELECT ST_SetSRID(ST_MakePoint($1, $2), 4326); ", ["double precision", "double precision"])
+      point = plpy.execute(plan, [coordinates[0], coordinates[1]], 1)[0]
+      return point['st_setsrid']
+    else:
+      service_manager.quota_service.increment_empty_service_use()
+      return None
+  except BaseException as e:
+    import sys
+    service_manager.quota_service.increment_failed_service_use()
+    service_manager.logger.error('Error trying to geocode street point using mapbox', sys.exc_info(), data={"username": username, "orgname": orgname})
+    raise Exception('Error trying to geocode street point using mapbox')
+  finally:
+    service_manager.quota_service.increment_total_service_use()
+$$ LANGUAGE plpythonu STABLE PARALLEL RESTRICTED;

server/extension/cdb_dataservices_server.control

@@ -1,5 +1,5 @@
 comment = 'CartoDB dataservices server extension'
-default_version = '0.30.0'
+default_version = '0.30.1'
 requires = 'plpythonu, plproxy, postgis, cdb_geocoder'
 superuser = true
 schema = cdb_dataservices_server

server/extension/sql/20_geocode_street.sql

@@ -188,6 +188,7 @@ RETURNS Geometry AS $$
   from iso3166 import countries
   from cartodb_services.tools import ServiceManager
   from cartodb_services.mapbox import MapboxGeocoder
+  from cartodb_services.tools.country import country_to_iso3
   from cartodb_services.refactor.service.mapbox_geocoder_config import MapboxGeocoderConfigBuilder
 
   import cartodb_services
@@ -201,11 +202,13 @@ RETURNS Geometry AS $$
 
     country_iso3166 = None
     if country:
-      country_iso3166 = countries.get(country).alpha2.lower()
+      country_iso3 = country_to_iso3(country)
+      if country_iso3:
+        country_iso3166 = countries.get(country_iso3).alpha2.lower()
 
     coordinates = geocoder.geocode(searchtext=searchtext, city=city,
-                                    state_province=state_province,
-                                    country=country_iso3166)
+                                   state_province=state_province,
+                                   country=country_iso3166)
     if coordinates:
       service_manager.quota_service.increment_success_service_use()
       plan = plpy.prepare("SELECT ST_SetSRID(ST_MakePoint($1, $2), 4326); ", ["double precision", "double precision"])

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 city:
-            address.append(city)
-        if state_province:
-            address.append(state_province)
+        if searchtext and searchtext.strip():
+            address = [searchtext]
+            if city:
+                address.append(city)
+            if state_province:
+                address.append(state_province)
+        else:
+            return []
 
         country = [country] if country else None
 

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

@@ -5,4 +5,5 @@ MAPBOX_ISOLINES_APIKEY_ROUNDROBIN = 'mapbox_isolines_apikey_roundrobin'
 TRANSPORT_MODE_TO_MAPBOX = {
     'car': 'driving',
     'walk': 'walking',
+    'bicycle': 'cycling',
 }

server/lib/python/cartodb_services/setup.py

@@ -10,7 +10,7 @@ from setuptools import setup, find_packages
 setup(
     name='cartodb_services',
 
-    version='0.16.1',
+    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)
-        client2 = GoogleMapsClientFactory.get(id, key)
+        client1 = GoogleMapsClientFactory.get(client_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)
-        client2 = GoogleMapsClientFactory.get(id, key2)
+        client1 = GoogleMapsClientFactory.get(client_id, key1)
+        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):
-            self.geocoder.geocode()
+        result = 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

@@ -22,8 +22,8 @@ class MapboxGeocoderTestCase(unittest.TestCase):
     def test_valid_request(self):
         place = self.geocoder.geocode(VALID_ADDRESS)
 
-        self.assertEqual(place[0], WELL_KNOWN_LONGITUDE)
-        self.assertEqual(place[1], WELL_KNOWN_LATITUDE)
+        self.assertEqual('%.3f' % place[0], '%.3f' % WELL_KNOWN_LONGITUDE)
+        self.assertEqual('%.3f' % place[1], '%.3f' % WELL_KNOWN_LATITUDE)
 
     def test_valid_request_namedplace(self):
         place = self.geocoder.geocode(searchtext='Barcelona')
@@ -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 == []