Merge pull request #404 from CartoDB/development

Release client 0.21.0 and server 0.28.0
Merge pull request #403 from CartoDB/numgeoms_getavailablegeometries
2017-10-18 10:28:18 +02:00 · 2017-10-18 10:26:48 +02:00 · 2017-10-18 10:25:14 +02:00 · 2017-10-18 10:25:04 +02:00 · 2017-10-18 10:24:50 +02:00 · 2017-10-10 16:23:15 +02:00
51 changed files with 7866 additions and 43 deletions
@@ -4,3 +4,4 @@
 cartodb_services.egg-info/
 build/
 dist/
+.vscode/
@@ -1,3 +1,19 @@
+October 18th, 2017
+=================
+* Version `0.21.0` of the client extension
+    * Added new parameter `number_geometries` to the `obs_getavailablegeometries` to improve the score calculation
+* Version `0.28.0` of the server extension
+    * Added new parameter `number_geometries` to the `obs_getavailablegeometries` to improve the score calculation
+
+October 6th, 2017
+=================
+* Version `0.15.5` of the python library
+    * googlemaps dependency updated to v2.5.1
+    * Google geocoder performance boost: client connections are now reused between queries. See #401
+    * Fixed issue with Google keys validity check. See #382
+    * Fixed inconsistency in service usage failed requests tracking. See f0a3249
+* Client extension tests are now compatible with PostgreSQL 9.5 and 9.6
+
 August 30th, 2017
 =============
 * Version `0.15.4` of the python library
@@ -21,7 +21,7 @@ DATA = $(NEW_EXTENSION_ARTIFACT) \
 SOURCES_DATA_DIR = sql/


-REGRESS = $(notdir $(basename $(wildcard test/sql/*test.sql)))
+REGRESS = $(notdir $(basename $(sort $(wildcard test/sql/*test.sql))))
 TEST_DIR = test/
 REGRESS_OPTS = --inputdir='$(TEST_DIR)' --outputdir='$(TEST_DIR)' --user='postgres'

@@ -0,0 +1,72 @@
+--DO NOT MODIFY THIS FILE, IT IS GENERATED AUTOMATICALLY FROM SOURCES
+-- Complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "ALTER EXTENSION cdb_dataservices_client UPDATE TO '0.21.0'" to load this file. \quit
+
+-- Make sure we have a sane search path to create/update the extension
+SET search_path = "$user",cartodb,public,cdb_dataservices_client;
+
+-- HERE goes your code to upgrade/downgrade
+CREATE OR REPLACE FUNCTION cdb_dataservices_client._obs_getavailablegeometries (username text, orgname text, bounds geometry(Geometry, 4326) DEFAULT NULL, filter_tags text[] DEFAULT NULL, numer_id text DEFAULT NULL, denom_id text DEFAULT NULL, timespan text DEFAULT NULL, number_geometries integer DEFAULT NULL)
+RETURNS SETOF cdb_dataservices_client.obs_meta_geometry AS $$
+  CONNECT cdb_dataservices_client._server_conn_str();
+
+  SELECT * FROM cdb_dataservices_server.obs_getavailablegeometries (username, orgname, bounds, filter_tags, numer_id, denom_id, timespan, number_geometries);
+
+$$ LANGUAGE plproxy;
+
+CREATE OR REPLACE FUNCTION cdb_dataservices_client.obs_getavailablegeometries (bounds geometry(Geometry, 4326) DEFAULT NULL ,filter_tags text[] DEFAULT NULL ,numer_id text DEFAULT NULL ,denom_id text DEFAULT NULL ,timespan text DEFAULT NULL ,number_geometries integer DEFAULT NULL)
+RETURNS SETOF cdb_dataservices_client.obs_meta_geometry AS $$
+DECLARE
+
+  username text;
+  orgname text;
+BEGIN
+  IF session_user = 'publicuser' OR session_user ~ 'cartodb_publicuser_*' THEN
+    RAISE EXCEPTION 'The api_key must be provided';
+  END IF;
+  SELECT u, o INTO username, orgname FROM cdb_dataservices_client._cdb_entity_config() AS (u text, o text);
+  -- JSON value stored "" is taken as literal
+  IF username IS NULL OR username = '' OR username = '""' THEN
+    RAISE EXCEPTION 'Username is a mandatory argument, check it out';
+  END IF;
+
+  RETURN QUERY SELECT * FROM cdb_dataservices_client._obs_getavailablegeometries(username, orgname, bounds, filter_tags, numer_id, denom_id, timespan, number_geometries);
+END;
+$$ LANGUAGE 'plpgsql' SECURITY DEFINER;
+
+CREATE OR REPLACE FUNCTION cdb_dataservices_client._obs_getavailablegeometries_exception_safe (bounds geometry(Geometry, 4326) DEFAULT NULL ,filter_tags text[] DEFAULT NULL ,numer_id text DEFAULT NULL ,denom_id text DEFAULT NULL ,timespan text DEFAULT NULL ,number_geometries integer DEFAULT NULL)
+RETURNS SETOF cdb_dataservices_client.obs_meta_geometry AS $$
+DECLARE
+
+  username text;
+  orgname text;
+  _returned_sqlstate TEXT;
+  _message_text TEXT;
+  _pg_exception_context TEXT;
+BEGIN
+  IF session_user = 'publicuser' OR session_user ~ 'cartodb_publicuser_*' THEN
+    RAISE EXCEPTION 'The api_key must be provided';
+  END IF;
+  SELECT u, o INTO username, orgname FROM cdb_dataservices_client._cdb_entity_config() AS (u text, o text);
+  -- JSON value stored "" is taken as literal
+  IF username IS NULL OR username = '' OR username = '""' THEN
+    RAISE EXCEPTION 'Username is a mandatory argument, check it out';
+  END IF;
+
+
+  BEGIN
+    RETURN QUERY SELECT * FROM cdb_dataservices_client._obs_getavailablegeometries(username, orgname, bounds, filter_tags, numer_id, denom_id, timespan, number_geometries);
+  EXCEPTION
+    WHEN OTHERS THEN
+        GET STACKED DIAGNOSTICS _returned_sqlstate = RETURNED_SQLSTATE,
+                                _message_text = MESSAGE_TEXT,
+                                _pg_exception_context = PG_EXCEPTION_CONTEXT;
+        RAISE WARNING USING ERRCODE = _returned_sqlstate, MESSAGE = _message_text, DETAIL = _pg_exception_context;
+
+  END;
+END;
+$$ LANGUAGE 'plpgsql' SECURITY DEFINER;
+
+DROP FUNCTION cdb_dataservices_client.obs_getavailablegeometries (geometry(Geometry, 4326), text[], text, text, text);
+DROP FUNCTION cdb_dataservices_client._obs_getavailablegeometries_exception_safe (geometry(Geometry, 4326), text[], text, text, text);
+DROP FUNCTION cdb_dataservices_client._obs_getavailablegeometries (text, text, geometry(Geometry, 4326), text[], text, text, text);
@@ -0,0 +1,65 @@
+--DO NOT MODIFY THIS FILE, IT IS GENERATED AUTOMATICALLY FROM SOURCES
+-- Complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "ALTER EXTENSION cdb_dataservices_client UPDATE TO '0.20.0'" to load this file. \quit
+
+-- Make sure we have a sane search path to create/update the extension
+SET search_path = "$user",cartodb,public,cdb_dataservices_client;
+
+-- HERE goes your code to upgrade/downgrade
+CREATE OR REPLACE FUNCTION cdb_dataservices_client._obs_getavailablegeometries (username text, orgname text, bounds geometry(Geometry, 4326) DEFAULT NULL, filter_tags text[] DEFAULT NULL, numer_id text DEFAULT NULL, denom_id text DEFAULT NULL, timespan text DEFAULT NULL)
+RETURNS SETOF cdb_dataservices_client.obs_meta_geometry AS $$
+  CONNECT cdb_dataservices_client._server_conn_str();
+  SELECT * FROM cdb_dataservices_server.obs_getavailablegeometries (username, orgname, bounds, filter_tags, numer_id, denom_id, timespan);
+$$ LANGUAGE plproxy;
+
+CREATE OR REPLACE FUNCTION cdb_dataservices_client.obs_getavailablegeometries (bounds geometry(Geometry, 4326) DEFAULT NULL ,filter_tags text[] DEFAULT NULL ,numer_id text DEFAULT NULL ,denom_id text DEFAULT NULL ,timespan text DEFAULT NULL)
+RETURNS SETOF cdb_dataservices_client.obs_meta_geometry AS $$
+DECLARE
+  username text;
+  orgname text;
+BEGIN
+  IF session_user = 'publicuser' OR session_user ~ 'cartodb_publicuser_*' THEN
+    RAISE EXCEPTION 'The api_key must be provided';
+  END IF;
+  SELECT u, o INTO username, orgname FROM cdb_dataservices_client._cdb_entity_config() AS (u text, o text);
+  -- JSON value stored "" is taken as literal
+  IF username IS NULL OR username = '' OR username = '""' THEN
+    RAISE EXCEPTION 'Username is a mandatory argument, check it out';
+  END IF;
+
+  RETURN QUERY SELECT * FROM cdb_dataservices_client._obs_getavailablegeometries(username, orgname, bounds, filter_tags, numer_id, denom_id, timespan);
+END;
+$$ LANGUAGE 'plpgsql' SECURITY DEFINER;
+
+CREATE OR REPLACE FUNCTION cdb_dataservices_client._obs_getavailablegeometries_exception_safe (bounds geometry(Geometry, 4326) DEFAULT NULL ,filter_tags text[] DEFAULT NULL ,numer_id text DEFAULT NULL ,denom_id text DEFAULT NULL ,timespan text DEFAULT NULL)
+RETURNS SETOF cdb_dataservices_client.obs_meta_geometry AS $$
+DECLARE
+  username text;
+  orgname text;
+  _returned_sqlstate TEXT;
+  _message_text TEXT;
+  _pg_exception_context TEXT;
+BEGIN
+  IF session_user = 'publicuser' OR session_user ~ 'cartodb_publicuser_*' THEN
+    RAISE EXCEPTION 'The api_key must be provided';
+  END IF;
+  SELECT u, o INTO username, orgname FROM cdb_dataservices_client._cdb_entity_config() AS (u text, o text);
+  -- JSON value stored "" is taken as literal
+  IF username IS NULL OR username = '' OR username = '""' THEN
+    RAISE EXCEPTION 'Username is a mandatory argument, check it out';
+  END IF;
+  BEGIN
+    RETURN QUERY SELECT * FROM cdb_dataservices_client._obs_getavailablegeometries(username, orgname, bounds, filter_tags, numer_id, denom_id, timespan);
+  EXCEPTION
+    WHEN OTHERS THEN
+        GET STACKED DIAGNOSTICS _returned_sqlstate = RETURNED_SQLSTATE,
+                                _message_text = MESSAGE_TEXT,
+                                _pg_exception_context = PG_EXCEPTION_CONTEXT;
+        RAISE WARNING USING ERRCODE = _returned_sqlstate, MESSAGE = _message_text, DETAIL = _pg_exception_context;
+  END;
+END;
+$$ LANGUAGE 'plpgsql' SECURITY DEFINER;
+
+DROP FUNCTION cdb_dataservices_client.obs_getavailablegeometries (geometry(Geometry, 4326), text[], text, text, text, integer);
+DROP FUNCTION cdb_dataservices_client._obs_getavailablegeometries_exception_safe (geometry(Geometry, 4326), text[], text, text, text, integer);
+DROP FUNCTION cdb_dataservices_client._obs_getavailablegeometries (text, text, geometry(Geometry, 4326), text[], text, text, text, integer);
@@ -1,5 +1,5 @@
 comment = 'CartoDB dataservices client API extension'
-default_version = '0.20.0'
+default_version = '0.21.0'
 requires = 'plproxy, cartodb'
 superuser = true
 schema = cdb_dataservices_client
@@ -427,6 +427,7 @@
    - { name: numer_id, type: text, default: 'NULL' }
    - { name: denom_id, type: text, default: 'NULL' }
    - { name: timespan, type: text, default: 'NULL'}
+    - { name: number_geometries, type: integer, default: 'NULL'}

 - name: obs_getavailabletimespans
  return_type: SETOF cdb_dataservices_client.obs_meta_timespan
@@ -1,6 +1,5 @@
 -- Install dependencies
 CREATE EXTENSION postgis;
-CREATE EXTENSION schema_triggers;
 CREATE EXTENSION plpythonu;
 CREATE EXTENSION cartodb;
 CREATE EXTENSION plproxy;
@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 -- Add to the search path the schema
 SET search_path TO public,cartodb,cdb_dataservices_client;
 -- Mock the server function
@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 -- Add to the search path the schema
 SET search_path TO public,cartodb,cdb_dataservices_client;
 -- Mock the server functions
@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 SET client_min_messages TO warning;
 SET search_path TO public,cartodb,cdb_dataservices_client;
 -- Mock the server functions to raise exceptions
@@ -31,8 +32,6 @@ SET ROLE test_regular_user;
 -- Exercise the exception safe and the proxied functions
 SELECT _cdb_geocode_street_point_exception_safe('One street, 1');
 WARNING:  cdb_dataservices_client._cdb_geocode_street_point(6): [contrib_regression] REMOTE ERROR: Not enough quota or any other exception whatsoever.
-DETAIL:  SQL statement "SELECT cdb_dataservices_client._cdb_geocode_street_point(username, orgname, searchtext, city, state_province, country)"
-PL/pgSQL function _cdb_geocode_street_point_exception_safe(text,text,text,text) line 21 at SQL statement
 _cdb_geocode_street_point_exception_safe 
 ------------------------------------------
 
@@ -40,15 +39,12 @@ PL/pgSQL function _cdb_geocode_street_point_exception_safe(text,text,text,text)

 SELECT * FROM _cdb_isodistance_exception_safe('POINT(-3.70568 40.42028)'::geometry, 'walk', ARRAY[300]::integer[]);
 WARNING:  cdb_dataservices_client._cdb_isodistance(6): [contrib_regression] REMOTE ERROR: Not enough quota or any other exception whatsoever.
-DETAIL:  PL/pgSQL function _cdb_isodistance_exception_safe(geometry,text,integer[],text[]) line 21 at RETURN QUERY
 center | data_range | the_geom 
 --------+------------+----------
 (0 rows)

 SELECT * FROM _cdb_route_point_to_point_exception_safe('POINT(-3.70237112 40.41706163)'::geometry,'POINT(-3.69909883 40.41236875)'::geometry, 'car', ARRAY['mode_type=shortest']::text[]);
 WARNING:  cdb_dataservices_client._cdb_route_point_to_point(7): [contrib_regression] REMOTE ERROR: Not enough quota or any other exception whatsoever.
-DETAIL:  SQL statement "SELECT * FROM cdb_dataservices_client._cdb_route_point_to_point(username, orgname, origin, destination, mode, options, units)"
-PL/pgSQL function _cdb_route_point_to_point_exception_safe(geometry,geometry,text,text[],text) line 21 at SQL statement
 shape | length | duration 
 -------+--------+----------
       |        |         
@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 -- Add to the search path the schema
 SET search_path TO public,cartodb,cdb_dataservices_client;
 -- Mock the server functions
@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 -- Add to the search path the schema
 SET search_path TO public,cartodb,cdb_dataservices_client;
 -- Mock the server functions
@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 -- Add to the search path the schema
 SET search_path TO public,cartodb,cdb_dataservices_client;
 -- Mock the server functions
@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 -- Add to the search path the schema
 SET search_path TO public,cartodb,cdb_dataservices_client;
 -- Mock the server functions
@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 -- Add to the search path the schema
 SET search_path TO public,cartodb,cdb_dataservices_client;
 -- Mock the server functions
@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 -- Add to the search path the schema
 SET search_path TO public,cartodb,cdb_dataservices_client;
 -- Mock the server functions
@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 -- Use regular user role
 SET ROLE test_regular_user;
 -- Add to the search path the schema
@@ -1,6 +1,5 @@
 -- Install dependencies
 CREATE EXTENSION postgis;
-CREATE EXTENSION schema_triggers;
 CREATE EXTENSION plpythonu;
 CREATE EXTENSION cartodb;
 CREATE EXTENSION plproxy;
@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 -- Add to the search path the schema
 SET search_path TO public,cartodb,cdb_dataservices_client;

@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 -- Add to the search path the schema
 SET search_path TO public,cartodb,cdb_dataservices_client;

@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 SET client_min_messages TO warning;
 SET search_path TO public,cartodb,cdb_dataservices_client;

@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 -- Add to the search path the schema
 SET search_path TO public,cartodb,cdb_dataservices_client;

@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 -- Add to the search path the schema
 SET search_path TO public,cartodb,cdb_dataservices_client;

@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 -- Add to the search path the schema
 SET search_path TO public,cartodb,cdb_dataservices_client;

@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 -- Add to the search path the schema
 SET search_path TO public,cartodb,cdb_dataservices_client;

@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 -- Add to the search path the schema
 SET search_path TO public,cartodb,cdb_dataservices_client;

@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 -- Add to the search path the schema
 SET search_path TO public,cartodb,cdb_dataservices_client;

@@ -1,3 +1,4 @@
+\set VERBOSITY terse
 -- Use regular user role
 SET ROLE test_regular_user;

@@ -0,0 +1,19 @@
+--DO NOT MODIFY THIS FILE, IT IS GENERATED AUTOMATICALLY FROM SOURCES
+-- Complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "ALTER EXTENSION cdb_dataservices_server UPDATE TO '0.28.0'" to load this file. \quit
+
+-- HERE goes your code to upgrade/downgrade
+CREATE OR REPLACE FUNCTION cdb_dataservices_server.OBS_GetAvailableGeometries(
+  username TEXT,
+  orgname TEXT,
+  bounds geometry(Geometry, 4326) DEFAULT NULL,
+  filter_tags TEXT[] DEFAULT NULL,
+  numer_id TEXT DEFAULT NULL,
+  denom_id TEXT DEFAULT NULL,
+  timespan TEXT DEFAULT NULL,
+  number_geometries INTEGER DEFAULT NULL)
+RETURNS SETOF cdb_dataservices_server.obs_meta_geometry AS $$
+  CONNECT cdb_dataservices_server._obs_server_conn_str(username, orgname);
+  SELECT * FROM cdb_observatory.OBS_GetAvailableGeometries(bounds, filter_tags, numer_id, denom_id, timespan, number_geometries);
+$$ LANGUAGE plproxy;
+DROP FUNCTION cdb_dataservices_server.OBS_GetAvailableGeometries(TEXT, TEXT, geometry(Geometry, 4326), TEXT[], TEXT, TEXT, TEXT);
@@ -0,0 +1,18 @@
+--DO NOT MODIFY THIS FILE, IT IS GENERATED AUTOMATICALLY FROM SOURCES
+-- Complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "ALTER EXTENSION cdb_dataservices_server UPDATE TO '0.27.0'" to load this file. \quit
+
+-- HERE goes your code to upgrade/downgrade
+CREATE OR REPLACE FUNCTION cdb_dataservices_server.OBS_GetAvailableGeometries(
+  username TEXT,
+  orgname TEXT,
+  bounds geometry(Geometry, 4326) DEFAULT NULL,
+  filter_tags TEXT[] DEFAULT NULL,
+  numer_id TEXT DEFAULT NULL,
+  denom_id TEXT DEFAULT NULL,
+  timespan TEXT DEFAULT NULL)
+RETURNS SETOF cdb_dataservices_server.obs_meta_geometry AS $$
+  CONNECT cdb_dataservices_server._obs_server_conn_str(username, orgname);
+  SELECT * FROM cdb_observatory.OBS_GetAvailableGeometries(bounds, filter_tags, numer_id, denom_id, timespan);
+$$ LANGUAGE plproxy;
+DROP FUNCTION cdb_dataservices_server.OBS_GetAvailableGeometries(TEXT, TEXT, geometry(Geometry, 4326), TEXT[], TEXT, TEXT, TEXT, INTEGER);
@@ -1,5 +1,5 @@
 comment = 'CartoDB dataservices server extension'
-default_version = '0.27.0'
+default_version = '0.28.0'
 requires = 'plpythonu, plproxy, postgis, cdb_geocoder'
 superuser = true
 schema = cdb_dataservices_server
@@ -61,10 +61,11 @@ CREATE OR REPLACE FUNCTION cdb_dataservices_server.OBS_GetAvailableGeometries(
  filter_tags TEXT[] DEFAULT NULL,
  numer_id TEXT DEFAULT NULL,
  denom_id TEXT DEFAULT NULL,
-  timespan TEXT DEFAULT NULL)
+  timespan TEXT DEFAULT NULL,
+  number_geometries INTEGER DEFAULT NULL)
 RETURNS SETOF cdb_dataservices_server.obs_meta_geometry AS $$
  CONNECT cdb_dataservices_server._obs_server_conn_str(username, orgname);
-  SELECT * FROM cdb_observatory.OBS_GetAvailableGeometries(bounds, filter_tags, numer_id, denom_id, timespan);
+  SELECT * FROM cdb_observatory.OBS_GetAvailableGeometries(bounds, filter_tags, numer_id, denom_id, timespan, number_geometries);
 $$ LANGUAGE plproxy;

 CREATE TYPE cdb_dataservices_server.obs_meta_timespan AS (timespan_id text, timespan_name text, timespan_description text, timespan_weight text, timespan_aggregate text, timespan_license text, timespan_source text, valid_numer boolean, valid_denom boolean, valid_geom boolean, timespan_type text, timespan_extra jsonb, timespan_tags jsonb);
@@ -245,7 +245,7 @@ SELECT exists(SELECT *
              INNER JOIN pg_namespace ns ON (p.pronamespace = ns.oid)
              WHERE ns.nspname = 'cdb_dataservices_server'
              AND proname = 'obs_getavailablegeometries'
-              AND oidvectortypes(p.proargtypes)  = 'text, text, geometry, text[], text, text, text');
+              AND oidvectortypes(p.proargtypes)  = 'text, text, geometry, text[], text, text, text, integer');
 exists 
 --------
 t
@@ -157,7 +157,7 @@ SELECT exists(SELECT *
              INNER JOIN pg_namespace ns ON (p.pronamespace = ns.oid)
              WHERE ns.nspname = 'cdb_dataservices_server'
              AND proname = 'obs_getavailablegeometries'
-              AND oidvectortypes(p.proargtypes)  = 'text, text, geometry, text[], text, text, text');
+              AND oidvectortypes(p.proargtypes)  = 'text, text, geometry, text[], text, text, text, integer');

 SELECT exists(SELECT *
              FROM pg_proc p
@@ -0,0 +1,38 @@
+#!/usr/local/bin/python
+# -*- coding: utf-8 -*-
+
+import googlemaps
+import base64
+from exceptions import InvalidGoogleCredentials
+
+class GoogleMapsClientFactory():
+    clients = {}
+
+    @classmethod
+    def get(cls, client_id, client_secret):
+        cache_key = "{}:{}".format(client_id, client_secret)
+        client = cls.clients.get(cache_key)
+        if not client:
+            cls.assert_valid_crendentials(client_secret)
+            client = googlemaps.Client(
+                client_id=client_id,
+                client_secret=client_secret)
+            cls.clients[cache_key] = client
+        return client
+
+    @classmethod
+    def assert_valid_crendentials(cls, client_secret):
+        if not cls.valid_credentials(client_secret):
+            raise InvalidGoogleCredentials
+
+    @staticmethod
+    def valid_credentials(client_secret):
+        try:
+            # Only fails if the string dont have a correct padding for b64
+            # but this way we could provide a more clear error than
+            # TypeError: Incorrect padding
+            b64_secret = client_secret.replace('-', '+').replace('_', '/')
+            base64.b64decode(b64_secret)
+            return True
+        except TypeError:
+            return False
@@ -1,22 +1,19 @@
 #!/usr/local/bin/python
 # -*- coding: utf-8 -*-

-import base64
 import googlemaps

-from exceptions import MalformedResult, InvalidGoogleCredentials
+from exceptions import MalformedResult
+from client_factory import GoogleMapsClientFactory


 class GoogleMapsGeocoder:
    """A Google Maps Geocoder wrapper for python"""

    def __init__(self, client_id, client_secret, logger):
-        if not self._valid_credentials(client_secret):
-            raise InvalidGoogleCredentials('Invalid google secret key')
        self.client_id = self._clean_client_id(client_id)
        self.client_secret = client_secret
-        self.geocoder = googlemaps.Client(
-            client_id=self.client_id, client_secret=self.client_secret)
+        self.geocoder = GoogleMapsClientFactory.get(self.client_id, self.client_secret)
        self._logger = logger

    def geocode(self, searchtext, city=None, state=None,
@@ -52,13 +49,3 @@ class GoogleMapsGeocoder:
    def _clean_client_id(self, client_id):
        # Consistency with how the client_id is saved in metadata
        return client_id.replace('client=', '')
-
-    def _valid_credentials(self, private_key):
-        try:
-            # Only fails if the string dont have a correct padding for b64
-            # but this way we could provide a more clear error than
-            # TypeError: Incorrect padding
-            base64.b64decode(private_key)
-            return True
-        except TypeError:
-            return False
@@ -32,7 +32,7 @@ class QuotaService:

    def increment_failed_service_use(self, amount=1):
        self._user_service.increment_service_use(
-            self._user_service_config.service_type, "fail_responses",
+            self._user_service_config.service_type, "failed_responses",
            amount=amount)
        self._log_service_process("fail")

@@ -1,7 +1,7 @@
 redis==2.10.5
 hiredis==0.1.5
 python-dateutil==2.2
-googlemaps==2.4.2
+googlemaps==2.5.1
 rollbar==0.13.2
 # Dependency for googlemaps package
 requests==2.9.1
@@ -10,7 +10,7 @@ from setuptools import setup, find_packages
 setup(
    name='cartodb_services',

-    version='0.15.4',
+    version='0.15.5',

    description='CartoDB Services API Python Library',

@@ -0,0 +1,61 @@
+#!/usr/local/bin/python
+# -*- coding: utf-8 -*-
+
+import googlemaps
+import unittest
+import base64
+
+from cartodb_services.google.client_factory import GoogleMapsClientFactory
+from cartodb_services.google.exceptions import InvalidGoogleCredentials
+
+class GoogleMapsClientFactoryTestCase(unittest.TestCase):
+
+    def setUp(self):
+        pass
+
+    def tearDown(self):
+        # reset client cache
+        GoogleMapsClientFactory.clients = {}
+
+    def test_consecutive_calls_with_same_params_return_same_client(self):
+        id = 'any_id'
+        key = base64.b64encode('any_key')
+        client1 = GoogleMapsClientFactory.get(id, key)
+        client2 = GoogleMapsClientFactory.get(id, key)
+        self.assertEqual(client1, client2)
+
+    def test_consecutive_calls_with_different_key_return_different_clients(self):
+        """
+        This requirement is important for security reasons as well as not to
+        cache a wrong key accidentally.
+        """
+        id = 'any_id'
+        key1 = base64.b64encode('any_key')
+        key2 = base64.b64encode('another_key')
+        client1 = GoogleMapsClientFactory.get(id, key1)
+        client2 = GoogleMapsClientFactory.get(id, key2)
+        self.assertNotEqual(client1, client2)
+
+    def test_consecutive_calls_with_different_ids_return_different_clients(self):
+        """
+        This requirement is important for security reasons as well as not to
+        cache a wrong key accidentally.
+        """
+        id1 = 'any_id'
+        id2 = 'another_id'
+        key = base64.b64encode('any_key')
+        client1 = GoogleMapsClientFactory.get(id1, key)
+        client2 = GoogleMapsClientFactory.get(id2, key)
+        self.assertNotEqual(client1, client2)
+
+    def test_invalid_credentials(self):
+        with self.assertRaises(InvalidGoogleCredentials):
+            GoogleMapsClientFactory.get('dummy_client_id', 'lalala')
+
+    def test_credentials_with_dashes_can_be_valid(self):
+        client = GoogleMapsClientFactory.get('yet_another_dummy_client_id', 'Ola-k-ase---')
+        self.assertIsInstance(client, googlemaps.Client)
+
+    def test_credentials_with_underscores_can_be_valid(self):
+        client = GoogleMapsClientFactory.get('yet_another_dummy_client_id', 'Ola_k_ase___')
+        self.assertIsInstance(client, googlemaps.Client)
@@ -6,7 +6,7 @@ import requests_mock
 from mock import Mock

 from cartodb_services.google import GoogleMapsGeocoder
-from cartodb_services.google.exceptions import MalformedResult, InvalidGoogleCredentials
+from cartodb_services.google.exceptions import MalformedResult

 requests_mock.Mocker.TEST_PREFIX = 'test_'

@@ -118,9 +118,3 @@ class GoogleGeocoderTestCase(unittest.TestCase):
                searchtext='Calle Eloy Gonzalo 27',
                city='Madrid',
                country='España')
-
-    def test_invalid_credentials(self, req_mock):
-        with self.assertRaises(InvalidGoogleCredentials):
-            GoogleMapsGeocoder('dummy_client_id',
-                               'lalala',
-                               None)
@@ -52,7 +52,7 @@ class TestUserService(TestCase):
        assert us.used_quota(self.NOKIA_GEOCODER, date.today()) == 1
        us.increment_service_use(self.NOKIA_GEOCODER, 'empty_responses')
        assert us.used_quota(self.NOKIA_GEOCODER, date.today()) == 2
-        us.increment_service_use(self.NOKIA_GEOCODER, 'fail_responses')
+        us.increment_service_use(self.NOKIA_GEOCODER, 'failed_responses')
        assert us.used_quota(self.NOKIA_GEOCODER, date.today()) == 2

    def test_should_increment_org_used_quota(self):
@@ -61,7 +61,7 @@ class TestUserService(TestCase):
        assert us.used_quota(self.NOKIA_GEOCODER, date.today()) == 1
        us.increment_service_use(self.NOKIA_GEOCODER, 'empty_responses')
        assert us.used_quota(self.NOKIA_GEOCODER, date.today()) == 2
-        us.increment_service_use(self.NOKIA_GEOCODER, 'fail_responses')
+        us.increment_service_use(self.NOKIA_GEOCODER, 'failed_responses')
        assert us.used_quota(self.NOKIA_GEOCODER, date.today()) == 2

    def test_should_increment_user_used_quota_in_for_multiples_dates(self):
@@ -81,7 +81,7 @@ class TestUserService(TestCase):
        us.increment_service_use(self.NOKIA_GEOCODER, 'empty_responses',
                                 date=one_day_after)
        assert us.used_quota(self.NOKIA_GEOCODER, date.today()) == 2
-        us.increment_service_use(self.NOKIA_GEOCODER, 'fail_responses')
+        us.increment_service_use(self.NOKIA_GEOCODER, 'failed_responses')
        assert us.used_quota(self.NOKIA_GEOCODER, date.today()) == 2

    @freeze_time("2015-06-01")
Author	SHA1	Message	Date
Mario de Frutos	d8a20a61b2	Merge pull request #404 from CartoDB/development Release client 0.21.0 and server 0.28.0	2017-10-18 10:28:18 +02:00
Mario de Frutos	423431460d	Merge pull request #403 from CartoDB/numgeoms_getavailablegeometries Number of geometries for OBS_GetAvailableGeometries	2017-10-18 10:26:48 +02:00
Mario de Frutos	90e3229e00	Updated NEWS.md	2017-10-18 10:25:14 +02:00
Mario de Frutos	68e87ca477	Release 0.28.0 server extension artifact	2017-10-18 10:25:04 +02:00
Mario de Frutos	b56a6bfa45	Release 0.21.0 client extension artifact	2017-10-18 10:24:50 +02:00
Mario de Frutos	4c7a3a25ec	Remove duplicated functions after the new ones are added	2017-10-10 16:23:15 +02:00
Mario de Frutos	e68cc3e820	OBS_GetAvailableGeometries now has number of geometries parameter We've add a new parameter, number of geometries, that would be used by DO to compute the geometry that better fit with the input provided	2017-10-09 17:20:57 +02:00
Mario de Frutos	baf9a48dad	Stub version 0.28.0 for server	2017-10-09 17:05:07 +02:00
Mario de Frutos	8ecd69d0bf	Stub version 0.21.0 for client	2017-10-09 17:03:40 +02:00
Rafa de la Torre	1936374124	Version 0.15.5 of the python library	2017-10-06 16:03:03 +02:00
Rafa de la Torre	988a0c31dc	Merge pull request #402 from CartoDB/401-persist-gloogle-client-connections GoogleMapsClientFactory to persist clients #401	2017-10-06 15:46:54 +02:00
Rafa de la Torre	0cd01ee9c9	Merge pull request #400 from CartoDB/383-upgrade-googlemaps-lib-v2.5.1 Upgrade googlemaps python library to v2.5.1	2017-10-06 15:45:05 +02:00
Rafa de la Torre	3ed41b065a	Marginally improve tests for #382	2017-10-06 15:27:51 +02:00
Rafa de la Torre	6c3260b6ee	Move credentials tests to GoogleMapsClientFactoryTestCase	2017-10-06 15:23:44 +02:00
Rafa de la Torre	5005572f89	Add tests for GoogleMapsClientFactory	2017-10-06 15:18:51 +02:00
Rafa de la Torre	27fde5a910	Use "{id}:{secret}" as caching key	2017-10-06 15:12:05 +02:00
Rafa de la Torre	b8e1120169	Revert "Use a different client_id for the test" This reverts commit `854fbb803c`.	2017-10-06 15:03:08 +02:00
Rafa de la Torre	8e33cf9723	Fix google services key validity check #382	2017-10-06 14:40:53 +02:00
Rafa de la Torre	7b988e3f78	Add tests for #382 The examples are not arbitrary: base64 by default ignores characters that are not in its alphabet. When decoding, the dashes and underscores are not b64-valid and therefore ignored, resulting in a sequence with the wrong padding.	2017-10-06 14:37:26 +02:00
Rafa de la Torre	f79ac9297d	Add again the check for valid credentials As our check is more strict that the one provided in googlemaps library.	2017-10-06 13:44:08 +02:00
Rafa de la Torre	854fbb803c	Use a different client_id for the test	2017-10-06 13:41:50 +02:00
Rafa de la Torre	d029ad7041	GoogleMapsClientFactory to persist clients #401 Add a GoogleMapsClientFactory and remove the check for valid key, which is no longer needed, as it is done in the google library.	2017-10-06 12:05:34 +02:00
Rafa de la Torre	24c29c0847	Make test order execution deterministic	2017-10-04 17:26:30 +02:00
Rafa de la Torre	31e79cb626	Remove installation of schema_triggers Schema triggers dependency was deprecated time ago. See https://github.com/CartoDB/cartodb-postgresql/blob/5d43faecaf5659cbc32f3e9468e10e029ea47cd7/NEWS.md#0120-2016-01-27	2017-10-04 17:26:30 +02:00
Rafa de la Torre	98eef7ae02	Improve robustness of client tests All tests that were failing in CI with this diffs showing traces with context, which correspond to a default `VERBOSITY verbose` in the server. Since we migrated the CI to postgres 9.6 it makes sense to see such differences in server configuration. Enforcing the verbosity level makes the tests more robust indeed.	2017-10-04 17:26:13 +02:00
Rafa de la Torre	445d4cf97d	Upgrade googlemaps python library to v2.5.1 This closes #383	2017-10-04 16:22:33 +02:00
Rafa de la Torre	3a86ffba71	Merge pull request #399 from CartoDB/keep-metrics-keys-consistent Keep metrics keys consistent	2017-09-07 11:54:55 +02:00
Rafa de la Torre	f0a3249b4e	Keep metrics keys consistent See cartodb repo and our internal documentation: https://github.com/CartoDB/cartodb/search?utf8=%E2%9C%93&q=failed_responses&type=	2017-09-07 11:24:38 +02:00