From 7e15d0c928525e11ad655d9edb3f9bb00876dd82 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 7 Aug 2020 17:31:29 +0100 Subject: [PATCH] delete template cache when reply_to stuff is updated sms sender, email reply to, letter contact blocks. These are all cached within template, under `template.reply_to` - if the template doesnt have a specific default, then that field stores the service default. So when service default changes, we need to clear the template cache so that it is updated to reflect that. We already use this pattern for deleting the template cache for a bunch of templates in `template_folder_api_client.move_to_folder` --- app/notify_client/service_api_client.py | 40 ++++++++++++++---- .../notify_client/test_service_api_client.py | 42 ++++++++++++++++++- 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index e0d78ad08..277720ce0 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -6,6 +6,14 @@ from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache class ServiceAPIClient(NotifyAdminAPIClient): + def _delete_template_cache_for_service(self, service_id): + templates_for_service = self.get_service_templates(service_id)['data'] + if templates_for_service: + redis_client.delete(*[ + f"template-{x['id']}-version-None" + for x in templates_for_service + ]) + @cache.delete('user-{user_id}') def create_service( self, @@ -430,7 +438,7 @@ class ServiceAPIClient(NotifyAdminAPIClient): @cache.delete('service-{service_id}') def update_reply_to_email_address(self, service_id, reply_to_email_id, email_address, is_default=False): - return self.post( + ret = self.post( "/service/{}/email-reply-to/{}".format( service_id, reply_to_email_id, @@ -440,13 +448,17 @@ class ServiceAPIClient(NotifyAdminAPIClient): "is_default": is_default } ) + self._delete_template_cache_for_service(service_id) + return ret @cache.delete('service-{service_id}') def delete_reply_to_email_address(self, service_id, reply_to_email_id): - return self.post( + ret = self.post( "/service/{}/email-reply-to/{}/archive".format(service_id, reply_to_email_id), data=None ) + self._delete_template_cache_for_service(service_id) + return ret def get_letter_contacts(self, service_id): return self.get("/service/{}/letter-contact".format(service_id)) @@ -456,17 +468,19 @@ class ServiceAPIClient(NotifyAdminAPIClient): @cache.delete('service-{service_id}') def add_letter_contact(self, service_id, contact_block, is_default=False): - return self.post( + ret = self.post( "/service/{}/letter-contact".format(service_id), data={ "contact_block": contact_block, "is_default": is_default } ) + self._delete_template_cache_for_service(service_id) + return ret @cache.delete('service-{service_id}') def update_letter_contact(self, service_id, letter_contact_id, contact_block, is_default=False): - return self.post( + ret = self.post( "/service/{}/letter-contact/{}".format( service_id, letter_contact_id, @@ -476,13 +490,17 @@ class ServiceAPIClient(NotifyAdminAPIClient): "is_default": is_default } ) + self._delete_template_cache_for_service(service_id) + return ret @cache.delete('service-{service_id}') def delete_letter_contact(self, service_id, letter_contact_id): - return self.post( + ret = self.post( "/service/{}/letter-contact/{}/archive".format(service_id, letter_contact_id), data=None ) + self._delete_template_cache_for_service(service_id) + return ret def get_sms_senders(self, service_id): return self.get( @@ -502,24 +520,30 @@ class ServiceAPIClient(NotifyAdminAPIClient): } if inbound_number_id: data["inbound_number_id"] = inbound_number_id - return self.post("/service/{}/sms-sender".format(service_id), data=data) + ret = self.post("/service/{}/sms-sender".format(service_id), data=data) + self._delete_template_cache_for_service(service_id) + return ret @cache.delete('service-{service_id}') def update_sms_sender(self, service_id, sms_sender_id, sms_sender, is_default=False): - return self.post( + ret = self.post( "/service/{}/sms-sender/{}".format(service_id, sms_sender_id), data={ "sms_sender": sms_sender, "is_default": is_default } ) + self._delete_template_cache_for_service(service_id) + return ret @cache.delete('service-{service_id}') def delete_sms_sender(self, service_id, sms_sender_id): - return self.post( + ret = self.post( "/service/{}/sms-sender/{}/archive".format(service_id, sms_sender_id), data=None ) + self._delete_template_cache_for_service(service_id) + return ret def get_service_callback_api(self, service_id, callback_api_id): return self.get( diff --git a/tests/app/notify_client/test_service_api_client.py b/tests/app/notify_client/test_service_api_client.py index ea4e45840..7bcc5a7aa 100644 --- a/tests/app/notify_client/test_service_api_client.py +++ b/tests/app/notify_client/test_service_api_client.py @@ -413,6 +413,7 @@ def test_returns_value_from_cache( def test_deletes_service_cache( app_, mock_get_user, + mock_get_service_templates, mocker, client, method, @@ -470,6 +471,7 @@ def test_deletes_caches_when_modifying_templates( method, extra_args, expected_cache_deletes, + mock_get_service_templates, ): mocker.patch('app.notify_client.current_user', id='1') mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete') @@ -477,7 +479,7 @@ def test_deletes_caches_when_modifying_templates( getattr(service_api_client, method)(*extra_args) - assert mock_redis_delete.call_args_list == list(map(call, expected_cache_deletes)) + assert mock_redis_delete.call_args_list == [call(x) for x in expected_cache_deletes] assert len(mock_request.call_args_list) == 1 @@ -512,3 +514,41 @@ def test_client_updates_guest_list(mocker): url='/service/foo/guest-list', data=['a', 'b', 'c'], ) + + +def test_client_doesnt_delete_service_template_cache_when_none_exist( + app_, + mock_get_user, + mock_get_service_templates_when_no_templates_exist, + mocker +): + mocker.patch('app.notify_client.current_user', id='1') + mocker.patch('notifications_python_client.base.BaseAPIClient.request') + mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete') + + service_api_client.update_reply_to_email_address(SERVICE_ONE_ID, uuid4(), 'foo@bar.com') + + assert len(mock_redis_delete.call_args_list) == 1 + assert mock_redis_delete.call_args_list[0] == call('service-{}'.format(SERVICE_ONE_ID)) + + +def test_client_deletes_service_template_cache_when_service_is_updated( + app_, + mock_get_user, + mock_get_service_templates, + mocker +): + mocker.patch('app.notify_client.current_user', id='1') + mocker.patch('notifications_python_client.base.BaseAPIClient.request') + mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete') + + service_api_client.update_reply_to_email_address(SERVICE_ONE_ID, uuid4(), 'foo@bar.com') + + assert len(mock_redis_delete.call_args_list) == 2 + assert mock_redis_delete.call_args_list[1] == call('service-{}'.format(SERVICE_ONE_ID)) + + templates_to_delete = mock_redis_delete.call_args_list[0][0] + assert len(templates_to_delete) == 6 + for template_key in templates_to_delete: + assert template_key.startswith('template-') + assert template_key.endswith('version-None')