From 501187a9f4612b87b9a37b9b908589297cf1ae3e Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 9 Jan 2017 16:22:27 +0000 Subject: [PATCH] bump utils to 13.0.1 brings in a fix to InvalidEmail/Phone/AddressExceptions not being instantiated correctly. `exception.message` is not a python standard, so we shouldn't be relying on it to transmit exception reasons - rather we should be using `str(exception)` instead. This involved a handful of small changes to the schema validation --- app/schema_validation/__init__.py | 17 ++++++------ app/schemas.py | 8 +++--- requirements.txt | 2 +- tests/app/clients/test_aws_ses.py | 4 +-- tests/app/invite/test_invite_rest.py | 2 +- .../rest/test_send_notification.py | 2 +- tests/app/user/test_rest.py | 2 +- .../test_notification_schemas.py | 26 +++++++++---------- 8 files changed, 31 insertions(+), 32 deletions(-) diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py index bd94755d4..674d83ff8 100644 --- a/app/schema_validation/__init__.py +++ b/app/schema_validation/__init__.py @@ -30,8 +30,10 @@ def validate(json_to_validate, schema): def build_error_message(errors): fields = [] for e in errors: - field = "{} {}".format(e.path[0], e.schema.get('validationMessage')) if e.schema.get( - 'validationMessage') else __format_message(e) + field = ( + "{} {}".format(e.path[0], e.schema['validationMessage']) + if 'validationMessage' in e.schema else __format_message(e) + ) fields.append({"error": "ValidationError", "message": field}) message = { "status_code": 400, @@ -52,13 +54,10 @@ def __format_message(e): return error_path def get_error_message(e): - error_message = None - try: - error_message = e.cause.message - except AttributeError: - error_message = e.message - finally: - return error_message.replace("'", '') + # e.cause is an exception (such as InvalidPhoneError). if it's not present it was a standard jsonschema error + # such as a required field not being present + error_message = str(e.cause) if e.cause else e.message + return error_message.replace("'", '') path = get_path(e) message = get_error_message(e) diff --git a/app/schemas.py b/app/schemas.py index 3a220ef94..a7fe2ff11 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -124,7 +124,7 @@ class UserUpdateAttributeSchema(BaseSchema): try: validate_email_address(value) except InvalidEmailError as e: - raise ValidationError(e.message) + raise ValidationError(str(e)) @validates('mobile_number') def validate_mobile_number(self, value): @@ -314,7 +314,7 @@ class EmailNotificationSchema(NotificationSchema): try: validate_email_address(value) except InvalidEmailError as e: - raise ValidationError(e.message) + raise ValidationError(str(e)) class SmsTemplateNotificationSchema(SmsNotificationSchema): @@ -413,7 +413,7 @@ class InvitedUserSchema(BaseSchema): try: validate_email_address(value) except InvalidEmailError as e: - raise ValidationError(e.message) + raise ValidationError(str(e)) class PermissionSchema(BaseSchema): @@ -446,7 +446,7 @@ class EmailDataSchema(ma.Schema): try: validate_email_address(value) except InvalidEmailError as e: - raise ValidationError(e.message) + raise ValidationError(str(e)) class NotificationsFilterSchema(ma.Schema): diff --git a/requirements.txt b/requirements.txt index a414d2549..ab817c0e1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -23,6 +23,6 @@ Flask-Redis==0.1.0 # pin to minor version 3.1.x notifications-python-client>=3.1,<3.2 -git+https://github.com/alphagov/notifications-utils.git@12.2.0#egg=notifications-utils==12.2.0 +git+https://github.com/alphagov/notifications-utils.git@13.0.1#egg=notifications-utils==13.0.1 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3 diff --git a/tests/app/clients/test_aws_ses.py b/tests/app/clients/test_aws_ses.py index 47b5ce9d0..1f4913bbc 100644 --- a/tests/app/clients/test_aws_ses.py +++ b/tests/app/clients/test_aws_ses.py @@ -92,8 +92,8 @@ def test_send_email_raises_bad_email_as_InvalidEmailError(mocker): body=Mock() ) - assert 'some error message from amazon' in excinfo.value.message - assert 'clearly@invalid@email.com' in excinfo.value.message + assert 'some error message from amazon' in str(excinfo.value) + assert 'clearly@invalid@email.com' in str(excinfo.value) def test_send_email_raises_other_errs_as_AwsSesClientException(mocker): diff --git a/tests/app/invite/test_invite_rest.py b/tests/app/invite/test_invite_rest.py index d55f5dadc..182f4b05b 100644 --- a/tests/app/invite/test_invite_rest.py +++ b/tests/app/invite/test_invite_rest.py @@ -60,7 +60,7 @@ def test_create_invited_user_invalid_email(client, sample_service, mocker): assert response.status_code == 400 json_resp = json.loads(response.get_data(as_text=True)) assert json_resp['result'] == 'error' - assert json_resp['message'] == {'email_address': ['Not a valid email address.']} + assert json_resp['message'] == {'email_address': ['Not a valid email address']} assert mocked.call_count == 0 diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 5cc2e5e6a..a1c532ff2 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -306,7 +306,7 @@ def test_should_reject_email_notification_with_bad_email(notify_api, sample_emai mocked.apply_async.assert_not_called() assert response.status_code == 400 assert data['result'] == 'error' - assert data['message']['to'][0] == 'Not a valid email address.' + assert data['message']['to'][0] == 'Not a valid email address' @freeze_time("2016-01-01 11:09:00.061258") diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index b9580827d..fde24bff8 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -475,7 +475,7 @@ def test_send_user_reset_password_should_return_400_when_data_is_not_email_addre headers=[('Content-Type', 'application/json'), auth_header]) assert resp.status_code == 400 - assert json.loads(resp.get_data(as_text=True))['message'] == {'email': ['Not a valid email address.']} + assert json.loads(resp.get_data(as_text=True))['message'] == {'email': ['Not a valid email address']} assert mocked.call_count == 0 diff --git a/tests/app/v2/notifications/test_notification_schemas.py b/tests/app/v2/notifications/test_notification_schemas.py index 713e94aea..44a743a1a 100644 --- a/tests/app/v2/notifications/test_notification_schemas.py +++ b/tests/app/v2/notifications/test_notification_schemas.py @@ -33,7 +33,7 @@ def test_get_notifications_request_invalid_statuses( with pytest.raises(ValidationError) as e: validate({'status': invalid_statuses + valid_statuses}, get_notifications_request) - errors = json.loads(e.value.message).get('errors') + errors = json.loads(str(e.value)).get('errors') assert len(errors) == len(invalid_statuses) for index, value in enumerate(invalid_statuses): assert errors[index]['message'] == "status {} {}".format(value, partial_error_status) @@ -55,7 +55,7 @@ def test_get_notifications_request_invalid_template_types( with pytest.raises(ValidationError) as e: validate({'template_type': invalid_template_types + valid_template_types}, get_notifications_request) - errors = json.loads(e.value.message).get('errors') + errors = json.loads(str(e.value)).get('errors') assert len(errors) == len(invalid_template_types) for index, value in enumerate(invalid_template_types): assert errors[index]['message'] == "template_type {} {}".format(value, partial_error_template_type) @@ -68,7 +68,7 @@ def test_get_notifications_request_invalid_statuses_and_template_types(): 'template_type': ["sms", "orange", "avocado"] }, get_notifications_request) - errors = json.loads(e.value.message).get('errors') + errors = json.loads(str(e.value)).get('errors') assert len(errors) == 4 @@ -104,7 +104,7 @@ def test_post_sms_json_schema_bad_uuid_and_missing_phone_number(): j = {"template_id": "notUUID"} with pytest.raises(ValidationError) as e: validate(j, post_sms_request_schema) - error = json.loads(e.value.message) + error = json.loads(str(e.value)) assert len(error.keys()) == 2 assert error.get('status_code') == 400 assert len(error.get('errors')) == 2 @@ -123,7 +123,7 @@ def test_post_sms_schema_with_personalisation_that_is_not_a_dict(): } with pytest.raises(ValidationError) as e: validate(j, post_sms_request_schema) - error = json.loads(e.value.message) + error = json.loads(str(e.value)) assert len(error.get('errors')) == 1 assert error['errors'] == [{'error': 'ValidationError', 'message': "personalisation should contain key value pairs"}] @@ -141,7 +141,7 @@ def test_post_sms_request_schema_invalid_phone_number(invalid_phone_number, err_ } with pytest.raises(ValidationError) as e: validate(j, post_sms_request_schema) - errors = json.loads(e.value.message).get('errors') + errors = json.loads(str(e.value)).get('errors') assert len(errors) == 1 assert {"error": "ValidationError", "message": err_msg} == errors[0] @@ -151,7 +151,7 @@ def test_post_sms_request_schema_invalid_phone_number_and_missing_template(): } with pytest.raises(ValidationError) as e: validate(j, post_sms_request_schema) - errors = json.loads(e.value.message).get('errors') + errors = json.loads(str(e.value)).get('errors') assert len(errors) == 2 assert {"error": "ValidationError", "message": "phone_number Not a UK mobile number"} in errors assert {"error": "ValidationError", "message": "template_id is a required property"} in errors @@ -264,7 +264,7 @@ def test_post_sms_response_schema_missing_uri_raises_validation_error(response, del response['uri'] with pytest.raises(ValidationError) as e: validate(response, schema) - error = json.loads(e.value.message) + error = json.loads(str(e.value)) assert error['status_code'] == 400 assert error['errors'] == [{'error': 'ValidationError', 'message': "uri is a required property"}] @@ -278,10 +278,10 @@ def test_post_sms_response_schema_invalid_uri_raises_validation_error(response, response['uri'] = 'invalid-uri' with pytest.raises(ValidationError) as e: validate(response, schema) - error = json.loads(e.value.message) + error = json.loads(str(e.value)) assert error['status_code'] == 400 assert error['errors'] == [{'error': 'ValidationError', - 'message': "uri invalid-uri is not a uri"}] + 'message': "uri invalid-uri is not a valid URI."}] @pytest.mark.parametrize('response, schema', [ @@ -292,7 +292,7 @@ def test_post_sms_response_schema_missing_template_uri_raises_validation_error(r del response['template']['uri'] with pytest.raises(ValidationError) as e: validate(response, schema) - error = json.loads(e.value.message) + error = json.loads(str(e.value)) assert error['status_code'] == 400 assert error['errors'] == [{'error': 'ValidationError', 'message': "template uri is a required property"}] @@ -306,10 +306,10 @@ def test_post_sms_response_schema_invalid_template_uri_raises_validation_error(r response['template']['uri'] = 'invalid-uri' with pytest.raises(ValidationError) as e: validate(response, schema) - error = json.loads(e.value.message) + error = json.loads(str(e.value)) assert error['status_code'] == 400 assert error['errors'] == [{'error': 'ValidationError', - 'message': "template invalid-uri is not a uri"}] + 'message': "template invalid-uri is not a valid URI."}] def test_get_notifications_response_with_email_and_phone_number():