From 8e430ce1c102f1911ea80caa6a67c28c55b18f1a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Ignacio=20S=C3=A1nchez=20Lara?=
 <juanignaciosl@gmail.com>
Date: Mon, 9 Jul 2018 15:28:28 +0200
Subject: [PATCH] Google geocoder works better concatenating all components

---
 .../cartodb_services/geocoder.py               |  4 ++++
 .../cartodb_services/google/geocoder.py        |  8 +++++---
 test/integration/test_street_functions.py      | 18 +++++++++++++++++-
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/server/lib/python/cartodb_services/cartodb_services/geocoder.py b/server/lib/python/cartodb_services/cartodb_services/geocoder.py
index 6a14594..6c2ce43 100644
--- a/server/lib/python/cartodb_services/cartodb_services/geocoder.py
+++ b/server/lib/python/cartodb_services/cartodb_services/geocoder.py
@@ -6,6 +6,10 @@ from collections import namedtuple
 import json
 
 
+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):
     plpy.execute("SELECT cdb_dataservices_server._get_logger_config()")
     logger_config = GD["logger_config"]
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 1cc61c9..956d09e 100644
--- a/server/lib/python/cartodb_services/cartodb_services/google/geocoder.py
+++ b/server/lib/python/cartodb_services/cartodb_services/google/geocoder.py
@@ -6,6 +6,7 @@ from urlparse import parse_qs
 
 from exceptions import MalformedResult
 from cartodb_services import StreetPointBulkGeocoder
+from cartodb_services.geocoder import compose_address
 from cartodb_services.google.exceptions import InvalidGoogleCredentials
 from client_factory import GoogleMapsClientFactory
 
@@ -36,8 +37,9 @@ class GoogleMapsGeocoder(StreetPointBulkGeocoder):
     def geocode(self, searchtext, city=None, state=None,
                 country=None):
         try:
+            address = compose_address(searchtext, city, state, country)
             opt_params = self._build_optional_parameters(city, state, country)
-            results = self.geocoder.geocode(address=searchtext,
+            results = self.geocoder.geocode(address=address,
                                             components=opt_params)
             if results:
                 return self._extract_lng_lat_from_result(results[0])
@@ -50,10 +52,10 @@ class GoogleMapsGeocoder(StreetPointBulkGeocoder):
         bulk_results = {}
         pool = Pool(processes=self.PARALLEL_PROCESSES)
         for search in searches:
-            (search_id, address, city, state, country) = search
+            (search_id, street, city, state, country) = search
             opt_params = self._build_optional_parameters(city, state, country)
             # Geocoding works better if components are also inside the address
-            address = ', '.join(filter(None, [address, city, state, country]))
+            address = compose_address(street, city, state, country)
             if address:
                 self._logger.debug('async geocoding --> {} {}'.format(address.encode('utf-8'), opt_params))
                 result = pool.apply_async(async_geocoder,
diff --git a/test/integration/test_street_functions.py b/test/integration/test_street_functions.py
index 12f5112..d939de1 100644
--- a/test/integration/test_street_functions.py
+++ b/test/integration/test_street_functions.py
@@ -18,11 +18,18 @@ class TestStreetFunctionsSetUp(TestCase):
             self.env_variables['api_key']
         )
 
+    def _run_authenticated(self, query):
+        authenticated_query = "{}&api_key={}".format(query,
+                                                     self.env_variables[
+                                                         'api_key'])
+        return IntegrationTestHelper.execute_query_raw(self.sql_api_url,
+                                                       authenticated_query)
+
 
 class TestStreetFunctions(TestStreetFunctionsSetUp):
 
     def test_if_select_with_street_point_is_ok(self):
-        query = "SELECT cdb_geocode_street_point(street) " \
+        query = "SELECT cdb_dataservices_client.cdb_geocode_street_point(street) " \
                 "as geometry FROM {0} LIMIT 1&api_key={1}".format(
             self.env_variables['table_name'],
             self.env_variables['api_key'])
@@ -39,6 +46,15 @@ class TestStreetFunctions(TestStreetFunctionsSetUp):
             assert_equal(e.message[0],
                          "permission denied for relation {}".format(table))
 
+    def test_component_aggregation(self):
+        query = "select st_x(the_geom), st_y(the_geom) from (" \
+                "select cdb_dataservices_client.cdb_geocode_street_point( " \
+                "'Plaza España 1', 'Barcelona', null, 'Spain') as the_geom) _x"
+        response = self._run_authenticated(query)
+        row = response['rows'][0]
+        x_y = [row['st_x'], row['st_y']]
+        # Wrong coordinates (Plaza España, Madrid): [-3.7138975, 40.4256762]
+        assert_close_enough(x_y, [2.1482563, 41.375485])
 
 class TestBulkStreetFunctions(TestStreetFunctionsSetUp):
     provider = None