From d16d06fdef680e5908ddd893b2cc5eaff6896d73 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 18 Jun 2020 15:09:17 +0100 Subject: [PATCH] Cache serialised services in Redis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same as we’re doing for templates. This means avoiding a database call, even for services that don’t hit our API so often. They’ll still need to go to the database for the API keys, because we’re not comfortable putting the API key secrets in Redis. But once a service has got its keys from the database we commit the transaction, so the connection can be freed up until we need it again to insert the notification. --- app/serialised_models.py | 5 ++- .../notifications/test_post_notifications.py | 44 +++++++++++++------ 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/app/serialised_models.py b/app/serialised_models.py index db8d0a3fe..7f88171d2 100644 --- a/app/serialised_models.py +++ b/app/serialised_models.py @@ -128,16 +128,17 @@ class SerialisedService(SerialisedModel): @classmethod @memory_cache def from_id(cls, service_id): - return cls(cls.get_dict(service_id)) + return cls(cls.get_dict(service_id)['data']) @staticmethod + @redis_cache.set('service-{service_id}') def get_dict(service_id): from app.schemas import service_schema service_dict = service_schema.dump(dao_fetch_service_by_id(service_id)).data db.session.commit() - return service_dict + return {'data': service_dict} @cached_property def api_keys(self): diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 76a342801..21b2345f4 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -240,9 +240,9 @@ def test_should_cache_template_lookups_in_memory(mocker, client, sample_template 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', @@ -266,34 +266,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') @@ -311,6 +326,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",