From 8af4b27fd6924b5540a996e67f31cde05a0d9585 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 9 Dec 2020 11:13:50 +0000 Subject: [PATCH] Separate functions for cbc clients Also move message_format to the clients. --- app/celery/broadcast_message_tasks.py | 7 +- app/clients/cbc_proxy.py | 96 +++++++++++---- app/models.py | 5 - .../celery/test_broadcast_message_tasks.py | 28 ++--- tests/app/clients/test_cbc_proxy.py | 111 ++++++++++++++++-- 5 files changed, 188 insertions(+), 59 deletions(-) diff --git a/app/celery/broadcast_message_tasks.py b/app/celery/broadcast_message_tasks.py index e27edbe08..ce80ea54f 100644 --- a/app/celery/broadcast_message_tasks.py +++ b/app/celery/broadcast_message_tasks.py @@ -6,7 +6,7 @@ from sqlalchemy.schema import Sequence from app import cbc_proxy_client, db, notify_celery from app.config import QueueNames -from app.models import BroadcastEventMessageType, BroadcastProvider, BroadcastProviderMessageType +from app.models import BroadcastEventMessageType, BroadcastProvider from app.dao.broadcast_message_dao import dao_get_broadcast_event_by_id, create_broadcast_provider_message from app.utils import format_sequential_number @@ -34,10 +34,8 @@ def send_broadcast_provider_message(broadcast_event_id, provider): broadcast_provider_message, message_number = create_broadcast_provider_message(broadcast_event, provider) formatted_message_number = None - message_format = BroadcastProviderMessageType.CBC if provider == BroadcastProvider.VODAFONE: formatted_message_number = format_sequential_number(message_number.broadcast_provider_message_number) - message_format = BroadcastProviderMessageType.IBAG current_app.logger.info( f'invoking cbc proxy to send ' @@ -56,7 +54,6 @@ def send_broadcast_provider_message(broadcast_event_id, provider): cbc_proxy_provider_client.create_and_send_broadcast( identifier=str(broadcast_provider_message.id), message_number=formatted_message_number, - message_format=message_format, headline="GOV.UK Notify Broadcast", description=broadcast_event.transmitted_content['body'], areas=areas, @@ -67,7 +64,6 @@ def send_broadcast_provider_message(broadcast_event_id, provider): cbc_proxy_provider_client.update_and_send_broadcast( identifier=str(broadcast_provider_message.id), message_number=formatted_message_number, - message_format=message_format, headline="GOV.UK Notify Broadcast", description=broadcast_event.transmitted_content['body'], areas=areas, @@ -79,7 +75,6 @@ def send_broadcast_provider_message(broadcast_event_id, provider): cbc_proxy_provider_client.cancel_broadcast( identifier=str(broadcast_provider_message.id), message_number=formatted_message_number, - message_format=message_format, headline="GOV.UK Notify Broadcast", description=broadcast_event.transmitted_content['body'], areas=areas, diff --git a/app/clients/cbc_proxy.py b/app/clients/cbc_proxy.py index 9cf9fb2bc..7b80b107c 100644 --- a/app/clients/cbc_proxy.py +++ b/app/clients/cbc_proxy.py @@ -65,34 +65,12 @@ class CBCProxyClientBase: sequential_number, message_format ): - """ - link test - open up a connection to a specific provider, and send them an xml payload with a of - test. - """ - payload = { - 'message_type': 'test', - 'identifier': identifier, - 'message_number': sequential_number, - 'message_format': message_format - } - - self._invoke_lambda(payload=payload) + pass def create_and_send_broadcast( self, identifier, message_number, message_format, headline, description, areas, sent, expires, ): - payload = { - 'message_type': 'alert', - 'identifier': identifier, - 'message_number': message_number, - 'message_format': message_format, - 'headline': headline, - 'description': description, - 'areas': areas, - 'sent': sent, - 'expires': expires, - } - self._invoke_lambda(payload=payload) + pass # We have not implementated updating a broadcast def update_and_send_broadcast( @@ -151,6 +129,76 @@ class CBCProxyCanary(CBCProxyClientBase): class CBCProxyEE(CBCProxyClientBase): lambda_name = 'bt-ee-1-proxy' + def send_link_test( + self, + identifier, + sequential_number=None, + ): + pass + """ + link test - open up a connection to a specific provider, and send them an xml payload with a of + test. + """ + payload = { + 'message_type': 'test', + 'identifier': identifier, + 'message_format': 'cbc' + } + + self._invoke_lambda(payload=payload) + + def create_and_send_broadcast( + self, identifier, headline, description, areas, sent, expires, message_number=None + ): + pass + payload = { + 'message_type': 'alert', + 'identifier': identifier, + 'message_format': 'cbc', + 'headline': headline, + 'description': description, + 'areas': areas, + 'sent': sent, + 'expires': expires, + } + self._invoke_lambda(payload=payload) + class CBCProxyVodafone(CBCProxyClientBase): lambda_name = 'vodafone-1-proxy' + + def send_link_test( + self, + identifier, + sequential_number, + ): + pass + """ + link test - open up a connection to a specific provider, and send them an xml payload with a of + test. + """ + payload = { + 'message_type': 'test', + 'identifier': identifier, + 'message_number': sequential_number, + 'message_format': 'ibag' + } + + self._invoke_lambda(payload=payload) + + def create_and_send_broadcast( + self, identifier, message_number, headline, description, areas, sent, expires, + ): + pass + payload = { + 'message_type': 'alert', + 'identifier': identifier, + 'message_number': message_number, + 'message_format': 'ibag', + 'headline': headline, + 'description': description, + 'areas': areas, + 'sent': sent, + 'expires': expires, + } + self._invoke_lambda(payload=payload) diff --git a/app/models.py b/app/models.py index e88308f99..494842b6b 100644 --- a/app/models.py +++ b/app/models.py @@ -2453,11 +2453,6 @@ class BroadcastProvider: PROVIDERS = [EE, VODAFONE, THREE, O2] -class BroadcastProviderMessageType: - CBC = 'cbc' - IBAG = 'ibag' - - class BroadcastProviderMessageStatus: TECHNICAL_FAILURE = 'technical-failure' # Couldn’t send (cbc proxy 5xx/4xx) SENDING = 'sending' # Sent to cbc, awaiting response diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index 547c95c84..7bfb9e334 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -105,12 +105,12 @@ def test_send_broadcast_event_does_nothing_if_cbc_proxy_disabled(mocker, notify_ @freeze_time('2020-08-01 12:00') -@pytest.mark.parametrize('provider,provider_capitalised,message_format', [ - ['ee', 'EE', 'cbc'], - ['vodafone', 'Vodafone', 'ibag'], +@pytest.mark.parametrize('provider,provider_capitalised', [ + ['ee', 'EE'], + ['vodafone', 'Vodafone'], ]) def test_send_broadcast_provider_message_sends_data_correctly( - mocker, sample_service, provider, provider_capitalised, message_format + mocker, sample_service, provider, provider_capitalised ): template = create_template(sample_service, BROADCAST_TYPE) broadcast_message = create_broadcast_message( @@ -140,7 +140,6 @@ def test_send_broadcast_provider_message_sends_data_correctly( mock_create_broadcast.assert_called_once_with( identifier=str(broadcast_provider_message.id), message_number=mocker.ANY, - message_format=message_format, headline='GOV.UK Notify Broadcast', description='this is an emergency broadcast message', areas=[{ @@ -157,12 +156,12 @@ def test_send_broadcast_provider_message_sends_data_correctly( ) -@pytest.mark.parametrize('provider,provider_capitalised,message_format', [ - ['ee', 'EE', 'cbc'], - ['vodafone', 'Vodafone', 'ibag'], +@pytest.mark.parametrize('provider,provider_capitalised', [ + ['ee', 'EE'], + ['vodafone', 'Vodafone'], ]) def test_send_broadcast_provider_message_sends_update_with_references( - mocker, sample_service, provider, provider_capitalised, message_format + mocker, sample_service, provider, provider_capitalised ): template = create_template(sample_service, BROADCAST_TYPE, content='content') @@ -193,7 +192,6 @@ def test_send_broadcast_provider_message_sends_update_with_references( mock_update_broadcast.assert_called_once_with( identifier=str(broadcast_provider_message.id), message_number=mocker.ANY, - message_format=message_format, headline="GOV.UK Notify Broadcast", description='this is an emergency broadcast message', areas=[{ @@ -207,12 +205,12 @@ def test_send_broadcast_provider_message_sends_update_with_references( ) -@pytest.mark.parametrize('provider,provider_capitalised,message_format', [ - ['ee', 'EE', 'cbc'], - ['vodafone', 'Vodafone', 'ibag'], +@pytest.mark.parametrize('provider,provider_capitalised', [ + ['ee', 'EE'], + ['vodafone', 'Vodafone'], ]) def test_send_broadcast_provider_message_sends_cancel_with_references( - mocker, sample_service, provider, provider_capitalised, message_format + mocker, sample_service, provider, provider_capitalised ): template = create_template(sample_service, BROADCAST_TYPE, content='content') @@ -246,7 +244,6 @@ def test_send_broadcast_provider_message_sends_cancel_with_references( mock_cancel_broadcast.assert_called_once_with( identifier=str(broadcast_provider_message.id), message_number=mocker.ANY, - message_format=message_format, headline="GOV.UK Notify Broadcast", description='this is an emergency broadcast message', areas=[{ @@ -290,7 +287,6 @@ def test_send_broadcast_provider_message_errors(mocker, sample_service): mock_create_broadcast.assert_called_once_with( identifier=ANY, message_number=mocker.ANY, - message_format='cbc', headline="GOV.UK Notify Broadcast", description='this is an emergency broadcast message', areas=[{ diff --git a/tests/app/clients/test_cbc_proxy.py b/tests/app/clients/test_cbc_proxy.py index eeb242727..1f0f5efaa 100644 --- a/tests/app/clients/test_cbc_proxy.py +++ b/tests/app/clients/test_cbc_proxy.py @@ -24,6 +24,11 @@ def cbc_proxy_ee(cbc_proxy_client): return cbc_proxy_client.get_proxy('ee') +@pytest.fixture +def cbc_proxy_vodafone(cbc_proxy_client): + return cbc_proxy_client.get_proxy('vodafone') + + @pytest.mark.parametrize('provider_name, expected_provider_class', [ ('ee', CBCProxyEE), ('canary', CBCProxyCanary), @@ -51,7 +56,7 @@ def test_cbc_proxy_lambda_client_has_correct_keys(cbc_proxy_ee): assert secret == 'cbc-proxy-aws-secret-access-key' -def test_cbc_proxy_create_and_send_invokes_function(mocker, cbc_proxy_ee): +def test_cbc_proxy_ee_create_and_send_invokes_function(mocker, cbc_proxy_ee): identifier = 'my-identifier' headline = 'my-headline' description = 'my-description' @@ -84,7 +89,6 @@ def test_cbc_proxy_create_and_send_invokes_function(mocker, cbc_proxy_ee): cbc_proxy_ee.create_and_send_broadcast( identifier=identifier, message_number='0000007b', - message_format='cbc', headline=headline, description=description, areas=areas, @@ -102,7 +106,7 @@ def test_cbc_proxy_create_and_send_invokes_function(mocker, cbc_proxy_ee): payload = json.loads(payload_bytes) assert payload['identifier'] == identifier - assert payload['message_number'] == '0000007b' + assert 'message_number' not in payload assert payload['message_format'] == 'cbc' assert payload['message_type'] == 'alert' assert payload['headline'] == headline @@ -112,6 +116,66 @@ def test_cbc_proxy_create_and_send_invokes_function(mocker, cbc_proxy_ee): assert payload['expires'] == expires +def test_cbc_proxy_vodafone_create_and_send_invokes_function(mocker, cbc_proxy_vodafone): + identifier = 'my-identifier' + headline = 'my-headline' + description = 'my-description' + + sent = 'a-passed-through-sent-value' + expires = 'a-passed-through-expires-value' + + # a single area which is a square including london + areas = [{ + 'description': 'london', + 'polygon': [ + [51.12, -1.2], + [51.12, 1.2], + [51.74, 1.2], + [51.74, -1.2], + [51.12, -1.2], + ], + }] + + ld_client_mock = mocker.patch.object( + cbc_proxy_vodafone, + '_lambda_client', + create=True, + ) + + ld_client_mock.invoke.return_value = { + 'StatusCode': 200, + } + + cbc_proxy_vodafone.create_and_send_broadcast( + identifier=identifier, + message_number='0000007b', + headline=headline, + description=description, + areas=areas, + sent=sent, expires=expires, + ) + + ld_client_mock.invoke.assert_called_once_with( + FunctionName='vodafone-1-proxy', + InvocationType='RequestResponse', + Payload=mocker.ANY, + ) + + kwargs = ld_client_mock.invoke.mock_calls[0][-1] + payload_bytes = kwargs['Payload'] + payload = json.loads(payload_bytes) + + assert payload['identifier'] == identifier + assert payload['message_number'] == '0000007b' + assert payload['message_format'] == 'ibag' + assert payload['message_type'] == 'alert' + assert payload['headline'] == headline + assert payload['description'] == description + assert payload['areas'] == areas + assert payload['sent'] == sent + assert payload['expires'] == expires + + def test_cbc_proxy_create_and_send_handles_invoke_error(mocker, cbc_proxy_ee): identifier = 'my-identifier' headline = 'my-headline' @@ -146,7 +210,6 @@ def test_cbc_proxy_create_and_send_handles_invoke_error(mocker, cbc_proxy_ee): cbc_proxy_ee.create_and_send_broadcast( identifier=identifier, message_number='0000007b', - message_format='cbc', headline=headline, description=description, areas=areas, @@ -197,7 +260,6 @@ def test_cbc_proxy_create_and_send_handles_function_error(mocker, cbc_proxy_ee): cbc_proxy_ee.create_and_send_broadcast( identifier=identifier, message_number='0000007b', - message_format='cbc', headline=headline, description=description, areas=areas, @@ -245,7 +307,7 @@ def test_cbc_proxy_send_canary_invokes_function(mocker, cbc_proxy_client): assert payload['identifier'] == identifier -def test_cbc_proxy_send_link_test_invokes_function(mocker, cbc_proxy_ee): +def test_cbc_proxy_ee_send_link_test_invokes_function(mocker, cbc_proxy_ee): identifier = str(uuid.uuid4()) ld_client_mock = mocker.patch.object( @@ -261,7 +323,6 @@ def test_cbc_proxy_send_link_test_invokes_function(mocker, cbc_proxy_ee): cbc_proxy_ee.send_link_test( identifier=identifier, sequential_number='0000007b', - message_format='cbc' ) ld_client_mock.invoke.assert_called_once_with( @@ -276,5 +337,39 @@ def test_cbc_proxy_send_link_test_invokes_function(mocker, cbc_proxy_ee): assert payload['identifier'] == identifier assert payload['message_type'] == 'test' - assert payload['message_number'] == '0000007b' + assert 'message_number' not in payload assert payload['message_format'] == 'cbc' + + +def test_cbc_proxy_vodafone_send_link_test_invokes_function(mocker, cbc_proxy_vodafone): + identifier = str(uuid.uuid4()) + + ld_client_mock = mocker.patch.object( + cbc_proxy_vodafone, + '_lambda_client', + create=True, + ) + + ld_client_mock.invoke.return_value = { + 'StatusCode': 200, + } + + cbc_proxy_vodafone.send_link_test( + identifier=identifier, + sequential_number='0000007b', + ) + + ld_client_mock.invoke.assert_called_once_with( + FunctionName='vodafone-1-proxy', + InvocationType='RequestResponse', + Payload=mocker.ANY, + ) + + kwargs = ld_client_mock.invoke.mock_calls[0][-1] + payload_bytes = kwargs['Payload'] + payload = json.loads(payload_bytes) + + assert payload['identifier'] == identifier + assert payload['message_type'] == 'test' + assert payload['message_number'] == '0000007b' + assert payload['message_format'] == 'ibag'