Merge pull request #2887 from alphagov/cache-the-serialised-things

Serialise and cache services and API keys
This commit is contained in:
Chris Hill-Scott
2020-06-26 09:18:45 +01:00
committed by GitHub
15 changed files with 215 additions and 50 deletions

View File

@@ -8,9 +8,18 @@ import pytest
from flask import json, current_app, request
from freezegun import freeze_time
from notifications_python_client.authentication import create_jwt_token
from unittest.mock import call
from app import api_user
from app.dao.api_key_dao import get_unsigned_secrets, save_model_api_key, get_unsigned_secret, expire_api_key
from app.dao.api_key_dao import (
get_unsigned_secrets,
save_model_api_key,
get_unsigned_secret,
expire_api_key,
get_model_api_keys,
)
from app.dao.services_dao import dao_fetch_service_by_id
from app.models import ApiKey, KEY_TYPE_NORMAL
from app.authentication.auth import AuthError, requires_admin_auth, requires_auth, GENERAL_TOKEN_ERROR_MESSAGE
@@ -300,7 +309,7 @@ def test_authentication_returns_token_expired_when_service_uses_expired_key_and_
with pytest.raises(AuthError) as exc:
requires_auth()
assert exc.value.short_message == 'Invalid token: API key revoked'
assert exc.value.service_id == expired_api_key.service_id
assert exc.value.service_id == str(expired_api_key.service_id)
assert exc.value.api_key_id == expired_api_key.id
@@ -376,7 +385,7 @@ def test_authentication_returns_error_when_service_has_no_secrets(client,
with pytest.raises(AuthError) as exc:
requires_auth()
assert exc.value.short_message == 'Invalid token: service has no API keys'
assert exc.value.service_id == sample_service.id
assert exc.value.service_id == str(sample_service.id)
def test_should_attach_the_current_api_key_to_current_app(notify_api, sample_service, sample_api_key):
@@ -387,7 +396,7 @@ def test_should_attach_the_current_api_key_to_current_app(notify_api, sample_ser
headers={'Authorization': 'Bearer {}'.format(token)}
)
assert response.status_code == 200
assert api_user == sample_api_key
assert str(api_user.id) == str(sample_api_key.id)
def test_should_return_403_when_token_is_expired(client,
@@ -399,8 +408,8 @@ def test_should_return_403_when_token_is_expired(client,
request.headers = {'Authorization': 'Bearer {}'.format(token)}
requires_auth()
assert exc.value.short_message == 'Error: Your system clock must be accurate to within 30 seconds'
assert exc.value.service_id == sample_api_key.service_id
assert exc.value.api_key_id == sample_api_key.id
assert exc.value.service_id == str(sample_api_key.service_id)
assert str(exc.value.api_key_id) == str(sample_api_key.id)
def __create_token(service_id):
@@ -457,3 +466,28 @@ def test_proxy_key_on_admin_auth_endpoint(notify_api, check_proxy_header, header
]
)
assert response.status_code == expected_status
def test_should_cache_service_and_api_key_lookups(mocker, client, sample_api_key):
mock_get_api_keys = mocker.patch(
'app.serialised_models.get_model_api_keys',
wraps=get_model_api_keys,
)
mock_get_service = mocker.patch(
'app.serialised_models.dao_fetch_service_by_id',
wraps=dao_fetch_service_by_id,
)
for i in range(5):
token = __create_token(sample_api_key.service_id)
client.get('/notifications', headers={
'Authorization': f'Bearer {token}'
})
assert mock_get_api_keys.call_args_list == [
call(str(sample_api_key.service_id))
]
assert mock_get_service.call_args_list == [
call(str(sample_api_key.service_id))
]

View File

@@ -23,7 +23,7 @@ from app.notifications.validators import (
validate_and_format_recipient,
validate_template,
)
from app.serialised_models import SerialisedTemplate
from app.serialised_models import SerialisedService, SerialisedTemplate
from app.utils import get_template_instance
from app.v2.errors import (
@@ -439,8 +439,9 @@ def test_rejects_api_calls_with_international_numbers_if_service_does_not_allow_
notify_db_session,
):
service = create_service(service_permissions=[SMS_TYPE])
service_model = SerialisedService.from_id(service.id)
with pytest.raises(BadRequestError) as e:
validate_and_format_recipient('20-12-1234-1234', key_type, service, SMS_TYPE)
validate_and_format_recipient('20-12-1234-1234', key_type, service_model, SMS_TYPE)
assert e.value.status_code == 400
assert e.value.message == 'Cannot send to international mobile numbers'
assert e.value.fields == []
@@ -449,7 +450,8 @@ def test_rejects_api_calls_with_international_numbers_if_service_does_not_allow_
@pytest.mark.parametrize('key_type', ['test', 'normal'])
def test_allows_api_calls_with_international_numbers_if_service_does_allow_int_sms(
key_type, sample_service_full_permissions):
result = validate_and_format_recipient('20-12-1234-1234', key_type, sample_service_full_permissions, SMS_TYPE)
service_model = SerialisedService.from_id(sample_service_full_permissions.id)
result = validate_and_format_recipient('20-12-1234-1234', key_type, service_model, SMS_TYPE)
assert result == '201212341234'

View File

@@ -232,14 +232,14 @@ def test_should_cache_template_lookups_in_memory(mocker, client, sample_template
assert mock_get_template.call_count == 1
assert mock_get_template.call_args_list == [
call(service_id=sample_template.service_id, template_id=str(sample_template.id))
call(service_id=str(sample_template.service_id), template_id=str(sample_template.id))
]
assert Notification.query.count() == 5
def test_should_cache_template_lookups_in_redis(mocker, client, sample_template):
def test_should_cache_template_and_service_in_redis(mocker, client, sample_template):
from app.schemas import template_schema
from app.schemas import service_schema, template_schema
mock_redis_get = mocker.patch(
'app.redis_store.get',
@@ -263,34 +263,49 @@ def test_should_cache_template_lookups_in_redis(mocker, client, sample_template)
headers=[('Content-Type', 'application/json'), auth_header]
)
expected_key = f'template-{sample_template.id}-version-None'
expected_service_key = f'service-{sample_template.service_id}'
expected_templates_key = f'template-{sample_template.id}-version-None'
assert mock_redis_get.call_args_list == [call(
expected_key,
)]
assert mock_redis_get.call_args_list == [
call(expected_service_key),
call(expected_templates_key),
]
service_dict = service_schema.dump(sample_template.service).data
template_dict = template_schema.dump(sample_template).data
assert len(mock_redis_set.call_args_list) == 1
assert mock_redis_set.call_args[0][0] == expected_key
assert json.loads(mock_redis_set.call_args[0][1]) == {
'data': template_dict,
}
assert mock_redis_set.call_args[1]['ex'] == 604_800
assert len(mock_redis_set.call_args_list) == 2
service_call, templates_call = mock_redis_set.call_args_list
assert service_call[0][0] == expected_service_key
assert json.loads(service_call[0][1]) == {'data': service_dict}
assert service_call[1]['ex'] == 604_800
assert templates_call[0][0] == expected_templates_key
assert json.loads(templates_call[0][1]) == {'data': template_dict}
assert templates_call[1]['ex'] == 604_800
def test_should_return_template_if_found_in_redis(mocker, client, sample_template):
from app.schemas import template_schema
from app.schemas import service_schema, template_schema
service_dict = service_schema.dump(sample_template.service).data
template_dict = template_schema.dump(sample_template).data
mocker.patch(
'app.redis_store.get',
return_value=json.dumps({'data': template_dict}).encode('utf-8')
side_effect=[
json.dumps({'data': service_dict}).encode('utf-8'),
json.dumps({'data': template_dict}).encode('utf-8'),
],
)
mock_get_template = mocker.patch(
'app.dao.templates_dao.dao_get_template_by_id_and_service_id'
)
mock_get_service = mocker.patch(
'app.dao.services_dao.dao_fetch_service_by_id'
)
mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
@@ -308,6 +323,7 @@ def test_should_return_template_if_found_in_redis(mocker, client, sample_templat
assert response.status_code == 201
assert mock_get_template.called is False
assert mock_get_service.called is False
@pytest.mark.parametrize("notification_type, key_send_to, send_to",
@@ -867,8 +883,8 @@ def test_post_notification_with_document_upload(client, notify_db_session, mocke
assert validate(resp_json, post_email_response) == resp_json
assert document_download_mock.upload_document.call_args_list == [
call(service.id, 'abababab', csv_param.get('is_csv')),
call(service.id, 'cdcdcdcd', csv_param.get('is_csv'))
call(str(service.id), 'abababab', csv_param.get('is_csv')),
call(str(service.id), 'cdcdcdcd', csv_param.get('is_csv'))
]
notification = Notification.query.one()
@@ -1017,7 +1033,10 @@ def test_post_notifications_saves_email_to_queue(client, notify_db_session, mock
save_email_task = mocker.patch("app.celery.tasks.save_api_email.apply_async")
mock_send_task = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
service = create_service(service_id='941b6f9a-50d7-4742-8d50-f365ca74bf27', service_name='high volume service')
service = create_service(
service_id=current_app.config['HIGH_VOLUME_SERVICE'][0],
service_name='high volume service',
)
template = create_template(service=service, content='((message))', template_type=EMAIL_TYPE)
data = {
"email_address": "joe.citizen@example.com",
@@ -1048,7 +1067,10 @@ def test_post_notifications_saves_email_normally_if_save_email_to_queue_fails(cl
)
mock_send_task = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
service = create_service(service_id='941b6f9a-50d7-4742-8d50-f365ca74bf27', service_name='high volume service')
service = create_service(
service_id=current_app.config['HIGH_VOLUME_SERVICE'][1],
service_name='high volume service',
)
template = create_template(service=service, content='((message))', template_type=EMAIL_TYPE)
data = {
"email_address": "joe.citizen@example.com",
@@ -1077,8 +1099,10 @@ def test_post_notifications_doesnt_save_email_to_queue_for_test_emails(client, n
save_email_task = mocker.patch("app.celery.tasks.save_api_email.apply_async")
mock_send_task = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
service = create_service(service_id='941b6f9a-50d7-4742-8d50-f365ca74bf27', service_name='high volume service')
# create_api_key(service=service, key_type='test')
service = create_service(
service_id=current_app.config['HIGH_VOLUME_SERVICE'][2],
service_name='high volume service',
)
template = create_template(service=service, content='((message))', template_type=EMAIL_TYPE)
data = {
"email_address": "joe.citizen@example.com",
@@ -1107,7 +1131,10 @@ def test_post_notifications_doesnt_save_email_to_queue_for_sms(client, notify_db
save_email_task = mocker.patch("app.celery.tasks.save_api_email.apply_async")
mock_send_task = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
service = create_service(service_id='941b6f9a-50d7-4742-8d50-f365ca74bf27', service_name='high volume service')
service = create_service(
service_id=current_app.config['HIGH_VOLUME_SERVICE'][3],
service_name='high volume service',
)
template = create_template(service=service, content='((message))', template_type=SMS_TYPE)
data = {
"phone_number": '+447700900855',