From 8493e29acc0949ebbc440750a697ab6d2faee080 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 1 Apr 2016 13:42:11 +0100 Subject: [PATCH] Added some more tests. Removed the validation in the schema - it was adding complexity, let the unique constraint on the db throw the exception. This should only ever happen on a race condition which seems unlikely (two people changing a service to the same name at the same time) Do no set debug=true to the test config. If debug=true it changes the behaviour of the error handlers, throwing the exception rather than returning a 500. --- app/schemas.py | 15 ------- config.py | 4 +- tests/app/service/test_rest.py | 79 +++++++++++++++++++++++++++++----- 3 files changed, 71 insertions(+), 27 deletions(-) diff --git a/app/schemas.py b/app/schemas.py index e943e8419..dc1ce3c48 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -73,21 +73,6 @@ class ServiceSchema(BaseSchema): model = models.Service exclude = ("updated_at", "created_at", "api_keys", "templates", "jobs", 'old_id') - @validates_schema - def validate_all(self, data): - # There are 2 instances where we want to check - # for duplicate service name. One when they updating - # an existing service and when they are creating a service - name = data.get('name', None) - service = models.Service.query.filter_by(name=name).first() - error_msg = "Duplicate service name '{}'".format(name) - if 'id' in data: - if service and str(service.id) != data['id']: - raise ValidationError(error_msg, 'name') - else: - if service: - raise ValidationError(error_msg, 'name') - class TemplateSchema(BaseSchema): class Meta: diff --git a/config.py b/config.py index dde70a44e..e188940b0 100644 --- a/config.py +++ b/config.py @@ -83,8 +83,8 @@ class Development(Config): DEBUG = True -class Test(Development): - DEBUG = True +class Test(Config): + pass configs = { 'live': 'config_live.Live', diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 8dc241fd5..60f1ed3cd 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1,6 +1,7 @@ import json import uuid from flask import url_for + from app.dao.users_dao import save_model_user from app.dao.services_dao import dao_remove_user_from_service from app.models import User @@ -301,7 +302,8 @@ def test_should_not_create_service_with_duplicate_name(notify_api, 'user_id': sample_service.users[0].id, 'limit': 1000, 'restricted': False, - 'active': False} + 'active': False, + 'email_from': 'sample.service2'} auth_header = create_authorization_header( path='/service', method='POST', @@ -312,10 +314,36 @@ def test_should_not_create_service_with_duplicate_name(notify_api, '/service', data=json.dumps(data), headers=headers) - json_resp = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == 400 - assert "Duplicate service name '{}'".format( - sample_service.name) in json_resp['message']['name'] + assert resp.status_code == 500 + assert 'Key (name)=(Sample service) already exists.' in resp.get_data(as_text=True) + + +def test_create_service_should_throw_duplicate_key_constraint_for_existing_email_from(notify_api, + notify_db, + notify_db_session, + service_factory): + first_service = service_factory.get('First service', email_from='first.service') + with notify_api.test_request_context(): + with notify_api.test_client() as client: + data = { + 'name': 'First SERVICE', + 'user_id': first_service.users[0].id, + 'limit': 1000, + 'restricted': False, + 'active': False, + 'email_from': 'first.service'} + auth_header = create_authorization_header( + path='/service', + method='POST', + request_body=json.dumps(data) + ) + headers = [('Content-Type', 'application/json'), auth_header] + resp = client.post( + '/service', + data=json.dumps(data), + headers=headers) + assert resp.status_code == 500 + assert 'Key (email_from)=(first.service) already exists.' in resp.get_data(as_text=True) def test_update_service(notify_api, sample_service): @@ -363,7 +391,7 @@ def test_should_not_update_service_with_duplicate_name(notify_api, with notify_api.test_request_context(): with notify_api.test_client() as client: service_name = "another name" - another_service = create_sample_service( + create_sample_service( notify_db, notify_db_session, service_name=service_name, @@ -384,10 +412,41 @@ def test_should_not_update_service_with_duplicate_name(notify_api, data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header] ) - json_resp = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == 400 - assert "Duplicate service name '{}'".format( - service_name) in json_resp['message']['name'] + assert resp.status_code == 500 + assert 'Key (name)=(another name) already exists.' in resp.get_data(as_text=True) + + +def test_should_not_update_service_with_duplicate_email_from(notify_api, + notify_db, + notify_db_session, + sample_user, + sample_service): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + email_from = "duplicate.name" + create_sample_service( + notify_db, + notify_db_session, + service_name="duplicate name", + user=sample_user, + email_from=email_from) + data = { + 'email_from': email_from + } + + auth_header = create_authorization_header( + path='/service/{}'.format(sample_service.id), + method='POST', + request_body=json.dumps(data) + ) + + resp = client.post( + '/service/{}'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + assert resp.status_code == 500 + assert 'Key (email_from)=(duplicate.name) already exists.' in resp.get_data(as_text=True) def test_update_service_should_404_if_id_is_invalid(notify_api, notify_db, notify_db_session):