Change code order for Redis delete decorator

Before, the delete decorator would delete the keys from Redis and then
we made the request to api to change the data. However, it is possible
that the cache could get re-populated in between these two things
happening, and so would cache outdated data.

This changes the order to send the api request first. We then always
delete the specified keys from Redis. Changing the order of the code in
the decorator changes the order in which the cache keys get deleted, so
the tests have been updated.
This commit is contained in:
Katie Smith
2019-07-26 16:19:03 +01:00
parent ccad068a48
commit 123b769771
7 changed files with 26 additions and 23 deletions

View File

@@ -61,9 +61,12 @@ def delete(key_format):
@wraps(client_method)
def new_client_method(client_instance, *args, **kwargs):
redis_key = _make_key(key_format, client_method, args, kwargs)
redis_client.delete(redis_key)
return client_method(client_instance, *args, **kwargs)
try:
api_response = client_method(client_instance, *args, **kwargs)
finally:
redis_key = _make_key(key_format, client_method, args, kwargs)
redis_client.delete(redis_key)
return api_response
return new_client_method

View File

@@ -85,6 +85,6 @@ def test_update_email_branding(mocker, fake_uuid):
data=org_data
)
assert mock_redis_delete.call_args_list == [
call('email_branding'),
call('email_branding-{}'.format(fake_uuid)),
call('email_branding'),
]

View File

@@ -68,6 +68,6 @@ def test_update_letter_branding(mocker, fake_uuid):
data=branding
)
assert mock_redis_delete.call_args_list == [
call('letter_branding'),
call('letter_branding-{}'.format(fake_uuid)),
call('letter_branding'),
]

View File

@@ -150,7 +150,7 @@ def test_update_organisation_when_not_updating_org_type(mocker, fake_uuid):
url='/organisations/{}'.format(fake_uuid),
data={'foo': 'bar'}
)
assert mock_redis_delete.call_args_list == [call('domains'), call('organisations')]
assert mock_redis_delete.call_args_list == [call('organisations'), call('domains')]
def test_update_organisation_when_updating_org_type_and_org_has_services(mocker, fake_uuid):
@@ -168,9 +168,9 @@ def test_update_organisation_when_updating_org_type_and_org_has_services(mocker,
data={'organisation_type': 'central'}
)
assert mock_redis_delete.call_args_list == [
call('domains'),
call('organisations'),
call('service-a', 'service-b', 'service-c'),
call('organisations'),
call('domains'),
]
@@ -189,6 +189,6 @@ def test_update_organisation_when_updating_org_type_but_org_has_no_services(mock
data={'organisation_type': 'central'}
)
assert mock_redis_delete.call_args_list == [
call('domains'),
call('organisations'),
call('domains'),
]

View File

@@ -388,29 +388,29 @@ def test_deletes_service_cache(
'service-{}-templates'.format(SERVICE_ONE_ID),
]),
('update_service_template', [FAKE_TEMPLATE_ID, 'foo', 'sms', 'bar', SERVICE_ONE_ID], [
'service-{}-templates'.format(SERVICE_ONE_ID),
'template-{}-version-None'.format(FAKE_TEMPLATE_ID),
'template-{}-versions'.format(FAKE_TEMPLATE_ID),
'template-{}-version-None'.format(FAKE_TEMPLATE_ID),
'service-{}-templates'.format(SERVICE_ONE_ID),
]),
('redact_service_template', [SERVICE_ONE_ID, FAKE_TEMPLATE_ID], [
'service-{}-templates'.format(SERVICE_ONE_ID),
'template-{}-version-None'.format(FAKE_TEMPLATE_ID),
'template-{}-versions'.format(FAKE_TEMPLATE_ID),
'template-{}-version-None'.format(FAKE_TEMPLATE_ID),
'service-{}-templates'.format(SERVICE_ONE_ID),
]),
('update_service_template_sender', [SERVICE_ONE_ID, FAKE_TEMPLATE_ID, 'foo'], [
'service-{}-templates'.format(SERVICE_ONE_ID),
'template-{}-version-None'.format(FAKE_TEMPLATE_ID),
'template-{}-versions'.format(FAKE_TEMPLATE_ID),
'template-{}-version-None'.format(FAKE_TEMPLATE_ID),
'service-{}-templates'.format(SERVICE_ONE_ID),
]),
('update_service_template_postage', [SERVICE_ONE_ID, FAKE_TEMPLATE_ID, 'first'], [
'service-{}-templates'.format(SERVICE_ONE_ID),
'template-{}-version-None'.format(FAKE_TEMPLATE_ID),
'template-{}-versions'.format(FAKE_TEMPLATE_ID),
'template-{}-version-None'.format(FAKE_TEMPLATE_ID),
'service-{}-templates'.format(SERVICE_ONE_ID),
]),
('delete_service_template', [SERVICE_ONE_ID, FAKE_TEMPLATE_ID], [
'service-{}-templates'.format(SERVICE_ONE_ID),
'template-{}-version-None'.format(FAKE_TEMPLATE_ID),
'template-{}-versions'.format(FAKE_TEMPLATE_ID),
'template-{}-version-None'.format(FAKE_TEMPLATE_ID),
'service-{}-templates'.format(SERVICE_ONE_ID),
]),
])
def test_deletes_caches_when_modifying_templates(

View File

@@ -73,13 +73,13 @@ def test_move_templates_and_folders(mocker):
},
)
assert mock_redis_delete.call_args_list == [
call('service-{}-template-folders'.format(some_service_id)),
call('service-{}-templates'.format(some_service_id)),
call(
'template-a-version-None',
'template-b-version-None',
'template-c-version-None',
),
call('service-{}-templates'.format(some_service_id)),
call('service-{}-template-folders'.format(some_service_id)),
]

View File

@@ -234,7 +234,7 @@ def test_add_user_to_service_calls_correct_endpoint_and_deletes_keys_from_cache(
mock_post.assert_called_once_with(expected_url, data=data)
assert mock_redis_delete.call_args_list == [
call('service-{service_id}'.format(service_id=service_id)),
call('service-{service_id}-template-folders'.format(service_id=service_id)),
call('user-{user_id}'.format(user_id=user_id)),
call('service-{service_id}-template-folders'.format(service_id=service_id)),
call('service-{service_id}'.format(service_id=service_id)),
]