darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2025-12-16 10:04:07 -05:00
Code Issues Packages Projects Releases Wiki Activity

Rewrite cache decorator to use format string

Browse Source 
This is easier to read than having to understand the arguments 1…n of
the cache decorator are ‘magic’, and gives us more flexibility about
how the cache keys are formatted, eg being able to add words in the
middle of them.

Also changes the key format for all templates to be
`service-{service_id}-templates` instead of `templates-{service_id}`
because then it’s clearer what the ID represents.
This commit is contained in:
Chris Hill-Scott
2018-04-20 16:32:02 +01:00
parent b28e8691a6
commit 06de94f1c5
6 changed files with 80 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
app/notify_client/cache.py
View File

@@ -7,13 +7,14 @@ from inspect import signature
TTL = int(timedelta(hours=24).total_seconds())
def _get_argument(argument_name, args, kwargs, client_method):
def _get_argument(argument_name, client_method, args, kwargs):
with suppress(KeyError):
return kwargs[argument_name]
with suppress(ValueError, IndexError):
return args[list(signature(client_method).parameters).index(argument_name) - 1]
argument_index = list(signature(client_method).parameters).index(argument_name)
return args[argument_index - 1] # -1 because `args` doesn’t include `self`
with suppress(KeyError):
return signature(client_method).parameters[argument_name].default
@@ -23,32 +24,20 @@ def _get_argument(argument_name, args, kwargs, client_method):
))
def list_of_strings(list_of_stuff):
return list(map(str, filter(None, list_of_stuff)))
def _make_key(key_format, client_method, args, kwargs):
return key_format.format(**{
argument_name: _get_argument(argument_name, client_method, args, kwargs)
for argument_name in list(signature(client_method).parameters)
})
def _make_key(prefix, key_from_args, local_variables):
return '-'.join(
[
local_variables['prefix']
] + list_of_strings(
_get_argument(
argument_name,
local_variables['args'],
local_variables['kwargs'],
local_variables['client_method']
) for argument_name in key_from_args
)
)
def set(prefix, *key_from_args):
def set(key_format):
def _set(client_method):
@wraps(client_method)
def new_client_method(client_instance, *args, **kwargs):
redis_key = _make_key(prefix, key_from_args, locals())
redis_key = _make_key(key_format, client_method, args, kwargs)
cached = client_instance.redis_client.get(redis_key)
if cached:
return json.loads(cached.decode('utf-8'))
@@ -64,13 +53,13 @@ def set(prefix, *key_from_args):
return _set
def delete(prefix, *key_from_args):
def delete(key_format):
def _delete(client_method):
@wraps(client_method)
def new_client_method(client_instance, *args, **kwargs):
redis_key = _make_key(prefix, key_from_args, locals())
redis_key = _make_key(key_format, client_method, args, kwargs)
client_instance.redis_client.delete(redis_key)
return client_method(client_instance, *args, **kwargs)

4
app/notify_client/invite_api_client.py
View File

@@ -44,8 +44,8 @@ class InviteApiClient(NotifyAdminAPIClient):
self.post(url='/service/{0}/invite/{1}'.format(service_id, invited_user_id),
data=data)
@cache.delete('service', 'service_id')
@cache.delete('user', 'invited_user_id')
@cache.delete('service-{service_id}')
@cache.delete('user-{invited_user_id}')
def accept_invite(self, service_id, invited_user_id):
data = {'status': 'accepted'}
self.post(url='/service/{0}/invite/{1}'.format(service_id, invited_user_id),

2
app/notify_client/organisations_api_client.py
View File

@@ -27,7 +27,7 @@ class OrganisationsClient(NotifyAdminAPIClient):
def get_service_organisation(self, service_id):
return self.get(url="/service/{}/organisation".format(service_id))
@cache.delete('service', 'service_id')
@cache.delete('service-{service_id}')
def update_service_organisation(self, service_id, org_id):
data = {
'service_id': service_id

70
app/notify_client/service_api_client.py
View File

@@ -9,7 +9,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
def __init__(self):
super().__init__("a" * 73, "b")
@cache.delete('user', 'user_id')
@cache.delete('user-{user_id}')
def create_service(
self,
service_name,
@@ -34,7 +34,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
data = _attach_current_user(data)
return self.post("/service", data)['data']['id']
@cache.set('service', 'service_id')
@cache.set('service-{service_id}')
def get_service(self, service_id):
return self._get_service(service_id, detailed=False, today_only=False)
@@ -74,7 +74,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
params_dict['only_active'] = True
return self.get_services(params_dict)
@cache.delete('service', 'service_id')
@cache.delete('service-{service_id}')
def update_service(
self,
service_id,
@@ -115,20 +115,20 @@ class ServiceAPIClient(NotifyAdminAPIClient):
def update_service_with_properties(self, service_id, properties):
return self.update_service(service_id, **properties)
@cache.delete('service', 'service_id')
@cache.delete('service-{service_id}')
def archive_service(self, service_id):
return self.post('/service/{}/archive'.format(service_id), data=None)
@cache.delete('service', 'service_id')
@cache.delete('service-{service_id}')
def suspend_service(self, service_id):
return self.post('/service/{}/suspend'.format(service_id), data=None)
@cache.delete('service', 'service_id')
@cache.delete('service-{service_id}')
def resume_service(self, service_id):
return self.post('/service/{}/resume'.format(service_id), data=None)
@cache.delete('service', 'service_id')
@cache.delete('user', 'user_id')
@cache.delete('service-{service_id}')
@cache.delete('user-{user_id}')
def remove_user_from_service(self, service_id, user_id):
"""
Remove a user from a service
@@ -139,7 +139,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
data = _attach_current_user({})
return self.delete(endpoint, data)
@cache.delete('templates', 'service_id')
@cache.delete('service-{service_id}-templates')
def create_service_template(self, name, type_, content, service_id, subject=None, process_type='normal'):
"""
Create a service template.
@@ -159,9 +159,9 @@ class ServiceAPIClient(NotifyAdminAPIClient):
endpoint = "/service/{0}/template".format(service_id)
return self.post(endpoint, data)
@cache.delete('templates', 'service_id')
@cache.delete('template', 'id_')
@cache.delete('template_versions', 'id_')
@cache.delete('service-{service_id}-templates')
@cache.delete('template-{id_}-version-None')
@cache.delete('template-{id_}-versions')
def update_service_template(self, id_, name, type_, content, service_id, subject=None, process_type=None):
"""
Update a service template.
@@ -185,9 +185,9 @@ class ServiceAPIClient(NotifyAdminAPIClient):
endpoint = "/service/{0}/template/{1}".format(service_id, id_)
return self.post(endpoint, data)
@cache.delete('templates', 'service_id')
@cache.delete('template', 'id_')
@cache.delete('template_versions', 'id_')
@cache.delete('service-{service_id}-templates')
@cache.delete('template-{id_}-version-None')
@cache.delete('template-{id_}-versions')
def redact_service_template(self, service_id, id_):
return self.post(
"/service/{}/template/{}".format(service_id, id_),
@@ -196,9 +196,9 @@ class ServiceAPIClient(NotifyAdminAPIClient):
),
)
@cache.delete('templates', 'service_id')
@cache.delete('template', 'template_id')
@cache.delete('template_versions', 'template_id')
@cache.delete('service-{service_id}-templates')
@cache.delete('template-{template_id}-version-None')
@cache.delete('template-{template_id}-versions')
def update_service_template_sender(self, service_id, template_id, reply_to):
data = {
'reply_to': reply_to,
@@ -209,7 +209,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
data
)
@cache.set('template', 'template_id', 'version')
@cache.set('template-{template_id}-version-{version}')
def get_service_template(self, service_id, template_id, version=None):
"""
Retrieve a service template.
@@ -221,7 +221,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
endpoint = '{base}/version/{version}'.format(base=endpoint, version=version)
return self.get(endpoint)
@cache.set('template_versions', 'template_id')
@cache.set('template-{template_id}-versions')
def get_service_template_versions(self, service_id, template_id):
"""
Retrieve a list of versions for a template
@@ -232,7 +232,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
)
return self.get(endpoint)
@cache.set('templates', 'service_id')
@cache.set('service-{service_id}-templates')
def get_service_templates(self, service_id):
"""
Retrieve all templates for service.
@@ -252,9 +252,9 @@ class ServiceAPIClient(NotifyAdminAPIClient):
)
])
@cache.delete('templates', 'service_id')
@cache.delete('template', 'template_id')
@cache.delete('template_versions', 'template_id')
@cache.delete('service-{service_id}-templates')
@cache.delete('template-{template_id}-version-None')
@cache.delete('template-{template_id}-versions')
def delete_service_template(self, service_id, template_id):
"""
Set a service template's archived flag to True
@@ -284,7 +284,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
def get_whitelist(self, service_id):
return self.get(url='/service/{}/whitelist'.format(service_id))
@cache.delete('service', 'service_id')
@cache.delete('service-{service_id}')
def update_whitelist(self, service_id, data):
return self.put(url='/service/{}/whitelist'.format(service_id), data=data)
@@ -322,7 +322,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
'/service/{}/inbound-sms/summary'.format(service_id)
)
@cache.delete('service', 'service_id')
@cache.delete('service-{service_id}')
def create_service_inbound_api(self, service_id, url, bearer_token, user_id):
data = {
"url": url,
@@ -331,7 +331,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
}
return self.post("/service/{}/inbound-api".format(service_id), data)
@cache.delete('service', 'service_id')
@cache.delete('service-{service_id}')
def update_service_inbound_api(self, service_id, url, bearer_token, user_id, inbound_api_id):
data = {
"url": url,
@@ -363,7 +363,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
)
)
@cache.delete('service', 'service_id')
@cache.delete('service-{service_id}')
def add_reply_to_email_address(self, service_id, email_address, is_default=False):
return self.post(
"/service/{}/email-reply-to".format(service_id),
@@ -373,7 +373,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
}
)
@cache.delete('service', 'service_id')
@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(
"/service/{}/email-reply-to/{}".format(
@@ -392,7 +392,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
def get_letter_contact(self, service_id, letter_contact_id):
return self.get("/service/{}/letter-contact/{}".format(service_id, letter_contact_id))
@cache.delete('service', 'service_id')
@cache.delete('service-{service_id}')
def add_letter_contact(self, service_id, contact_block, is_default=False):
return self.post(
"/service/{}/letter-contact".format(service_id),
@@ -402,7 +402,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
}
)
@cache.delete('service', 'service_id')
@cache.delete('service-{service_id}')
def update_letter_contact(self, service_id, letter_contact_id, contact_block, is_default=False):
return self.post(
"/service/{}/letter-contact/{}".format(
@@ -428,7 +428,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
"/service/{}/sms-sender/{}".format(service_id, sms_sender_id)
)
@cache.delete('service', 'service_id')
@cache.delete('service-{service_id}')
def add_sms_sender(self, service_id, sms_sender, is_default=False, inbound_number_id=None):
data = {
"sms_sender": sms_sender,
@@ -438,7 +438,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
data["inbound_number_id"] = inbound_number_id
return self.post("/service/{}/sms-sender".format(service_id), data=data)
@cache.delete('service', 'service_id')
@cache.delete('service-{service_id}')
def update_sms_sender(self, service_id, sms_sender_id, sms_sender, is_default=False):
return self.post(
"/service/{}/sms-sender/{}".format(service_id, sms_sender_id),
@@ -455,7 +455,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
)
)['data']
@cache.delete('service', 'service_id')
@cache.delete('service-{service_id}')
def update_service_callback_api(self, service_id, url, bearer_token, user_id, callback_api_id):
data = {
"url": url,
@@ -465,7 +465,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
data['bearer_token'] = bearer_token
return self.post("/service/{}/delivery-receipt-api/{}".format(service_id, callback_api_id), data)
@cache.delete('service', 'service_id')
@cache.delete('service-{service_id}')
def create_service_callback_api(self, service_id, url, bearer_token, user_id):
data = {
"url": url,

22
app/notify_client/user_api_client.py
View File

@@ -39,7 +39,7 @@ class UserApiClient(NotifyAdminAPIClient):
def get_user(self, user_id):
return User(self._get_user(user_id)['data'], max_failed_login_count=self.max_failed_login_count)
@cache.set('user', 'user_id')
@cache.set('user-{user_id}')
def _get_user(self, user_id):
return self.get("/user/{}".format(user_id))
@@ -61,7 +61,7 @@ class UserApiClient(NotifyAdminAPIClient):
users.append(User(user, max_failed_login_count=self.max_failed_login_count))
return users
@cache.delete('user', 'user_id')
@cache.delete('user-{user_id}')
def update_user_attribute(self, user_id, **kwargs):
data = dict(kwargs)
disallowed_attributes = set(data.keys()) - ALLOWED_ATTRIBUTES
@@ -75,20 +75,20 @@ class UserApiClient(NotifyAdminAPIClient):
user_data = self.post(url, data=data)
return User(user_data['data'], max_failed_login_count=self.max_failed_login_count)
@cache.delete('user', 'user_id')
@cache.delete('user-{user_id}')
def reset_failed_login_count(self, user_id):
url = "/user/{}/reset-failed-login-count".format(user_id)
user_data = self.post(url, data={})
return User(user_data['data'], max_failed_login_count=self.max_failed_login_count)
@cache.delete('user', 'user_id')
@cache.delete('user-{user_id}')
def update_password(self, user_id, password):
data = {"_password": password}
url = "/user/{}/update-password".format(user_id)
user_data = self.post(url, data=data)
return User(user_data['data'], max_failed_login_count=self.max_failed_login_count)
@cache.delete('user', 'user_id')
@cache.delete('user-{user_id}')
def verify_password(self, user_id, password):
try:
url = "/user/{}/verify/password".format(user_id)
@@ -118,7 +118,7 @@ class UserApiClient(NotifyAdminAPIClient):
endpoint = '/user/{0}/email-already-registered'.format(user_id)
self.post(endpoint, data=data)
@cache.delete('user', 'user_id')
@cache.delete('user-{user_id}')
def check_verify_code(self, user_id, code, code_type):
data = {'code_type': code_type, 'code': code}
endpoint = '/user/{}/verify/code'.format(user_id)
@@ -154,20 +154,20 @@ class UserApiClient(NotifyAdminAPIClient):
resp = self.get(endpoint)
return [User(data) for data in resp['data']]
@cache.delete('service', 'service_id')
@cache.delete('user', 'user_id')
@cache.delete('service-{service_id}')
@cache.delete('user-{user_id}')
def add_user_to_service(self, service_id, user_id, permissions):
# permissions passed in are the combined admin roles, not db permissions
endpoint = '/service/{}/users/{}'.format(service_id, user_id)
data = [{'permission': x} for x in translate_permissions_from_admin_roles_to_db(permissions)]
self.post(endpoint, data=data)
@cache.delete('user', 'user_id')
@cache.delete('user-{user_id}')
def add_user_to_organisation(self, org_id, user_id):
resp = self.post('/organisations/{}/users/{}'.format(org_id, user_id), data={})
return User(resp['data'], max_failed_login_count=self.max_failed_login_count)
@cache.delete('user', 'user_id')
@cache.delete('user-{user_id}')
def set_user_permissions(self, user_id, service_id, permissions):
# permissions passed in are the combined admin roles, not db permissions
data = [{'permission': x} for x in translate_permissions_from_admin_roles_to_db(permissions)]
@@ -191,7 +191,7 @@ class UserApiClient(NotifyAdminAPIClient):
else:
return user
@cache.delete('user', 'user_id')
@cache.delete('user-{user_id}')
def _activate_user(self, user_id):
return self.post("/user/{}/activate".format(user_id), data=None)

38
tests/app/notify_client/test_service_api_client.py
View File

@@ -184,7 +184,7 @@ def test_client_returns_count_of_service_templates(
service_api_client.get_service_template,
[SERVICE_ONE_ID, FAKE_TEMPLATE_ID],
[
call('template-{}'.format(FAKE_TEMPLATE_ID))
call('template-{}-version-None'.format(FAKE_TEMPLATE_ID))
],
b'{"data_from": "cache"}',
[],
@@ -195,7 +195,7 @@ def test_client_returns_count_of_service_templates(
service_api_client.get_service_template,
[SERVICE_ONE_ID, FAKE_TEMPLATE_ID],
[
call('template-{}'.format(FAKE_TEMPLATE_ID))
call('template-{}-version-None'.format(FAKE_TEMPLATE_ID))
],
None,
[
@@ -203,7 +203,7 @@ def test_client_returns_count_of_service_templates(
],
[
call(
'template-{}'.format(FAKE_TEMPLATE_ID),
'template-{}-version-None'.format(FAKE_TEMPLATE_ID),
'{"data_from": "api"}',
ex=86400
)
@@ -214,7 +214,7 @@ def test_client_returns_count_of_service_templates(
service_api_client.get_service_template,
[SERVICE_ONE_ID, FAKE_TEMPLATE_ID, 1],
[
call('template-{}-1'.format(FAKE_TEMPLATE_ID))
call('template-{}-version-1'.format(FAKE_TEMPLATE_ID))
],
b'{"data_from": "cache"}',
[],
@@ -225,7 +225,7 @@ def test_client_returns_count_of_service_templates(
service_api_client.get_service_template,
[SERVICE_ONE_ID, FAKE_TEMPLATE_ID, 1],
[
call('template-{}-1'.format(FAKE_TEMPLATE_ID))
call('template-{}-version-1'.format(FAKE_TEMPLATE_ID))
],
None,
[
@@ -233,7 +233,7 @@ def test_client_returns_count_of_service_templates(
],
[
call(
'template-{}-1'.format(FAKE_TEMPLATE_ID),
'template-{}-version-1'.format(FAKE_TEMPLATE_ID),
'{"data_from": "api"}',
ex=86400
)
@@ -314,27 +314,27 @@ def test_deletes_service_cache(
@pytest.mark.parametrize('method, extra_args, expected_cache_deletes', [
('create_service_template', ['name', 'type_', 'content', SERVICE_ONE_ID], [
'templates-{}'.format(SERVICE_ONE_ID),
'service-{}-templates'.format(SERVICE_ONE_ID),
]),
('update_service_template', [FAKE_TEMPLATE_ID, 'foo', 'sms', 'bar', SERVICE_ONE_ID], [
'templates-{}'.format(SERVICE_ONE_ID),
'template-{}'.format(FAKE_TEMPLATE_ID),
'template_versions-{}'.format(FAKE_TEMPLATE_ID),
'service-{}-templates'.format(SERVICE_ONE_ID),
'template-{}-version-None'.format(FAKE_TEMPLATE_ID),
'template-{}-versions'.format(FAKE_TEMPLATE_ID),
]),
('redact_service_template', [SERVICE_ONE_ID, FAKE_TEMPLATE_ID], [
'templates-{}'.format(SERVICE_ONE_ID),
'template-{}'.format(FAKE_TEMPLATE_ID),
'template_versions-{}'.format(FAKE_TEMPLATE_ID),
'service-{}-templates'.format(SERVICE_ONE_ID),
'template-{}-version-None'.format(FAKE_TEMPLATE_ID),
'template-{}-versions'.format(FAKE_TEMPLATE_ID),
]),
('update_service_template_sender', [SERVICE_ONE_ID, FAKE_TEMPLATE_ID, 'foo'], [
'templates-{}'.format(SERVICE_ONE_ID),
'template-{}'.format(FAKE_TEMPLATE_ID),
'template_versions-{}'.format(FAKE_TEMPLATE_ID),
'service-{}-templates'.format(SERVICE_ONE_ID),
'template-{}-version-None'.format(FAKE_TEMPLATE_ID),
'template-{}-versions'.format(FAKE_TEMPLATE_ID),
]),
('delete_service_template', [SERVICE_ONE_ID, FAKE_TEMPLATE_ID], [
'templates-{}'.format(SERVICE_ONE_ID),
'template-{}'.format(FAKE_TEMPLATE_ID),
'template_versions-{}'.format(FAKE_TEMPLATE_ID),
'service-{}-templates'.format(SERVICE_ONE_ID),
'template-{}-version-None'.format(FAKE_TEMPLATE_ID),
'template-{}-versions'.format(FAKE_TEMPLATE_ID),
]),
])
def test_deletes_caches_when_modifying_templates(