From daff1c3f535c370d836b9e707f201069f6a6071e Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 27 Jan 2016 14:18:11 +0000 Subject: [PATCH 1/5] Add queue name to service table. Set queue name when creating the service --- app/models.py | 2 +- app/service/rest.py | 3 +++ .../0010_add_queue_name_to_service.py | 26 +++++++++++++++++++ tests/app/conftest.py | 4 ++- tests/app/service/test_rest.py | 1 + 5 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 migrations/versions/0010_add_queue_name_to_service.py diff --git a/app/models.py b/app/models.py index 578b8ce7c..40acb7b66 100644 --- a/app/models.py +++ b/app/models.py @@ -84,7 +84,7 @@ class Service(db.Model): secondary=user_to_service, backref=db.backref('user_to_service', lazy='dynamic')) restricted = db.Column(db.Boolean, index=False, unique=False, nullable=False) - + queue_name = db.Column(UUID(as_uuid=True)) class ApiKey(db.Model): __tablename__ = 'api_key' diff --git a/app/service/rest.py b/app/service/rest.py index c114b5555..81cfc8e4d 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -28,6 +28,9 @@ def create_service(): # I believe service is already added to the session but just needs a # db.session.commit try: + # add service name to here or in dao? + import uuid + service.queue_name = uuid.uuid4() save_model_service(service) except DAOException as e: return jsonify(result="error", message=str(e)), 400 diff --git a/migrations/versions/0010_add_queue_name_to_service.py b/migrations/versions/0010_add_queue_name_to_service.py new file mode 100644 index 000000000..774b4d3ba --- /dev/null +++ b/migrations/versions/0010_add_queue_name_to_service.py @@ -0,0 +1,26 @@ +"""empty message + +Revision ID: 0010_add_queue_name_to_service +Revises: 0009_unique_service_to_key_name +Create Date: 2016-01-27 13:53:34.717916 + +""" + +# revision identifiers, used by Alembic. + +revision = '0010_add_queue_name_to_service' +down_revision = '0009_unique_service_to_key_name' + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +def upgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.add_column('services', sa.Column('queue_name', postgresql.UUID(as_uuid=True), nullable=True)) + ### end Alembic commands ### + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.drop_column('services', 'queue_name') + ### end Alembic commands ### diff --git a/tests/app/conftest.py b/tests/app/conftest.py index d016b7daf..9241873ca 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -1,4 +1,6 @@ import pytest +from flask import jsonify + from app.models import (User, Service, Template, ApiKey, Job, VerifyCode) from app.dao.users_dao import (save_model_user, create_user_code, create_secret_code) from app.dao.services_dao import save_model_service @@ -156,7 +158,7 @@ def sample_admin_service_id(notify_db, notify_db_session): @pytest.fixture(scope='function') def mock_notify_client_send_sms(mocker): def _send(mobile_number, message): - pass + return jsonify('sms sent'), 200 mock_class = mocker.patch('app.notify_alpha_client.send_sms', side_effect=_send) return mock_class diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 6c2b2843e..a21b04107 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -93,6 +93,7 @@ def test_post_service(notify_api, notify_db, notify_db_session, sample_user, sam json_resp = json.loads(resp.get_data(as_text=True)) assert json_resp['data']['name'] == service.name assert json_resp['data']['limit'] == service.limit + assert service.queue_name is not None def test_post_service_multiple_users(notify_api, notify_db, notify_db_session, sample_user, sample_admin_service_id): From 42a4c8b0b1ed9876b3b86e65b2157f240ff12683 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 27 Jan 2016 17:42:05 +0000 Subject: [PATCH 2/5] Add sms notifications from a service to a queue. --- app/dao/services_dao.py | 6 +- app/models.py | 1 + app/notifications/rest.py | 120 +++++++++++++----- app/schemas.py | 2 +- config.py | 2 + .../0010_add_queue_name_to_service.py | 2 + requirements.txt | 1 + requirements_for_test.txt | 3 +- tests/app/conftest.py | 5 +- tests/app/notifications/test_rest.py | 33 ++++- 10 files changed, 131 insertions(+), 44 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 54fb5dd01..80ff0b815 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -35,13 +35,15 @@ def delete_model_service(service): db.session.commit() -def get_model_services(service_id=None, user_id=None): +def get_model_services(service_id=None, user_id=None, _raise=True): # TODO need better mapping from function params to sql query. if user_id and service_id: return Service.query.filter( Service.users.any(id=user_id)).filter_by(id=service_id).one() elif service_id: - return Service.query.filter_by(id=service_id).one() + result = Service.query.filter_by(id=service_id).one() if _raise else Service.query.filter_by( + id=service_id).first() + return result elif user_id: return Service.query.filter(Service.users.any(id=user_id)).all() return Service.query.all() diff --git a/app/models.py b/app/models.py index 40acb7b66..e081653cf 100644 --- a/app/models.py +++ b/app/models.py @@ -86,6 +86,7 @@ class Service(db.Model): restricted = db.Column(db.Boolean, index=False, unique=False, nullable=False) queue_name = db.Column(UUID(as_uuid=True)) + class ApiKey(db.Model): __tablename__ = 'api_key' diff --git a/app/notifications/rest.py b/app/notifications/rest.py index d8da3202a..bddd13532 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -1,10 +1,13 @@ +import json + +import boto3 from flask import ( Blueprint, jsonify, request, current_app ) - +from itsdangerous import URLSafeSerializer from app import notify_alpha_client from app import api_user from app.dao import (templates_dao, services_dao) @@ -20,23 +23,54 @@ def get_notifications(notification_id): return jsonify(notify_alpha_client.fetch_notification_by_id(notification_id)), 200 +def _add_notification_to_queue(template_id, service, msg_type, to): + q = boto3.resource('sqs', region_name=current_app.config['AWS_REGION']).create_queue( + QueueName=str(service.queue_name)) + import uuid + notification = json.dumps({'message_id': str(uuid.uuid4()), + 'service_id': service.id, + 'to': to, + 'message_type': msg_type, + 'template_id': template_id}) + serializer = URLSafeSerializer(current_app.config.get('SECRET_KEY')) + encrypted = serializer.dumps(notification, current_app.config.get('DANGEROUS_SALT')) + q.send_message(MessageBody=encrypted, + MessageAttributes={'type': {'StringValue': msg_type, 'DataType': 'String'}, + 'service_id': {'StringValue': str(service.id), 'DataType': 'String'}, + 'template_id': {'StringValue': str(template_id), 'DataType': 'String'}}) + + @notifications.route('/sms', methods=['POST']) def create_sms_notification(): notification = request.get_json()['notification'] errors = {} - to, to_errors = validate_to(notification, api_user['client']) - print("create sms") - print(notification) - template, template_errors = validate_template(notification, api_user['client']) - if to_errors['to']: - errors.update(to_errors) - if template_errors['template']: - errors.update(template_errors) - if errors: - return jsonify(result="error", message=errors), 400 - return jsonify(notify_alpha_client.send_sms( - mobile_number=to, - message=template)), 200 + # TODO: should create a different endpoint for the admin client to send verify codes. + if api_user['client'] == current_app.config.get('ADMIN_CLIENT_USER_NAME'): + to, to_errors = validate_to_for_admin_client(notification) + content, content_errors = validate_content_for_admin_client(notification) + if to_errors['to']: + errors.update(to_errors) + if content_errors['content']: + errors.update(content_errors) + if errors: + return jsonify(result="error", message=errors), 400 + + return jsonify(notify_alpha_client.send_sms(mobile_number=to, message=content)), 200 + + else: + to, to_errors = validate_to(notification, api_user['client']) + template, template_errors = validate_template(notification, api_user['client']) + if to_errors['to']: + errors.update(to_errors) + if template_errors['template']: + errors.update(template_errors) + if errors: + return jsonify(result="error", message=errors), 400 + + # add notification to the queue + service = services_dao.get_model_services(api_user['client'], _raise=False) + _add_notification_to_queue(template.id, service, 'sms', to) + return jsonify(notify_alpha_client.send_sms(mobile_number=to, message=template.content)), 200 @notifications.route('/email', methods=['POST']) @@ -66,36 +100,52 @@ def validate_to(json_body, service_id): else: if not mobile_regex.match(mob): errors['to'].append('invalid phone number, must be of format +441234123123') - if service_id != current_app.config.get('ADMIN_CLIENT_USER_NAME'): - service = services_dao.get_model_services(service_id=service_id) - if service.restricted: - valid = False - for usr in service.users: - if mob == usr.mobile_number: - valid = True - break - if not valid: - errors['to'].append('Invalid phone number for restricted service') + + service = services_dao.get_model_services(service_id=service_id) + if service.restricted: + valid = False + for usr in service.users: + if mob == usr.mobile_number: + valid = True + break + if not valid: + errors['to'].append('Invalid phone number for restricted service') + return mob, errors + + +def validate_to_for_admin_client(json_body): + errors = {"to": []} + mob = json_body.get('to', None) + if not mob: + errors['to'].append('Required data missing') + else: + if not mobile_regex.match(mob): + errors['to'].append('invalid phone number, must be of format +441234123123') return mob, errors def validate_template(json_body, service_id): errors = {"template": []} template_id = json_body.get('template', None) - content = '' + template = '' if not template_id: errors['template'].append('Required data missing') else: - if service_id == current_app.config.get('ADMIN_CLIENT_USER_NAME'): - content = json_body['template'] - else: - try: - template = templates_dao.get_model_templates( - template_id=json_body['template'], - service_id=service_id) - content = template.content - except: - errors['template'].append("Unable to load template.") + try: + template = templates_dao.get_model_templates( + template_id=json_body['template'], + service_id=service_id) + except: + errors['template'].append("Unable to load template.") + return template, errors + + +def validate_content_for_admin_client(json_body): + errors = {"content": []} + content = json_body.get('template', None) + if not content: + errors['content'].append('Required content') + return content, errors diff --git a/app/schemas.py b/app/schemas.py index a6e6c1377..d6ab588ba 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -20,7 +20,7 @@ class UserSchema(ma.ModelSchema): class ServiceSchema(ma.ModelSchema): class Meta: model = models.Service - exclude = ("updated_at", "created_at", "api_keys", "templates", "jobs") + exclude = ("updated_at", "created_at", "api_keys", "templates", "jobs", "queue_name") class TemplateSchema(ma.ModelSchema): diff --git a/config.py b/config.py index b26a526a8..1dd256571 100644 --- a/config.py +++ b/config.py @@ -14,6 +14,8 @@ class Config(object): ADMIN_CLIENT_USER_NAME = None ADMIN_CLIENT_SECRET = None + AWS_REGION = 'eu-west-1' + class Development(Config): DEBUG = True diff --git a/migrations/versions/0010_add_queue_name_to_service.py b/migrations/versions/0010_add_queue_name_to_service.py index 774b4d3ba..636847cd3 100644 --- a/migrations/versions/0010_add_queue_name_to_service.py +++ b/migrations/versions/0010_add_queue_name_to_service.py @@ -18,6 +18,8 @@ from sqlalchemy.dialects import postgresql def upgrade(): ### commands auto generated by Alembic - please adjust! ### op.add_column('services', sa.Column('queue_name', postgresql.UUID(as_uuid=True), nullable=True)) + op.get_bind() + op.execute('update services set queue_name = (SELECT uuid_in(md5(random()::text || now()::text)::cstring))') ### end Alembic commands ### def downgrade(): diff --git a/requirements.txt b/requirements.txt index b85bac9a7..96d3663b6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,6 +12,7 @@ flask-marshmallow==0.6.2 itsdangerous==0.24 Flask-Bcrypt==0.6.2 credstash==1.8.0 +boto3==1.2.3 git+https://github.com/alphagov/notifications-python-client.git@0.2.1#egg=notifications-python-client==0.2.1 diff --git a/requirements_for_test.txt b/requirements_for_test.txt index 263c2f4da..60ceea35b 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -3,4 +3,5 @@ pep8==1.5.7 pytest==2.8.1 pytest-mock==0.8.1 pytest-cov==2.2.0 -mock==1.0.1 \ No newline at end of file +mock==1.0.1 +moto==0.4.19 \ No newline at end of file diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 9241873ca..8ab965606 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -78,7 +78,8 @@ def sample_service(notify_db, 'users': [user], 'limit': 1000, 'active': False, - 'restricted': False} + 'restricted': False, + 'queue_name': str(uuid.uuid4())} service = Service.query.filter_by(name=service_name).first() if not service: service = Service(**data) @@ -158,7 +159,7 @@ def sample_admin_service_id(notify_db, notify_db_session): @pytest.fixture(scope='function') def mock_notify_client_send_sms(mocker): def _send(mobile_number, message): - return jsonify('sms sent'), 200 + pass mock_class = mocker.patch('app.notify_alpha_client.send_sms', side_effect=_send) return mock_class diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index 8e3a4d11b..c8c13d5db 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -1,3 +1,6 @@ +import boto3 +import moto + from tests import create_authorization_header from flask import url_for, json from app import notify_alpha_client @@ -70,6 +73,7 @@ def test_get_notifications_empty_result( notify_alpha_client.fetch_notification_by_id.assert_called_with("123") +@moto.mock_sqs def test_should_reject_if_no_phone_numbers( notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id, mocker): """ @@ -77,6 +81,7 @@ def test_should_reject_if_no_phone_numbers( """ with notify_api.test_request_context(): with notify_api.test_client() as client: + set_up_mock_queue() mocker.patch( 'app.notify_alpha_client.send_sms', return_value='success' @@ -104,6 +109,7 @@ def test_should_reject_if_no_phone_numbers( assert not notify_alpha_client.send_sms.called +@moto.mock_sqs def test_should_reject_bad_phone_numbers( notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id, mocker): """ @@ -111,6 +117,7 @@ def test_should_reject_bad_phone_numbers( """ with notify_api.test_request_context(): with notify_api.test_client() as client: + set_up_mock_queue() mocker.patch( 'app.notify_alpha_client.send_sms', return_value='success' @@ -139,6 +146,7 @@ def test_should_reject_bad_phone_numbers( assert not notify_alpha_client.send_sms.called +@moto.mock_sqs def test_should_reject_missing_template( notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id, mocker): """ @@ -146,6 +154,7 @@ def test_should_reject_missing_template( """ with notify_api.test_request_context(): with notify_api.test_client() as client: + set_up_mock_queue() mocker.patch( 'app.notify_alpha_client.send_sms', return_value='success' @@ -173,6 +182,7 @@ def test_should_reject_missing_template( assert not notify_alpha_client.send_sms.called +@moto.mock_sqs def test_send_template_content(notify_api, notify_db, notify_db_session, @@ -185,6 +195,7 @@ def test_send_template_content(notify_api, """ with notify_api.test_request_context(): with notify_api.test_client() as client: + set_up_mock_queue() mocker.patch( 'app.notify_alpha_client.send_sms', return_value={ @@ -224,6 +235,7 @@ def test_send_template_content(notify_api, message=sample_template.content) +@moto.mock_sqs def test_send_notification_restrict_mobile(notify_api, notify_db, notify_db_session, @@ -237,6 +249,7 @@ def test_send_notification_restrict_mobile(notify_api, """ with notify_api.test_request_context(): with notify_api.test_client() as client: + set_up_mock_queue() Service.query.filter_by( id=sample_template.service.id).update({'restricted': True}) invalid_mob = '+449999999999' @@ -267,13 +280,15 @@ def test_send_notification_restrict_mobile(notify_api, assert 'Invalid phone number for restricted service' in json_resp['message']['to'] +@moto.mock_sqs def test_should_allow_valid_message( - notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id, mocker): + notify_api, notify_db, notify_db_session, sample_service, mocker): """ Tests POST endpoint '/sms' with notifications-admin notification. """ with notify_api.test_request_context(): with notify_api.test_client() as client: + set_up_mock_queue() mocker.patch( 'app.notify_alpha_client.send_sms', return_value={ @@ -307,7 +322,7 @@ def test_should_allow_valid_message( json_resp = json.loads(response.get_data(as_text=True)) assert response.status_code == 200 assert json_resp['notification']['id'] == 100 - notify_alpha_client.send_sms.assert_called_with(mobile_number='+441234123123', message='valid') + notify_alpha_client.send_sms.assert_called_with(mobile_number='+441234123123', message="valid") def test_send_email_valid_data(notify_api, @@ -318,7 +333,6 @@ def test_send_email_valid_data(notify_api, mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: - to_address = "to@notify.com" from_address = "from@notify.com" subject = "This is the subject" @@ -362,3 +376,16 @@ def test_send_email_valid_data(notify_api, assert json_resp['notification']['id'] == 100 notify_alpha_client.send_email.assert_called_with( to_address, message, from_address, subject) + + +@moto.mock_sqs +def test_add_notification_to_queue(notify_api, notify_db, notify_db_session, sample_service): + set_up_mock_queue() + from app.notifications.rest import _add_notification_to_queue + _add_notification_to_queue('some message', sample_service, 'sms', '+447515349060') + + +def set_up_mock_queue(): + # set up mock queue + boto3.setup_default_session(region_name='eu-west-1') + conn = boto3.resource('sqs') From a5466651884691d6399ca56a12d573cd1d1ef03c Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 28 Jan 2016 11:06:24 +0000 Subject: [PATCH 3/5] Some code clean up. Clean up the unit test. --- app/notifications/rest.py | 81 +++++++++++++--------------- tests/app/notifications/test_rest.py | 66 +++++++++-------------- 2 files changed, 62 insertions(+), 85 deletions(-) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index bddd13532..3a74d9341 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -23,33 +23,17 @@ def get_notifications(notification_id): return jsonify(notify_alpha_client.fetch_notification_by_id(notification_id)), 200 -def _add_notification_to_queue(template_id, service, msg_type, to): - q = boto3.resource('sqs', region_name=current_app.config['AWS_REGION']).create_queue( - QueueName=str(service.queue_name)) - import uuid - notification = json.dumps({'message_id': str(uuid.uuid4()), - 'service_id': service.id, - 'to': to, - 'message_type': msg_type, - 'template_id': template_id}) - serializer = URLSafeSerializer(current_app.config.get('SECRET_KEY')) - encrypted = serializer.dumps(notification, current_app.config.get('DANGEROUS_SALT')) - q.send_message(MessageBody=encrypted, - MessageAttributes={'type': {'StringValue': msg_type, 'DataType': 'String'}, - 'service_id': {'StringValue': str(service.id), 'DataType': 'String'}, - 'template_id': {'StringValue': str(template_id), 'DataType': 'String'}}) - - @notifications.route('/sms', methods=['POST']) def create_sms_notification(): notification = request.get_json()['notification'] errors = {} + to, to_errors = validate_to(notification) + if to_errors['to']: + errors.update(to_errors) + # TODO: should create a different endpoint for the admin client to send verify codes. if api_user['client'] == current_app.config.get('ADMIN_CLIENT_USER_NAME'): - to, to_errors = validate_to_for_admin_client(notification) content, content_errors = validate_content_for_admin_client(notification) - if to_errors['to']: - errors.update(to_errors) if content_errors['content']: errors.update(content_errors) if errors: @@ -58,16 +42,17 @@ def create_sms_notification(): return jsonify(notify_alpha_client.send_sms(mobile_number=to, message=content)), 200 else: - to, to_errors = validate_to(notification, api_user['client']) + to, restricted_errors = validate_to_for_service(notification, api_user['client']) + if restricted_errors['restricted']: + errors.update(restricted_errors) + template, template_errors = validate_template(notification, api_user['client']) - if to_errors['to']: - errors.update(to_errors) if template_errors['template']: errors.update(template_errors) if errors: return jsonify(result="error", message=errors), 400 - # add notification to the queue + # add notification to the queue service = services_dao.get_model_services(api_user['client'], _raise=False) _add_notification_to_queue(template.id, service, 'sms', to) return jsonify(notify_alpha_client.send_sms(mobile_number=to, message=template.content)), 200 @@ -92,28 +77,21 @@ def create_email_notification(): notification['subject'])) -def validate_to(json_body, service_id): - errors = {"to": []} - mob = json_body.get('to', None) - if not mob: - errors['to'].append('Required data missing') - else: - if not mobile_regex.match(mob): - errors['to'].append('invalid phone number, must be of format +441234123123') - - service = services_dao.get_model_services(service_id=service_id) - if service.restricted: - valid = False - for usr in service.users: - if mob == usr.mobile_number: - valid = True - break - if not valid: - errors['to'].append('Invalid phone number for restricted service') +def validate_to_for_service(mob, service_id): + errors = {"restricted": []} + service = services_dao.get_model_services(service_id=service_id) + if service.restricted: + valid = False + for usr in service.users: + if mob == usr.mobile_number: + valid = True + break + if not valid: + errors['restricted'].append('Invalid phone number for restricted service') return mob, errors -def validate_to_for_admin_client(json_body): +def validate_to(json_body): errors = {"to": []} mob = json_body.get('to', None) if not mob: @@ -154,3 +132,20 @@ def validate_required_and_something(json_body, field): if field not in json_body and json_body[field]: errors.append('Required data for field.') return {field: errors} if errors else None + + +def _add_notification_to_queue(template_id, service, msg_type, to): + q = boto3.resource('sqs', region_name=current_app.config['AWS_REGION']).create_queue( + QueueName=str(service.queue_name)) + import uuid + notification = json.dumps({'message_id': str(uuid.uuid4()), + 'service_id': service.id, + 'to': to, + 'message_type': msg_type, + 'template_id': template_id}) + serializer = URLSafeSerializer(current_app.config.get('SECRET_KEY')) + encrypted = serializer.dumps(notification, current_app.config.get('DANGEROUS_SALT')) + q.send_message(MessageBody=encrypted, + MessageAttributes={'type': {'StringValue': msg_type, 'DataType': 'String'}, + 'service_id': {'StringValue': str(service.id), 'DataType': 'String'}, + 'template_id': {'StringValue': str(template_id), 'DataType': 'String'}}) diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index c8c13d5db..e940a8e6e 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -8,7 +8,7 @@ from app.models import Service def test_get_notifications( - notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id, mocker): + notify_api, notify_db, notify_db_session, sample_api_key, mocker): """ Tests GET endpoint '/' to retrieve entire service list. """ @@ -27,7 +27,7 @@ def test_get_notifications( ) auth_header = create_authorization_header( - service_id=sample_admin_service_id, + service_id=sample_api_key.service_id, path=url_for('notifications.get_notifications', notification_id=123), method='GET') @@ -44,7 +44,7 @@ def test_get_notifications( def test_get_notifications_empty_result( - notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id, mocker): + notify_api, notify_db, notify_db_session, sample_api_key, mocker): """ Tests GET endpoint '/' to retrieve entire service list. """ @@ -59,7 +59,7 @@ def test_get_notifications_empty_result( ) auth_header = create_authorization_header( - service_id=sample_admin_service_id, + service_id=sample_api_key.service_id, path=url_for('notifications.get_notifications', notification_id=123), method='GET') @@ -73,15 +73,13 @@ def test_get_notifications_empty_result( notify_alpha_client.fetch_notification_by_id.assert_called_with("123") -@moto.mock_sqs -def test_should_reject_if_no_phone_numbers( - notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id, mocker): +def test_create_sms_should_reject_if_no_phone_numbers( + notify_api, notify_db, notify_db_session, sample_api_key, mocker): """ Tests GET endpoint '/' to retrieve entire service list. """ with notify_api.test_request_context(): with notify_api.test_client() as client: - set_up_mock_queue() mocker.patch( 'app.notify_alpha_client.send_sms', return_value='success' @@ -92,7 +90,7 @@ def test_should_reject_if_no_phone_numbers( } } auth_header = create_authorization_header( - service_id=sample_admin_service_id, + service_id=sample_api_key.service_id, request_body=json.dumps(data), path=url_for('notifications.create_sms_notification'), method='POST') @@ -109,15 +107,13 @@ def test_should_reject_if_no_phone_numbers( assert not notify_alpha_client.send_sms.called -@moto.mock_sqs def test_should_reject_bad_phone_numbers( - notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id, mocker): + notify_api, notify_db, notify_db_session, mocker): """ Tests GET endpoint '/' to retrieve entire service list. """ with notify_api.test_request_context(): with notify_api.test_client() as client: - set_up_mock_queue() mocker.patch( 'app.notify_alpha_client.send_sms', return_value='success' @@ -129,7 +125,6 @@ def test_should_reject_bad_phone_numbers( } } auth_header = create_authorization_header( - service_id=sample_admin_service_id, request_body=json.dumps(data), path=url_for('notifications.create_sms_notification'), method='POST') @@ -146,15 +141,13 @@ def test_should_reject_bad_phone_numbers( assert not notify_alpha_client.send_sms.called -@moto.mock_sqs -def test_should_reject_missing_template( - notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id, mocker): +def test_should_reject_missing_content( + notify_api, notify_db, notify_db_session, mocker): """ Tests GET endpoint '/' to retrieve entire service list. """ with notify_api.test_request_context(): with notify_api.test_client() as client: - set_up_mock_queue() mocker.patch( 'app.notify_alpha_client.send_sms', return_value='success' @@ -165,7 +158,6 @@ def test_should_reject_missing_template( } } auth_header = create_authorization_header( - service_id=sample_admin_service_id, request_body=json.dumps(data), path=url_for('notifications.create_sms_notification'), method='POST') @@ -178,7 +170,7 @@ def test_should_reject_missing_template( json_resp = json.loads(response.get_data(as_text=True)) assert response.status_code == 400 assert json_resp['result'] == 'error' - assert 'Required data missing' in json_resp['message']['template'] + assert 'Required content' in json_resp['message']['content'] assert not notify_alpha_client.send_sms.called @@ -186,9 +178,6 @@ def test_should_reject_missing_template( def test_send_template_content(notify_api, notify_db, notify_db_session, - sample_api_key, - sample_template, - sample_user, mocker): """ Test POST endpoint '/sms' with service notification. @@ -196,6 +185,8 @@ def test_send_template_content(notify_api, with notify_api.test_request_context(): with notify_api.test_client() as client: set_up_mock_queue() + mobile = '+447719087678' + msg = 'Message content' mocker.patch( 'app.notify_alpha_client.send_sms', return_value={ @@ -203,21 +194,20 @@ def test_send_template_content(notify_api, "createdAt": "2015-11-03T09:37:27.414363Z", "id": 100, "jobId": 65, - "message": sample_template.content, + "message": msg, "method": "sms", "status": "created", - "to": sample_user.mobile_number + "to": mobile } } ) data = { 'notification': { - 'to': sample_user.mobile_number, - 'template': sample_template.id + 'to': mobile, + 'template': msg } } auth_header = create_authorization_header( - service_id=sample_template.service.id, request_body=json.dumps(data), path=url_for('notifications.create_sms_notification'), method='POST') @@ -231,11 +221,10 @@ def test_send_template_content(notify_api, assert response.status_code == 200 assert json_resp['notification']['id'] == 100 notify_alpha_client.send_sms.assert_called_with( - mobile_number=sample_user.mobile_number, - message=sample_template.content) + mobile_number=mobile, + message=msg) -@moto.mock_sqs def test_send_notification_restrict_mobile(notify_api, notify_db, notify_db_session, @@ -249,7 +238,7 @@ def test_send_notification_restrict_mobile(notify_api, """ with notify_api.test_request_context(): with notify_api.test_client() as client: - set_up_mock_queue() + Service.query.filter_by( id=sample_template.service.id).update({'restricted': True}) invalid_mob = '+449999999999' @@ -277,12 +266,12 @@ def test_send_notification_restrict_mobile(notify_api, json_resp = json.loads(response.get_data(as_text=True)) assert response.status_code == 400 - assert 'Invalid phone number for restricted service' in json_resp['message']['to'] + assert 'Invalid phone number for restricted service' in json_resp['message']['restricted'] + assert not notify_alpha_client.send_sms.called @moto.mock_sqs -def test_should_allow_valid_message( - notify_api, notify_db, notify_db_session, sample_service, mocker): +def test_should_allow_valid_message(notify_api, notify_db, notify_db_session, mocker): """ Tests POST endpoint '/sms' with notifications-admin notification. """ @@ -296,7 +285,7 @@ def test_should_allow_valid_message( "createdAt": "2015-11-03T09:37:27.414363Z", "id": 100, "jobId": 65, - "message": "This is the message", + "message": "valid", "method": "sms", "status": "created", "to": "+449999999999" @@ -378,13 +367,6 @@ def test_send_email_valid_data(notify_api, to_address, message, from_address, subject) -@moto.mock_sqs -def test_add_notification_to_queue(notify_api, notify_db, notify_db_session, sample_service): - set_up_mock_queue() - from app.notifications.rest import _add_notification_to_queue - _add_notification_to_queue('some message', sample_service, 'sms', '+447515349060') - - def set_up_mock_queue(): # set up mock queue boto3.setup_default_session(region_name='eu-west-1') From 03989c09de2b54f09e76adce7d19c7cea4523dc0 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 28 Jan 2016 11:09:25 +0000 Subject: [PATCH 4/5] Add message id to message attributes --- app/notifications/rest.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 3a74d9341..9b7b73a4e 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -138,7 +138,8 @@ def _add_notification_to_queue(template_id, service, msg_type, to): q = boto3.resource('sqs', region_name=current_app.config['AWS_REGION']).create_queue( QueueName=str(service.queue_name)) import uuid - notification = json.dumps({'message_id': str(uuid.uuid4()), + message_id = str(uuid.uuid4()) + notification = json.dumps({'message_id': message_id, 'service_id': service.id, 'to': to, 'message_type': msg_type, @@ -147,5 +148,6 @@ def _add_notification_to_queue(template_id, service, msg_type, to): encrypted = serializer.dumps(notification, current_app.config.get('DANGEROUS_SALT')) q.send_message(MessageBody=encrypted, MessageAttributes={'type': {'StringValue': msg_type, 'DataType': 'String'}, + 'message_id': {'StringValue': message_id, 'DataType': 'String'}, 'service_id': {'StringValue': str(service.id), 'DataType': 'String'}, 'template_id': {'StringValue': str(template_id), 'DataType': 'String'}}) From 4010ed61ce10582e6fb949ffdfd95e88a774eeca Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 28 Jan 2016 11:42:13 +0000 Subject: [PATCH 5/5] Moved creating queue name to model --- app/models.py | 4 +++- app/service/rest.py | 3 --- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/models.py b/app/models.py index e081653cf..83f80bbda 100644 --- a/app/models.py +++ b/app/models.py @@ -1,3 +1,5 @@ +import uuid + from sqlalchemy import UniqueConstraint from . import db @@ -84,7 +86,7 @@ class Service(db.Model): secondary=user_to_service, backref=db.backref('user_to_service', lazy='dynamic')) restricted = db.Column(db.Boolean, index=False, unique=False, nullable=False) - queue_name = db.Column(UUID(as_uuid=True)) + queue_name = db.Column(UUID(as_uuid=True), default=uuid.uuid4) class ApiKey(db.Model): diff --git a/app/service/rest.py b/app/service/rest.py index 81cfc8e4d..c114b5555 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -28,9 +28,6 @@ def create_service(): # I believe service is already added to the session but just needs a # db.session.commit try: - # add service name to here or in dao? - import uuid - service.queue_name = uuid.uuid4() save_model_service(service) except DAOException as e: return jsonify(result="error", message=str(e)), 400