From a538329d0e1a4959f27fe7a762170aeaa3814c2b Mon Sep 17 00:00:00 2001 From: David McDonald Date: Mon, 21 Sep 2020 16:29:35 +0100 Subject: [PATCH] Use new redis key for service templates and broadcasts We can drop use of the old key as we no longer need to read data from the old key. Either data exists in the new key and we read it from there or data doesn't exist in the new key and we go to the API to get it and then set it in redis. Note, the previous commit is important because it means we aren't at risk of when this commit is being deployed out, of us getting stale data from the old key. --- .../broadcast_message_api_client.py | 2 +- app/notify_client/service_api_client.py | 4 ++-- .../test_broadcast_message_client.py | 2 +- .../notify_client/test_service_api_client.py | 24 +++++++++---------- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/app/notify_client/broadcast_message_api_client.py b/app/notify_client/broadcast_message_api_client.py index eeb3bc63e..aaa8f6887 100644 --- a/app/notify_client/broadcast_message_api_client.py +++ b/app/notify_client/broadcast_message_api_client.py @@ -27,7 +27,7 @@ class BroadcastMessageAPIClient(NotifyAdminAPIClient): def get_broadcast_messages(self, service_id): return self.get(f'/service/{service_id}/broadcast-message')['broadcast_messages'] - @cache.set('broadcast-message-{broadcast_message_id}') + @cache.set('service-{service_id}-broadcast-message-{broadcast_message_id}') def get_broadcast_message(self, *, service_id, broadcast_message_id): return self.get(f'/service/{service_id}/broadcast-message/{broadcast_message_id}') diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 0a8b9e771..7c4065c4e 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -258,7 +258,7 @@ class ServiceAPIClient(NotifyAdminAPIClient): _attach_current_user({'postage': postage}) ) - @cache.set('template-{template_id}-version-{version}') + @cache.set('service-{service_id}-template-{template_id}-version-{version}') def get_service_template(self, service_id, template_id, version=None): """ Retrieve a service template. @@ -270,7 +270,7 @@ class ServiceAPIClient(NotifyAdminAPIClient): endpoint = '{base}/version/{version}'.format(base=endpoint, version=version) return self.get(endpoint) - @cache.set('template-{template_id}-versions') + @cache.set('service-{service_id}-template-{template_id}-versions') def get_service_template_versions(self, service_id, template_id): """ Retrieve a list of versions for a template diff --git a/tests/app/notify_client/test_broadcast_message_client.py b/tests/app/notify_client/test_broadcast_message_client.py index eac8e6c0e..3f9a53ffb 100644 --- a/tests/app/notify_client/test_broadcast_message_client.py +++ b/tests/app/notify_client/test_broadcast_message_client.py @@ -50,7 +50,7 @@ def test_get_broadcast_message(mocker): '/service/12345/broadcast-message/67890', ) mock_redis_set.assert_called_once_with( - 'broadcast-message-67890', + 'service-12345-broadcast-message-67890', '{"abc": "def"}', ex=604_800, ) diff --git a/tests/app/notify_client/test_service_api_client.py b/tests/app/notify_client/test_service_api_client.py index e172b9569..294a8c6fb 100644 --- a/tests/app/notify_client/test_service_api_client.py +++ b/tests/app/notify_client/test_service_api_client.py @@ -200,7 +200,7 @@ def test_client_returns_count_of_service_templates( service_api_client.get_service_template, [SERVICE_ONE_ID, FAKE_TEMPLATE_ID], [ - call('template-{}-version-None'.format(FAKE_TEMPLATE_ID)) + call('service-{}-template-{}-version-None'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID)) ], b'{"data_from": "cache"}', [], @@ -211,7 +211,7 @@ def test_client_returns_count_of_service_templates( service_api_client.get_service_template, [SERVICE_ONE_ID, FAKE_TEMPLATE_ID], [ - call('template-{}-version-None'.format(FAKE_TEMPLATE_ID)) + call('service-{}-template-{}-version-None'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID)), ], None, [ @@ -219,10 +219,10 @@ def test_client_returns_count_of_service_templates( ], [ call( - 'template-{}-version-None'.format(FAKE_TEMPLATE_ID), + 'service-{}-template-{}-version-None'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID), '{"data_from": "api"}', ex=604800, - ) + ), ], {'data_from': 'api'}, ), @@ -230,7 +230,7 @@ def test_client_returns_count_of_service_templates( service_api_client.get_service_template, [SERVICE_ONE_ID, FAKE_TEMPLATE_ID, 1], [ - call('template-{}-version-1'.format(FAKE_TEMPLATE_ID)) + call('service-{}-template-{}-version-1'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID)) ], b'{"data_from": "cache"}', [], @@ -241,7 +241,7 @@ def test_client_returns_count_of_service_templates( service_api_client.get_service_template, [SERVICE_ONE_ID, FAKE_TEMPLATE_ID, 1], [ - call('template-{}-version-1'.format(FAKE_TEMPLATE_ID)) + call('service-{}-template-{}-version-1'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID)), ], None, [ @@ -249,10 +249,10 @@ def test_client_returns_count_of_service_templates( ], [ call( - 'template-{}-version-1'.format(FAKE_TEMPLATE_ID), + 'service-{}-template-{}-version-1'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID), '{"data_from": "api"}', ex=604800, - ) + ), ], {'data_from': 'api'}, ), @@ -290,7 +290,7 @@ def test_client_returns_count_of_service_templates( service_api_client.get_service_template_versions, [SERVICE_ONE_ID, FAKE_TEMPLATE_ID], [ - call('template-{}-versions'.format(FAKE_TEMPLATE_ID)) + call('service-{}-template-{}-versions'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID)) ], b'{"data_from": "cache"}', [], @@ -301,7 +301,7 @@ def test_client_returns_count_of_service_templates( service_api_client.get_service_template_versions, [SERVICE_ONE_ID, FAKE_TEMPLATE_ID], [ - call('template-{}-versions'.format(FAKE_TEMPLATE_ID)) + call('service-{}-template-{}-versions'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID)), ], None, [ @@ -309,10 +309,10 @@ def test_client_returns_count_of_service_templates( ], [ call( - 'template-{}-versions'.format(FAKE_TEMPLATE_ID), + 'service-{}-template-{}-versions'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID), '{"data_from": "api"}', ex=604800, - ) + ), ], {'data_from': 'api'}, ),