Serialise service, API keys and permissions

By serialising these straight away we can:
- not go back to the database later, potentially closing the connection
  sooner
- potentially cache the serialised data, meaning we don’t touch the
  database at all

app/authentication/auth.py

@@ -8,7 +8,7 @@ from sqlalchemy.exc import DataError
 from sqlalchemy.orm.exc import NoResultFound
 from gds_metrics import Histogram
 
-from app.dao.services_dao import dao_fetch_service_by_id_with_api_keys
+from app.serialised_models import SerialisedService
 
 
 GENERAL_TOKEN_ERROR_MESSAGE = 'Invalid token: make sure your API token matches the example at https://docs.notifications.service.gov.uk/rest-api.html#authorisation-header'  # noqa
@@ -94,7 +94,7 @@ def requires_auth():
 
     try:
         with AUTH_DB_CONNECTION_DURATION_SECONDS.time():
-            service = dao_fetch_service_by_id_with_api_keys(issuer)
+            service = SerialisedService.from_id(issuer)
     except DataError:
         raise AuthError("Invalid token: service id is not the right data type", 403)
     except NoResultFound:
@@ -129,7 +129,7 @@ def requires_auth():
         if api_key.expiry_date:
             raise AuthError("Invalid token: API key revoked", 403, service_id=service.id, api_key_id=api_key.id)
 
-        g.service_id = api_key.service_id
+        g.service_id = service.id
         _request_ctx_stack.top.authenticated_service = service
         _request_ctx_stack.top.api_user = api_key
 

app/notifications/process_notifications.py

@@ -123,7 +123,6 @@ def persist_notification(
         template_version=template_version,
         to=recipient,
         service_id=service.id,
-        service=service,
         personalisation=personalisation,
         notification_type=notification_type,
         api_key_id=api_key_id,

app/notifications/validators.py

@@ -90,7 +90,7 @@ def service_can_send_to_recipient(send_to, key_type, service, allow_whitelisted_
 
 
 def service_has_permission(notify_type, permissions):
-    return notify_type in [p.permission for p in permissions]
+    return notify_type in permissions
 
 
 def check_service_has_permission(notify_type, permissions):
@@ -137,7 +137,7 @@ def check_if_service_can_send_to_number(service, number):
     if (
         # if number is international and not a crown dependency
         international_phone_info.international and not international_phone_info.crown_dependency
-    ) and INTERNATIONAL_SMS_TYPE not in [p.permission for p in service.permissions]:
+    ) and INTERNATIONAL_SMS_TYPE not in service.permissions:
         raise BadRequestError(message="Cannot send to international mobile numbers")
     else:
         return international_phone_info

app/serialised_models.py

@@ -5,9 +5,13 @@ from threading import RLock
 
 import cachetools
 from notifications_utils.clients.redis import RequestCache
+from werkzeug.utils import cached_property
 
 from app import redis_store
+
 from app.dao import templates_dao
+from app.dao.api_key_dao import get_model_api_keys
+from app.dao.services_dao import dao_fetch_service_by_id
 
 caches = defaultdict(partial(cachetools.TTLCache, maxsize=1024, ttl=2))
 locks = defaultdict(RLock)
@@ -53,6 +57,29 @@ class SerialisedModel(ABC):
         return super().__dir__() + list(sorted(self.ALLOWED_PROPERTIES))
 
 
+class SerialisedModelCollection(ABC):
+
+    """
+    A SerialisedModelCollection takes a list of dictionaries, typically
+    created by serialising database objects. When iterated over it
+    returns a SerialisedModel instance for each of the items in the list.
+    """
+
+    @property
+    @abstractmethod
+    def model(self):
+        pass
+
+    def __init__(self, items):
+        self.items = items
+
+    def __bool__(self):
+        return bool(self.items)
+
+    def __getitem__(self, index):
+        return self.model(self.items[index])
+
+
 class SerialisedTemplate(SerialisedModel):
     ALLOWED_PROPERTIES = {
         'archived',
@@ -84,3 +111,49 @@ class SerialisedTemplate(SerialisedModel):
         template_dict = template_schema.dump(fetched_template).data
 
         return {'data': template_dict}
+
+
+class SerialisedService(SerialisedModel):
+    ALLOWED_PROPERTIES = {
+        'id',
+        'active',
+        'contact_link',
+        'email_from',
+        'permissions',
+        'research_mode',
+        'restricted',
+    }
+
+    @classmethod
+    def from_id(cls, service_id):
+        return cls(cls.get_dict(service_id))
+
+    @staticmethod
+    def get_dict(service_id):
+        from app.schemas import service_schema
+
+        return service_schema.dump(dao_fetch_service_by_id(service_id)).data
+
+    @cached_property
+    def api_keys(self):
+        return SerialisedAPIKeyCollection.from_service_id(self.id)
+
+
+class SerialisedAPIKey(SerialisedModel):
+    ALLOWED_PROPERTIES = {
+        'id',
+        'secret',
+        'expiry_date',
+        'key_type',
+    }
+
+
+class SerialisedAPIKeyCollection(SerialisedModelCollection):
+    model = SerialisedAPIKey
+
+    @classmethod
+    def from_service_id(cls, service_id):
+        return cls([
+            {k: getattr(key, k) for k in SerialisedAPIKey.ALLOWED_PROPERTIES}
+            for key in get_model_api_keys(service_id)
+        ])

app/service/send_notification.py

@@ -137,7 +137,9 @@ def get_reply_to_text(notification_type, sender_id, service, template):
 def send_pdf_letter_notification(service_id, post_data):
     service = dao_fetch_service_by_id(service_id)
 
-    check_service_has_permission(LETTER_TYPE, service.permissions)
+    check_service_has_permission(LETTER_TYPE, [
+        p.permission for p in service.permissions
+    ])
     check_service_over_daily_message_limit(KEY_TYPE_NORMAL, service)
     validate_created_by(service, post_data['created_by'])
     validate_and_format_recipient(

app/service/utils.py

@@ -7,6 +7,8 @@ from app.models import (
     MOBILE_TYPE, EMAIL_TYPE,
     KEY_TYPE_TEST, KEY_TYPE_TEAM, KEY_TYPE_NORMAL)
 
+from app.dao.services_dao import dao_fetch_service_by_id
+
 
 def get_recipients_from_request(request_json, key, type):
     return [(type, recipient) for recipient in request_json.get(key)]
@@ -33,6 +35,10 @@ def service_allowed_to_send_to(recipient, service, key_type, allow_whitelisted_r
     if key_type == KEY_TYPE_NORMAL and not service.restricted:
         return True
 
+    # Revert back to the ORM model here so we can get some things which
+    # aren’t in the serialised model
+    service = dao_fetch_service_by_id(service.id)
+
     team_members = itertools.chain.from_iterable(
         [user.mobile_number, user.email_address] for user in service.users
     )

app/template/rest.py

@@ -69,7 +69,9 @@ def validate_parent_folder(template_json):
 def create_template(service_id):
     fetched_service = dao_fetch_service_by_id(service_id=service_id)
     # permissions needs to be placed here otherwise marshmallow will interfere with versioning
-    permissions = fetched_service.permissions
+    permissions = [
+        p.permission for p in fetched_service.permissions
+    ]
     template_json = validate(request.get_json(), post_create_template_schema)
     folder = validate_parent_folder(template_json=template_json)
     new_template = Template.from_json(template_json, folder)
@@ -102,7 +104,12 @@ def create_template(service_id):
 def update_template(service_id, template_id):
     fetched_template = dao_get_template_by_id_and_service_id(template_id=template_id, service_id=service_id)
 
-    if not service_has_permission(fetched_template.template_type, fetched_template.service.permissions):
+    if not service_has_permission(
+        fetched_template.template_type,
+        [
+            p.permission for p in fetched_template.service.permissions
+        ]
+    ):
         message = "Updating {} templates is not allowed".format(
             get_public_notify_type_text(fetched_template.template_type))
         errors = {'template_type': [message]}

app/v2/notifications/post_notifications.py

@@ -342,7 +342,7 @@ def process_letter_notification(
     if api_key.key_type == KEY_TYPE_TEAM:
         raise BadRequestError(message='Cannot send letters with a team api key', status_code=403)
 
-    if not api_key.service.research_mode and api_key.service.restricted and api_key.key_type != KEY_TYPE_TEST:
+    if not service.research_mode and service.restricted and api_key.key_type != KEY_TYPE_TEST:
         raise BadRequestError(message='Cannot send letters when service is in trial mode', status_code=403)
 
     if precompiled:
@@ -352,7 +352,7 @@ def process_letter_notification(
                                                         template=template,
                                                         reply_to_text=reply_to_text)
 
-    validate_address(api_key, letter_data)
+    validate_address(service, letter_data)
 
     test_key = api_key.key_type == KEY_TYPE_TEST
 
@@ -402,10 +402,10 @@ def process_letter_notification(
     return resp
 
 
-def validate_address(api_key, letter_data):
+def validate_address(service, letter_data):
     address = PostalAddress.from_personalisation(
         letter_data['personalisation'],
-        allow_international_letters=api_key.service.has_permission(INTERNATIONAL_LETTERS),
+        allow_international_letters=(INTERNATIONAL_LETTERS in service.permissions),
     )
     if not address.has_enough_lines:
         raise ValidationError(

tests/app/authentication/test_authentication.py

@@ -300,7 +300,7 @@ def test_authentication_returns_token_expired_when_service_uses_expired_key_and_
     with pytest.raises(AuthError) as exc:
         requires_auth()
     assert exc.value.short_message == 'Invalid token: API key revoked'
-    assert exc.value.service_id == expired_api_key.service_id
+    assert exc.value.service_id == str(expired_api_key.service_id)
     assert exc.value.api_key_id == expired_api_key.id
 
 
@@ -376,7 +376,7 @@ def test_authentication_returns_error_when_service_has_no_secrets(client,
     with pytest.raises(AuthError) as exc:
         requires_auth()
     assert exc.value.short_message == 'Invalid token: service has no API keys'
-    assert exc.value.service_id == sample_service.id
+    assert exc.value.service_id == str(sample_service.id)
 
 
 def test_should_attach_the_current_api_key_to_current_app(notify_api, sample_service, sample_api_key):
@@ -387,7 +387,7 @@ def test_should_attach_the_current_api_key_to_current_app(notify_api, sample_ser
             headers={'Authorization': 'Bearer {}'.format(token)}
         )
         assert response.status_code == 200
-        assert api_user == sample_api_key
+        assert str(api_user.id) == str(sample_api_key.id)
 
 
 def test_should_return_403_when_token_is_expired(client,
@@ -399,8 +399,8 @@ def test_should_return_403_when_token_is_expired(client,
             request.headers = {'Authorization': 'Bearer {}'.format(token)}
             requires_auth()
     assert exc.value.short_message == 'Error: Your system clock must be accurate to within 30 seconds'
-    assert exc.value.service_id == sample_api_key.service_id
-    assert exc.value.api_key_id == sample_api_key.id
+    assert exc.value.service_id == str(sample_api_key.service_id)
+    assert str(exc.value.api_key_id) == str(sample_api_key.id)
 
 
 def __create_token(service_id):

tests/app/notifications/test_validators.py

@@ -23,7 +23,7 @@ from app.notifications.validators import (
     validate_and_format_recipient,
     validate_template,
 )
-from app.serialised_models import SerialisedTemplate
+from app.serialised_models import SerialisedService, SerialisedTemplate
 from app.utils import get_template_instance
 
 from app.v2.errors import (
@@ -439,8 +439,9 @@ def test_rejects_api_calls_with_international_numbers_if_service_does_not_allow_
         notify_db_session,
 ):
     service = create_service(service_permissions=[SMS_TYPE])
+    service_model = SerialisedService.from_id(service.id)
     with pytest.raises(BadRequestError) as e:
-        validate_and_format_recipient('20-12-1234-1234', key_type, service, SMS_TYPE)
+        validate_and_format_recipient('20-12-1234-1234', key_type, service_model, SMS_TYPE)
     assert e.value.status_code == 400
     assert e.value.message == 'Cannot send to international mobile numbers'
     assert e.value.fields == []
@@ -449,7 +450,8 @@ def test_rejects_api_calls_with_international_numbers_if_service_does_not_allow_
 @pytest.mark.parametrize('key_type', ['test', 'normal'])
 def test_allows_api_calls_with_international_numbers_if_service_does_allow_int_sms(
         key_type, sample_service_full_permissions):
-    result = validate_and_format_recipient('20-12-1234-1234', key_type, sample_service_full_permissions, SMS_TYPE)
+    service_model = SerialisedService.from_id(sample_service_full_permissions.id)
+    result = validate_and_format_recipient('20-12-1234-1234', key_type, service_model, SMS_TYPE)
     assert result == '201212341234'
 
 

tests/app/v2/notifications/test_post_notifications.py

@@ -235,7 +235,7 @@ def test_should_cache_template_lookups_in_memory(mocker, client, sample_template
 
     assert mock_get_template.call_count == 1
     assert mock_get_template.call_args_list == [
-        call(service_id=sample_template.service_id, template_id=str(sample_template.id))
+        call(service_id=str(sample_template.service_id), template_id=str(sample_template.id))
     ]
     assert Notification.query.count() == 5
 
@@ -920,8 +920,8 @@ def test_post_notification_with_document_upload(client, notify_db_session, mocke
     assert validate(resp_json, post_email_response) == resp_json
 
     assert document_download_mock.upload_document.call_args_list == [
-        call(service.id, 'abababab', csv_param.get('is_csv')),
-        call(service.id, 'cdcdcdcd', csv_param.get('is_csv'))
+        call(str(service.id), 'abababab', csv_param.get('is_csv')),
+        call(str(service.id), 'cdcdcdcd', csv_param.get('is_csv'))
     ]
 
     notification = Notification.query.one()