diff --git a/NEWS.md b/NEWS.md index 9883417..4571e01 100644 --- a/NEWS.md +++ b/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 diff --git a/server/lib/python/cartodb_services/cartodb_services/geocoder.py b/server/lib/python/cartodb_services/cartodb_services/geocoder.py index 54e80f1..a2c250c 100644 --- a/server/lib/python/cartodb_services/cartodb_services/geocoder.py +++ b/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) - if geocode_results: - results = [] + geocode_results = geocoder.bulk_geocode(searches) + results = [] + a_failed_one = None + if not geocode_results == EMPTY_BATCH_RESPONSE: for result in geocode_results: - if len(result) > 2: - metadata = json.dumps(result[2]) - else: - logger.warning('Geocoding for {} without metadata'.format(username)) - metadata = '{}' + metadata = result[2] if len(result) > 2 else {} + try: + if metadata.get('error', None): + results.append([result[0], None, json.dumps(metadata)]) + a_failed_one = result + failed_count += 1 + 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'], json.dumps(metadata)]) + success_count += 1 + else: + results.append([result[0], None, json.dumps(metadata)]) + 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 - if 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]) - else: - results.append([result[0], None, metadata]) - service_manager.quota_service.increment_success_service_use(len(results)) - return results + 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: - service_manager.quota_service.increment_empty_service_use(len(searches)) - return [] + 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 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)) - if self._should_use_batch(street_geocoder_searches): - return self._batch_geocode(street_geocoder_searches) - else: - return self._serial_geocode(street_geocoder_searches) + 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') diff --git a/server/lib/python/cartodb_services/cartodb_services/google/bulk_geocoder.py b/server/lib/python/cartodb_services/cartodb_services/google/bulk_geocoder.py index 0e1ab1f..cde9f1c 100644 --- a/server/lib/python/cartodb_services/cartodb_services/google/bulk_geocoder.py +++ b/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 - lng_lat, metadata = self.geocode_meta(street, city, state, country) + 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) - lng_lat, metadata = [[], {}] + msg = 'Error at Google async_geocoder' + 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 diff --git a/server/lib/python/cartodb_services/cartodb_services/google/geocoder.py b/server/lib/python/cartodb_services/cartodb_services/google/geocoder.py index fb75c76..30a62cf 100644 --- a/server/lib/python/cartodb_services/cartodb_services/google/geocoder.py +++ b/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, diff --git a/server/lib/python/cartodb_services/cartodb_services/here/bulk_geocoder.py b/server/lib/python/cartodb_services/cartodb_services/here/bulk_geocoder.py index 8316c44..772787c 100644 --- a/server/lib/python/cartodb_services/cartodb_services/here/bulk_geocoder.py +++ b/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 - result = self.geocode_meta(searchtext=address, city=city, state=state, country=country) + 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 diff --git a/server/lib/python/cartodb_services/cartodb_services/here/geocoder.py b/server/lib/python/cartodb_services/cartodb_services/here/geocoder.py index 23dd871..cbf2576 100644 --- a/server/lib/python/cartodb_services/cartodb_services/here/geocoder.py +++ b/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": diff --git a/server/lib/python/cartodb_services/cartodb_services/mapbox/bulk_geocoder.py b/server/lib/python/cartodb_services/cartodb_services/mapbox/bulk_geocoder.py index 2563d43..a939716 100644 --- a/server/lib/python/cartodb_services/cartodb_services/mapbox/bulk_geocoder.py +++ b/server/lib/python/cartodb_services/cartodb_services/mapbox/bulk_geocoder.py @@ -1,8 +1,6 @@ -import json, requests, time -from requests.adapters import HTTPAdapter +import requests 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 diff --git a/server/lib/python/cartodb_services/cartodb_services/mapbox/geocoder.py b/server/lib/python/cartodb_services/cartodb_services/mapbox/geocoder.py index d76937a..f50eff1 100644 --- a/server/lib/python/cartodb_services/cartodb_services/mapbox/geocoder.py +++ b/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', - te) - raise ServiceException('Error geocoding {0} using Mapbox'.format( - free_search), None) + msg = 'Timeout connecting to Mapbox geocoding server' + self._logger.error(msg, te) + return [geocoder_error_response(msg)] * len(free_searches) 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 diff --git a/server/lib/python/cartodb_services/cartodb_services/tomtom/bulk_geocoder.py b/server/lib/python/cartodb_services/cartodb_services/tomtom/bulk_geocoder.py index 8fc2e92..400cc2a 100644 --- a/server/lib/python/cartodb_services/cartodb_services/tomtom/bulk_geocoder.py +++ b/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._download_results(location) + full_results = self._geocode_searches(searches) 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 = { diff --git a/server/lib/python/cartodb_services/cartodb_services/tomtom/geocoder.py b/server/lib/python/cartodb_services/cartodb_services/tomtom/geocoder.py index 46cdbda..b5ec592 100644 --- a/server/lib/python/cartodb_services/cartodb_services/tomtom/geocoder.py +++ b/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', - te) - raise ServiceException('Error geocoding {0} using TomTom'.format( - searchtext), None) + msg = 'Timeout connecting to TomTom geocoding server' + self._logger.error(msg, te) + return geocoder_error_response(msg) 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) \ diff --git a/server/lib/python/cartodb_services/setup.py b/server/lib/python/cartodb_services/setup.py index 3ea820e..7699f06 100644 --- a/server/lib/python/cartodb_services/setup.py +++ b/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', diff --git a/server/lib/python/cartodb_services/test/mock_plpy.py b/server/lib/python/cartodb_services/test/mock_plpy.py index 0fe067f..c088670 100644 --- a/server/lib/python/cartodb_services/test/mock_plpy.py +++ b/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) diff --git a/server/lib/python/cartodb_services/test/test_cartodb_services_geocoder.py b/server/lib/python/cartodb_services/test/test_cartodb_services_geocoder.py new file mode 100644 index 0000000..ad36f72 --- /dev/null +++ b/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) + + + diff --git a/server/lib/python/cartodb_services/test/test_mapboxrouting.py b/server/lib/python/cartodb_services/test/test_mapboxrouting.py index 160b2a5..2101535 100644 --- a/server/lib/python/cartodb_services/test/test_mapboxrouting.py +++ b/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.73531, -73.99412), (40.73467, -73.99459), - (40.73442, -73.99477), (40.73435, -73.99482), - (40.73403, -73.99505), (40.73344, -73.99549), - (40.73286, -73.9959), (40.73226, -73.99635), - (40.73186, -73.99664), (40.73147, -73.99693), - (40.73141, -73.99698), (40.73147, -73.99707), - (40.73219, -73.99856), (40.73222, -73.99861), - (40.73225, -73.99868), (40.73293, -74.00007), - (40.733, -74.00001)] -WELL_KNOWN_LENGTH = 1317.9 + (40.73398, -73.99095), (40.73321, -73.99111), + (40.73245, -73.99129), (40.7333, -73.99332), + (40.7338, -73.99449), (40.73403, -73.99505), + (40.73344, -73.99549), (40.73286, -73.9959), + (40.73226, -73.99635), (40.73186, -73.99664), + (40.73147, -73.99693), (40.73141, -73.99698), + (40.73147, -73.99707), (40.73219, -73.99856), + (40.73222, -73.99861), (40.73225, -73.99868), + (40.73293, -74.00007), (40.733, -74.00001)] +WELL_KNOWN_LENGTH = 1384.8 class MapboxRoutingTestCase(unittest.TestCase): diff --git a/test/integration/test_street_functions.py b/test/integration/test_street_functions.py index 2d02bdb..b7557c9 100644 --- a/test/integration/test_street_functions.py +++ b/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)