Proper empty count on bulk geocoding

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
+        empty_count = len(searches) - success_count - failed_count
-        else:
+        service_manager.quota_service.increment_success_service_use(success_count)
-            service_manager.quota_service.increment_empty_service_use(len(searches))
+        service_manager.quota_service.increment_empty_service_use(empty_count)
-            return []
+        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})

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 = [
+        searches = 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)
-        ]
         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,
+        result = run_street_point_geocoder(self.plpy_mock, self.gd_mock,
-                                  self.geocoder_mock,
+                                           self.geocoder_mock,
-                                  self.service_manager_mock,
+                                           self.service_manager_mock,
-                                  'any_username', None, searches)
+                                           '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']))
+