Merge pull request #509 from CartoDB/development

Version `0.19.1` of the Python library

NEWS.md

@@ -1,6 +1,6 @@
 Jul 19th, 2018
 ==============
-* Version `0.25.0` of the client, `0.32.0` of the server, and `0.19.0` of the Python library.
+* Version `0.25.0` of the client, `0.32.0` of the server, and `0.19.1` of the Python library.
     * Support for batch street-level geocoding.
 
 May 7th, 2018

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

@@ -9,7 +9,6 @@ import json
 PRECISION_PRECISE = 'precise'
 PRECISION_INTERPOLATED = 'interpolated'
 
-
 def geocoder_metadata(relevance, precision, match_types):
     return {
         'relevance': round(relevance, 2),
@@ -18,49 +17,104 @@ def geocoder_metadata(relevance, precision, match_types):
     }
 
 
+def geocoder_error_response(message):
+    return [[], {'error': message}]
+
+
+# Single empty result
+EMPTY_RESPONSE = [[], {}]
+# HTTP 429 and related
+TOO_MANY_REQUESTS_ERROR_RESPONSE = geocoder_error_response('Rate limit exceeded')
+# Full empty _batch_geocode response
+EMPTY_BATCH_RESPONSE = []
+
+
 def compose_address(street, city=None, state=None, country=None):
     return ', '.join(filter(None, [street, city, state, country]))
 
 
-def run_street_point_geocoder(plpy, GD, geocoder, service_manager, username, orgname, searches):
+def run_street_point_geocoder(plpy, GD, geocoder, service_manager, username, orgname, searches_string):
     plpy.execute("SELECT cdb_dataservices_server._get_logger_config()")
     logger_config = GD["logger_config"]
 
     logger = Logger(logger_config)
 
+    success_count, failed_count, empty_count = 0, 0, 0
+
+    try:
+        searches = json.loads(searches_string)
+    except Exception as e:
+        logger.error('Parsing searches', exception=e, data={'searches': searches_string})
+        raise e
+
     try:
         service_manager.assert_within_limits(quota=False)
-        geocode_results = geocoder.bulk_geocode(searches=searches)
+        geocode_results = geocoder.bulk_geocode(searches)
-        if geocode_results:
         results = []
+        a_failed_one = None
+        if not geocode_results == EMPTY_BATCH_RESPONSE:
             for result in geocode_results:
-                if len(result) > 2:
+                metadata = result[2] if len(result) > 2 else {}
-                    metadata = json.dumps(result[2])
+                try:
-                else:
+                    if metadata.get('error', None):
-                    logger.warning('Geocoding for {} without metadata'.format(username))
+                        results.append([result[0], None, json.dumps(metadata)])
-                    metadata = '{}'
+                        a_failed_one = result
-
+                        failed_count += 1
-                if result[1] and len(result[1]) == 2:
+                    elif result[1] and len(result[1]) == 2:
                         plan = plpy.prepare("SELECT ST_SetSRID(ST_MakePoint($1, $2), 4326) as the_geom; ", ["double precision", "double precision"])
                         point = plpy.execute(plan, result[1], 1)[0]
-                    results.append([result[0], point['the_geom'], metadata])
+                        results.append([result[0], point['the_geom'], json.dumps(metadata)])
+                        success_count += 1
                     else:
-                    results.append([result[0], None, metadata])
+                        results.append([result[0], None, json.dumps(metadata)])
-            service_manager.quota_service.increment_success_service_use(len(results))
+                        empty_count += 1
+                except Exception as e:
+                    import sys
+                    logger.error("Error processing geocode", sys.exc_info(), data={"username": username, "orgname": orgname})
+                    metadata['processing_error'] = 'Error: {}'.format(e.message)
+                    results.append([result[0], None, json.dumps(metadata)])
+                    failed_count += 1
+
+        missing_count = len(searches) - success_count - failed_count - empty_count
+
+        if a_failed_one:
+            logger.warning("failed geocoding",
+                           data={
+                               "username": username,
+                               "orgname": orgname,
+                               "failed": str(a_failed_one),
+                               "success_count": success_count,
+                               "empty_count": empty_count,
+                               "missing_count": missing_count,
+                               "failed_count": failed_count
+                           })
+        else:
+            logger.debug("finished geocoding",
+                           data={
+                               "username": username,
+                               "orgname": orgname,
+                               "success_count": success_count,
+                               "empty_count": empty_count,
+                               "missing_count": missing_count,
+                               "failed_count": failed_count
+                           })
+        service_manager.quota_service.increment_success_service_use(success_count)
+        service_manager.quota_service.increment_empty_service_use(empty_count + missing_count)
+        service_manager.quota_service.increment_failed_service_use(failed_count)
+
         return results
-        else:
-            service_manager.quota_service.increment_empty_service_use(len(searches))
-            return []
     except QuotaExceededException as qe:
+        logger.debug('QuotaExceededException at run_street_point_geocoder', qe,
+                     data={"username": username, "orgname": orgname})
         service_manager.quota_service.increment_failed_service_use(len(searches))
         return []
     except BaseException as e:
         import sys
-        service_manager.quota_service.increment_failed_service_use()
+        service_manager.quota_service.increment_failed_service_use(len(searches))
         service_manager.logger.error('Error trying to bulk geocode street point', sys.exc_info(), data={"username": username, "orgname": orgname})
         raise Exception('Error trying to bulk geocode street')
     finally:
-        service_manager.quota_service.increment_total_service_use()
+        service_manager.quota_service.increment_total_service_use(len(searches))
 
 
 StreetGeocoderSearch = namedtuple('StreetGeocoderSearch', 'id address city state country')
@@ -79,20 +133,14 @@ class StreetPointBulkGeocoder:
 
     SEARCH_KEYS = ['id', 'address', 'city', 'state', 'country']
 
-    def bulk_geocode(self, searches):
+    def bulk_geocode(self, decoded_searches):
         """
-        :param searches: array of StreetGeocoderSearch
+        :param decoded_searches: JSON array
         :return: array of tuples with three elements:
             * id
             * latitude and longitude (array of two elements)
             * empty array (future use: metadata)
         """
-        try:
-            decoded_searches = json.loads(searches)
-        except Exception as e:
-            self._logger.error('General error', exception=e)
-            raise e
-
         street_geocoder_searches = []
         for search in decoded_searches:
             search_id, address, city, state, country = \
@@ -102,10 +150,19 @@ class StreetPointBulkGeocoder:
 
         if len(street_geocoder_searches) > self.MAX_BATCH_SIZE:
             raise Exception("Batch size can't be larger than {}".format(self.MAX_BATCH_SIZE))
+        try:
             if self._should_use_batch(street_geocoder_searches):
                 return self._batch_geocode(street_geocoder_searches)
             else:
                 return self._serial_geocode(street_geocoder_searches)
+        except Exception as e:
+            msg = "Error running geocode: {}".format(e)
+            self._logger.error(msg, e)
+            errors = [geocoder_error_response(msg)] * len(decoded_searches)
+            results = []
+            for s, r in zip(decoded_searches, errors):
+                results.append((s['id'], r[0], r[1]))
+            return results
 
     def _batch_geocode(self, street_geocoder_searches):
         raise NotImplementedError('Subclasses must implement _batch_geocode')

server/lib/python/cartodb_services/cartodb_services/google/bulk_geocoder.py

@@ -1,7 +1,7 @@
 from multiprocessing import Pool
 from exceptions import MalformedResult
 from cartodb_services import StreetPointBulkGeocoder
-from cartodb_services.geocoder import compose_address
+from cartodb_services.geocoder import compose_address, geocoder_error_response
 from cartodb_services.google import GoogleMapsGeocoder
 
 
@@ -25,7 +25,11 @@ class GoogleMapsBulkGeocoder(GoogleMapsGeocoder, StreetPointBulkGeocoder):
         results = []
         for search in searches:
             (cartodb_id, street, city, state, country) = search
+            try:
                 lng_lat, metadata = self.geocode_meta(street, city, state, country)
+            except Exception as e:
+                self._logger.error("Error geocoding", e)
+                lng_lat, metadata = geocoder_error_response("Error geocoding")
             results.append((cartodb_id, lng_lat, metadata))
         return results
 
@@ -49,14 +53,12 @@ class GoogleMapsBulkGeocoder(GoogleMapsGeocoder, StreetPointBulkGeocoder):
                 try:
                     lng_lat, metadata = self._process_results(bulk_result.get())
                 except Exception as e:
-                    self._logger.error('Error at Google async_geocoder', e)
+                    msg = 'Error at Google async_geocoder'
-                    lng_lat, metadata = [[], {}]
+                    self._logger.error(msg, e)
+                    lng_lat, metadata = geocoder_error_response(msg)
 
                 results.append((cartodb_id, lng_lat, metadata))
             return results
-        except KeyError as e:
-            self._logger.error('KeyError error', exception=e)
-            raise MalformedResult()
         except Exception as e:
             self._logger.error('General error', exception=e)
             raise e

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

@@ -4,11 +4,10 @@
 from urlparse import parse_qs
 
 from exceptions import MalformedResult
-from cartodb_services.geocoder import compose_address, geocoder_metadata, PRECISION_PRECISE, PRECISION_INTERPOLATED
+from cartodb_services.geocoder import compose_address, geocoder_metadata, PRECISION_PRECISE, PRECISION_INTERPOLATED, EMPTY_RESPONSE
 from cartodb_services.google.exceptions import InvalidGoogleCredentials
 from client_factory import GoogleMapsClientFactory
 
-EMPTY_RESPONSE = [[], {}]
 PARTIAL_FACTOR = 0.8
 RELEVANCE_BY_LOCATION_TYPE = {
     'ROOFTOP': 1,

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

@@ -8,7 +8,7 @@ from collections import namedtuple
 from requests.adapters import HTTPAdapter
 from cartodb_services import StreetPointBulkGeocoder
 from cartodb_services.here import HereMapsGeocoder
-from cartodb_services.geocoder import geocoder_metadata
+from cartodb_services.geocoder import geocoder_metadata, geocoder_error_response
 from cartodb_services.metrics import Traceable
 from cartodb_services.tools.exceptions import ServiceException
 
@@ -42,7 +42,11 @@ class HereMapsBulkGeocoder(HereMapsGeocoder, StreetPointBulkGeocoder):
         results = []
         for search in searches:
             (search_id, address, city, state, country) = search
+            try:
                 result = self.geocode_meta(searchtext=address, city=city, state=state, country=country)
+            except Exception as e:
+                self._logger.error("Error geocoding", e)
+                result = geocoder_error_response("Error geocoding")
             results.append((search_id, result[0], result[1]))
         return results
 

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

@@ -6,7 +6,7 @@ import requests
 
 from requests.adapters import HTTPAdapter
 from exceptions import *
-from cartodb_services.geocoder import PRECISION_PRECISE, PRECISION_INTERPOLATED, geocoder_metadata
+from cartodb_services.geocoder import PRECISION_PRECISE, PRECISION_INTERPOLATED, geocoder_metadata, EMPTY_RESPONSE
 from cartodb_services.metrics import Traceable
 
 class HereMapsGeocoder(Traceable):
@@ -90,7 +90,7 @@ class HereMapsGeocoder(Traceable):
             if value and value.strip():
                 params[key] = value
         if not params:
-            return [[], {}]
+            return EMPTY_RESPONSE
         return self._execute_geocode(params)
 
     def _execute_geocode(self, params):
@@ -102,7 +102,7 @@ class HereMapsGeocoder(Traceable):
             return [self._extract_lng_lat_from_result(result),
                     self._extract_metadata_from_result(result)]
         except IndexError:
-            return [[], {}]
+            return EMPTY_RESPONSE
         except KeyError as e:
             self._logger.error('params: {}'.format(params), e)
             raise MalformedResult()
@@ -127,7 +127,7 @@ class HereMapsGeocoder(Traceable):
             self._logger.warning('Error 4xx trying to geocode street using HERE',
                                data={"response": response.json(), "params":
                                      params})
-            return []
+            return EMPTY_RESPONSE
         else:
             self._logger.error('Error trying to geocode street using HERE',
                                data={"response": response.json(), "params":

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

@@ -1,8 +1,6 @@
-import json, requests, time
+import requests
-from requests.adapters import HTTPAdapter
 from cartodb_services import StreetPointBulkGeocoder
 from cartodb_services.mapbox import MapboxGeocoder
-from cartodb_services.tools.exceptions import ServiceException
 from iso3166 import countries
 from cartodb_services.tools.country import country_to_iso3
 

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

@@ -5,7 +5,7 @@ Python client for the Mapbox Geocoder service.
 import json
 import requests
 from mapbox import Geocoder
-from cartodb_services.geocoder import PRECISION_PRECISE, PRECISION_INTERPOLATED, geocoder_metadata
+from cartodb_services.geocoder import PRECISION_PRECISE, PRECISION_INTERPOLATED, geocoder_metadata, EMPTY_RESPONSE, EMPTY_BATCH_RESPONSE, TOO_MANY_REQUESTS_ERROR_RESPONSE, geocoder_error_response
 from cartodb_services.metrics import Traceable
 from cartodb_services.tools.exceptions import ServiceException
 from cartodb_services.tools.qps import qps_retry
@@ -23,8 +23,6 @@ ENTRY_COORDINATES = 'coordinates'
 ENTRY_TYPE = 'type'
 TYPE_POINT = 'Point'
 
-EMPTY_RESPONSE = [[], {}]
-
 MATCH_TYPE_BY_MATCH_LEVEL = {
     'poi': 'point_of_interest',
     'poi.landmark': 'point_of_interest',
@@ -71,7 +69,7 @@ class MapboxGeocoder(Traceable):
                     result.append(EMPTY_RESPONSE)
             return result
         else:
-            return EMPTY_RESPONSE
+            return EMPTY_BATCH_RESPONSE
 
     def _extract_lng_lat_from_feature(self, feature):
         geometry = feature[ENTRY_GEOMETRY]
@@ -120,9 +118,17 @@ class MapboxGeocoder(Traceable):
         :param city:
         :param state_province:
         :param country: Country ISO 3166 code
-        :return: [x, y] on success, [] on error
+        :return: [x, y] on success, raises ServiceException on error
         """
-        return self.geocode_meta(searchtext, city, state_province, country)[0]
+        response = self.geocode_meta(searchtext, city, state_province, country)
+        if response:
+            error_message = response[1].get('error', None)
+            if error_message:
+                raise ServiceException(error_message, None)
+            else:
+                return response[0]
+        else:
+            return EMPTY_RESPONSE
 
     @qps_retry(qps=10)
     def geocode_meta(self, searchtext, city=None, state_province=None,
@@ -140,7 +146,8 @@ class MapboxGeocoder(Traceable):
 
         free_search = ', '.join(address)
 
-        return self.geocode_free_text_meta([free_search], country)[0]
+        response = self.geocode_free_text_meta([free_search], country)
+        return response[0] if response else EMPTY_RESPONSE
 
     @qps_retry(qps=10)
     def geocode_free_text_meta(self, free_searches, country=None):
@@ -154,28 +161,31 @@ class MapboxGeocoder(Traceable):
         try:
             free_search = ';'.join([self._escape(fs) for fs in free_searches])
             response = self._geocoder.forward(address=free_search.decode('utf-8'),
+                                              limit=1,
                                               country=country)
-
             if response.status_code == requests.codes.ok:
                 return self._parse_geocoder_response(response.text)
+            elif response.status_code == requests.codes.too_many_requests:
+                return [TOO_MANY_REQUESTS_ERROR_RESPONSE] * len(free_searches)
             elif response.status_code == requests.codes.bad_request:
-                return EMPTY_RESPONSE
+                return EMPTY_BATCH_RESPONSE
             elif response.status_code == requests.codes.unprocessable_entity:
-                return EMPTY_RESPONSE
+                return EMPTY_BATCH_RESPONSE
             else:
-                raise ServiceException(response.status_code, response)
+                msg = "Unkown status: {}".format(response.status_code)
+                self._logger.warning(msg, data={"searches": free_searches})
+                return [geocoder_error_response(msg)] * len(free_searches)
         except requests.Timeout as te:
             # In case of timeout we want to stop the job because the server
             # could be down
-            self._logger.error('Timeout connecting to Mapbox geocoding server',
+            msg = 'Timeout connecting to Mapbox geocoding server'
-                               te)
+            self._logger.error(msg, te)
-            raise ServiceException('Error geocoding {0} using Mapbox'.format(
+            return [geocoder_error_response(msg)] * len(free_searches)
-                free_search), None)
         except requests.ConnectionError as ce:
             # Don't raise the exception to continue with the geocoding job
             self._logger.error('Error connecting to Mapbox geocoding server',
                                exception=ce)
-            return EMPTY_RESPONSE
+            return EMPTY_BATCH_RESPONSE
 
     def _escape(self, free_search):
         # Semicolon is used to separate batch geocoding; there's no documented

server/lib/python/cartodb_services/cartodb_services/tomtom/bulk_geocoder.py

@@ -1,6 +1,7 @@
 import json, requests, time
 from requests.adapters import HTTPAdapter
 from cartodb_services import StreetPointBulkGeocoder
+from cartodb_services.geocoder import geocoder_error_response
 from cartodb_services.tomtom import TomTomGeocoder
 from cartodb_services.tools.exceptions import ServiceException
 
@@ -43,13 +44,21 @@ class TomTomBulkGeocoder(TomTomGeocoder, StreetPointBulkGeocoder):
         return results
 
     def _batch_geocode(self, searches):
-        location = self._send_batch(searches)
+        full_results = self._geocode_searches(searches)
-        full_results = self._download_results(location)
         results = []
         for s, r in zip(searches, full_results):
             results.append((s[0], r[0], r[1]))
         return results
 
+    def _geocode_searches(self, searches):
+        try:
+            location = self._send_batch(searches)
+            return self._download_results(location)
+        except Exception as e:
+            msg = "Error running TomTom batch geocode: {}".format(e)
+            self._logger.error(msg, e)
+            return [geocoder_error_response(msg)] * len(searches)
+
     def _send_batch(self, searches):
         body = {'batchItems': [{'query': self._query(s)} for s in searches]}
         request_params = {

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

@@ -5,7 +5,7 @@ import json
 import requests
 from uritemplate import URITemplate
 from math import tanh
-from cartodb_services.geocoder import PRECISION_PRECISE, PRECISION_INTERPOLATED, geocoder_metadata
+from cartodb_services.geocoder import PRECISION_PRECISE, PRECISION_INTERPOLATED, geocoder_metadata, EMPTY_RESPONSE, geocoder_error_response
 from cartodb_services.metrics import Traceable
 from cartodb_services.tools.exceptions import ServiceException
 from cartodb_services.tools.qps import qps_retry
@@ -20,7 +20,6 @@ ENTRY_RESULTS = 'results'
 ENTRY_POSITION = 'position'
 ENTRY_LON = 'lon'
 ENTRY_LAT = 'lat'
-EMPTY_RESPONSE = [[], {}]
 
 SCORE_NORMALIZATION_FACTOR = 0.15
 PRECISION_SCORE_THRESHOLD = 0.5
@@ -74,7 +73,12 @@ class TomTomGeocoder(Traceable):
     @qps_retry(qps=5)
     def geocode(self, searchtext, city=None, state_province=None,
                 country=None):
-        return self.geocode_meta(searchtext, city, state_province, country)[0]
+        response = self.geocode_meta(searchtext, city, state_province, country)
+        error_message = response[1].get('error', None)
+        if error_message:
+            raise ServiceException(error_message, None)
+        else:
+            return response[0]
 
     @qps_retry(qps=5)
     def geocode_meta(self, searchtext, city=None, state_province=None,
@@ -107,10 +111,9 @@ class TomTomGeocoder(Traceable):
         except requests.Timeout as te:
             # In case of timeout we want to stop the job because the server
             # could be down
-            self._logger.error('Timeout connecting to TomTom geocoding server',
+            msg = 'Timeout connecting to TomTom geocoding server'
-                               te)
+            self._logger.error(msg, te)
-            raise ServiceException('Error geocoding {0} using TomTom'.format(
+            return geocoder_error_response(msg)
-                searchtext), None)
         except requests.ConnectionError as ce:
             # Don't raise the exception to continue with the geocoding job
             self._logger.error('Error connecting to TomTom geocoding server',
@@ -126,7 +129,9 @@ class TomTomGeocoder(Traceable):
             return EMPTY_RESPONSE
         else:
             msg = 'Unknown response {}: {}'.format(str(status_code), text)
-            raise ServiceException(msg, None)
+            self._logger.warning('Error parsing TomTom geocoding response',
+                                 data={'msg': msg})
+            return geocoder_error_response(msg)
 
     def _parse_geocoder_response(self, response):
         json_response = json.loads(response) \

server/lib/python/cartodb_services/setup.py

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

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

@@ -52,11 +52,14 @@ class MockPlPy:
         self._logged_queries = []
         self._log_executed_queries = True
 
+    def warning(self, msg):
+        self.warnings.append(msg)
+
     def notice(self, msg):
         self.notices.append(msg)
 
     def debug(self, msg):
-        self.notices.append(msg)
+        self.debugs.append(msg)
 
     def info(self, msg):
         self.infos.append(msg)

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

@@ -0,0 +1,211 @@
+#!/usr/local/bin/python
+# -*- coding: utf-8 -*-
+
+import json
+from unittest import TestCase
+from mock import Mock, MagicMock
+from nose.tools import assert_not_equal, assert_equal, assert_true
+from cartodb_services.tools import QuotaExceededException
+from cartodb_services.geocoder import run_street_point_geocoder, StreetGeocoderSearch
+
+
+SEARCH_FIXTURES = {
+    'two': [
+        StreetGeocoderSearch(id=1, address='Paseo Zorrilla 1, Valladolid',
+                             city=None, state=None, country=None),
+        StreetGeocoderSearch(id=2, address='Paseo Zorrilla 2, Valladolid',
+                             city=None, state=None, country=None)
+    ],
+    'wrong': [
+        StreetGeocoderSearch(id=100, address='deowpfjoepwjfopejwpofjewpojgf',
+                             city=None, state=None, country=None),
+    ],
+    'error': [
+        StreetGeocoderSearch(id=200, address=None, city=None, state=None,
+                             country=None),
+    ],
+    'broken_middle': [
+        StreetGeocoderSearch(id=301, address='Paseo Zorrilla 1, Valladolid',
+                             city=None, state=None, country=None),
+        StreetGeocoderSearch(id=302, address='Marsopolis',
+                             city=None, state=None, country=None),
+        StreetGeocoderSearch(id=303, address='Paseo Zorrilla 2, Valladolid',
+                             city=None, state=None, country=None)
+    ],
+}
+
+BULK_RESULTS_FIXTURES = {
+    'two': [
+        (1, [0, 0], {}),
+        (2, [0, 0], {}),
+    ],
+    'wrong': [
+        (100, [], {})
+    ],
+    'error': [
+        (200, [], {'error': 'Something wrong happened'})
+    ],
+    'broken_middle': [
+        (301, [0, 0], {}),
+        (302, ['a', 'b'], {}),
+        (303, [0, 0], {}),
+    ]
+}
+
+EXPECTED_RESULTS_FIXTURES = {
+    'two': [
+        [1, [0, 0], '{}'],
+        [2, [0, 0], '{}'],
+    ],
+    'wrong': [
+        [100, None, '{}']
+    ],
+    'error': [
+        [200, None, '{"error": "Something wrong happened"}']
+    ],
+    'broken_middle': [
+        [301, [0, 0], '{}'],
+        [302, None, '{"processing_error": "Error: NO!"}'],
+        [303, [0, 0], '{}'],
+    ]
+}
+
+
+class TestRunStreetPointGeocoder(TestCase):
+    def _run_geocoder(self, plpy=None, gd=None, geocoder=None,
+                      service_manager=None, username=None, orgname=None,
+                      searches=None):
+        return run_street_point_geocoder(
+            plpy if plpy else self.plpy_mock,
+            gd if gd else self.gd_mock,
+            geocoder if geocoder else self.geocoder_mock,
+            service_manager if service_manager else self.service_manager_mock,
+            username if username else 'any_username',
+            orgname if orgname else None,
+            json.dumps(searches) if searches else '[]')
+
+    def setUp(self):
+        point = [0,0]
+        self.plpy_mock = Mock()
+        self.plpy_mock.execute = MagicMock(return_value=[{'the_geom': point}])
+
+        self.logger_config_mock = MagicMock(min_log_level='debug',
+                                            log_file_path='/tmp/ptest.log',
+                                            rollbar_api_key=None)
+        self.gd_mock = {'logger_config': self.logger_config_mock}
+
+        self.geocoder_mock = Mock()
+
+        self.quota_service_mock = Mock()
+
+        self.service_manager_mock = Mock()
+        self.service_manager_mock.quota_service = self.quota_service_mock
+        self.service_manager_mock.assert_within_limits = MagicMock()
+
+    def test_count_increment_total_and_failed_service_use_on_error(self):
+        self.service_manager_mock.assert_within_limits = \
+            Mock(side_effect=Exception('Fail!'))
+        searches = []
+
+        with(self.assertRaises(BaseException)):
+            self._run_geocoder(service_manager=self.service_manager_mock,
+                               searches=searches)
+
+        self.quota_service_mock.increment_total_service_use. \
+            assert_called_once_with(len(searches))
+        self.quota_service_mock.increment_failed_service_use. \
+            assert_called_once_with(len(searches))
+
+    def test_count_increment_failed_service_use_on_quota_error(self):
+        self.service_manager_mock.assert_within_limits = \
+            Mock(side_effect=QuotaExceededException())
+        searches = SEARCH_FIXTURES['two']
+
+        result = self._run_geocoder(service_manager=self.service_manager_mock,
+                                    searches=searches)
+        assert_equal(result, [])
+        self.quota_service_mock.increment_failed_service_use. \
+            assert_called_once_with(len(searches))
+
+    def test_increment_success_service_use_on_complete_response(self):
+        searches = SEARCH_FIXTURES['two']
+        results = [
+            (1, [0, 0], {}),
+            (2, [0, 0], {}),
+        ]
+        expected_results = [
+            [1, [0, 0], '{}'],
+            [2, [0, 0], '{}'],
+        ]
+        self.geocoder_mock.bulk_geocode = MagicMock(return_value=results)
+
+        result = self._run_geocoder(geocoder=self.geocoder_mock,
+                                    searches=searches)
+        assert_equal(result, expected_results)
+        self.quota_service_mock.increment_success_service_use. \
+            assert_called_once_with(len(results))
+
+    def test_increment_empty_service_use_on_complete_response(self):
+        searches = SEARCH_FIXTURES['two']
+        results = []
+        self.geocoder_mock.bulk_geocode = MagicMock(return_value=results)
+
+        result = self._run_geocoder(geocoder=self.geocoder_mock,
+                                    searches=searches)
+
+        assert_equal(result, results)
+        self.quota_service_mock.increment_empty_service_use. \
+            assert_called_once_with(len(searches))
+
+    def test_increment_mixed_empty_service_use_on_complete_response(self):
+        searches = SEARCH_FIXTURES['two'] + SEARCH_FIXTURES['wrong']
+        bulk_results = BULK_RESULTS_FIXTURES['two'] + BULK_RESULTS_FIXTURES['wrong']
+        self.geocoder_mock.bulk_geocode = MagicMock(return_value=bulk_results)
+
+        result = self._run_geocoder(geocoder=self.geocoder_mock,
+                                    searches=searches)
+
+        assert_equal(result, EXPECTED_RESULTS_FIXTURES['two'] + EXPECTED_RESULTS_FIXTURES['wrong'])
+        self.quota_service_mock.increment_success_service_use. \
+            assert_called_once_with(len(SEARCH_FIXTURES['two']))
+        self.quota_service_mock.increment_empty_service_use. \
+            assert_called_once_with(len(SEARCH_FIXTURES['wrong']))
+
+    def test_increment_mixed_error_service_use_on_complete_response(self):
+        searches = SEARCH_FIXTURES['two'] + SEARCH_FIXTURES['error']
+        bulk_results = BULK_RESULTS_FIXTURES['two'] + BULK_RESULTS_FIXTURES['error']
+        self.geocoder_mock.bulk_geocode = MagicMock(return_value=bulk_results)
+
+        result = self._run_geocoder(geocoder=self.geocoder_mock,
+                                    searches=searches)
+
+        assert_equal(result, EXPECTED_RESULTS_FIXTURES['two'] + EXPECTED_RESULTS_FIXTURES['error'])
+        self.quota_service_mock.increment_success_service_use. \
+            assert_called_once_with(len(SEARCH_FIXTURES['two']))
+        self.quota_service_mock.increment_failed_service_use. \
+            assert_called_once_with(len(SEARCH_FIXTURES['error']))
+
+    def test_controlled_failure_on_query_break(self):
+        searches = SEARCH_FIXTURES['broken_middle']
+        bulk_results = BULK_RESULTS_FIXTURES['broken_middle']
+        self.geocoder_mock.bulk_geocode = MagicMock(return_value=bulk_results)
+        def break_on_302(*args):
+            if len(args) == 3:
+                plan, values, limit = args
+                if values[0] == 'a':
+                    raise Exception('NO!')
+
+            return [{'the_geom': [0,0]}]
+        self.plpy_mock.execute = break_on_302
+
+        result = self._run_geocoder(geocoder=self.geocoder_mock,
+                                    searches=searches)
+
+        assert_equal(result, EXPECTED_RESULTS_FIXTURES['broken_middle'])
+        self.quota_service_mock.increment_success_service_use. \
+            assert_called_once_with(2)
+        self.quota_service_mock.increment_failed_service_use. \
+            assert_called_once_with(1)
+
+
+

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

@@ -17,17 +17,16 @@ VALID_PROFILE = DEFAULT_PROFILE
 INVALID_PROFILE = 'invalid_profile'
 
 WELL_KNOWN_SHAPE = [(40.73312, -73.98891), (40.73353, -73.98987),
-                    (40.73398, -73.99095), (40.73453, -73.99227),
+                    (40.73398, -73.99095), (40.73321, -73.99111),
-                    (40.73531, -73.99412), (40.73467, -73.99459),
+                    (40.73245, -73.99129), (40.7333, -73.99332),
-                    (40.73442, -73.99477), (40.73435, -73.99482),
+                    (40.7338, -73.99449),  (40.73403, -73.99505),
-                    (40.73403, -73.99505), (40.73344, -73.99549),
+                    (40.73344, -73.99549), (40.73286, -73.9959),
-                    (40.73286, -73.9959), (40.73226, -73.99635),
+                    (40.73226, -73.99635), (40.73186, -73.99664),
-                    (40.73186, -73.99664), (40.73147, -73.99693),
+                    (40.73147, -73.99693), (40.73141, -73.99698),
-                    (40.73141, -73.99698), (40.73147, -73.99707),
+                    (40.73147, -73.99707), (40.73219, -73.99856),
-                    (40.73219, -73.99856), (40.73222, -73.99861),
+                    (40.73222, -73.99861), (40.73225, -73.99868),
-                    (40.73225, -73.99868), (40.73293, -74.00007),
+                    (40.73293, -74.00007), (40.733, -74.00001)]
-                    (40.733, -74.00001)]
+WELL_KNOWN_LENGTH = 1384.8
-WELL_KNOWN_LENGTH = 1317.9
 
 
 class MapboxRoutingTestCase(unittest.TestCase):

test/integration/test_street_functions.py

@@ -11,7 +11,7 @@ class TestStreetFunctionsSetUp(TestCase):
     fixture_points = None
 
     GOOGLE_POINTS = {
-        'Plaza Mayor, Valladolid': [-4.728252, 41.6517025],
+        'Plaza Mayor 1, Valladolid': [-4.728252, 41.6517025],
         'Paseo Zorrilla, Valladolid': [-4.7404453, 41.6314339],
         '1900 amphitheatre parkway': [-122.0875324, 37.4227968],
         '1901 amphitheatre parkway': [-122.0885504, 37.4238657],
@@ -26,7 +26,7 @@ class TestStreetFunctionsSetUp(TestCase):
     }
 
     HERE_POINTS = {
-        'Plaza Mayor, Valladolid': [-4.72979, 41.65258],
+        'Plaza Mayor 1, Valladolid': [-4.729, 41.65258],
         'Paseo Zorrilla, Valladolid': [-4.73869, 41.63817],
         '1900 amphitheatre parkway': [-122.0879468, 37.4234763],
         '1901 amphitheatre parkway': [-122.0879253, 37.4238725],
@@ -42,13 +42,13 @@ class TestStreetFunctionsSetUp(TestCase):
 
     TOMTOM_POINTS = HERE_POINTS.copy()
     TOMTOM_POINTS.update({
-        'Plaza Mayor, Valladolid': [-4.72183, 41.5826],
+        'Plaza Mayor 1, Valladolid': [-4.7286, 41.6523],
         'Paseo Zorrilla, Valladolid': [-4.74031, 41.63181],
         'Valladolid': [-4.72838, 41.6542],
         'Valladolid, Spain': [-4.72838, 41.6542],
         'Madrid': [-3.70035, 40.42028],
         'Logroño, Spain': [-2.44998, 42.46592],
-        'Plaza España, Barcelona': [2.1497, 41.37516]
+        'Plaza España, Barcelona': [2.14856, 41.37516]
     })
 
     MAPBOX_POINTS = GOOGLE_POINTS.copy()
@@ -174,7 +174,7 @@ class TestBulkStreetFunctions(TestStreetFunctionsSetUp):
                 "FROM cdb_dataservices_client.cdb_bulk_geocode_street_point(" \
                 "'select 1 as cartodb_id, ''Spain'' as country, " \
                 "''Castilla y León'' as state, ''Valladolid'' as city, " \
-                "''Plaza Mayor'' as street  " \
+                "''Plaza Mayor 1'' as street  " \
                 "UNION " \
                 "select 2 as cartodb_id, ''Spain'' as country, " \
                 "''Castilla y León'' as state, ''Valladolid'' as city, " \
@@ -183,7 +183,7 @@ class TestBulkStreetFunctions(TestStreetFunctionsSetUp):
         response = self._run_authenticated(query)
 
         points_by_cartodb_id = {
-            1: self.fixture_points['Plaza Mayor, Valladolid'],
+            1: self.fixture_points['Plaza Mayor 1, Valladolid'],
             2: self.fixture_points['Paseo Zorrilla, Valladolid']
         }
         self.assert_close_points(self._x_y_by_cartodb_id(response), points_by_cartodb_id)