Merge pull request #41 from alphagov/client-params

allow api_key to be positional
alphagov · Nov 14, 2016 · 7f00e5f · 7f00e5f
2 parents e846c0d + 947b1c9
commit 7f00e5f
Show file tree

Hide file tree

Showing 7 changed files with 67 additions and 66 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -0,0 +1,14 @@
+## 3.0.0 (2016-11-14)
+
+### Changed
+* `BaseClient` method signature: `base_url` is now optional. See [#41](https://github.com/alphagov/notifications-python-client/pull/41)
+* `BaseClient` method signature: `api_key` is now a positional argument. See [#41](https://github.com/alphagov/notifications-python-client/pull/41)
+
+## 2.0.0 (2016-11-09)
+
+### Changed
+* Replace asserts with proper exceptions in jwt token code. See [#40](https://github.com/alphagov/notifications-python-client/pull/40)
+
+# Prior versions
+
+Changelog not recorded - please see pull requests on github.
diff --git a/integration_test/integration_tests.py b/integration_test/integration_tests.py
@@ -47,9 +47,9 @@ def get_all_notifications(client):
 
 if __name__ == "__main__":
     client = NotificationsAPIClient(
-        os.environ['NOTIFY_API_URL'],
-        os.environ['SERVICE_ID'],
-        os.environ['API_KEY']
+        base_url=os.environ['NOTIFY_API_URL'],
+        service_id=os.environ['SERVICE_ID'],
+        api_key=os.environ['API_KEY']
     )
 
     sms_id = send_sms_notification_test_response(client)

diff --git a/notifications_python_client/base.py b/notifications_python_client/base.py
@@ -1,10 +1,11 @@
 from __future__ import absolute_import
+import logging
+import json
 from time import monotonic
+
+from notifications_python_client.version import __version__
 from notifications_python_client.errors import HTTPError, InvalidResponse
 from notifications_python_client.authentication import create_jwt_token
-from notifications_python_client.version import __version__
-import logging
-import json
 
 try:
     import urlparse
@@ -20,9 +21,9 @@
 class BaseAPIClient(object):
     def __init__(
         self,
+        api_key,
         base_url='https://api.notifications.service.gov.uk',
         service_id=None,
-        api_key=None
     ):
         """
         Initialise the client

diff --git a/notifications_python_client/version.py b/notifications_python_client/version.py
@@ -1 +1 @@
-__version__ = '2.0.0'
+__version__ = '3.0.0'
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -1,13 +1,15 @@
+from unittest import mock
+
 import requests_mock
 import pytest
+
 from notifications_python_client.base import BaseAPIClient
 from notifications_python_client.notifications import NotificationsAPIClient
-import mock
 
 
 TEST_HOST = 'http://test-host'
-TEST_CLIENT_ID = "tests"
-TEST_SECRET = 'very'
+SERVICE_ID = 'c745a8d8-b48a-4b0d-96e5-dbea0165ebd1'
+API_KEY_ID = '8b3aa916-ec82-434e-b0c5-d5d9b371d6a3'
 
 
 @pytest.yield_fixture
@@ -25,12 +27,12 @@ def rmock_patch():
 @pytest.yield_fixture
 def base_client():
     yield BaseAPIClient(base_url=TEST_HOST,
-                        service_id=TEST_CLIENT_ID,
-                        api_key=TEST_SECRET)
+                        service_id=SERVICE_ID,
+                        api_key=API_KEY_ID)
 
 
 @pytest.yield_fixture
 def notifications_client():
     yield NotificationsAPIClient(base_url=TEST_HOST,
-                                 service_id=TEST_CLIENT_ID,
-                                 api_key=TEST_SECRET)
+                                 service_id=SERVICE_ID,
+                                 api_key=API_KEY_ID)
diff --git a/tests/notifications_python_client/test_base_api_client.py b/tests/notifications_python_client/test_base_api_client.py
@@ -1,66 +1,50 @@
+import requests
+from unittest import mock
+
 import pytest
-import mock
 from notifications_python_client.errors import HTTPError, InvalidResponse
 from notifications_python_client.base import BaseAPIClient
-import requests
+from tests.conftest import API_KEY_ID, SERVICE_ID
+
+
+COMBINED_API_KEY = '-'.join(['name_of_key', SERVICE_ID, API_KEY_ID])
+EMOJI_API_KEY = '😬'.join(['name_of_key', SERVICE_ID, API_KEY_ID])
 
 
 @pytest.mark.parametrize('client', [
-    BaseAPIClient(
-        service_id='c745a8d8-b48a-4b0d-96e5-dbea0165ebd1',
-        api_key='8b3aa916-ec82-434e-b0c5-d5d9b371d6a3'
-    ),
-    BaseAPIClient(
-        api_key=(
-            'name_of_key'
-            '-'
-            'c745a8d8-b48a-4b0d-96e5-dbea0165ebd1'  # service ID
-            '-'
-            '8b3aa916-ec82-434e-b0c5-d5d9b371d6a3'  # secret
-        )
-    ),
-    BaseAPIClient(
-        api_key=(
-            'name_of_key'
-            '😬'
-            'c745a8d8-b48a-4b0d-96e5-dbea0165ebd1'  # service ID
-            '😬'
-            '8b3aa916-ec82-434e-b0c5-d5d9b371d6a3'  # secret
-        )
-    ),
-    BaseAPIClient(
-        service_id='c745a8d8-b48a-4b0d-96e5-dbea0165ebd1',
-        api_key=(
-            'name_of_key'
-            '-'
-            'c745a8d8-b48a-4b0d-96e5-dbea0165ebd1'  # service ID
-            '-'
-            '8b3aa916-ec82-434e-b0c5-d5d9b371d6a3'  # secret
-        )
-    )
+    BaseAPIClient(service_id=SERVICE_ID, api_key=API_KEY_ID),
+    BaseAPIClient(api_key=COMBINED_API_KEY),
+    BaseAPIClient(api_key=EMOJI_API_KEY),
+    BaseAPIClient(service_id=SERVICE_ID, api_key=COMBINED_API_KEY),
+    BaseAPIClient(COMBINED_API_KEY),
+], ids=[
+    'service and api as kwargs',
+    'combined api key',
+    'key with emoji',
+    'service id and combined api key',
+    'positional api key'
 ])
-@mock.patch('notifications_python_client.base.create_jwt_token')
-def test_passes_through_service_id_and_key(mock_create_token, rmock, client):
-    rmock.request("GET", "/", status_code=204)
-    client.request("GET", '/')
-    mock_create_token.assert_called_once_with(
-        '8b3aa916-ec82-434e-b0c5-d5d9b371d6a3',
-        'c745a8d8-b48a-4b0d-96e5-dbea0165ebd1'
-    )
+def test_passes_through_service_id_and_key(rmock, client):
+    with mock.patch('notifications_python_client.base.create_jwt_token') as mock_create_token:
+        rmock.request("GET", "/", status_code=204)
+        client.request("GET", '/')
+    mock_create_token.assert_called_once_with(API_KEY_ID, SERVICE_ID)
+    assert client.base_url == 'https://api.notifications.service.gov.uk'
+
+
+def test_can_set_base_url():
+    client = BaseAPIClient(base_url='foo', service_id=SERVICE_ID, api_key=COMBINED_API_KEY)
+    assert client.base_url == 'foo'
 
 
 def test_fails_if_client_id_missing():
     with pytest.raises(AssertionError) as err:
-        BaseAPIClient(
-            api_key='8b3aa916-ec82-434e-b0c5-d5d9b371d6a3'
-        )
+        BaseAPIClient(api_key=API_KEY_ID)
     assert str(err.value) == "Missing service ID"
 
 
 def test_connection_error_raises_api_error(base_client, rmock_patch):
-    rmock_patch.side_effect = requests.exceptions.ConnectionError(
-        None
-    )
+    rmock_patch.side_effect = requests.exceptions.ConnectionError(None)
 
     with pytest.raises(HTTPError) as e:
         base_client.request("GET", '/')

diff --git a/utils/make_api_call.py b/utils/make_api_call.py
@@ -89,9 +89,9 @@ def get_template_version(notifications_client):
     arguments = docopt(__doc__)
 
     client = NotificationsAPIClient(
-        arguments['<base_url>'],
-        arguments['<service_id>'],
-        arguments['<secret>']
+        base_url=arguments['<base_url>'],
+        service_id=arguments['<service_id>'],
+        api_key=arguments['<secret>']
     )
 
     if arguments['<call>'] == 'create':