Rewrite cache decorator to reference args by name

`@cache.delete('user', 'user_id')` is easier to read and understand than
`@cache.delete('user', key_from_args=[1])`. This will become even more
apparent if we have to start doing stuff like `key_from_args=[1, 5]`,
which is a lot more opaque than just saying
`'service_id', 'template_id'`.

It does make the implementation a bit more complex, but I’m not too
worried about that because:
- the tests are solid
- it’s nicely encapsulated

app/notify_client/cache.py

@@ -1,27 +1,51 @@
 import json
+from contextlib import suppress
 from datetime import timedelta
 from functools import wraps
+from inspect import signature
 
 TTL = int(timedelta(hours=24).total_seconds())
 
 
-def _make_key(prefix, args, key_from_args):
+def _get_argument(argument_name, args, kwargs, client_method):
 
-    if key_from_args is None:
+    with suppress(KeyError):
-        key_from_args = [0]
+        return kwargs[argument_name]
 
+    with suppress(ValueError):
+        return args[list(signature(client_method).parameters).index(argument_name) - 1]
+
+    raise TypeError("{}() takes no argument called '{}'".format(
+        client_method.__name__, argument_name
+    ))
+
+
+def list_of_strings(list_of_stuff):
+    return list(map(str, filter(None, list_of_stuff)))
+
+
+def _make_key(prefix, key_from_args, local_variables):
     return '-'.join(
-        [prefix] + [args[index] for index in key_from_args]
+        [
+            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=None):
+def set(prefix, *key_from_args):
 
     def _set(client_method):
 
         @wraps(client_method)
         def new_client_method(client_instance, *args, **kwargs):
-            redis_key = _make_key(prefix, args, key_from_args)
+            redis_key = _make_key(prefix, key_from_args, locals())
             cached = client_instance.redis_client.get(redis_key)
             if cached:
                 return json.loads(cached.decode('utf-8'))
@@ -37,13 +61,13 @@ def set(prefix, key_from_args=None):
     return _set
 
 
-def delete(prefix, key_from_args=None):
+def delete(prefix, *key_from_args):
 
     def _delete(client_method):
 
         @wraps(client_method)
         def new_client_method(client_instance, *args, **kwargs):
-            redis_key = _make_key(prefix, args, key_from_args)
+            redis_key = _make_key(prefix, key_from_args, locals())
             client_instance.redis_client.delete(redis_key)
             return client_method(client_instance, *args, **kwargs)
 

app/notify_client/invite_api_client.py

@@ -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')
+    @cache.delete('service', 'service_id')
-    @cache.delete('user', key_from_args=[1])
+    @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),

app/notify_client/organisations_api_client.py

@@ -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')
+    @cache.delete('service', 'service_id')
     def update_service_organisation(self, service_id, org_id):
         data = {
             'service_id': service_id

app/notify_client/service_api_client.py

@@ -9,7 +9,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
     def __init__(self):
         super().__init__("a" * 73, "b")
 
-    @cache.delete('user', key_from_args=[4])
+    @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')
+    @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')
+    @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')
+    @cache.delete('service', 'service_id')
     def archive_service(self, service_id):
         return self.post('/service/{}/archive'.format(service_id), data=None)
 
-    @cache.delete('service')
+    @cache.delete('service', 'service_id')
     def suspend_service(self, service_id):
         return self.post('/service/{}/suspend'.format(service_id), data=None)
 
-    @cache.delete('service')
+    @cache.delete('service', 'service_id')
     def resume_service(self, service_id):
         return self.post('/service/{}/resume'.format(service_id), data=None)
 
-    @cache.delete('service')
+    @cache.delete('service', 'service_id')
-    @cache.delete('user', key_from_args=[1])
+    @cache.delete('user', 'user_id')
     def remove_user_from_service(self, service_id, user_id):
         """
         Remove a user from a service
@@ -267,7 +267,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
     def get_whitelist(self, service_id):
         return self.get(url='/service/{}/whitelist'.format(service_id))
 
-    @cache.delete('service')
+    @cache.delete('service', 'service_id')
     def update_whitelist(self, service_id, data):
         return self.put(url='/service/{}/whitelist'.format(service_id), data=data)
 
@@ -305,7 +305,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
             '/service/{}/inbound-sms/summary'.format(service_id)
         )
 
-    @cache.delete('service')
+    @cache.delete('service', 'service_id')
     def create_service_inbound_api(self, service_id, url, bearer_token, user_id):
         data = {
             "url": url,
@@ -314,7 +314,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
         }
         return self.post("/service/{}/inbound-api".format(service_id), data)
 
-    @cache.delete('service')
+    @cache.delete('service', 'service_id')
     def update_service_inbound_api(self, service_id, url, bearer_token, user_id, inbound_api_id):
         data = {
             "url": url,
@@ -346,7 +346,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
             )
         )
 
-    @cache.delete('service')
+    @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),
@@ -356,7 +356,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
             }
         )
 
-    @cache.delete('service')
+    @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(
@@ -375,7 +375,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')
+    @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),
@@ -385,7 +385,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
             }
         )
 
-    @cache.delete('service')
+    @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(
@@ -411,7 +411,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
             "/service/{}/sms-sender/{}".format(service_id, sms_sender_id)
         )
 
-    @cache.delete('service')
+    @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,
@@ -421,7 +421,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')
+    @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),
@@ -438,7 +438,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
             )
         )['data']
 
-    @cache.delete('service')
+    @cache.delete('service', 'service_id')
     def update_service_callback_api(self, service_id, url, bearer_token, user_id, callback_api_id):
         data = {
             "url": url,
@@ -448,7 +448,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')
+    @cache.delete('service', 'service_id')
     def create_service_callback_api(self, service_id, url, bearer_token, user_id):
         data = {
             "url": url,

app/notify_client/user_api_client.py

@@ -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')
+    @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')
+    @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')
+    @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')
+    @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')
+    @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')
+    @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')
+    @cache.delete('service', 'service_id')
-    @cache.delete('user', key_from_args=[1])
+    @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', key_from_args=[1])
+    @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')
+    @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')
+    @cache.delete('user', 'user_id')
     def _activate_user(self, user_id):
         return self.post("/user/{}/activate".format(user_id), data=None)