From 2862c8002530c9b291bd9bbad67efa94e9f71833 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Ignacio=20S=C3=A1nchez=20Lara?= Date: Mon, 23 Jul 2018 11:53:02 +0200 Subject: [PATCH] Proper empty count on bulk geocoding --- .../cartodb_services/geocoder.py | 17 +-- .../test/test_cartodb_services_geocoder.py | 102 ++++++++++++++++-- 2 files changed, 102 insertions(+), 17 deletions(-) diff --git a/server/lib/python/cartodb_services/cartodb_services/geocoder.py b/server/lib/python/cartodb_services/cartodb_services/geocoder.py index 1678552..edb33bc 100644 --- a/server/lib/python/cartodb_services/cartodb_services/geocoder.py +++ b/server/lib/python/cartodb_services/cartodb_services/geocoder.py @@ -30,11 +30,13 @@ def run_street_point_geocoder(plpy, GD, geocoder, service_manager, username, org logger = Logger(logger_config) + success_count, failed_count = 0, 0 + try: service_manager.assert_within_limits(quota=False) geocode_results = geocoder.bulk_geocode(searches=searches) + results = [] if geocode_results: - results = [] for result in geocode_results: if len(result) > 2: metadata = json.dumps(result[2]) @@ -46,13 +48,16 @@ def run_street_point_geocoder(plpy, GD, geocoder, service_manager, username, org 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]) + success_count += 1 else: results.append([result[0], None, metadata]) - service_manager.quota_service.increment_success_service_use(len(results)) - return results - else: - service_manager.quota_service.increment_empty_service_use(len(searches)) - return [] + + empty_count = len(searches) - success_count - failed_count + service_manager.quota_service.increment_success_service_use(success_count) + service_manager.quota_service.increment_empty_service_use(empty_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}) 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 index 4670fe6..240e6f7 100644 --- a/server/lib/python/cartodb_services/test/test_cartodb_services_geocoder.py +++ b/server/lib/python/cartodb_services/test/test_cartodb_services_geocoder.py @@ -4,9 +4,44 @@ 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), + ] +} + +BULK_RESULTS_FIXTURES = { + 'two': [ + (1, [0, 0], {}), + (2, [0, 0], {}), + ], + 'wrong': [ + (100, [], {}) + ] +} + +EXPECTED_RESULTS_FIXTURES = { + 'two': [ + [1, [0, 0], '{}'], + [2, [0, 0], '{}'], + ], + 'wrong': [ + [100, None, '{}'] + ] +} + + class TestRunStreetPointGeocoder(TestCase): def setUp(self): @@ -44,24 +79,69 @@ class TestRunStreetPointGeocoder(TestCase): 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 = run_street_point_geocoder(self.plpy_mock, self.gd_mock, + self.geocoder_mock, + self.service_manager_mock, + 'any_username', None, 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 = [ - 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) - ] + 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) - run_street_point_geocoder(self.plpy_mock, self.gd_mock, - self.geocoder_mock, - self.service_manager_mock, - 'any_username', None, searches) + result = run_street_point_geocoder(self.plpy_mock, self.gd_mock, + self.geocoder_mock, + self.service_manager_mock, + 'any_username', None, searches) + assert_equal(result, expected_results) - self.quota_service_mock.increment_success_service_use.\ + 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 = run_street_point_geocoder(self.plpy_mock, self.gd_mock, + self.geocoder_mock, + self.service_manager_mock, + 'any_username', None, 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 = run_street_point_geocoder(self.plpy_mock, self.gd_mock, + self.geocoder_mock, + self.service_manager_mock, + 'any_username', None, 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'])) +