refactor performance platform code

so that it doesn't appear generic when it's actually specific to sending the daily notification totals. To do this, split it out into a separate performance_platform directory, containing the business logic, and make the performance_platform_client incredibly thin - all it handles is adding ids to payloads, and sending stats. Also, some changes to the config (not all done yet) since there is one token per endpoint, not one for the whole platform as we'd previously coded
2026-02-04 18:31:13 -05:00 · 2017-08-23 15:04:37 +01:00
parent 3794722203
commit 89f4f5173e
8 changed files with 193 additions and 166 deletions
--- a/app/celery/scheduled_tasks.py
+++ b/app/celery/scheduled_tasks.py
@@ -8,6 +8,7 @@ from sqlalchemy.exc import SQLAlchemyError

 from app.aws import s3
 from app import notify_celery
+from app.performance_platform import total_sent_notifications
 from app import performance_platform_client
 from app.dao.date_util import get_month_start_and_end_date_in_utc
 from app.dao.inbound_sms_dao import delete_inbound_sms_created_more_than_a_week_ago
@@ -175,7 +176,7 @@ def timeout_notifications():
@statsd(namespace="tasks")
 def send_daily_performance_platform_stats():
    if performance_platform_client.active:
-        count_dict = performance_platform_client.get_total_sent_notifications_yesterday()
+        count_dict = total_sent_notifications.get_total_sent_notifications_yesterday()
        email_sent_count = count_dict.get('email').get('count')
        sms_sent_count = count_dict.get('sms').get('count')
        start_date = count_dict.get('start_date')
@@ -185,14 +186,14 @@ def send_daily_performance_platform_stats():
            .format(start_date, email_sent_count, sms_sent_count)
        )

-        performance_platform_client.send_performance_stats(
+        total_sent_notifications.send_total_notifications_sent_for_day_stats(
            start_date,
            'sms',
            sms_sent_count,
            'day'
        )

-        performance_platform_client.send_performance_stats(
+        total_sent_notifications.send_total_notifications_sent_for_day_stats(
            start_date,
            'email',
            email_sent_count,
--- a/app/clients/performance_platform/performance_platform_client.py
+++ b/app/clients/performance_platform/performance_platform_client.py
@@ -1,11 +1,8 @@
 import base64
 import json
-from datetime import datetime

-import requests
 from flask import current_app
-
-from app.utils import get_midnight_for_day_before, get_london_midnight_in_utc, convert_utc_to_bst
+import requests


 class PerformancePlatformClient:
@@ -14,72 +11,40 @@ class PerformancePlatformClient:
    def active(self):
        return self._active

-    @active.setter
-    def active(self, value):
-        self._active = value
-
    def init_app(self, app):
        self._active = app.config.get('PERFORMANCE_PLATFORM_ENABLED')
        if self.active:
-            self.bearer_token = app.config.get('PERFORMANCE_PLATFORM_TOKEN')
            self.performance_platform_url = app.config.get('PERFORMANCE_PLATFORM_URL')
+            self.performance_platform_endpoints = app.config.get('PERFORMANCE_PLATFORM_ENDPOINTS')

-    def send_performance_stats(self, date, channel, count, period):
+    def send_stats_to_performance_platform(self, dataset, payload):
        if self.active:
-            payload = {
-                '_timestamp': convert_utc_to_bst(date).isoformat(),
-                'service': 'govuk-notify',
-                'channel': channel,
-                'count': count,
-                'dataType': 'notifications',
-                'period': period
+            bearer_token = self.performance_platform_endpoints[dataset]
+            headers = {
+                'Content-Type': "application/json",
+                'Authorization': 'Bearer {}'.format(bearer_token)
            }
-            self._add_id_for_payload(payload)
-            self._send_stats_to_performance_platform(payload)
-
-    def get_total_sent_notifications_yesterday(self):
-        today = datetime.utcnow()
-        start_date = get_midnight_for_day_before(today)
-        end_date = get_london_midnight_in_utc(today)
-
-        from app.dao.notifications_dao import get_total_sent_notifications_in_date_range
-        email_count = get_total_sent_notifications_in_date_range(start_date, end_date, 'email')
-        sms_count = get_total_sent_notifications_in_date_range(start_date, end_date, 'sms')
-
-        return {
-            "start_date": start_date,
-            "email": {
-                "count": email_count
-            },
-            "sms": {
-                "count": sms_count
-            }
-        }
-
-    def _send_stats_to_performance_platform(self, payload):
-        headers = {
-            'Content-Type': "application/json",
-            'Authorization': 'Bearer {}'.format(self.bearer_token)
-        }
-        resp = requests.post(
-            self.performance_platform_url,
-            json=payload,
-            headers=headers
-        )
-
-        if resp.status_code == 200:
-            current_app.logger.info(
-                "Updated performance platform successfully with payload {}".format(json.dumps(payload))
-            )
-        else:
-            current_app.logger.error(
-                "Performance platform update request failed for payload with response details: {} '{}'".format(
-                    json.dumps(payload),
-                    resp.status_code,
-                    resp.json())
+            resp = requests.post(
+                self.performance_platform_url + dataset,
+                json=payload,
+                headers=headers
            )

-    def _add_id_for_payload(self, payload):
+            if resp.status_code == 200:
+                current_app.logger.info(
+                    "Updated performance platform successfully with payload {}".format(json.dumps(payload))
+                )
+            else:
+                current_app.logger.error(
+                    "Performance platform update request failed for payload with response details: {} '{}'".format(
+                        json.dumps(payload),
+                        resp.status_code
+                    )
+                )
+                resp.raise_for_status()
+
+    @staticmethod
+    def add_id_to_payload(payload):
        payload_string = '{}{}{}{}{}'.format(
            payload['_timestamp'],
            payload['service'],
--- a/app/config.py
+++ b/app/config.py
@@ -93,7 +93,7 @@ class Config(object):

    # Performance platform
    PERFORMANCE_PLATFORM_ENABLED = False
-    PERFORMANCE_PLATFORM_URL = 'https://www.performance.service.gov.uk/data/govuk-notify/notifications'
+    PERFORMANCE_PLATFORM_URL = 'https://www.performance.service.gov.uk/data/govuk-notify/'
    PERFORMANCE_PLATFORM_TOKEN = os.getenv('PERFORMANCE_PLATFORM_TOKEN')

    # Logging
--- a/app/performance_platform/init.py
+++ b/app/performance_platform/init.py
--- a/app/performance_platform/total_sent_notifications.py
+++ b/app/performance_platform/total_sent_notifications.py
@@ -0,0 +1,44 @@
+from datetime import datetime
+
+from app import performance_platform_client
+from app.dao.notifications_dao import get_total_sent_notifications_in_date_range
+from app.utils import (
+    get_london_midnight_in_utc,
+    get_midnight_for_day_before,
+    convert_utc_to_bst,
+)
+
+
+def send_total_notifications_sent_for_day_stats(date, channel, count, period):
+    payload = {
+        '_timestamp': convert_utc_to_bst(date).isoformat(),
+        'service': 'govuk-notify',
+        'channel': channel,
+        'count': count,
+        'dataType': 'notifications',
+        'period': period
+    }
+    performance_platform_client.add_id_to_payload(payload)
+
+    performance_platform_client.send_stats_to_performance_platform(
+        dataset='notifications',
+        payload=payload
+    )
+
+def get_total_sent_notifications_yesterday():
+    today = datetime.utcnow()
+    start_date = get_midnight_for_day_before(today)
+    end_date = get_london_midnight_in_utc(today)
+
+    email_count = get_total_sent_notifications_in_date_range(start_date, end_date, 'email')
+    sms_count = get_total_sent_notifications_in_date_range(start_date, end_date, 'sms')
+
+    return {
+        "start_date": start_date,
+        "email": {
+            "count": email_count
+        },
+        "sms": {
+            "count": sms_count
+        }
+    }
--- a/tests/app/celery/test_scheduled_tasks.py
+++ b/tests/app/celery/test_scheduled_tasks.py
@@ -276,7 +276,7 @@ def test_send_daily_performance_stats_calls_does_not_send_if_inactive(
    sample_template,
    mocker
 ):
-    send_mock = mocker.patch('app.celery.scheduled_tasks.performance_platform_client.send_performance_stats')
+    send_mock = mocker.patch('app.celery.scheduled_tasks.total_sent_notifications.send_total_notifications_sent_for_day_stats')

    with patch.object(
        PerformancePlatformClient,
@@ -296,7 +296,7 @@ def test_send_daily_performance_stats_calls_with_correct_totals(
    sample_template,
    mocker
 ):
-    perf_mock = mocker.patch('app.celery.scheduled_tasks.performance_platform_client.send_performance_stats')
+    perf_mock = mocker.patch('app.celery.scheduled_tasks.total_sent_notifications.send_total_notifications_sent_for_day_stats')

    notification_history = partial(
        create_notification_history,
--- a/tests/app/clients/test_performance_platform.py
+++ b/tests/app/clients/test_performance_platform.py
@@ -1,119 +1,59 @@
+import requests
 import requests_mock
 import pytest
-from datetime import datetime
-from freezegun import freeze_time
-from functools import partial

 from app.clients.performance_platform.performance_platform_client import PerformancePlatformClient
-from app.utils import (
-    get_london_midnight_in_utc,
-    get_midnight_for_day_before
-)
-from tests.app.conftest import sample_notification_history as create_notification_history


@pytest.fixture(scope='function')
-def client(mocker):
-    client = PerformancePlatformClient()
+def perf_client(mocker):
+    perf_client = PerformancePlatformClient()
    current_app = mocker.Mock(config={
        'PERFORMANCE_PLATFORM_ENABLED': True,
-        'PERFORMANCE_PLATFORM_URL': 'https://performance-platform-url/',
-        'PERFORMANCE_PLATFORM_TOKEN': 'token'
+        'PERFORMANCE_PLATFORM_ENDPOINTS': {
+            'foo': 'my_token',
+            'bar': 'other_token'
+        },
+        'PERFORMANCE_PLATFORM_URL': 'https://performance-platform-url/'
    })
-    client.init_app(current_app)
-    return client
+    perf_client.init_app(current_app)
+    return perf_client


-def test_should_not_call_if_not_enabled(notify_api, client, mocker):
-    mocker.patch.object(client, '_send_stats_to_performance_platform')
-    client.active = False
-    client.send_performance_stats(
-        date=datetime(2016, 10, 16, 0, 0, 0),
-        channel='sms',
-        count=142,
-        period='day'
-    )
-
-    client._send_stats_to_performance_platform.assert_not_called()
-
-
-def test_should_call_if_enabled(notify_api, client, mocker):
-    mocker.patch.object(client, '_send_stats_to_performance_platform')
-    client.send_performance_stats(
-        date=datetime(2016, 10, 16, 0, 0, 0),
-        channel='sms',
-        count=142,
-        period='day'
-    )
-
-    assert client._send_stats_to_performance_platform.call_count == 1
-
-
-def test_send_platform_stats_creates_correct_call(notify_api, client):
+def test_should_not_call_if_not_enabled(perf_client):
    with requests_mock.Mocker() as request_mock:
-        request_mock.post(
-            client.performance_platform_url,
-            json={},
-            status_code=200
-        )
-        client.send_performance_stats(
-            date=datetime(2016, 10, 15, 23, 0, 0),
-            channel='sms',
-            count=142,
-            period='day'
-        )
+        request_mock.post('https://performance-platform-url/foo', json={}, status_code=200)
+        perf_client._active = False
+        perf_client.send_stats_to_performance_platform(dataset='foo', payload={})
+
+    assert request_mock.called is False
+
+
+
+def test_should_call_if_enabled(perf_client):
+    with requests_mock.Mocker() as request_mock:
+        request_mock.post('https://performance-platform-url/foo', json={}, status_code=200)
+        perf_client.send_stats_to_performance_platform(dataset='foo', payload={})

    assert request_mock.call_count == 1
-
-    assert request_mock.request_history[0].url == client.performance_platform_url
-    assert request_mock.request_history[0].method == 'POST'
-
-    request_args = request_mock.request_history[0].json()
-    assert request_args['dataType'] == 'notifications'
-    assert request_args['service'] == 'govuk-notify'
-    assert request_args['period'] == 'day'
-    assert request_args['channel'] == 'sms'
-    assert request_args['_timestamp'] == '2016-10-16T00:00:00'
-    assert request_args['count'] == 142
-    expected_base64_id = 'MjAxNi0xMC0xNlQwMDowMDowMGdvdnVrLW5vdGlmeXNtc25vdGlmaWNhdGlvbnNkYXk='
-    assert request_args['_id'] == expected_base64_id
+    assert request_mock.last_request.method == 'POST'


-@freeze_time("2016-01-11 12:30:00")
-def test_get_total_sent_notifications_yesterday_returns_expected_totals_dict(
-    notify_db,
-    notify_db_session,
-    client,
-    sample_template
-):
-    notification_history = partial(
-        create_notification_history,
-        notify_db,
-        notify_db_session,
-        sample_template,
-        status='delivered'
-    )
+@pytest.mark.parametrize('dataset, token', [
+    ('foo', 'my_token'),
+    ('bar', 'other_token')
+])
+def test_should_use_correct_token(perf_client, dataset, token):
+    with requests_mock.Mocker() as request_mock:
+        request_mock.post('https://performance-platform-url/foo', json={}, status_code=200)
+        request_mock.post('https://performance-platform-url/bar', json={}, status_code=200)
+        perf_client.send_stats_to_performance_platform(dataset=dataset, payload={})

-    notification_history(notification_type='email')
-    notification_history(notification_type='sms')
+    assert request_mock.call_count == 1
+    assert request_mock.last_request.headers.get('authorization') == 'Bearer {}'.format(token)

-    # Create some notifications for the day before
-    yesterday = datetime(2016, 1, 10, 15, 30, 0, 0)
-    with freeze_time(yesterday):
-        notification_history(notification_type='sms')
-        notification_history(notification_type='sms')
-        notification_history(notification_type='email')
-        notification_history(notification_type='email')
-        notification_history(notification_type='email')

-    total_count_dict = client.get_total_sent_notifications_yesterday()
-
-    assert total_count_dict == {
-        "start_date": get_midnight_for_day_before(datetime.utcnow()),
-        "email": {
-            "count": 3
-        },
-        "sms": {
-            "count": 2
-        }
-    }
+def test_should_raise_for_status(perf_client):
+    with pytest.raises(requests.HTTPError), requests_mock.Mocker() as request_mock:
+        request_mock.post('https://performance-platform-url/foo', json={}, status_code=403)
+        perf_client.send_stats_to_performance_platform(dataset='foo', payload={})
--- a/tests/app/performance_platform/test_total_sent_notifications.py
+++ b/tests/app/performance_platform/test_total_sent_notifications.py
@@ -0,0 +1,77 @@
+from datetime import datetime
+from functools import partial
+
+from freezegun import freeze_time
+
+from app.utils import get_midnight_for_day_before
+from app.performance_platform.total_sent_notifications import(
+    send_total_notifications_sent_for_day_stats,
+    get_total_sent_notifications_yesterday
+)
+
+from tests.app.conftest import (
+    sample_notification_history as create_notification_history
+)
+
+
+def test_send_total_notifications_sent_for_day_stats_stats_creates_correct_call(mocker, client):
+    send_stats = mocker.patch('app.performance_platform.total_sent_notifications.performance_platform_client.send_stats_to_performance_platform')  # noqa
+
+    send_total_notifications_sent_for_day_stats(
+        date=datetime(2016, 10, 15, 23, 0, 0),
+        channel='sms',
+        count=142,
+        period='day'
+    )
+
+    assert send_stats.call_count == 1
+    assert send_stats.call_args[1]['dataset'] == 'notifications'
+
+    request_args = send_stats.call_args[1]['payload']
+    assert request_args['dataType'] == 'notifications'
+    assert request_args['service'] == 'govuk-notify'
+    assert request_args['period'] == 'day'
+    assert request_args['channel'] == 'sms'
+    assert request_args['_timestamp'] == '2016-10-16T00:00:00'
+    assert request_args['count'] == 142
+    expected_base64_id = 'MjAxNi0xMC0xNlQwMDowMDowMGdvdnVrLW5vdGlmeXNtc25vdGlmaWNhdGlvbnNkYXk='
+    assert request_args['_id'] == expected_base64_id
+
+
+@freeze_time("2016-01-11 12:30:00")
+def test_get_total_sent_notifications_yesterday_returns_expected_totals_dict(
+    notify_db,
+    notify_db_session,
+    sample_template
+):
+    notification_history = partial(
+        create_notification_history,
+        notify_db,
+        notify_db_session,
+        sample_template,
+        status='delivered'
+    )
+
+    notification_history(notification_type='email')
+    notification_history(notification_type='sms')
+
+    # Create some notifications for the day before
+    yesterday = datetime(2016, 1, 10, 15, 30, 0, 0)
+    with freeze_time(yesterday):
+        notification_history(notification_type='sms')
+        notification_history(notification_type='sms')
+        notification_history(notification_type='email')
+        notification_history(notification_type='email')
+        notification_history(notification_type='email')
+
+    total_count_dict = get_total_sent_notifications_yesterday()
+
+    assert total_count_dict == {
+        "start_date": get_midnight_for_day_before(datetime.utcnow()),
+        "email": {
+            "count": 3
+        },
+        "sms": {
+            "count": 2
+        }
+    }