From d00223b11695c522b60497f4fdcd2d8bf0704f17 Mon Sep 17 00:00:00 2001 From: antoniocarlon Date: Wed, 8 May 2019 14:26:51 +0200 Subject: [PATCH 1/2] Added Google Maps API key support --- .../cartodb_services/google/client_factory.py | 10 +++-- .../cartodb_services/google/geocoder.py | 7 +++- server/lib/python/cartodb_services/setup.py | 2 +- .../cartodb_services/test/credentials.py | 5 +++ .../test/test_google_client_factory.py | 14 ++++--- .../test/test_googlegeocoder.py | 37 +++++++++++++++---- 6 files changed, 55 insertions(+), 20 deletions(-) diff --git a/server/lib/python/cartodb_services/cartodb_services/google/client_factory.py b/server/lib/python/cartodb_services/cartodb_services/google/client_factory.py index 9593f4f0..2f74412c 100644 --- a/server/lib/python/cartodb_services/cartodb_services/google/client_factory.py +++ b/server/lib/python/cartodb_services/cartodb_services/google/client_factory.py @@ -5,23 +5,25 @@ import base64 from exceptions import InvalidGoogleCredentials +CLIENT_SECRET_APIKEY = 'key=' + class GoogleMapsClientFactory(): clients = {} @classmethod - def get(cls, client_id, client_secret, channel=None): + def get(cls, client_id=None, client_secret=None, channel=None): cache_key = "{}:{}:{}".format(client_id, client_secret, channel) client = cls.clients.get(cache_key) if not client: - if client_id: + if client_secret.startswith(CLIENT_SECRET_APIKEY): + client = googlemaps.Client(key=client_secret.replace(CLIENT_SECRET_APIKEY, '')) + else: cls.assert_valid_crendentials(client_secret) client = googlemaps.Client( client_id=client_id, client_secret=client_secret, channel=channel) - else: - client = googlemaps.Client(key=client_secret) cls.clients[cache_key] = client return client 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 30a62cf9..c457974f 100644 --- a/server/lib/python/cartodb_services/cartodb_services/google/geocoder.py +++ b/server/lib/python/cartodb_services/cartodb_services/google/geocoder.py @@ -38,12 +38,12 @@ class GoogleMapsGeocoder(): def __init__(self, client_id, client_secret, logger): - if client_id is None: + self._logger = logger + if client_id is None and client_secret is None: raise InvalidGoogleCredentials self.client_id, self.channel = self.parse_client_id(client_id) self.client_secret = client_secret self.geocoder = GoogleMapsClientFactory.get(self.client_id, self.client_secret, self.channel) - self._logger = logger def geocode(self, searchtext, city=None, state=None, country=None): return self.geocode_meta(searchtext, city, state, country)[0] @@ -100,6 +100,9 @@ def _build_optional_parameters(self, city=None, state=None, return optional_params def parse_client_id(self, client_id): + if not client_id: + return None, None + arguments = parse_qs(client_id) client = arguments['client'][0] if arguments.has_key('client') else client_id channel = arguments['channel'][0] if arguments.has_key('channel') else None diff --git a/server/lib/python/cartodb_services/setup.py b/server/lib/python/cartodb_services/setup.py index 4a272ce2..026a5f59 100644 --- a/server/lib/python/cartodb_services/setup.py +++ b/server/lib/python/cartodb_services/setup.py @@ -10,7 +10,7 @@ setup( name='cartodb_services', - version='0.21.4', + version='0.21.5', description='CartoDB Services API Python Library', diff --git a/server/lib/python/cartodb_services/test/credentials.py b/server/lib/python/cartodb_services/test/credentials.py index 52a2a4d9..20d63d79 100644 --- a/server/lib/python/cartodb_services/test/credentials.py +++ b/server/lib/python/cartodb_services/test/credentials.py @@ -9,3 +9,8 @@ def mapbox_api_key(): def tomtom_api_key(): """Returns TomTom API key. Requires setting TOMTOM_API_KEY environment variable.""" return os.environ['TOMTOM_API_KEY'] + + +def google_api_key(): + """Returns Google API key. Requires setting GOOGLE_API_KEY environment variable.""" + return os.environ['GOOGLE_API_KEY'] diff --git a/server/lib/python/cartodb_services/test/test_google_client_factory.py b/server/lib/python/cartodb_services/test/test_google_client_factory.py index 7fd001bd..ce9daeb6 100644 --- a/server/lib/python/cartodb_services/test/test_google_client_factory.py +++ b/server/lib/python/cartodb_services/test/test_google_client_factory.py @@ -48,10 +48,18 @@ def test_consecutive_calls_with_different_ids_return_different_clients(self): client2 = GoogleMapsClientFactory.get(id2, key) self.assertNotEqual(client1, client2) - def test_invalid_credentials(self): + def test_invalid_credentials_client_secret(self): with self.assertRaises(InvalidGoogleCredentials): GoogleMapsClientFactory.get('dummy_client_id', 'lalala') + def test_invalid_credentials_api_key_client(self): + with self.assertRaises(ValueError): + GoogleMapsClientFactory.get('fake_client', 'key=fake_apikey') + + def test_invalid_credentials_api_key_noclient(self): + with self.assertRaises(ValueError): + GoogleMapsClientFactory.get(None, 'key=fake_apikey') + 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) @@ -60,10 +68,6 @@ 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) - def test_invalid_credentials(self): - with self.assertRaises(InvalidGoogleCredentials): - GoogleMapsClientFactory.get('dummy_client_id', 'lalala') - def test_credentials_with_channel(self): client = GoogleMapsClientFactory.get('yet_another_dummy_client_id', 'Ola_k_ase___', 'channel') self.assertIsInstance(client, googlemaps.Client) diff --git a/server/lib/python/cartodb_services/test/test_googlegeocoder.py b/server/lib/python/cartodb_services/test/test_googlegeocoder.py index bf56a604..69118942 100644 --- a/server/lib/python/cartodb_services/test/test_googlegeocoder.py +++ b/server/lib/python/cartodb_services/test/test_googlegeocoder.py @@ -5,14 +5,14 @@ import requests_mock from mock import Mock +from credentials import google_api_key from cartodb_services.google import GoogleMapsGeocoder from cartodb_services.google.exceptions import MalformedResult requests_mock.Mocker.TEST_PREFIX = 'test_' -@requests_mock.Mocker() -class GoogleGeocoderTestCase(unittest.TestCase): +class GoogleGeocoderTestCase(): GOOGLE_MAPS_GEOCODER_URL = 'https://maps.googleapis.com/maps/api/geocode/json' EMPTY_RESPONSE = """{ @@ -87,12 +87,6 @@ class GoogleGeocoderTestCase(unittest.TestCase): MALFORMED_RESPONSE = """{"manolo": "escobar"}""" - def setUp(self): - logger = Mock() - self.geocoder = GoogleMapsGeocoder('dummy_client_id', - 'MgxyOFxjZXIyOGO52jJlMzEzY1Oqy4hsO49E', - logger) - def test_geocode_address_with_valid_params(self, req_mock): req_mock.register_uri('GET', self.GOOGLE_MAPS_GEOCODER_URL, text=self.GOOD_RESPONSE) @@ -119,6 +113,15 @@ def test_geocode_with_malformed_result(self, req_mock): city='Madrid', country='España') + +@requests_mock.Mocker() +class GoogleGeocoderClientSecretTestCase(unittest.TestCase, GoogleGeocoderTestCase): + def setUp(self): + logger = Mock() + self.geocoder = GoogleMapsGeocoder('dummy_client_id', + 'MgxyOFxjZXIyOGO52jJlMzEzY1Oqy4hsO49E', + logger) + def test_client_data_extraction(self, req_mock): client_id, channel = self.geocoder.parse_client_id('dummy_client_id') self.assertEqual(client_id, 'dummy_client_id') @@ -133,3 +136,21 @@ def test_client_data_extraction_with_client_and_channel_parameter(self, req_mock client_id, channel = self.geocoder.parse_client_id('client=gme-test&channel=testchannel') self.assertEqual(client_id, 'gme-test') self.assertEqual(channel, 'testchannel') + + +@requests_mock.Mocker() +class GoogleGeocoderApiKeyTestCase(unittest.TestCase, GoogleGeocoderTestCase): + def setUp(self): + logger = Mock() + self.geocoder = GoogleMapsGeocoder(None, + google_api_key(), + logger) + + +@requests_mock.Mocker() +class GoogleGeocoderClientApiKeyTestCase(unittest.TestCase, GoogleGeocoderTestCase): + def setUp(self): + logger = Mock() + self.geocoder = GoogleMapsGeocoder('fake_client_id', + google_api_key(), + logger) From cfa493c8bddd88e0b96f0401165b2185bcb6ff35 Mon Sep 17 00:00:00 2001 From: antoniocarlon Date: Mon, 27 May 2019 12:43:34 +0200 Subject: [PATCH 2/2] Check for quota increment in Google geocoder --- .../cartodb_services/test/test_quota_service.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/server/lib/python/cartodb_services/test/test_quota_service.py b/server/lib/python/cartodb_services/test/test_quota_service.py index ce1ca21e..7efb76ef 100644 --- a/server/lib/python/cartodb_services/test/test_quota_service.py +++ b/server/lib/python/cartodb_services/test/test_quota_service.py @@ -114,6 +114,23 @@ def test_should_check_org_mapbox_geocoder_quota_correctly(self): qs.increment_success_service_use(amount=1500000) assert qs.check_user_quota() is False + def test_should_check_user_google_geocoder_quota_correctly(self): + qs = self.__build_geocoder_quota_service('test_user', + provider='google') + qs.increment_success_service_use() + assert qs.check_user_quota() is True + qs.increment_success_service_use(amount=1500000) + assert qs.check_user_quota() is True # We don't check quota for google geocoder + + def test_should_check_org_google_geocoder_quota_correctly(self): + qs = self.__build_geocoder_quota_service('test_user', + orgname='testorg', + provider='google') + qs.increment_success_service_use() + assert qs.check_user_quota() is True + qs.increment_success_service_use(amount=1500000) + assert qs.check_user_quota() is True # We don't check quota for google geocoder + def test_should_check_user_mapzen_routing_quota_correctly(self): qs = self.__build_routing_quota_service('test_user', provider='mapzen') qs.increment_success_service_use()