From 9a1b1e2832030a91704c0dafb9e1fcc89f339ab2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Ignacio=20S=C3=A1nchez=20Lara?=
 <juanignaciosl@gmail.com>
Date: Mon, 23 Jul 2018 12:18:26 +0200
Subject: [PATCH] Error count

---
 .../cartodb_services/geocoder.py              | 18 ++++++++----
 .../test/test_cartodb_services_geocoder.py    | 29 ++++++++++++++++++-
 2 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/server/lib/python/cartodb_services/cartodb_services/geocoder.py b/server/lib/python/cartodb_services/cartodb_services/geocoder.py
index edb33bc..8d5d456 100644
--- a/server/lib/python/cartodb_services/cartodb_services/geocoder.py
+++ b/server/lib/python/cartodb_services/cartodb_services/geocoder.py
@@ -39,18 +39,24 @@ def run_street_point_geocoder(plpy, GD, geocoder, service_manager, username, org
         if geocode_results:
             for result in geocode_results:
                 if len(result) > 2:
-                    metadata = json.dumps(result[2])
+                    metadata = result[2]
                 else:
-                    logger.warning('Geocoding for {} without metadata'.format(username))
-                    metadata = '{}'
+                    logger.warning('Geocoding without metadata',
+                                   data={"username": username, "orgname": orgname})
+                    metadata = {}
 
-                if result[1] and len(result[1]) == 2:
+                if metadata.get('error', None):
+                    logger.warning('Geocoding error',
+                                   data={"username": username, "orgname": orgname, "error": metadata['error']})
+                    results.append([result[0], None, json.dumps(metadata)])
+                    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'], 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)])
 
         empty_count = len(searches) - success_count - failed_count
         service_manager.quota_service.increment_success_service_use(success_count)
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 240e6f7..6dd199c 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
@@ -18,6 +18,10 @@ SEARCH_FIXTURES = {
     'wrong': [
         StreetGeocoderSearch(id=100, address='deowpfjoepwjfopejwpofjewpojgf',
                              city=None, state=None, country=None),
+    ],
+    'error': [
+        StreetGeocoderSearch(id=200, address=None, city=None, state=None,
+                             country=None),
     ]
 }
 
@@ -28,6 +32,9 @@ BULK_RESULTS_FIXTURES = {
     ],
     'wrong': [
         (100, [], {})
+    ],
+    'error': [
+        (200, [], {'error': 'Something wrong happened'})
     ]
 }
 
@@ -38,6 +45,9 @@ EXPECTED_RESULTS_FIXTURES = {
     ],
     'wrong': [
         [100, None, '{}']
+    ],
+    'error': [
+        [200, None, '{"error": "Something wrong happened"}']
     ]
 }
 
@@ -50,7 +60,8 @@ class TestRunStreetPointGeocoder(TestCase):
         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')
+                                            log_file_path='/tmp/ptest.log',
+                                            rollbar_api_key=None)
         self.gd_mock = {'logger_config': self.logger_config_mock}
 
         self.geocoder_mock = Mock()
@@ -144,4 +155,20 @@ class TestRunStreetPointGeocoder(TestCase):
         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 = 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['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']))
+