Update schemas for breaking changes in flask-marshmallow 0.12.0

See https://flask-marshmallow.readthedocs.io/en/latest/changelog.html#id2

app/__init__.py

@@ -1,20 +1,23 @@
+import time
 import os
 import random
 import string
 import uuid
 
-from flask import _request_ctx_stack, request, g, jsonify, make_response
+from celery import current_task
+from flask import _request_ctx_stack, request, g, jsonify, make_response, current_app, has_request_context
 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
 
@@ -56,7 +59,6 @@ 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()
@@ -66,6 +68,11 @@ 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
@@ -110,6 +117,9 @@ def create_app(application):
     from app.commands import setup_commands
     setup_commands(application)
 
+    # set up sqlalchemy events
+    setup_sqlalchemy_events(application)
+
     return application
 
 
@@ -257,17 +267,22 @@ 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')
@@ -313,3 +328,83 @@ 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,12 +6,18 @@ 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):
@@ -87,7 +93,8 @@ def requires_auth():
     issuer = __get_token_issuer(auth_token)  # ie the `iss` claim which should be a service ID
 
     try:
-        service = dao_fetch_service_by_id_with_api_keys(issuer)
+        with AUTH_DB_CONNECTION_DURATION_SECONDS.time():
+            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,5 +1,6 @@
 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
@@ -19,6 +20,12 @@ 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
@@ -52,7 +59,8 @@ def make_task(app):
             if has_request_context() and hasattr(request, 'request_id'):
                 kwargs['request_id'] = request.request_id
 
-            return super().apply_async(args, kwargs, task_id, producer, link, link_error, **options)
+            with SQS_APPLY_ASYNC_DURATION_SECONDS.labels(self.name).time():
+                return super().apply_async(args, kwargs, task_id, producer, link, link_error, **options)
 
     return NotifyTask
 

app/notifications/process_letter_notifications.py

@@ -5,22 +5,14 @@ from app.models import LETTER_TYPE
 from app.notifications.process_notifications import persist_notification
 
 
-def create_letter_notification(
-    letter_data,
-    template,
-    service,
-    api_key,
-    status,
-    reply_to_text=None,
-    billable_units=None,
-):
+def create_letter_notification(letter_data, template, api_key, status, reply_to_text=None, billable_units=None):
     notification = persist_notification(
         template_id=template.id,
-        template_version=template._template['version'],
-        template_postage=template._template['postage'],
+        template_version=template.version,
+        template_postage=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=service,
+        service=template.service,
         personalisation=letter_data['personalisation'],
         notification_type=LETTER_TYPE,
         api_key_id=api_key.id,

app/notifications/process_notifications.py

@@ -9,11 +9,6 @@ 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
@@ -39,18 +34,17 @@ from app.dao.notifications_dao import (
 from app.v2.errors import BadRequestError
 
 
-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'],
-        )
+from gds_metrics import Histogram
 
+
+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
@@ -130,8 +124,9 @@ def persist_notification(
     if not simulated:
         dao_create_notification(notification)
         if key_type != KEY_TYPE_TEST:
-            if redis_store.get(redis.daily_limit_cache_key(service.id)):
-                redis_store.incr(redis.daily_limit_cache_key(service.id))
+            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))
 
         current_app.logger.info(
             "{} {} created at {}".format(notification_type, notification_id, notification_created_at)

app/notifications/rest.py

@@ -100,13 +100,12 @@ def send_notification(notification_type):
 
     check_rate_limiting(authenticated_service, api_user)
 
-    template_with_content = validate_template(
+    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):
@@ -119,9 +118,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_dict['id'],
-                                              template_version=template_dict['version'],
-                                              template_postage=template_dict['postage'],
+    notification_model = persist_notification(template_id=template.id,
+                                              template_version=template.version,
+                                              template_postage=template.postage,
                                               recipient=request.get_json()['to'],
                                               service=authenticated_service,
                                               personalisation=notification_form.get('personalisation', None),
@@ -129,16 +128,16 @@ def send_notification(notification_type):
                                               api_key_id=api_user.id,
                                               key_type=api_user.key_type,
                                               simulated=simulated,
-                                              reply_to_text=template_dict['reply_to_text']
+                                              reply_to_text=template.get_reply_to_text()
                                               )
     if not simulated:
-        queue_name = QueueNames.PRIORITY if template_dict['process_type'] == PRIORITY else None
+        queue_name = QueueNames.PRIORITY if template.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_dict['version']})
+    notification_form.update({"template_version": template.version})
 
     return jsonify(
         data=get_notification_return_data(

app/notifications/validators.py

@@ -16,21 +16,30 @@ 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, request_cache
+from app import redis_store
 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
-        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)
+        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)
 
 
 def check_service_over_daily_message_limit(key_type, service):
@@ -62,7 +71,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")
 
@@ -138,26 +147,18 @@ def check_notification_content_is_not_empty(template_with_content):
         raise BadRequestError(message=message)
 
 
-@request_cache.set('template-{template_id}-version-None')
-def get_template_dict(template_id, service_id):
-    from app.schemas import template_schema
+def validate_template(template_id, personalisation, service, notification_type):
     try:
-        fetched_template = templates_dao.dao_get_template_by_id_and_service_id(
+        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}])
 
-    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_for_notification_type(notification_type, template.template_type)
     check_template_is_active(template)
 
     template_with_content = create_content_for_notification(template, personalisation)
@@ -166,7 +167,7 @@ def validate_template(template_id, personalisation, service, notification_type):
 
     check_content_char_count(template_with_content)
 
-    return template_with_content
+    return template, template_with_content
 
 
 def check_reply_to(service_id, reply_to_id, type_):

app/schemas.py

@@ -2,7 +2,6 @@ from datetime import (
     datetime,
     date,
     timedelta)
-from uuid import UUID
 from flask_marshmallow.fields import fields
 from marshmallow import (
     post_load,
@@ -13,7 +12,7 @@ from marshmallow import (
     pre_dump,
     post_dump
 )
-from marshmallow_sqlalchemy import field_for
+from marshmallow_sqlalchemy import auto_field
 
 from notifications_utils.recipients import (
     validate_email_address,
@@ -59,7 +58,7 @@ def _validate_datetime_not_in_past(dte, msg="Date cannot be in the past"):
         raise ValidationError(msg)
 
 
-class BaseSchema(ma.ModelSchema):
+class BaseSchema(ma.SQLAlchemyAutoSchema):
 
     def __init__(self, load_json=False, *args, **kwargs):
         self.load_json = load_json
@@ -76,13 +75,17 @@ class BaseSchema(ma.ModelSchema):
             return data
         return super(BaseSchema, self).make_instance(data)
 
+    class Meta:
+        include_relationships = True
+        load_instance = True
+
 
 class UserSchema(BaseSchema):
 
     permissions = fields.Method("user_permissions", dump_only=True)
-    password_changed_at = field_for(models.User, 'password_changed_at', format=DATETIME_FORMAT_NO_TIMEZONE)
-    created_at = field_for(models.User, 'created_at', format=DATETIME_FORMAT_NO_TIMEZONE)
-    auth_type = field_for(models.User, 'auth_type')
+    password_changed_at = auto_field(format=DATETIME_FORMAT_NO_TIMEZONE)
+    created_at = auto_field(format=DATETIME_FORMAT_NO_TIMEZONE)
+    auth_type = auto_field()
 
     def user_permissions(self, usr):
         retval = {}
@@ -93,7 +96,7 @@ class UserSchema(BaseSchema):
             retval[service_id].append(x.permission)
         return retval
 
-    class Meta:
+    class Meta(BaseSchema.Meta):
         model = models.User
         exclude = (
             "updated_at",
@@ -128,9 +131,9 @@ class UserSchema(BaseSchema):
 
 
 class UserUpdateAttributeSchema(BaseSchema):
-    auth_type = field_for(models.User, 'auth_type')
+    auth_type = auto_field()
 
-    class Meta:
+    class Meta(BaseSchema.Meta):
         model = models.User
         exclude = (
             'id', 'updated_at', 'created_at', 'user_to_service',
@@ -167,7 +170,7 @@ class UserUpdateAttributeSchema(BaseSchema):
 
 class UserUpdatePasswordSchema(BaseSchema):
 
-    class Meta:
+    class Meta(BaseSchema.Meta):
         model = models.User
         only = ('password')
         strict = True
@@ -182,7 +185,7 @@ class UserUpdatePasswordSchema(BaseSchema):
 class ProviderDetailsSchema(BaseSchema):
     created_by = fields.Nested(UserSchema, only=['id', 'name', 'email_address'], dump_only=True)
 
-    class Meta:
+    class Meta(BaseSchema.Meta):
         model = models.ProviderDetails
         exclude = ("provider_rates", "provider_stats")
         strict = True
@@ -191,7 +194,7 @@ class ProviderDetailsSchema(BaseSchema):
 class ProviderDetailsHistorySchema(BaseSchema):
     created_by = fields.Nested(UserSchema, only=['id', 'name', 'email_address'], dump_only=True)
 
-    class Meta:
+    class Meta(BaseSchema.Meta):
         model = models.ProviderDetailsHistory
         exclude = ("provider_rates", "provider_stats")
         strict = True
@@ -199,15 +202,15 @@ class ProviderDetailsHistorySchema(BaseSchema):
 
 class ServiceSchema(BaseSchema):
 
-    created_by = field_for(models.Service, 'created_by', required=True)
-    organisation_type = field_for(models.Service, 'organisation_type')
+    created_by = auto_field(required=True)
+    organisation_type = auto_field()
     letter_logo_filename = fields.Method(dump_only=True, serialize='get_letter_logo_filename')
     permissions = fields.Method("service_permissions")
-    email_branding = field_for(models.Service, 'email_branding')
-    organisation = field_for(models.Service, 'organisation')
+    email_branding = auto_field()
+    organisation = auto_field()
     override_flag = False
     letter_contact_block = fields.Method(serialize="get_letter_contact")
-    go_live_at = field_for(models.Service, 'go_live_at', format=DATETIME_FORMAT_NO_TIMEZONE)
+    go_live_at = auto_field(format=DATETIME_FORMAT_NO_TIMEZONE)
 
     def get_letter_logo_filename(self, service):
         return service.letter_branding and service.letter_branding.filename
@@ -218,7 +221,7 @@ class ServiceSchema(BaseSchema):
     def get_letter_contact(self, service):
         return service.get_default_letter_contact()
 
-    class Meta:
+    class Meta(BaseSchema.Meta):
         model = models.Service
         dump_only = ['letter_contact_block']
         exclude = (
@@ -264,9 +267,9 @@ class ServiceSchema(BaseSchema):
 
 class DetailedServiceSchema(BaseSchema):
     statistics = fields.Dict()
-    organisation_type = field_for(models.Service, 'organisation_type')
+    organisation_type = auto_field()
 
-    class Meta:
+    class Meta(BaseSchema.Meta):
         model = models.Service
         exclude = (
             'api_keys',
@@ -295,7 +298,7 @@ class DetailedServiceSchema(BaseSchema):
 
 
 class NotificationModelSchema(BaseSchema):
-    class Meta:
+    class Meta(BaseSchema.Meta):
         model = models.Notification
         strict = True
         exclude = ('_personalisation', 'job', 'service', 'template', 'api_key',)
@@ -313,7 +316,7 @@ class BaseTemplateSchema(BaseSchema):
     def get_reply_to_text(self, template):
         return template.get_reply_to_text()
 
-    class Meta:
+    class Meta(BaseSchema.Meta):
         model = models.Template
         exclude = ("service_id", "jobs", "service_letter_contact_id")
         strict = True
@@ -321,8 +324,8 @@ class BaseTemplateSchema(BaseSchema):
 
 class TemplateSchema(BaseTemplateSchema):
 
-    created_by = field_for(models.Template, 'created_by', required=True)
-    process_type = field_for(models.Template, 'process_type')
+    created_by = auto_field(required=True)
+    process_type = auto_field()
     redact_personalisation = fields.Method("redact")
 
     def redact(self, template):
@@ -335,16 +338,6 @@ 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):
 
@@ -352,7 +345,7 @@ class TemplateHistorySchema(BaseSchema):
     reply_to_text = fields.Method("get_reply_to_text", allow_none=True)
 
     created_by = fields.Nested(UserSchema, only=['id', 'name', 'email_address'], dump_only=True)
-    created_at = field_for(models.Template, 'created_at', format=DATETIME_FORMAT_NO_TIMEZONE)
+    created_at = auto_field(format=DATETIME_FORMAT_NO_TIMEZONE)
 
     def get_reply_to(self, template):
         return template.reply_to
@@ -360,16 +353,16 @@ class TemplateHistorySchema(BaseSchema):
     def get_reply_to_text(self, template):
         return template.get_reply_to_text()
 
-    class Meta:
+    class Meta(BaseSchema.Meta):
         model = models.TemplateHistory
 
 
 class ApiKeySchema(BaseSchema):
 
-    created_by = field_for(models.ApiKey, 'created_by', required=True)
-    key_type = field_for(models.ApiKey, 'key_type', required=True)
+    created_by = auto_field(required=True)
+    key_type = auto_field(required=True)
 
-    class Meta:
+    class Meta(BaseSchema.Meta):
         model = models.ApiKey
         exclude = ("service", "_secret")
         strict = True
@@ -378,9 +371,9 @@ class ApiKeySchema(BaseSchema):
 class JobSchema(BaseSchema):
     created_by_user = fields.Nested(UserSchema, attribute="created_by",
                                     dump_to="created_by", only=["id", "name"], dump_only=True)
-    created_by = field_for(models.Job, 'created_by', required=True, load_only=True)
+    created_by = auto_field(required=True, load_only=True)
 
-    job_status = field_for(models.JobStatus, 'name', required=False)
+    job_status = auto_field("name", model=models.JobStatus, required=False)
 
     scheduled_for = fields.DateTime()
     service_name = fields.Nested(
@@ -388,7 +381,7 @@ class JobSchema(BaseSchema):
 
     template_name = fields.Method('get_template_name', dump_only=True)
     template_type = fields.Method('get_template_type', dump_only=True)
-    contact_list_id = field_for(models.Job, 'contact_list_id')
+    contact_list_id = auto_field()
 
     def get_template_name(self, job):
         return job.template.name
@@ -401,7 +394,7 @@ class JobSchema(BaseSchema):
         _validate_datetime_not_in_past(value)
         _validate_datetime_not_more_than_96_hours_in_future(value)
 
-    class Meta:
+    class Meta(BaseSchema.Meta):
         model = models.Job
         exclude = (
             'notifications',
@@ -413,7 +406,7 @@ class JobSchema(BaseSchema):
 
 class NotificationSchema(ma.Schema):
 
-    class Meta:
+    class Meta(BaseSchema.Meta):
         strict = True
 
     status = fields.String(required=False)
@@ -454,7 +447,7 @@ class SmsTemplateNotificationSchema(SmsNotificationSchema):
 
 
 class NotificationWithTemplateSchema(BaseSchema):
-    class Meta:
+    class Meta(BaseSchema.Meta):
         model = models.Notification
         strict = True
         exclude = ('_personalisation', 'scheduled_notification')
@@ -477,7 +470,7 @@ class NotificationWithTemplateSchema(BaseSchema):
     created_by = fields.Nested(UserSchema, only=['id', 'name', 'email_address'], dump_only=True)
     status = fields.String(required=False)
     personalisation = fields.Dict(required=False)
-    key_type = field_for(models.Notification, 'key_type', required=True)
+    key_type = auto_field(required=True)
     key_name = fields.String()
 
     @pre_dump
@@ -531,9 +524,9 @@ class NotificationWithPersonalisationSchema(NotificationWithTemplateSchema):
 
 
 class InvitedUserSchema(BaseSchema):
-    auth_type = field_for(models.InvitedUser, 'auth_type')
+    auth_type = auto_field()
 
-    class Meta:
+    class Meta(BaseSchema.Meta):
         model = models.InvitedUser
         strict = True
 
@@ -547,7 +540,7 @@ class InvitedUserSchema(BaseSchema):
 
 class EmailDataSchema(ma.Schema):
 
-    class Meta:
+    class Meta(BaseSchema.Meta):
         strict = True
 
     email = fields.Str(required=True)
@@ -568,7 +561,7 @@ class EmailDataSchema(ma.Schema):
 
 class NotificationsFilterSchema(ma.Schema):
 
-    class Meta:
+    class Meta(BaseSchema.Meta):
         strict = True
 
     template_type = fields.Nested(BaseTemplateSchema, only=['template_type'], many=True)
@@ -636,7 +629,7 @@ class ApiKeyHistorySchema(ma.Schema):
 
 
 class EventSchema(BaseSchema):
-    class Meta:
+    class Meta(BaseSchema.Meta):
         model = models.Event
         strict = True
 

app/v2/notifications/post_notifications.py

@@ -7,6 +7,7 @@ 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,
@@ -42,7 +43,6 @@ 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,9 +75,14 @@ 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)
@@ -90,9 +95,6 @@ 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
@@ -100,12 +102,13 @@ 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,
-        service=authenticated_service,
-        reply_to_text=template._template['reply_to_text'],
+        reply_to_text=reply_to,
         precompiled=True
     )
 
@@ -120,16 +123,17 @@ def post_precompiled_letter_notification():
 
 @v2_notification_blueprint.route('/<notification_type>', methods=['POST'])
 def post_notification(notification_type):
-    request_json = get_valid_json()
+    with POST_NOTIFICATION_JSON_PARSE_DURATION_SECONDS.time():
+        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)
 
@@ -139,21 +143,20 @@ def post_notification(notification_type):
 
     check_rate_limiting(authenticated_service, api_user)
 
-    template_with_content = validate_template(
+    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_with_content)
+    reply_to = get_reply_to_text(notification_type, form, template)
 
     if notification_type == LETTER_TYPE:
         notification = process_letter_notification(
             letter_data=form,
             api_key=api_user,
-            template=template_with_content,
-            service=authenticated_service,
+            template=template,
             reply_to_text=reply_to
         )
     else:
@@ -161,12 +164,11 @@ def post_notification(notification_type):
             form=form,
             notification_type=notification_type,
             api_key=api_user,
-            template=template_with_content,
+            template=template,
             service=authenticated_service,
             reply_to_text=reply_to
         )
 
-        # Think this is redundant
         template_with_content.values = notification.personalisation
 
     if notification_type == SMS_TYPE:
@@ -243,7 +245,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._template['version'],
+        template_version=template.version,
         recipient=form_send_to,
         service=service,
         personalisation=personalisation,
@@ -261,7 +263,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._template['process_type'] == PRIORITY else None
+            queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None
             send_notification_to_queue(
                 notification=notification,
                 research_mode=service.research_mode,
@@ -288,7 +290,7 @@ def save_email_to_queue(
     data = {
         "id": notification_id,
         "template_id": str(template.id),
-        "template_version": template._template['version'],
+        "template_version": template.version,
         "to": form['email_address'],
         "service_id": str(service_id),
         "personalisation": personalisation,
@@ -339,7 +341,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, service, reply_to_text, precompiled=False):
+def process_letter_notification(*, letter_data, api_key, template, 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)
 
@@ -350,7 +352,6 @@ def process_letter_notification(*, letter_data, api_key, template, service, repl
         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(
@@ -385,7 +386,6 @@ def process_letter_notification(*, letter_data, api_key, template, service, repl
 
     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, service, repl
     return notification
 
 
-def process_precompiled_letter_notifications(*, letter_data, api_key, template, service, reply_to_text):
+def process_precompiled_letter_notifications(*, letter_data, api_key, template, reply_to_text):
     try:
         status = NOTIFICATION_PENDING_VIRUS_CHECK
         letter_content = base64.b64decode(letter_data['content'])
@@ -416,7 +416,6 @@ 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)
@@ -448,7 +447,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._template['reply_to_text']
+        ) or template.get_reply_to_text()
 
     elif notification_type == SMS_TYPE:
         service_sms_sender_id = form.get("sms_sender_id", None)
@@ -458,9 +457,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._template['reply_to_text']
+            reply_to = template.get_reply_to_text()
 
     elif notification_type == LETTER_TYPE:
-        reply_to = template._template['reply_to_text']
+        reply_to = template.get_reply_to_text()
 
     return reply_to

requirements-app.txt

@@ -5,7 +5,7 @@ cffi==1.14.0
 celery[sqs]==3.1.26.post2 # pyup: <4
 docopt==0.6.2
 Flask-Bcrypt==0.7.1
-flask-marshmallow==0.11.0
+flask-marshmallow==0.12.0
 Flask-Migrate==2.5.3
 git+https://github.com/mitsuhiko/flask-sqlalchemy.git@500e732dd1b975a56ab06a46bd1a20a21e682262#egg=Flask-SQLAlchemy==2.3.2.dev20190108
 Flask==1.1.2
@@ -28,4 +28,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

requirements.txt

@@ -7,7 +7,7 @@ cffi==1.14.0
 celery[sqs]==3.1.26.post2 # pyup: <4
 docopt==0.6.2
 Flask-Bcrypt==0.7.1
-flask-marshmallow==0.11.0
+flask-marshmallow==0.12.0
 Flask-Migrate==2.5.3
 git+https://github.com/mitsuhiko/flask-sqlalchemy.git@500e732dd1b975a56ab06a46bd1a20a21e682262#egg=Flask-SQLAlchemy==2.3.2.dev20190108
 Flask==1.1.2
@@ -30,6 +30,8 @@ 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:
@@ -66,7 +68,6 @@ 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/notifications/test_process_letter_notifications.py

@@ -2,8 +2,6 @@ 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):
@@ -15,17 +13,7 @@ def test_create_letter_notification_creates_notification(sample_letter_template,
         }
     }
 
-    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,
-    )
+    notification = create_letter_notification(data, sample_letter_template, sample_api_key, NOTIFICATION_CREATED)
 
     assert notification == Notification.query.one()
     assert notification.job is None
@@ -50,17 +38,7 @@ def test_create_letter_notification_sets_reference(sample_letter_template, sampl
         'reference': 'foo'
     }
 
-    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,
-    )
+    notification = create_letter_notification(data, sample_letter_template, sample_api_key, NOTIFICATION_CREATED)
 
     assert notification.client_reference == 'foo'
 
@@ -74,17 +52,7 @@ def test_create_letter_notification_sets_billable_units(sample_letter_template,
         },
     }
 
-    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,
-    )
+    notification = create_letter_notification(data, sample_letter_template, sample_api_key, NOTIFICATION_CREATED,
+                                              billable_units=3)
 
     assert notification.billable_units == 3

tests/app/notifications/test_process_notification.py

@@ -21,7 +21,6 @@ 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
@@ -29,30 +28,26 @@ 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)
-    template_dict = get_template_dict(template.id, template.service_id)
-    content = create_content_for_notification(template_dict, None)
+    content = create_content_for_notification(template, 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)
-    template_dict = get_template_dict(template.id, template.service_id)
-    content = create_content_for_notification(template_dict, {'name': 'Bobby'})
+    content = create_content_for_notification(template, {'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_dict, None)
+        create_content_for_notification(template, None)
 
 
 def test_create_content_for_notification_allows_additional_personalisation(sample_template_with_placeholders):
     template = Template.query.get(sample_template_with_placeholders.id)
-    template_dict = get_template_dict(template.id, template.service_id)
-    create_content_for_notification(template_dict, {'name': 'Bobby', 'Additional placeholder': 'Data'})
+    create_content_for_notification(template, {'name': 'Bobby', 'Additional placeholder': 'Data'})
 
 
 @freeze_time("2016-01-01 11:09:00.061258")

tests/app/notifications/test_validators.py

@@ -19,7 +19,6 @@ 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,
@@ -176,17 +175,15 @@ def test_check_template_is_for_notification_type_fails_when_template_type_does_n
 
 
 def test_check_template_is_active_passes(sample_template):
-    template_dict = get_template_dict(sample_template.id, sample_template.service_id)
-    assert check_template_is_active(template_dict) is None
+    assert check_template_is_active(sample_template) 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(template_dict)
+        check_template_is_active(sample_template)
     assert e.value.status_code == 400
     assert e.value.message == 'Template has been deleted'
     assert e.value.fields == [{'template': 'Template has been deleted'}]
@@ -317,11 +314,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_dict = get_template_dict(
+    template = templates_dao.dao_get_template_by_id_and_service_id(
         template_id=template_id,
         service_id=sample_service.id
     )
-    template_with_content = create_content_for_notification(template_dict, {})
+    template_with_content = create_content_for_notification(template, {})
     assert check_notification_content_is_not_empty(template_with_content) is None
 
 
@@ -333,11 +330,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_dict = get_template_dict(
+    template = templates_dao.dao_get_template_by_id_and_service_id(
         template_id=template_id,
         service_id=sample_service.id
     )
-    template_with_content = create_content_for_notification(template_dict, notification_values)
+    template_with_content = create_content_for_notification(template, notification_values)
     with pytest.raises(BadRequestError) as e:
         check_notification_content_is_not_empty(template_with_content)
     assert e.value.status_code == 400
@@ -352,10 +349,6 @@ 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(
@@ -366,8 +359,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_dict)
-    mock_create_conent.assert_called_once_with(template_dict, {})
+    mock_check_if_active.assert_called_once_with(template)
+    mock_create_conent.assert_called_once_with(template, {})
     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(mocker):
+def app_for_test():
     import flask
     from flask import Blueprint
     from app.authentication.auth import AuthError