From 0a8cb679d77b4ddc22b3b568bdaea8a00f1bb31a Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 1 Jun 2016 15:24:19 +0100 Subject: [PATCH 1/2] make mmg and firetext client params consistent --- app/__init__.py | 2 +- app/clients/sms/firetext.py | 22 ++++++++++++++++------ app/clients/sms/mmg.py | 23 +++++++++++++++-------- 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index be7bd43ed..77bf25a47 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -52,7 +52,7 @@ def create_app(app_name=None): statsd_client.init_app(application) firetext_client.init_app(application, statsd_client=statsd_client) loadtest_client.init_app(application, statsd_client=statsd_client) - mmg_client.init_app(application.config, statsd_client=statsd_client) + mmg_client.init_app(application, statsd_client=statsd_client) aws_ses_client.init_app(application.config['AWS_REGION'], statsd_client=statsd_client) notify_celery.init_app(application) encryption.init_app(application) diff --git a/app/clients/sms/firetext.py b/app/clients/sms/firetext.py index 2d77c777d..2dfba89f0 100644 --- a/app/clients/sms/firetext.py +++ b/app/clients/sms/firetext.py @@ -1,11 +1,12 @@ import logging + from monotonic import monotonic +from requests import request, RequestException, HTTPError + from app.clients.sms import ( SmsClient, SmsClientException ) -from flask import current_app -from requests import request, RequestException, HTTPError from app.clients import STATISTICS_DELIVERED, STATISTICS_FAILURE logger = logging.getLogger(__name__) @@ -55,10 +56,11 @@ class FiretextClient(SmsClient): FireText sms client. ''' - def init_app(self, config, statsd_client, *args, **kwargs): + def init_app(self, current_app, statsd_client, *args, **kwargs): super(SmsClient, self).__init__(*args, **kwargs) - self.api_key = config.config.get('FIRETEXT_API_KEY') - self.from_number = config.config.get('FIRETEXT_NUMBER') + self.current_app = current_app + self.api_key = current_app.config.get('FIRETEXT_API_KEY') + self.from_number = current_app.config.get('FIRETEXT_NUMBER') self.name = 'firetext' self.statsd_client = statsd_client @@ -86,6 +88,14 @@ class FiretextClient(SmsClient): if firetext_response['code'] != 0: raise FiretextClientException(firetext_response) response.raise_for_status() + self.current_app.logger.info( + "API {} request on {} succeeded with {} '{}'".format( + "POST", + "https://www.firetext.co.uk/api/sendsms", + response.status_code, + firetext_response + ) + ) except RequestException as e: api_error = HTTPError.create(e) logger.error( @@ -100,6 +110,6 @@ class FiretextClient(SmsClient): raise api_error finally: elapsed_time = monotonic() - start_time - current_app.logger.info("Firetext request finished in {}".format(elapsed_time)) + self.current_app.logger.info("Firetext request finished in {}".format(elapsed_time)) self.statsd_client.timing("notifications.clients.firetext.request-time", elapsed_time) return response diff --git a/app/clients/sms/mmg.py b/app/clients/sms/mmg.py index 70148e2c3..5d4698ba8 100644 --- a/app/clients/sms/mmg.py +++ b/app/clients/sms/mmg.py @@ -1,5 +1,4 @@ import json -from flask import current_app from monotonic import monotonic from requests import (request, RequestException, HTTPError) from app.clients import (STATISTICS_DELIVERED, STATISTICS_FAILURE) @@ -57,10 +56,11 @@ class MMGClient(SmsClient): MMG sms client ''' - def init_app(self, config, statsd_client, *args, **kwargs): + def init_app(self, current_app, statsd_client, *args, **kwargs): super(SmsClient, self).__init__(*args, **kwargs) - self.api_key = config.get('MMG_API_KEY') - self.from_number = config.get('MMG_FROM_NUMBER') + self.current_app = current_app + self.api_key = current_app.config.get('MMG_API_KEY') + self.from_number = current_app.config.get('MMG_FROM_NUMBER') self.name = 'mmg' self.statsd_client = statsd_client @@ -84,12 +84,19 @@ class MMGClient(SmsClient): headers={'Content-Type': 'application/json', 'Authorization': 'Basic {}'.format(self.api_key)}) if response.status_code != 200: - error = response.text - raise MMGClientException(json.loads(error)) + raise MMGClientException(response.json()) response.raise_for_status() + self.current_app.logger.info( + "API {} request on {} succeeded with {} '{}'".format( + "POST", + "https://www.mmgrp.co.uk/API/json/api.php", + response.status_code, + response.json() + ) + ) except RequestException as e: api_error = HTTPError.create(e) - current_app.logger.error( + self.current_app.logger.error( "API {} request on {} failed with {} '{}'".format( "POST", "https://www.mmgrp.co.uk/API/json/api.php", @@ -102,5 +109,5 @@ class MMGClient(SmsClient): finally: elapsed_time = monotonic() - start_time self.statsd_client.timing("notifications.clients.mmg.request-time", elapsed_time) - current_app.logger.info("MMG request finished in {}".format(elapsed_time)) + self.current_app.logger.info("MMG request finished in {}".format(elapsed_time)) return response From a3b847bf646058e5dd0ed34f3b4af00e95ffdd6c Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 1 Jun 2016 15:59:44 +0100 Subject: [PATCH 2/2] tests for send_sms through mmg/firetext --- requirements_for_test.txt | 1 + tests/app/clients/test_firetext.py | 67 +++++++++++++++++++++++++++++- tests/app/clients/test_mmg.py | 56 ++++++++++++++++++++++++- tests/app/conftest.py | 32 +++++++++++++- 4 files changed, 151 insertions(+), 5 deletions(-) diff --git a/requirements_for_test.txt b/requirements_for_test.txt index 66f6f39eb..3e0cf151d 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -6,3 +6,4 @@ pytest-cov==2.2.0 mock==1.0.1 moto==0.4.19 freezegun==0.3.6 +requests-mock==0.7.0 diff --git a/tests/app/clients/test_firetext.py b/tests/app/clients/test_firetext.py index 96627068f..7804eaee9 100644 --- a/tests/app/clients/test_firetext.py +++ b/tests/app/clients/test_firetext.py @@ -1,6 +1,9 @@ -import pytest +from urllib.parse import parse_qs -from app.clients.sms.firetext import get_firetext_responses +import pytest +import requests_mock + +from app.clients.sms.firetext import (get_firetext_responses, FiretextClientException) def test_should_return_correct_details_for_delivery(): @@ -31,3 +34,63 @@ def test_should_be_none_if_unrecognised_status_code(): with pytest.raises(KeyError) as e: get_firetext_responses('99') assert '99' in str(e.value) + + +def test_send_sms_successful_returns_firetext_response(mocker, mock_firetext_client): + to = content = reference = 'foo' + response_dict = { + 'data': [], + 'description': 'SMS successfully queued', + 'code': 0, + 'responseData': 1 + } + + with requests_mock.Mocker() as request_mock: + request_mock.post('https://www.firetext.co.uk/api/sendsms/json', json=response_dict, status_code=200) + response = mock_firetext_client.send_sms(to, content, reference) + + response_json = response.json() + assert response.status_code == 200 + assert response_json['code'] == 0 + assert response_json['description'] == 'SMS successfully queued' + + +def test_send_sms_calls_firetext_correctly(mocker, mock_firetext_client): + to = '+447234567890' + content = 'my message' + reference = 'my reference' + response_dict = { + 'code': 0, + } + + with requests_mock.Mocker() as request_mock: + request_mock.post('https://www.firetext.co.uk/api/sendsms/json', json=response_dict, status_code=200) + mock_firetext_client.send_sms(to, content, reference) + + assert request_mock.call_count == 1 + assert request_mock.request_history[0].url == 'https://www.firetext.co.uk/api/sendsms/json' + assert request_mock.request_history[0].method == 'POST' + + request_args = parse_qs(request_mock.request_history[0].text) + assert request_args['apiKey'][0] == 'foo' + assert request_args['from'][0] == 'bar' + assert request_args['to'][0] == '447234567890' + assert request_args['message'][0] == content + assert request_args['reference'][0] == reference + + +def test_send_sms_raises_if_firetext_rejects(mocker, mock_firetext_client): + to = content = reference = 'foo' + response_dict = { + 'data': [], + 'description': 'Some kind of error', + 'code': 1, + 'responseData': '' + } + + with pytest.raises(FiretextClientException) as exc, requests_mock.Mocker() as request_mock: + request_mock.post('https://www.firetext.co.uk/api/sendsms/json', json=response_dict, status_code=200) + mock_firetext_client.send_sms(to, content, reference) + + assert exc.value.code == 1 + assert exc.value.description == 'Some kind of error' diff --git a/tests/app/clients/test_mmg.py b/tests/app/clients/test_mmg.py index 60210afd8..aa5bb9f22 100644 --- a/tests/app/clients/test_mmg.py +++ b/tests/app/clients/test_mmg.py @@ -1,4 +1,7 @@ -from app.clients.sms.mmg import get_mmg_responses +import pytest +import requests_mock + +from app.clients.sms.mmg import (get_mmg_responses, MMGClientException) def test_should_return_correct_details_for_delivery(): @@ -23,3 +26,54 @@ def test_should_be_none_if_unrecognised_status_code(): assert response_dict['notification_status'] == 'failed' assert response_dict['notification_statistics_status'] == 'failure' assert not response_dict['success'] + + +def test_send_sms_successful_returns_mmg_response(mocker, mock_mmg_client): + to = content = reference = 'foo' + response_dict = {'Reference': 12345678} + + with requests_mock.Mocker() as request_mock: + request_mock.post('https://www.mmgrp.co.uk/API/json/api.php', json=response_dict, status_code=200) + response = mock_mmg_client.send_sms(to, content, reference) + + response_json = response.json() + assert response.status_code == 200 + assert response_json['Reference'] == 12345678 + + +def test_send_sms_calls_mmg_correctly(mocker, mock_mmg_client): + to = '+447234567890' + content = 'my message' + reference = 'my reference' + response_dict = {'Reference': 12345678} + + with requests_mock.Mocker() as request_mock: + request_mock.post('https://www.mmgrp.co.uk/API/json/api.php', json=response_dict, status_code=200) + mock_mmg_client.send_sms(to, content, reference) + + assert request_mock.call_count == 1 + assert request_mock.request_history[0].url == 'https://www.mmgrp.co.uk/API/json/api.php' + assert request_mock.request_history[0].method == 'POST' + + request_args = request_mock.request_history[0].json() + assert request_args['reqType'] == 'BULK' + assert request_args['MSISDN'] == to + assert request_args['msg'] == content + assert request_args['sender'] == 'bar' + assert request_args['cid'] == reference + assert request_args['multi'] is True + + +def test_send_sms_raises_if_mmg_rejects(mocker, mock_mmg_client): + to = content = reference = 'foo' + response_dict = { + 'Error': 206, + 'Description': 'Some kind of error' + } + + with pytest.raises(MMGClientException) as exc, requests_mock.Mocker() as request_mock: + request_mock.post('https://www.mmgrp.co.uk/API/json/api.php', json=response_dict, status_code=400) + mock_mmg_client.send_sms(to, content, reference) + + assert exc.value.code == 206 + assert exc.value.description == 'Some kind of error' diff --git a/tests/app/conftest.py b/tests/app/conftest.py index cdb4cb0ec..61d2d8c3f 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -1,5 +1,8 @@ -import pytest +import uuid from datetime import (datetime, date) + +import pytest + from app import db from app.models import ( User, @@ -20,7 +23,8 @@ from app.dao.api_key_dao import save_model_api_key from app.dao.jobs_dao import dao_create_job from app.dao.notifications_dao import dao_create_notification from app.dao.invited_user_dao import save_invited_user -import uuid +from app.clients.sms.firetext import FiretextClient +from app.clients.sms.mmg import MMGClient @pytest.fixture(scope='function') @@ -527,3 +531,27 @@ def sample_notification_statistics(notify_db, notify_db.session.add(stats) notify_db.session.commit() return stats + + +@pytest.fixture(scope='function') +def mock_firetext_client(mocker, statsd_client=None): + client = FiretextClient() + statsd_client = statsd_client or mocker.Mock() + current_app = mocker.Mock(config={ + 'FIRETEXT_API_KEY': 'foo', + 'FIRETEXT_NUMBER': 'bar' + }) + client.init_app(current_app, statsd_client) + return client + + +@pytest.fixture(scope='function') +def mock_mmg_client(mocker, statsd_client=None): + client = MMGClient() + statsd_client = statsd_client or mocker.Mock()() + current_app = mocker.Mock(config={ + 'MMG_API_KEY': 'foo', + 'MMG_FROM_NUMBER': 'bar' + }) + client.init_app(current_app, statsd_client) + return client