Cache template in redis

This commit changes the code in post notification endpoint to handle a
serialised template (ie a `dict`) rather than a database object.

This is the first step towards being able to cache the template and not
hit the database on every request.

There should be no functional changes here, it’s just refactoring.

There are some changes to the tests where the signature of functions
has changed.

Importing of the template schema has to be done at a function level,
otherwise Marshmallow gets weird.

app/__init__.py

@@ -1,23 +1,20 @@
-import time
 import os
 import random
 import string
 import uuid
 
-from celery import current_task
-from flask import _request_ctx_stack, request, g, jsonify, make_response, current_app, has_request_context
+from flask import _request_ctx_stack, request, g, jsonify, make_response
 from flask_sqlalchemy import SQLAlchemy as _SQLAlchemy
 from flask_marshmallow import Marshmallow
 from flask_migrate import Migrate
 from gds_metrics import GDSMetrics
-from gds_metrics.metrics import Gauge, Histogram
 from time import monotonic
 from notifications_utils.clients.zendesk.zendesk_client import ZendeskClient
 from notifications_utils.clients.statsd.statsd_client import StatsdClient
+from notifications_utils.clients.redis import RequestCache
 from notifications_utils.clients.redis.redis_client import RedisClient
 from notifications_utils.clients.encryption.encryption_client import Encryption
 from notifications_utils import logging, request_helper
-from sqlalchemy import event
 from werkzeug.exceptions import HTTPException as WerkzeugHTTPException
 from werkzeug.local import LocalProxy
 
@@ -59,6 +56,7 @@ encryption = Encryption()
 zendesk_client = ZendeskClient()
 statsd_client = StatsdClient()
 redis_store = RedisClient()
+request_cache = RequestCache(redis_store)
 performance_platform_client = PerformancePlatformClient()
 document_download_client = DocumentDownloadClient()
 metrics = GDSMetrics()
@@ -68,11 +66,6 @@ clients = Clients()
 api_user = LocalProxy(lambda: _request_ctx_stack.top.api_user)
 authenticated_service = LocalProxy(lambda: _request_ctx_stack.top.authenticated_service)
 
-CONCURRENT_REQUESTS = Gauge(
-    'concurrent_web_request_count',
-    'How many concurrent requests are currently being served',
-)
-
 
 def create_app(application):
     from app.config import configs
@@ -117,9 +110,6 @@ def create_app(application):
     from app.commands import setup_commands
     setup_commands(application)
 
-    # set up sqlalchemy events
-    setup_sqlalchemy_events(application)
-
     return application
 
 
@@ -267,22 +257,17 @@ def register_v2_blueprints(application):
 
 
 def init_app(app):
-
     @app.before_request
     def record_user_agent():
         statsd_client.incr("user-agent.{}".format(process_user_agent(request.headers.get('User-Agent', None))))
 
     @app.before_request
     def record_request_details():
-        CONCURRENT_REQUESTS.inc()
-
         g.start = monotonic()
         g.endpoint = request.endpoint
 
     @app.after_request
     def after_request(response):
-        CONCURRENT_REQUESTS.dec()
-
         response.headers.add('Access-Control-Allow-Origin', '*')
         response.headers.add('Access-Control-Allow-Headers', 'Content-Type,Authorization')
         response.headers.add('Access-Control-Allow-Methods', 'GET,PUT,POST,DELETE')
@@ -328,83 +313,3 @@ def process_user_agent(user_agent_string):
         return "non-notify-user-agent"
     else:
         return "unknown"
-
-
-def setup_sqlalchemy_events(app):
-
-    TOTAL_DB_CONNECTIONS = Gauge(
-        'db_connection_total_connected',
-        'How many db connections are currently held (potentially idle) by the server',
-    )
-
-    TOTAL_CHECKED_OUT_DB_CONNECTIONS = Gauge(
-        'db_connection_total_checked_out',
-        'How many db connections are currently checked out by web requests',
-    )
-
-    DB_CONNECTION_OPEN_DURATION_SECONDS = Histogram(
-        'db_connection_open_duration_seconds',
-        'How long db connections are held open for in seconds',
-        ['method', 'host', 'path']
-    )
-
-    # need this or db.engine isn't accessible
-    with app.app_context():
-        @event.listens_for(db.engine, 'connect')
-        def connect(dbapi_connection, connection_record):
-            # connection first opened with db
-            TOTAL_DB_CONNECTIONS.inc()
-
-        @event.listens_for(db.engine, 'close')
-        def close(dbapi_connection, connection_record):
-            # connection closed (probably only happens with overflow connections)
-            TOTAL_DB_CONNECTIONS.dec()
-
-        @event.listens_for(db.engine, 'checkout')
-        def checkout(dbapi_connection, connection_record, connection_proxy):
-            # connection given to a web worker
-            TOTAL_CHECKED_OUT_DB_CONNECTIONS.inc()
-
-            # this will overwrite any previous checkout_at timestamp
-            connection_record.info['checkout_at'] = time.monotonic()
-
-            # checkin runs after the request is already torn down, therefore we add the request_data onto the
-            # connection_record as otherwise it won't have that information when checkin actually runs.
-            # Note: this is not a problem for checkouts as the checkout always happens within a web request or task
-
-            # web requests
-            if has_request_context():
-                connection_record.info['request_data'] = {
-                    'method': request.method,
-                    'host': request.host,
-                    'url_rule': request.url_rule.rule if request.url_rule else 'No endpoint'
-                }
-            # celery apps
-            elif current_task:
-                connection_record.info['request_data'] = {
-                    'method': 'celery',
-                    'host': current_app.config['NOTIFY_APP_NAME'],  # worker name
-                    'url_rule': current_task.name,  # task name
-                }
-            # anything else. migrations possibly.
-            else:
-                current_app.logger.warning('Checked out sqlalchemy connection from outside of request/task')
-                connection_record.info['request_data'] = {
-                    'method': 'unknown',
-                    'host': 'unknown',
-                    'url_rule': 'unknown',
-                }
-
-        @event.listens_for(db.engine, 'checkin')
-        def checkin(dbapi_connection, connection_record):
-            # connection returned by a web worker
-            TOTAL_CHECKED_OUT_DB_CONNECTIONS.dec()
-
-            # duration that connection was held by a single web request
-            duration = time.monotonic() - connection_record.info['checkout_at']
-
-            DB_CONNECTION_OPEN_DURATION_SECONDS.labels(
-                connection_record.info['request_data']['method'],
-                connection_record.info['request_data']['host'],
-                connection_record.info['request_data']['url_rule']
-            ).observe(duration)

app/authentication/auth.py

@@ -6,18 +6,12 @@ from notifications_python_client.errors import (
 from notifications_utils import request_helper
 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
 
 
 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
 
-AUTH_DB_CONNECTION_DURATION_SECONDS = Histogram(
-    'auth_db_connection_duration_seconds',
-    'Time taken to get DB connection and fetch service from database',
-)
-
 
 class AuthError(Exception):
     def __init__(self, message, code, service_id=None, api_key_id=None):
@@ -93,8 +87,7 @@ def requires_auth():
     issuer = __get_token_issuer(auth_token)  # ie the `iss` claim which should be a service ID
 
     try:
-        with AUTH_DB_CONNECTION_DURATION_SECONDS.time():
-            service = dao_fetch_service_by_id_with_api_keys(issuer)
+        service = dao_fetch_service_by_id_with_api_keys(issuer)
     except DataError:
         raise AuthError("Invalid token: service id is not the right data type", 403)
     except NoResultFound:

app/celery/celery.py

@@ -1,6 +1,5 @@
 import time
 
-from gds_metrics.metrics import Histogram
 from celery import Celery, Task
 from celery.signals import worker_process_shutdown
 from flask import g, request
@@ -20,12 +19,6 @@ def log_on_worker_shutdown(sender, signal, pid, exitcode, **kwargs):
 
 
 def make_task(app):
-    SQS_APPLY_ASYNC_DURATION_SECONDS = Histogram(
-        'sqs_apply_async_duration_seconds',
-        'Time taken to put task on queue',
-        ['task_name']
-    )
-
     class NotifyTask(Task):
         abstract = True
         start = None
@@ -59,8 +52,7 @@ def make_task(app):
             if has_request_context() and hasattr(request, 'request_id'):
                 kwargs['request_id'] = request.request_id
 
-            with SQS_APPLY_ASYNC_DURATION_SECONDS.labels(self.name).time():
-                return super().apply_async(args, kwargs, task_id, producer, link, link_error, **options)
+            return super().apply_async(args, kwargs, task_id, producer, link, link_error, **options)
 
     return NotifyTask
 

app/celery/letters_pdf_tasks.py

@@ -42,7 +42,6 @@ from app.models import (
     NOTIFICATION_TECHNICAL_FAILURE,
     NOTIFICATION_VALIDATION_FAILED,
     NOTIFICATION_VIRUS_SCAN_FAILED,
-    LETTER_TYPE
 )
 from app.cronitor import cronitor
 
@@ -217,7 +216,7 @@ def group_letters(letter_pdfs):
 def sanitise_letter(self, filename):
     try:
         reference = get_reference_from_filename(filename)
-        notification = dao_get_notification_by_reference(reference=reference, notification_type=LETTER_TYPE)
+        notification = dao_get_notification_by_reference(reference)
 
         current_app.logger.info('Notification ID {} Virus scan passed: {}'.format(notification.id, filename))
 
@@ -353,7 +352,7 @@ def _move_invalid_letter_and_update_status(
 def process_virus_scan_failed(filename):
     move_failed_pdf(filename, ScanErrorType.FAILURE)
     reference = get_reference_from_filename(filename)
-    notification = dao_get_notification_by_reference(reference=reference, notification_type=LETTER_TYPE)
+    notification = dao_get_notification_by_reference(reference)
     updated_count = update_letter_pdf_status(reference, NOTIFICATION_VIRUS_SCAN_FAILED, billable_units=0)
 
     if updated_count != 1:
@@ -372,7 +371,7 @@ def process_virus_scan_failed(filename):
 def process_virus_scan_error(filename):
     move_failed_pdf(filename, ScanErrorType.ERROR)
     reference = get_reference_from_filename(filename)
-    notification = dao_get_notification_by_reference(reference=reference, notification_type=LETTER_TYPE)
+    notification = dao_get_notification_by_reference(reference)
     updated_count = update_letter_pdf_status(reference, NOTIFICATION_TECHNICAL_FAILURE, billable_units=0)
 
     if updated_count != 1:

app/celery/process_ses_receipts_tasks.py

@@ -10,7 +10,7 @@ from app import notify_celery, statsd_client
 from app.config import QueueNames
 from app.clients.email.aws_ses import get_aws_responses
 from app.dao import notifications_dao
-from app.models import NOTIFICATION_SENDING, NOTIFICATION_PENDING, EMAIL_TYPE
+from app.models import NOTIFICATION_SENDING, NOTIFICATION_PENDING
 
 from app.notifications.notifications_ses_callback import (
     determine_notification_bounce_type,
@@ -39,9 +39,7 @@ def process_ses_results(self, response):
         reference = ses_message['mail']['messageId']
 
         try:
-            notification = notifications_dao.dao_get_notification_or_history_by_reference(
-                reference=reference, notification_type=EMAIL_TYPE
-            )
+            notification = notifications_dao.dao_get_notification_or_history_by_reference(reference=reference)
         except NoResultFound:
             message_time = iso8601.parse_date(ses_message['mail']['timestamp']).replace(tzinfo=None)
             if datetime.utcnow() - message_time < timedelta(minutes=5):

app/celery/tasks.py

@@ -536,7 +536,7 @@ def update_letter_notification(filename, temporary_failures, update):
 
 
 def check_billable_units(notification_update):
-    notification = dao_get_notification_or_history_by_reference(notification_update.reference, LETTER_TYPE)
+    notification = dao_get_notification_or_history_by_reference(notification_update.reference)
 
     if int(notification_update.page_count) != notification.billable_units:
         msg = 'Notification with id {} has {} billable_units but DVLA says page count is {}'.format(

app/dao/notifications_dao.py

@@ -650,29 +650,33 @@ def dao_get_notifications_by_recipient_or_reference(
 
 
 @statsd(namespace="dao")
-def dao_get_notification_by_reference(reference, notification_type):
+def dao_get_notification_by_reference(reference):
     return Notification.query.filter(
-        Notification.reference == reference,
-        Notification.notification_type == notification_type
+        Notification.reference == reference
     ).one()
 
 
 @statsd(namespace="dao")
-def dao_get_notification_or_history_by_reference(reference, notification_type):
+def dao_get_notification_or_history_by_reference(reference):
     try:
         # This try except is necessary because in test keys and research mode does not create notification history.
         # Otherwise we could just search for the NotificationHistory object
         return Notification.query.filter(
-            Notification.reference == reference,
-            Notification.notification_type == notification_type
+            Notification.reference == reference
         ).one()
     except NoResultFound:
         return NotificationHistory.query.filter(
-            NotificationHistory.reference == reference,
-            NotificationHistory.notification_type == notification_type
+            NotificationHistory.reference == reference
         ).one()
 
 
+@statsd(namespace="dao")
+def dao_get_notifications_by_references(references):
+    return Notification.query.filter(
+        Notification.reference.in_(references)
+    ).all()
+
+
 @statsd(namespace="dao")
 def dao_created_scheduled_notification(scheduled_notification):
     db.session.add(scheduled_notification)

app/notifications/notifications_ses_callback.py

@@ -5,7 +5,7 @@ from app.dao.notifications_dao import dao_get_notification_or_history_by_referen
 from app.dao.service_callback_api_dao import (
     get_service_delivery_status_callback_api_for_service, get_service_complaint_callback_api_for_service
 )
-from app.models import Complaint, EMAIL_TYPE
+from app.models import Complaint
 from app.celery.service_callback_tasks import (
     send_delivery_status_to_service,
     send_complaint_to_service,
@@ -33,7 +33,7 @@ def handle_complaint(ses_message):
     except KeyError as e:
         current_app.logger.exception("Complaint from SES failed to get reference from message", e)
         return
-    notification = dao_get_notification_or_history_by_reference(reference, EMAIL_TYPE)
+    notification = dao_get_notification_or_history_by_reference(reference)
     ses_complaint = ses_message.get('complaint', None)
 
     complaint = Complaint(

app/notifications/process_letter_notifications.py

@@ -5,14 +5,22 @@ from app.models import LETTER_TYPE
 from app.notifications.process_notifications import persist_notification
 
 
-def create_letter_notification(letter_data, template, api_key, status, reply_to_text=None, billable_units=None):
+def create_letter_notification(
+    letter_data,
+    template,
+    service,
+    api_key,
+    status,
+    reply_to_text=None,
+    billable_units=None,
+):
     notification = persist_notification(
         template_id=template.id,
-        template_version=template.version,
-        template_postage=template.postage,
+        template_version=template._template['version'],
+        template_postage=template._template['postage'],
         # we only accept addresses_with_underscores from the API (from CSV we also accept dashes, spaces etc)
         recipient=PostalAddress.from_personalisation(letter_data['personalisation']).normalised,
-        service=template.service,
+        service=service,
         personalisation=letter_data['personalisation'],
         notification_type=LETTER_TYPE,
         api_key_id=api_key.id,

app/notifications/process_notifications.py

@@ -9,6 +9,11 @@ from notifications_utils.recipients import (
     validate_and_format_phone_number,
     format_email_address
 )
+from notifications_utils.template import (
+    PlainTextEmailTemplate,
+    SMSMessageTemplate,
+    LetterPrintTemplate,
+)
 from notifications_utils.timezones import convert_bst_to_utc
 
 from app import redis_store
@@ -34,17 +39,18 @@ from app.dao.notifications_dao import (
 from app.v2.errors import BadRequestError
 
 
-from gds_metrics import Histogram
+def create_content_for_notification(template_dict, personalisation):
+    if template_dict['template_type'] == EMAIL_TYPE:
+        template_object = PlainTextEmailTemplate(template_dict, personalisation)
+    if template_dict['template_type'] == SMS_TYPE:
+        template_object = SMSMessageTemplate(template_dict, personalisation)
+    if template_dict['template_type'] == LETTER_TYPE:
+        template_object = LetterPrintTemplate(
+            template_dict,
+            personalisation,
+            contact_block=template_dict['reply_to_text'],
+        )
 
-
-REDIS_GET_AND_INCR_DAILY_LIMIT_DURATION_SECONDS = Histogram(
-    'redis_get_and_incr_daily_limit_duration_seconds',
-    'Time taken to get and possibly incremement the daily limit cache key',
-)
-
-
-def create_content_for_notification(template, personalisation):
-    template_object = template._as_utils_template_with_personalisation(personalisation)
     check_placeholders(template_object)
 
     return template_object
@@ -124,9 +130,8 @@ def persist_notification(
     if not simulated:
         dao_create_notification(notification)
         if key_type != KEY_TYPE_TEST:
-            with REDIS_GET_AND_INCR_DAILY_LIMIT_DURATION_SECONDS.time():
-                if redis_store.get(redis.daily_limit_cache_key(service.id)):
-                    redis_store.incr(redis.daily_limit_cache_key(service.id))
+            if redis_store.get(redis.daily_limit_cache_key(service.id)):
+                redis_store.incr(redis.daily_limit_cache_key(service.id))
 
         current_app.logger.info(
             "{} {} created at {}".format(notification_type, notification_id, notification_created_at)

app/notifications/rest.py

@@ -100,12 +100,13 @@ def send_notification(notification_type):
 
     check_rate_limiting(authenticated_service, api_user)
 
-    template, template_with_content = validate_template(
+    template_with_content = validate_template(
         template_id=notification_form['template'],
         personalisation=notification_form.get('personalisation', {}),
         service=authenticated_service,
         notification_type=notification_type
     )
+    template_dict = template_with_content._template
 
     _service_allowed_to_send_to(notification_form, authenticated_service)
     if not service_has_permission(notification_type, authenticated_service.permissions):
@@ -118,9 +119,9 @@ def send_notification(notification_type):
         _service_can_send_internationally(authenticated_service, notification_form['to'])
     # Do not persist or send notification to the queue if it is a simulated recipient
     simulated = simulated_recipient(notification_form['to'], notification_type)
-    notification_model = persist_notification(template_id=template.id,
-                                              template_version=template.version,
-                                              template_postage=template.postage,
+    notification_model = persist_notification(template_id=template_dict['id'],
+                                              template_version=template_dict['version'],
+                                              template_postage=template_dict['postage'],
                                               recipient=request.get_json()['to'],
                                               service=authenticated_service,
                                               personalisation=notification_form.get('personalisation', None),
@@ -128,16 +129,16 @@ def send_notification(notification_type):
                                               api_key_id=api_user.id,
                                               key_type=api_user.key_type,
                                               simulated=simulated,
-                                              reply_to_text=template.get_reply_to_text()
+                                              reply_to_text=template_dict['reply_to_text']
                                               )
     if not simulated:
-        queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None
+        queue_name = QueueNames.PRIORITY if template_dict['process_type'] == PRIORITY else None
         send_notification_to_queue(notification=notification_model,
                                    research_mode=authenticated_service.research_mode,
                                    queue=queue_name)
     else:
         current_app.logger.debug("POST simulated notification for id: {}".format(notification_model.id))
-    notification_form.update({"template_version": template.version})
+    notification_form.update({"template_version": template_dict['version']})
 
     return jsonify(
         data=get_notification_return_data(

app/notifications/validators.py

@@ -16,30 +16,21 @@ from app.models import (
 )
 from app.service.utils import service_allowed_to_send_to
 from app.v2.errors import TooManyRequestsError, BadRequestError, RateLimitError
-from app import redis_store
+from app import redis_store, request_cache
 from app.notifications.process_notifications import create_content_for_notification
 from app.utils import get_public_notify_type_text
 from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id
 from app.dao.service_letter_contact_dao import dao_get_letter_contact_by_id
 
-from gds_metrics.metrics import Histogram
-
-
-REDIS_EXCEEDED_RATE_LIMIT_DURATION_SECONDS = Histogram(
-    'redis_exceeded_rate_limit_duration_seconds',
-    'Time taken to check rate limit',
-)
-
 
 def check_service_over_api_rate_limit(service, api_key):
     if current_app.config['API_RATE_LIMIT_ENABLED'] and current_app.config['REDIS_ENABLED']:
         cache_key = rate_limit_cache_key(service.id, api_key.key_type)
         rate_limit = service.rate_limit
         interval = 60
-        with REDIS_EXCEEDED_RATE_LIMIT_DURATION_SECONDS.time():
-            if redis_store.exceeded_rate_limit(cache_key, rate_limit, interval):
-                current_app.logger.info("service {} has been rate limited for throughput".format(service.id))
-                raise RateLimitError(rate_limit, interval, api_key.key_type)
+        if redis_store.exceeded_rate_limit(cache_key, rate_limit, interval):
+            current_app.logger.info("service {} has been rate limited for throughput".format(service.id))
+            raise RateLimitError(rate_limit, interval, api_key.key_type)
 
 
 def check_service_over_daily_message_limit(key_type, service):
@@ -71,7 +62,7 @@ def check_template_is_for_notification_type(notification_type, template_type):
 
 
 def check_template_is_active(template):
-    if template.archived:
+    if template['archived']:
         raise BadRequestError(fields=[{'template': 'Template has been deleted'}],
                               message="Template has been deleted")
 
@@ -147,18 +138,26 @@ def check_notification_content_is_not_empty(template_with_content):
         raise BadRequestError(message=message)
 
 
-def validate_template(template_id, personalisation, service, notification_type):
+@request_cache.set('template-{template_id}-version-None')
+def get_template_dict(template_id, service_id):
+    from app.schemas import template_schema
     try:
-        template = templates_dao.dao_get_template_by_id_and_service_id(
+        fetched_template = templates_dao.dao_get_template_by_id_and_service_id(
             template_id=template_id,
-            service_id=service.id
+            service_id=service_id
         )
     except NoResultFound:
         message = 'Template not found'
         raise BadRequestError(message=message,
                               fields=[{'template': message}])
 
-    check_template_is_for_notification_type(notification_type, template.template_type)
+    return template_schema.dump(fetched_template).data
+
+
+def validate_template(template_id, personalisation, service, notification_type):
+    template = get_template_dict(template_id, service.id)
+
+    check_template_is_for_notification_type(notification_type, template['template_type'])
     check_template_is_active(template)
 
     template_with_content = create_content_for_notification(template, personalisation)
@@ -167,7 +166,7 @@ def validate_template(template_id, personalisation, service, notification_type):
 
     check_content_char_count(template_with_content)
 
-    return template, template_with_content
+    return template_with_content
 
 
 def check_reply_to(service_id, reply_to_id, type_):

app/schemas.py

@@ -2,6 +2,7 @@ from datetime import (
     datetime,
     date,
     timedelta)
+from uuid import UUID
 from flask_marshmallow.fields import fields
 from marshmallow import (
     post_load,
@@ -334,6 +335,16 @@ class TemplateSchema(BaseTemplateSchema):
             if not subject or subject.strip() == '':
                 raise ValidationError('Invalid template subject', 'subject')
 
+    @post_dump()
+    def __post_dump(self, data):
+        for field in (
+            'service',
+            'created_by',
+            'template_redacted',
+        ):
+            if isinstance(data[field], UUID):
+                data[field] = str(data[field])
+
 
 class TemplateHistorySchema(BaseSchema):
 

app/v2/notifications/post_notifications.py

@@ -7,7 +7,6 @@ from boto.exception import SQSError
 from flask import request, jsonify, current_app, abort
 from notifications_utils.postal_address import PostalAddress
 from notifications_utils.recipients import try_validate_and_format_phone_number
-from gds_metrics import Histogram
 
 from app import (
     api_user,
@@ -43,6 +42,7 @@ from app.notifications.process_letter_notifications import (
     create_letter_notification
 )
 from app.notifications.process_notifications import (
+    create_content_for_notification,
     persist_notification,
     persist_scheduled_notification,
     send_notification_to_queue,
@@ -75,14 +75,9 @@ from app.v2.notifications.notification_schemas import (
 from app.v2.utils import get_valid_json
 
 
-POST_NOTIFICATION_JSON_PARSE_DURATION_SECONDS = Histogram(
-    'post_notification_json_parse_duration_seconds',
-    'Time taken to parse and validate post request json',
-)
-
-
 @v2_notification_blueprint.route('/{}'.format(LETTER_TYPE), methods=['POST'])
 def post_precompiled_letter_notification():
+    from app.schemas import template_schema
     request_json = get_valid_json()
     if 'content' not in (request_json or {}):
         return post_notification(LETTER_TYPE)
@@ -95,6 +90,9 @@ def post_precompiled_letter_notification():
     check_rate_limiting(authenticated_service, api_user)
 
     template = get_precompiled_letter_template(authenticated_service.id)
+    template = create_content_for_notification(
+        template_schema.dump(template).data, {}
+    )
 
     # For precompiled letters the to field will be set to Provided as PDF until the validation passes,
     # then the address of the letter will be set as the to field
@@ -102,13 +100,12 @@ def post_precompiled_letter_notification():
         'address_line_1': 'Provided as PDF'
     }
 
-    reply_to = get_reply_to_text(LETTER_TYPE, form, template)
-
     notification = process_letter_notification(
         letter_data=form,
         api_key=api_user,
         template=template,
-        reply_to_text=reply_to,
+        service=authenticated_service,
+        reply_to_text=template._template['reply_to_text'],
         precompiled=True
     )
 
@@ -123,17 +120,16 @@ def post_precompiled_letter_notification():
 
 @v2_notification_blueprint.route('/<notification_type>', methods=['POST'])
 def post_notification(notification_type):
-    with POST_NOTIFICATION_JSON_PARSE_DURATION_SECONDS.time():
-        request_json = get_valid_json()
+    request_json = get_valid_json()
 
-        if notification_type == EMAIL_TYPE:
-            form = validate(request_json, post_email_request)
-        elif notification_type == SMS_TYPE:
-            form = validate(request_json, post_sms_request)
-        elif notification_type == LETTER_TYPE:
-            form = validate(request_json, post_letter_request)
-        else:
-            abort(404)
+    if notification_type == EMAIL_TYPE:
+        form = validate(request_json, post_email_request)
+    elif notification_type == SMS_TYPE:
+        form = validate(request_json, post_sms_request)
+    elif notification_type == LETTER_TYPE:
+        form = validate(request_json, post_letter_request)
+    else:
+        abort(404)
 
     check_service_has_permission(notification_type, authenticated_service.permissions)
 
@@ -143,20 +139,21 @@ def post_notification(notification_type):
 
     check_rate_limiting(authenticated_service, api_user)
 
-    template, template_with_content = validate_template(
+    template_with_content = validate_template(
         form['template_id'],
         form.get('personalisation', {}),
         authenticated_service,
         notification_type,
     )
 
-    reply_to = get_reply_to_text(notification_type, form, template)
+    reply_to = get_reply_to_text(notification_type, form, template_with_content)
 
     if notification_type == LETTER_TYPE:
         notification = process_letter_notification(
             letter_data=form,
             api_key=api_user,
-            template=template,
+            template=template_with_content,
+            service=authenticated_service,
             reply_to_text=reply_to
         )
     else:
@@ -164,11 +161,12 @@ def post_notification(notification_type):
             form=form,
             notification_type=notification_type,
             api_key=api_user,
-            template=template,
+            template=template_with_content,
             service=authenticated_service,
             reply_to_text=reply_to
         )
 
+        # Think this is redundant
         template_with_content.values = notification.personalisation
 
     if notification_type == SMS_TYPE:
@@ -245,7 +243,7 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ
     notification = persist_notification(
         notification_id=notification_id,
         template_id=template.id,
-        template_version=template.version,
+        template_version=template._template['version'],
         recipient=form_send_to,
         service=service,
         personalisation=personalisation,
@@ -263,7 +261,7 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ
         persist_scheduled_notification(notification.id, form["scheduled_for"])
     else:
         if not simulated:
-            queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None
+            queue_name = QueueNames.PRIORITY if template._template['process_type'] == PRIORITY else None
             send_notification_to_queue(
                 notification=notification,
                 research_mode=service.research_mode,
@@ -290,7 +288,7 @@ def save_email_to_queue(
     data = {
         "id": notification_id,
         "template_id": str(template.id),
-        "template_version": template.version,
+        "template_version": template._template['version'],
         "to": form['email_address'],
         "service_id": str(service_id),
         "personalisation": personalisation,
@@ -341,7 +339,7 @@ def process_document_uploads(personalisation_data, service, simulated=False):
     return personalisation_data, len(file_keys)
 
 
-def process_letter_notification(*, letter_data, api_key, template, reply_to_text, precompiled=False):
+def process_letter_notification(*, letter_data, api_key, template, service, reply_to_text, precompiled=False):
     if api_key.key_type == KEY_TYPE_TEAM:
         raise BadRequestError(message='Cannot send letters with a team api key', status_code=403)
 
@@ -352,6 +350,7 @@ def process_letter_notification(*, letter_data, api_key, template, reply_to_text
         return process_precompiled_letter_notifications(letter_data=letter_data,
                                                         api_key=api_key,
                                                         template=template,
+                                                        service=service,
                                                         reply_to_text=reply_to_text)
 
     address = PostalAddress.from_personalisation(
@@ -386,6 +385,7 @@ def process_letter_notification(*, letter_data, api_key, template, reply_to_text
 
     notification = create_letter_notification(letter_data=letter_data,
                                               template=template,
+                                              service=service,
                                               api_key=api_key,
                                               status=status,
                                               reply_to_text=reply_to_text)
@@ -407,7 +407,7 @@ def process_letter_notification(*, letter_data, api_key, template, reply_to_text
     return notification
 
 
-def process_precompiled_letter_notifications(*, letter_data, api_key, template, reply_to_text):
+def process_precompiled_letter_notifications(*, letter_data, api_key, template, service, reply_to_text):
     try:
         status = NOTIFICATION_PENDING_VIRUS_CHECK
         letter_content = base64.b64decode(letter_data['content'])
@@ -416,6 +416,7 @@ def process_precompiled_letter_notifications(*, letter_data, api_key, template,
 
     notification = create_letter_notification(letter_data=letter_data,
                                               template=template,
+                                              service=service,
                                               api_key=api_key,
                                               status=status,
                                               reply_to_text=reply_to_text)
@@ -447,7 +448,7 @@ def get_reply_to_text(notification_type, form, template):
         service_email_reply_to_id = form.get("email_reply_to_id", None)
         reply_to = check_service_email_reply_to_id(
             str(authenticated_service.id), service_email_reply_to_id, notification_type
-        ) or template.get_reply_to_text()
+        ) or template._template['reply_to_text']
 
     elif notification_type == SMS_TYPE:
         service_sms_sender_id = form.get("sms_sender_id", None)
@@ -457,9 +458,9 @@ def get_reply_to_text(notification_type, form, template):
         if sms_sender_id:
             reply_to = try_validate_and_format_phone_number(sms_sender_id)
         else:
-            reply_to = template.get_reply_to_text()
+            reply_to = template._template['reply_to_text']
 
     elif notification_type == LETTER_TYPE:
-        reply_to = template.get_reply_to_text()
+        reply_to = template._template['reply_to_text']
 
     return reply_to

requirements-app.txt

@@ -28,6 +28,4 @@ awscli-cwlogs>=1.4,<1.5
 
 git+https://github.com/alphagov/notifications-utils.git@39.4.4#egg=notifications-utils==39.4.4
 
-# gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
-prometheus-client==0.7.1
 gds-metrics==0.2.0

requirements.txt

@@ -30,8 +30,6 @@ awscli-cwlogs>=1.4,<1.5
 
 git+https://github.com/alphagov/notifications-utils.git@39.4.4#egg=notifications-utils==39.4.4
 
-# gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
-prometheus-client==0.7.1
 gds-metrics==0.2.0
 
 ## The following requirements were added by pip freeze:
@@ -68,6 +66,7 @@ mistune==0.8.4
 monotonic==1.5
 orderedset==2.0.1
 phonenumbers==8.11.2
+prometheus-client==0.2.0
 pyasn1==0.4.8
 pycparser==2.20
 PyPDF2==1.26.0

tests/app/dao/notification_dao/test_notification_dao.py

@@ -28,6 +28,7 @@ from app.dao.notifications_dao import (
     update_notification_status_by_id,
     update_notification_status_by_reference,
     dao_get_notification_by_reference,
+    dao_get_notifications_by_references,
     dao_get_notification_or_history_by_reference,
     notifications_not_yet_sent,
 )
@@ -1612,7 +1613,7 @@ def test_dao_update_notifications_by_reference_updates_history_when_one_of_two_n
 
 def test_dao_get_notification_by_reference_with_one_match_returns_notification(sample_letter_template, notify_db):
     create_notification(template=sample_letter_template, reference='REF1')
-    notification = dao_get_notification_by_reference('REF1', 'letter')
+    notification = dao_get_notification_by_reference('REF1')
 
     assert notification.reference == 'REF1'
 
@@ -1622,25 +1623,30 @@ def test_dao_get_notification_by_reference_with_multiple_matches_raises_error(sa
     create_notification(template=sample_letter_template, reference='REF1')
 
     with pytest.raises(SQLAlchemyError):
-        dao_get_notification_by_reference('REF1', 'letter')
+        dao_get_notification_by_reference('REF1')
 
 
 def test_dao_get_notification_by_reference_with_no_matches_raises_error(notify_db):
     with pytest.raises(SQLAlchemyError):
-        dao_get_notification_by_reference('REF1', 'email')
+        dao_get_notification_by_reference('REF1')
 
 
-def test_dao_get_notification_by_reference_with_no_matches_for_type_raises_error(sample_email_template):
-    create_notification(template=sample_email_template, reference='REF1')
-    with pytest.raises(SQLAlchemyError):
-        dao_get_notification_by_reference('REF1', 'letter')
+def test_dao_get_notifications_by_references(sample_template):
+    create_notification(template=sample_template, reference='noref')
+    notification_1 = create_notification(template=sample_template, reference='ref')
+    notification_2 = create_notification(template=sample_template, reference='ref')
+
+    notifications = dao_get_notifications_by_references(['ref'])
+    assert len(notifications) == 2
+    assert notifications[0].id in [notification_1.id, notification_2.id]
+    assert notifications[1].id in [notification_1.id, notification_2.id]
 
 
 def test_dao_get_notification_or_history_by_reference_with_one_match_returns_notification(
         sample_letter_template
 ):
     create_notification(template=sample_letter_template, reference='REF1')
-    notification = dao_get_notification_or_history_by_reference('REF1', 'letter')
+    notification = dao_get_notification_or_history_by_reference('REF1')
 
     assert notification.reference == 'REF1'
 
@@ -1652,18 +1658,12 @@ def test_dao_get_notification_or_history_by_reference_with_multiple_matches_rais
     create_notification(template=sample_letter_template, reference='REF1')
 
     with pytest.raises(SQLAlchemyError):
-        dao_get_notification_or_history_by_reference('REF1', 'letter')
+        dao_get_notification_or_history_by_reference('REF1')
 
 
-def test_dao_get_notification_or_history_by_reference_with_no_matches_raises_error(sample_letter_template):
-    create_notification(template=sample_letter_template, reference='REF1')
+def test_dao_get_notification_or_history_by_reference_with_no_matches_raises_error(notify_db):
     with pytest.raises(SQLAlchemyError):
-        dao_get_notification_or_history_by_reference('REF1', 'email')
-
-
-def test_dao_get_notification_or_history_by_reference_with_no_matches_for_type_raises_error(notify_db):
-    with pytest.raises(SQLAlchemyError):
-        dao_get_notification_or_history_by_reference('REF1', 'email')
+        dao_get_notification_or_history_by_reference('REF1')
 
 
 @pytest.mark.parametrize("notification_type",

tests/app/notifications/test_process_letter_notifications.py

@@ -2,6 +2,8 @@ from app.models import LETTER_TYPE
 from app.models import Notification
 from app.models import NOTIFICATION_CREATED
 from app.notifications.process_letter_notifications import create_letter_notification
+from app.notifications.process_notifications import create_content_for_notification
+from app.notifications.validators import get_template_dict
 
 
 def test_create_letter_notification_creates_notification(sample_letter_template, sample_api_key):
@@ -13,7 +15,17 @@ def test_create_letter_notification_creates_notification(sample_letter_template,
         }
     }
 
-    notification = create_letter_notification(data, sample_letter_template, sample_api_key, NOTIFICATION_CREATED)
+    template = create_content_for_notification(get_template_dict(
+        sample_letter_template.id, sample_letter_template.service_id
+    ), {})
+
+    notification = create_letter_notification(
+        data,
+        template,
+        sample_letter_template.service,
+        sample_api_key,
+        NOTIFICATION_CREATED,
+    )
 
     assert notification == Notification.query.one()
     assert notification.job is None
@@ -38,7 +50,17 @@ def test_create_letter_notification_sets_reference(sample_letter_template, sampl
         'reference': 'foo'
     }
 
-    notification = create_letter_notification(data, sample_letter_template, sample_api_key, NOTIFICATION_CREATED)
+    template = create_content_for_notification(get_template_dict(
+        sample_letter_template.id, sample_letter_template.service_id
+    ), {})
+
+    notification = create_letter_notification(
+        data,
+        template,
+        sample_letter_template.service,
+        sample_api_key,
+        NOTIFICATION_CREATED,
+    )
 
     assert notification.client_reference == 'foo'
 
@@ -52,7 +74,17 @@ def test_create_letter_notification_sets_billable_units(sample_letter_template,
         },
     }
 
-    notification = create_letter_notification(data, sample_letter_template, sample_api_key, NOTIFICATION_CREATED,
-                                              billable_units=3)
+    template = create_content_for_notification(get_template_dict(
+        sample_letter_template.id, sample_letter_template.service_id
+    ), {})
+
+    notification = create_letter_notification(
+        data,
+        template,
+        sample_letter_template.service,
+        sample_api_key,
+        NOTIFICATION_CREATED,
+        billable_units=3,
+    )
 
     assert notification.billable_units == 3

tests/app/notifications/test_process_notification.py

@@ -21,6 +21,7 @@ from app.notifications.process_notifications import (
     send_notification_to_queue,
     simulated_recipient
 )
+from app.notifications.validators import get_template_dict
 from notifications_utils.recipients import validate_and_format_phone_number, validate_and_format_email_address
 from app.v2.errors import BadRequestError
 from tests.app.db import create_service, create_template
@@ -28,26 +29,30 @@ from tests.app.db import create_service, create_template
 
 def test_create_content_for_notification_passes(sample_email_template):
     template = Template.query.get(sample_email_template.id)
-    content = create_content_for_notification(template, None)
+    template_dict = get_template_dict(template.id, template.service_id)
+    content = create_content_for_notification(template_dict, None)
     assert str(content) == template.content + '\n'
 
 
 def test_create_content_for_notification_with_placeholders_passes(sample_template_with_placeholders):
     template = Template.query.get(sample_template_with_placeholders.id)
-    content = create_content_for_notification(template, {'name': 'Bobby'})
+    template_dict = get_template_dict(template.id, template.service_id)
+    content = create_content_for_notification(template_dict, {'name': 'Bobby'})
     assert content.content == template.content
     assert 'Bobby' in str(content)
 
 
 def test_create_content_for_notification_fails_with_missing_personalisation(sample_template_with_placeholders):
     template = Template.query.get(sample_template_with_placeholders.id)
+    template_dict = get_template_dict(template.id, template.service_id)
     with pytest.raises(BadRequestError):
-        create_content_for_notification(template, None)
+        create_content_for_notification(template_dict, None)
 
 
 def test_create_content_for_notification_allows_additional_personalisation(sample_template_with_placeholders):
     template = Template.query.get(sample_template_with_placeholders.id)
-    create_content_for_notification(template, {'name': 'Bobby', 'Additional placeholder': 'Data'})
+    template_dict = get_template_dict(template.id, template.service_id)
+    create_content_for_notification(template_dict, {'name': 'Bobby', 'Additional placeholder': 'Data'})
 
 
 @freeze_time("2016-01-01 11:09:00.061258")

tests/app/notifications/test_validators.py

@@ -19,6 +19,7 @@ from app.notifications.validators import (
     check_service_sms_sender_id,
     check_service_letter_contact_id,
     check_reply_to,
+    get_template_dict,
     service_can_send_to_recipient,
     validate_and_format_recipient,
     validate_template,
@@ -175,15 +176,17 @@ def test_check_template_is_for_notification_type_fails_when_template_type_does_n
 
 
 def test_check_template_is_active_passes(sample_template):
-    assert check_template_is_active(sample_template) is None
+    template_dict = get_template_dict(sample_template.id, sample_template.service_id)
+    assert check_template_is_active(template_dict) is None
 
 
 def test_check_template_is_active_fails(sample_template):
     sample_template.archived = True
     from app.dao.templates_dao import dao_update_template
     dao_update_template(sample_template)
+    template_dict = get_template_dict(sample_template.id, sample_template.service_id)
     with pytest.raises(BadRequestError) as e:
-        check_template_is_active(sample_template)
+        check_template_is_active(template_dict)
     assert e.value.status_code == 400
     assert e.value.message == 'Template has been deleted'
     assert e.value.fields == [{'template': 'Template has been deleted'}]
@@ -314,11 +317,11 @@ def test_check_content_char_count_passes_for_long_email_or_letter(sample_service
 
 def test_check_notification_content_is_not_empty_passes(notify_api, mocker, sample_service):
     template_id = create_template(sample_service, content="Content is not empty").id
-    template = templates_dao.dao_get_template_by_id_and_service_id(
+    template_dict = get_template_dict(
         template_id=template_id,
         service_id=sample_service.id
     )
-    template_with_content = create_content_for_notification(template, {})
+    template_with_content = create_content_for_notification(template_dict, {})
     assert check_notification_content_is_not_empty(template_with_content) is None
 
 
@@ -330,11 +333,11 @@ def test_check_notification_content_is_not_empty_fails(
     notify_api, mocker, sample_service, template_content, notification_values
 ):
     template_id = create_template(sample_service, content=template_content).id
-    template = templates_dao.dao_get_template_by_id_and_service_id(
+    template_dict = get_template_dict(
         template_id=template_id,
         service_id=sample_service.id
     )
-    template_with_content = create_content_for_notification(template, notification_values)
+    template_with_content = create_content_for_notification(template_dict, notification_values)
     with pytest.raises(BadRequestError) as e:
         check_notification_content_is_not_empty(template_with_content)
     assert e.value.status_code == 400
@@ -349,6 +352,10 @@ def test_validate_template(sample_service):
 
 def test_validate_template_calls_all_validators(mocker, fake_uuid, sample_service):
     template = create_template(sample_service, template_type="email")
+    template_dict = get_template_dict(
+        template_id=template.id,
+        service_id=sample_service.id
+    )
     mock_check_type = mocker.patch('app.notifications.validators.check_template_is_for_notification_type')
     mock_check_if_active = mocker.patch('app.notifications.validators.check_template_is_active')
     mock_create_conent = mocker.patch(
@@ -359,8 +366,8 @@ def test_validate_template_calls_all_validators(mocker, fake_uuid, sample_servic
     validate_template(template.id, {}, sample_service, "email")
 
     mock_check_type.assert_called_once_with("email", "email")
-    mock_check_if_active.assert_called_once_with(template)
-    mock_create_conent.assert_called_once_with(template, {})
+    mock_check_if_active.assert_called_once_with(template_dict)
+    mock_create_conent.assert_called_once_with(template_dict, {})
     mock_check_not_empty.assert_called_once_with("content")
     mock_check_message_is_too_long.assert_called_once_with("content")
 

tests/app/v2/test_errors.py

@@ -4,7 +4,7 @@ from sqlalchemy.exc import DataError
 
 
 @pytest.fixture(scope='function')
-def app_for_test():
+def app_for_test(mocker):
     import flask
     from flask import Blueprint
     from app.authentication.auth import AuthError