From b6774bf0f7853c145421c6fdd9e70f7b5928fc98 Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Mon, 19 Jul 2021 12:48:18 +0100 Subject: [PATCH] Generate Vodafone link test sequence nos in proxy Previously the Celery task to trigger a link test had to know about the special case of a sequence number for Vodafone. Since we're about to change the client to perform multiple tests it makes sense to give it the knowledge of how to generate number itself. Note that we have to import the db inline to avoid a circular import, since this module is itself imported by app/__init__.py. Other invocations of the Vodafone client use stored sequence numbers from the DB, which are called "message numbers" in that context. Since the two use cases are very different (even the names are different!), having them in two places shouldn't cause any confusion. --- app/celery/broadcast_message_tasks.py | 10 ++------- app/clients/cbc_proxy.py | 11 ++++++---- .../celery/test_broadcast_message_tasks.py | 22 ++++--------------- tests/app/clients/test_cbc_proxy.py | 9 +++++--- 4 files changed, 19 insertions(+), 33 deletions(-) diff --git a/app/celery/broadcast_message_tasks.py b/app/celery/broadcast_message_tasks.py index fcdd8e89b..07cced707 100644 --- a/app/celery/broadcast_message_tasks.py +++ b/app/celery/broadcast_message_tasks.py @@ -2,9 +2,8 @@ import uuid from datetime import datetime from flask import current_app -from sqlalchemy.schema import Sequence -from app import cbc_proxy_client, db, notify_celery, zendesk_client +from app import cbc_proxy_client, notify_celery, zendesk_client from app.clients.cbc_proxy import CBCProxyRetryableException from app.config import QueueNames from app.dao.broadcast_message_dao import ( @@ -252,11 +251,6 @@ def send_broadcast_provider_message(self, broadcast_event_id, provider): @notify_celery.task(name='trigger-link-test') def trigger_link_test(provider): identifier = str(uuid.uuid4()) - formatted_seq_number = None - if provider == BroadcastProvider.VODAFONE: - sequence = Sequence('broadcast_provider_message_number_seq') - sequential_number = db.session.connection().execute(sequence) - formatted_seq_number = format_sequential_number(sequential_number) message = f"Sending a link test to CBC proxy for provider {provider}. Identifier in payload is {identifier}" current_app.logger.info(message) - cbc_proxy_client.get_proxy(provider).send_link_test(identifier, formatted_seq_number) + cbc_proxy_client.get_proxy(provider).send_link_test(identifier) diff --git a/app/clients/cbc_proxy.py b/app/clients/cbc_proxy.py index 63a69f892..e475ab8fd 100644 --- a/app/clients/cbc_proxy.py +++ b/app/clients/cbc_proxy.py @@ -5,6 +5,7 @@ import boto3 import botocore from flask import current_app from notifications_utils.template import non_gsm_characters +from sqlalchemy.schema import Sequence from app.config import BroadcastProvider from app.utils import DATETIME_FORMAT, format_sequential_number @@ -79,7 +80,6 @@ class CBCProxyClientBase(ABC): def send_link_test( self, identifier, - sequential_number ): pass @@ -158,7 +158,6 @@ class CBCProxyOne2ManyClient(CBCProxyClientBase): def send_link_test( self, identifier, - sequential_number=None, ): """ link test - open up a connection to a specific provider, and send them an xml payload with a of @@ -234,16 +233,20 @@ class CBCProxyVodafone(CBCProxyClientBase): def send_link_test( self, identifier, - sequential_number, ): """ link test - open up a connection to a specific provider, and send them an xml payload with a of test. """ + from app import db + sequence = Sequence('broadcast_provider_message_number_seq') + sequential_number = db.session.connection().execute(sequence) + formatted_seq_number = format_sequential_number(sequential_number) + payload = { 'message_type': 'test', 'identifier': identifier, - 'message_number': sequential_number, + 'message_number': formatted_seq_number, 'message_format': 'ibag' } diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index cb6da59fb..3e44fd18a 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -1,4 +1,3 @@ -import uuid from datetime import datetime from unittest.mock import ANY, Mock, call @@ -561,29 +560,16 @@ def test_send_broadcast_provider_message_delays_retry_exponentially( ['vodafone', 'Vodafone'], ]) def test_trigger_link_tests_invokes_cbc_proxy_client( - mocker, provider, provider_capitalised + mocker, provider, provider_capitalised, client, ): mock_send_link_test = mocker.patch( f'app.clients.cbc_proxy.CBCProxy{provider_capitalised}.send_link_test', ) + mocker.patch('app.celery.broadcast_message_tasks.uuid.uuid4', return_value=123) + trigger_link_test(provider) - - assert mock_send_link_test.called - # the 0th argument of the call to send_link_test - identifier = mock_send_link_test.mock_calls[0][1][0] - - try: - uuid.UUID(identifier) - except BaseException: - pytest.fail(f"{identifier} is not a valid uuid") - - # testing sequential number: - if provider == 'vodafone': - assert type(mock_send_link_test.mock_calls[0][1][1]) is str - assert len(mock_send_link_test.mock_calls[0][1][1]) == 8 - else: - assert not mock_send_link_test.mock_calls[0][1][1] + assert mock_send_link_test.called_once_with('123') @pytest.mark.parametrize('retry_count, expected_delay', [ diff --git a/tests/app/clients/test_cbc_proxy.py b/tests/app/clients/test_cbc_proxy.py index 708fe99f2..c3261a2dd 100644 --- a/tests/app/clients/test_cbc_proxy.py +++ b/tests/app/clients/test_cbc_proxy.py @@ -8,6 +8,7 @@ from unittest.mock import Mock, call import pytest from botocore.exceptions import ClientError as BotoClientError +from app import db from app.clients.cbc_proxy import ( CBCProxyClient, CBCProxyEE, @@ -573,7 +574,6 @@ def test_cbc_proxy_one_2_many_send_link_test_invokes_function(mocker, cbc_proxy_ cbc_proxy.send_link_test( identifier=identifier, - sequential_number='0000007b', ) ld_client_mock.invoke.assert_called_once_with( @@ -595,6 +595,10 @@ def test_cbc_proxy_one_2_many_send_link_test_invokes_function(mocker, cbc_proxy_ def test_cbc_proxy_vodafone_send_link_test_invokes_function(mocker, cbc_proxy_vodafone): identifier = str(uuid.uuid4()) + db.session.connection().execute( + 'ALTER SEQUENCE broadcast_provider_message_number_seq RESTART WITH 1' + ) + ld_client_mock = mocker.patch.object( cbc_proxy_vodafone, '_lambda_client', @@ -607,7 +611,6 @@ def test_cbc_proxy_vodafone_send_link_test_invokes_function(mocker, cbc_proxy_vo cbc_proxy_vodafone.send_link_test( identifier=identifier, - sequential_number='0000007b', ) ld_client_mock.invoke.assert_called_once_with( @@ -622,5 +625,5 @@ def test_cbc_proxy_vodafone_send_link_test_invokes_function(mocker, cbc_proxy_vo assert payload['identifier'] == identifier assert payload['message_type'] == 'test' - assert payload['message_number'] == '0000007b' + assert payload['message_number'] == '00000001' assert payload['message_format'] == 'ibag'