Merge pull request #735 from alphagov/caching-with-redis

Caching with redis

app/__init__.py

@@ -1,19 +1,21 @@
-import uuid
 import os
+import uuid
 
-from flask import request, url_for, g, jsonify
 from flask import Flask, _request_ctx_stack
+from flask import request, url_for, g, jsonify
 from flask.ext.sqlalchemy import SQLAlchemy
 from flask_marshmallow import Marshmallow
 from monotonic import monotonic
-from werkzeug.local import LocalProxy
 from notifications_utils import logging
+from werkzeug.local import LocalProxy
+
 from app.celery.celery import NotifyCelery
 from app.clients import Clients
-from app.clients.sms.mmg import MMGClient
+from app.clients.email.aws_ses import AwsSesClient
+from app.clients.redis.redis_client import RedisClient
 from app.clients.sms.firetext import FiretextClient
 from app.clients.sms.loadtesting import LoadtestingClient
-from app.clients.email.aws_ses import AwsSesClient
+from app.clients.sms.mmg import MMGClient
 from app.clients.statsd.statsd_client import StatsdClient
 from app.encryption import Encryption
 
@@ -30,6 +32,7 @@ mmg_client = MMGClient()
 aws_ses_client = AwsSesClient()
 encryption = Encryption()
 statsd_client = StatsdClient()
+redis_store = RedisClient()
 
 clients = Clients()
 
@@ -55,6 +58,7 @@ def create_app(app_name=None):
     aws_ses_client.init_app(application.config['AWS_REGION'], statsd_client=statsd_client)
     notify_celery.init_app(application)
     encryption.init_app(application)
+    redis_store.init_app(application)
     clients.init_app(sms_clients=[firetext_client, mmg_client, loadtest_client], email_clients=[aws_ses_client])
 
     register_blueprint(application)

app/celery/tasks.py

@@ -27,6 +27,8 @@ from app.models import (
 from app.notifications.process_notifications import persist_notification
 from app.service.utils import service_allowed_to_send_to
 from app.statsd_decorators import statsd
+from app import redis_store
+from app.clients.redis import daily_limit_cache_key
 
 
 @notify_celery.task(name="process-job")
@@ -152,14 +154,16 @@ def send_sms(self,
 
     except SQLAlchemyError as e:
         current_app.logger.exception(
-            "RETRY: send_sms notification for job {} row number {}".format(notification.get('job', None),
+            "RETRY: send_sms notification for job {} row number {}".format(
-                                                                           notification.get('row_number', None)), e)
+                notification.get('job', None),
+                notification.get('row_number', None)), e)
         try:
             raise self.retry(queue="retry", exc=e)
         except self.MaxRetriesExceededError:
             current_app.logger.exception(
-                "RETRY FAILED: task send_sms failed for notification".format(notification.get('job', None),
+                "RETRY FAILED: task send_sms failed for notification".format(
-                                                                             notification.get('row_number', None)), e)
+                    notification.get('job', None),
+                    notification.get('row_number', None)), e)
 
 
 @notify_celery.task(bind=True, name="send-email", max_retries=5, default_retry_delay=300)
@@ -207,5 +211,6 @@ def send_email(self, service_id,
             raise self.retry(queue="retry", exc=e)
         except self.MaxRetriesExceededError:
             current_app.logger.error(
-                "RETRY FAILED: task send_email failed for notification".format(notification.get('job', None),
+                "RETRY FAILED: task send_email failed for notification".format(
-                                                                               notification.get('row_number', None)), e)
+                    notification.get('job', None),
+                    notification.get('row_number', None)), e)

app/clients/redis/__init__.py

@@ -0,0 +1,5 @@
+from datetime import datetime
+
+
+def daily_limit_cache_key(service_id):
+    return "{}-{}-{}".format(str(service_id), datetime.utcnow().strftime("%Y-%m-%d"), "count")

app/clients/redis/redis_client.py

@@ -0,0 +1,40 @@
+from flask.ext.redis import FlaskRedis
+from flask import current_app
+
+
+class RedisClient:
+    redis_store = FlaskRedis()
+    active = False
+
+    def init_app(self, app):
+        self.active = app.config.get('REDIS_ENABLED')
+        if self.active:
+            self.redis_store.init_app(app)
+
+    def set(self, key, value, ex=None, px=None, nx=False, xx=False, raise_exception=False):
+        if self.active:
+            try:
+                self.redis_store.set(key, value, ex, px, nx, xx)
+            except Exception as e:
+                current_app.logger.exception(e)
+                if raise_exception:
+                    raise e
+
+    def incr(self, key, raise_exception=False):
+        if self.active:
+            try:
+                return self.redis_store.incr(key)
+            except Exception as e:
+                current_app.logger.exception(e)
+                if raise_exception:
+                    raise e
+
+    def get(self, key, raise_exception=False):
+        if self.active:
+            try:
+                return self.redis_store.get(key)
+            except Exception as e:
+                current_app.logger.exception(e)
+                if raise_exception:
+                    raise e
+        return None

app/errors.py

@@ -6,6 +6,7 @@ from sqlalchemy.exc import SQLAlchemyError, DataError
 from sqlalchemy.orm.exc import NoResultFound
 from marshmallow import ValidationError
 from app.authentication.auth import AuthError
+from app.notifications import SendNotificationToQueueError
 
 
 class InvalidRequest(Exception):
@@ -83,6 +84,11 @@ def register_errors(blueprint):
         current_app.logger.exception(e)
         return jsonify(result='error', message="No result found"), 404
 
+    @blueprint.errorhandler(SendNotificationToQueueError)
+    def failed_to_create_notification(e):
+        current_app.logger.exception(e)
+        return jsonify(result='error', message=e.message), 500
+
     @blueprint.errorhandler(SQLAlchemyError)
     def db_error(e):
         current_app.logger.exception(e)

app/notifications/__init__.py

@@ -0,0 +1,7 @@
+
+
+class SendNotificationToQueueError(Exception):
+    status_code = 500
+
+    def __init__(self):
+        self.message = "Failed to create the notification"

app/notifications/process_notifications.py

@@ -4,12 +4,13 @@ from flask import current_app
 from notifications_utils.renderers import PassThrough
 from notifications_utils.template import Template
 
-from app import DATETIME_FORMAT
+from app import DATETIME_FORMAT, redis_store
 from app.celery import provider_tasks
+from app.clients import redis
 from app.dao.notifications_dao import dao_create_notification, dao_delete_notifications_and_history_by_id
 from app.models import SMS_TYPE, Notification, KEY_TYPE_TEST, EMAIL_TYPE
 from app.notifications.validators import check_sms_content_char_count
-from app.v2.errors import BadRequestError
+from app.v2.errors import BadRequestError, SendNotificationToQueueError
 
 
 def create_content_for_notification(template, personalisation):
@@ -63,6 +64,7 @@ def persist_notification(template_id,
         client_reference=reference
     )
     dao_create_notification(notification)
+    redis_store.incr(redis.daily_limit_cache_key(service_id))
     return notification
 
 
@@ -79,10 +81,10 @@ def send_notification_to_queue(notification, research_mode):
                 [str(notification.id)],
                 queue='send-email' if not research_mode else 'research-mode'
             )
-    except Exception:
+    except Exception as e:
-        current_app.logger.exception("Failed to send to SQS exception")
+        current_app.logger.exception(e)
         dao_delete_notifications_and_history_by_id(notification.id)
-        raise
+        raise SendNotificationToQueueError()
 
     current_app.logger.info(
         "{} {} created at {}".format(notification.notification_type, notification.id, notification.created_at)

app/notifications/rest.py

@@ -36,6 +36,8 @@ from app.schemas import (
 )
 from app.service.utils import service_allowed_to_send_to
 from app.utils import pagination_links
+from app import redis_store
+from app.clients import redis
 
 notifications = Blueprint('notifications', __name__)
 
@@ -44,6 +46,7 @@ from app.errors import (
     InvalidRequest
 )
 
+
 register_errors(notifications)
 
 
@@ -204,6 +207,7 @@ def get_notification_statistics_for_day():
 
 @notifications.route('/notifications/<string:notification_type>', methods=['POST'])
 def send_notification(notification_type):
+
     if notification_type not in ['sms', 'email']:
         assert False
 
@@ -242,7 +246,6 @@ def send_notification(notification_type):
 
     notification_id = create_uuid() if saved_notification is None else saved_notification.id
     notification.update({"template_version": template.version})
-
     return jsonify(
         data=get_notification_return_data(
             notification_id,

app/notifications/validators.py

@@ -4,13 +4,18 @@ from app.dao import services_dao
 from app.models import KEY_TYPE_TEST, KEY_TYPE_TEAM
 from app.service.utils import service_allowed_to_send_to
 from app.v2.errors import TooManyRequestsError, BadRequestError
+from app import redis_store
+from app.clients import redis
 
 
 def check_service_message_limit(key_type, service):
-    if all((key_type != KEY_TYPE_TEST,
+    if key_type != KEY_TYPE_TEST:
-            service.restricted)):
+        cache_key = redis.daily_limit_cache_key(service.id)
-        service_stats = services_dao.fetch_todays_total_message_count(service.id)
+        service_stats = redis_store.get(cache_key)
-        if service_stats >= service.message_limit:
+        if not service_stats:
+            service_stats = services_dao.fetch_todays_total_message_count(service.id)
+            redis_store.set(cache_key, service_stats, ex=3600)
+        if int(service_stats) >= service.message_limit:
             raise TooManyRequestsError(service.message_limit)
 
 

app/v2/errors.py

@@ -5,6 +5,7 @@ from sqlalchemy.exc import DataError
 from sqlalchemy.orm.exc import NoResultFound
 from app.authentication.auth import AuthError
 from app.errors import InvalidRequest
+from app.notifications import SendNotificationToQueueError
 
 
 class TooManyRequestsError(InvalidRequest):
@@ -47,6 +48,13 @@ def register_errors(blueprint):
     def auth_error(error):
         return jsonify(error.to_dict_v2()), error.code
 
+    @blueprint.errorhandler(SendNotificationToQueueError)
+    def failed_to_create_notification(error):
+        current_app.logger.exception(error)
+        return jsonify(
+            status_code=500,
+            errors=[{"error": error.__class__.__name__, "message": error.message}]), 500
+
     @blueprint.errorhandler(Exception)
     def internal_server_error(error):
         current_app.logger.exception(error)

config.py

@@ -132,6 +132,9 @@ class Config(object):
     STATSD_HOST = "statsd.hostedgraphite.com"
     STATSD_PORT = 8125
 
+    REDIS_ENABLED = False
+    REDIS_URL = "redis://localhost:6379/0"
+
     SENDING_NOTIFICATIONS_TIMEOUT_PERIOD = 259200
 
     SIMULATED_EMAIL_ADDRESSES = ('simulate-delivered@notifications.service.gov.uk',
@@ -167,6 +170,7 @@ class Test(Config):
     FROM_NUMBER = 'testing'
     NOTIFY_ENVIRONMENT = 'test'
     DEBUG = True
+    REDIS_ENABLED = True
     CSV_UPLOAD_BUCKET_NAME = 'test-notifications-csv-upload'
     STATSD_ENABLED = True
     STATSD_HOST = "localhost"

requirements.txt

@@ -18,6 +18,7 @@ celery==3.1.23
 monotonic==1.2
 statsd==3.2.1
 jsonschema==2.5.1
+Flask-Redis==0.1.0
 
 git+https://github.com/alphagov/notifications-python-client.git@3.0.0#egg=notifications-python-client==3.0.0
 

tests/app/celery/test_tasks.py

@@ -631,27 +631,28 @@ def test_should_not_send_sms_if_team_key_and_recipient_not_in_team(notify_db, no
 
 
 def test_should_use_email_template_and_persist(sample_email_template_with_placeholders, sample_api_key, mocker):
-    notification = _notification_json(
+    with freeze_time("2016-01-01 12:00:00.000000"):
-        sample_email_template_with_placeholders,
+        notification = _notification_json(
-        'my_email@my_email.com',
+            sample_email_template_with_placeholders,
-        {"name": "Jo"},
+            'my_email@my_email.com',
-        row_number=1)
+            {"name": "Jo"},
-    mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
+            row_number=1)
+        mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
 
-    notification_id = uuid.uuid4()
+        notification_id = uuid.uuid4()
 
-    with freeze_time("2016-01-01 11:09:00.00000"):
+        with freeze_time("2016-01-01 11:09:00.00000"):
-        now = datetime.utcnow()
+            now = datetime.utcnow()
 
-    with freeze_time("2016-01-01 11:10:00.00000"):
+        with freeze_time("2016-01-01 11:10:00.00000"):
-        send_email(
+            send_email(
-            sample_email_template_with_placeholders.service_id,
+                sample_email_template_with_placeholders.service_id,
-            notification_id,
+                notification_id,
-            encryption.encrypt(notification),
+                encryption.encrypt(notification),
-            now.strftime(DATETIME_FORMAT),
+                now.strftime(DATETIME_FORMAT),
-            api_key_id=str(sample_api_key.id),
+                api_key_id=str(sample_api_key.id),
-            key_type=sample_api_key.key_type
+                key_type=sample_api_key.key_type
-        )
+            )
 
     persisted_notification = Notification.query.one()
     assert persisted_notification.to == 'my_email@my_email.com'

tests/app/clients/test_redis_client.py

@@ -0,0 +1,81 @@
+import pytest
+from unittest.mock import Mock
+
+from app.clients.redis.redis_client import RedisClient
+from app.clients.redis import daily_limit_cache_key
+from freezegun import freeze_time
+
+
+@pytest.fixture(scope='function')
+def enabled_redis_client(notify_api, mocker):
+    notify_api.config['REDIS_ENABLED'] = True
+    return build_redis_client(notify_api, mocker)
+
+
+@pytest.fixture(scope='function')
+def disabled_redis_client(notify_api, mocker):
+    notify_api.config['REDIS_ENABLED'] = False
+    return build_redis_client(notify_api, mocker)
+
+
+def build_redis_client(notify_api, mocker):
+    redis_client = RedisClient()
+    redis_client.init_app(notify_api)
+    mocker.patch.object(redis_client.redis_store, 'get', return_value=100)
+    mocker.patch.object(redis_client.redis_store, 'set')
+    return redis_client
+
+
+def test_should_not_raise_exception_if_raise_set_to_false(notify_api):
+    notify_api.config['REDIS_ENABLED'] = True
+    redis_client = RedisClient()
+    redis_client.init_app(notify_api)
+    redis_client.redis_store.get = Mock(side_effect=Exception())
+    redis_client.redis_store.set = Mock(side_effect=Exception())
+    redis_client.redis_store.incr = Mock(side_effect=Exception())
+    assert redis_client.get('test') is None
+    assert redis_client.set('test', 'test') is None
+    assert redis_client.incr('test') is None
+
+
+def test_should_raise_exception_if_raise_set_to_true(notify_api):
+    notify_api.config['REDIS_ENABLED'] = True
+    redis_client = RedisClient()
+    redis_client.init_app(notify_api)
+    redis_client.redis_store.get = Mock(side_effect=Exception('get failed'))
+    redis_client.redis_store.set = Mock(side_effect=Exception('set failed'))
+    redis_client.redis_store.incr = Mock(side_effect=Exception('inc failed'))
+    with pytest.raises(Exception) as e:
+        redis_client.get('test', raise_exception=True)
+    assert str(e.value) == 'get failed'
+    with pytest.raises(Exception) as e:
+        redis_client.set('test', 'test', raise_exception=True)
+    assert str(e.value) == 'set failed'
+    with pytest.raises(Exception) as e:
+        redis_client.incr('test', raise_exception=True)
+    assert str(e.value) == 'inc failed'
+
+
+def test_should_not_call_set_if_not_enabled(disabled_redis_client):
+    assert not disabled_redis_client.set('key', 'value')
+    disabled_redis_client.redis_store.set.assert_not_called()
+
+
+def test_should_call_set_if_enabled(enabled_redis_client):
+    enabled_redis_client.set('key', 'value')
+    enabled_redis_client.redis_store.set.assert_called_with('key', 'value', None, None, False, False)
+
+
+def test_should_not_call_get_if_not_enabled(disabled_redis_client):
+    disabled_redis_client.set('key', 'value')
+    disabled_redis_client.redis_store.get.assert_not_called()
+
+
+def test_should_call_get_if_enabled(enabled_redis_client):
+    assert enabled_redis_client.get('key') == 100
+    enabled_redis_client.redis_store.get.assert_called_with('key')
+
+
+def test_should_build_cache_key_service_and_action(sample_service):
+    with freeze_time("2016-01-01 12:00:00.000000"):
+        assert daily_limit_cache_key(sample_service.id) == '{}-2016-01-01-count'.format(sample_service.id)

tests/app/notifications/rest/test_send_notification.py

@@ -343,7 +343,7 @@ def test_should_allow_valid_email_notification(notify_api, sample_email_template
 
 
 @freeze_time("2016-01-01 12:00:00.061258")
-def test_should_not_block_api_call_if_over_day_limit_for_live_service(
+def test_should_block_api_call_if_over_day_limit_for_live_service(
         notify_db,
         notify_db_session,
         notify_api,
@@ -371,8 +371,7 @@ def test_should_not_block_api_call_if_over_day_limit_for_live_service(
                 data=json.dumps(data),
                 headers=[('Content-Type', 'application/json'), auth_header])
             json.loads(response.get_data(as_text=True))
-
+            assert response.status_code == 429
-            assert response.status_code == 201
 
 
 @freeze_time("2016-01-01 12:00:00.061258")
@@ -698,15 +697,13 @@ def test_should_delete_notification_and_return_error_if_sqs_fails(
         save_model_api_key(api_key)
         auth_header = create_jwt_token(secret=api_key.unsigned_secret, client_id=str(api_key.service_id))
 
-        with pytest.raises(Exception) as exc:
+        response = client.post(
-            response = client.post(
+            path='/notifications/{}'.format(template_type),
-                path='/notifications/{}'.format(template_type),
+            data=json.dumps(data),
-                data=json.dumps(data),
+            headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))])
-                headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))])
 
         mocked.assert_called_once_with([fake_uuid], queue='send-{}'.format(template_type))
-        assert str(exc.value) == 'failed to talk to SQS'
+        assert response.status_code == 500
-
         assert not notifications_dao.get_notification_by_id(fake_uuid)
         assert not NotificationHistory.query.get(fake_uuid)
 

tests/app/notifications/test_process_notification.py

@@ -3,8 +3,10 @@ import datetime
 import pytest
 from boto3.exceptions import Boto3Error
 from sqlalchemy.exc import SQLAlchemyError
+from freezegun import freeze_time
 
 from app.models import Template, Notification, NotificationHistory
+from app.notifications import SendNotificationToQueueError
 from app.notifications.process_notifications import (create_content_for_notification,
                                                      persist_notification, send_notification_to_queue)
 from app.v2.errors import BadRequestError
@@ -37,7 +39,10 @@ def test_create_content_for_notification_fails_with_additional_personalisation(s
     assert e.value.message == 'Template personalisation not needed for template: Additional placeholder'
 
 
-def test_persist_notification_creates_and_save_to_db(sample_template, sample_api_key):
+@freeze_time("2016-01-01 11:09:00.061258")
+def test_persist_notification_creates_and_save_to_db(sample_template, sample_api_key, mocker):
+    mocked_redis = mocker.patch('app.notifications.process_notifications.redis_store.incr')
+
     assert Notification.query.count() == 0
     assert NotificationHistory.query.count() == 0
     notification = persist_notification(sample_template.id, sample_template.version, '+447111111111',
@@ -46,6 +51,7 @@ def test_persist_notification_creates_and_save_to_db(sample_template, sample_api
     assert Notification.query.count() == 1
     assert Notification.query.get(notification.id) is not None
     assert NotificationHistory.query.count() == 1
+    mocked_redis.assert_called_once_with(str(sample_template.service_id) + "-2016-01-01-count")
 
 
 def test_persist_notification_throws_exception_when_missing_template(sample_api_key):
@@ -64,9 +70,45 @@ def test_persist_notification_throws_exception_when_missing_template(sample_api_
     assert NotificationHistory.query.count() == 0
 
 
-def test_persist_notification_with_optionals(sample_job, sample_api_key):
+def test_exception_thown_by_redis_store_get_should_not_be_fatal(sample_template, sample_api_key, mocker):
+    mocker.patch(
+        'app.notifications.process_notifications.redis_store.redis_store.incr',
+        side_effect=Exception("broken redis"))
+
+    notification = persist_notification(
+        sample_template.id,
+        sample_template.version,
+        '+447111111111',
+        sample_template.service.id,
+        {},
+        'sms',
+        sample_api_key.id,
+        sample_api_key.key_type)
+    assert Notification.query.count() == 1
+    assert Notification.query.get(notification.id) is not None
+    assert NotificationHistory.query.count() == 1
+
+
+def test_cache_is_not_incremented_on_failure_to_persist_notification(sample_api_key, mocker):
+    mocked_redis = mocker.patch('app.notifications.process_notifications.redis_store.incr')
+    with pytest.raises(SQLAlchemyError):
+        persist_notification(template_id=None,
+                             template_version=None,
+                             recipient='+447111111111',
+                             service_id=sample_api_key.service_id,
+                             personalisation=None,
+                             notification_type='sms',
+                             api_key_id=sample_api_key.id,
+                             key_type=sample_api_key.key_type)
+    mocked_redis.assert_not_called()
+
+
+@freeze_time("2016-01-01 11:09:00.061258")
+def test_persist_notification_with_optionals(sample_job, sample_api_key, mocker):
     assert Notification.query.count() == 0
     assert NotificationHistory.query.count() == 0
+    mocked_redis = mocker.patch('app.notifications.process_notifications.redis_store.incr')
+
     created_at = datetime.datetime(2016, 11, 11, 16, 8, 18)
     persist_notification(template_id=sample_job.template.id,
                          template_version=sample_job.template.version,
@@ -85,6 +127,7 @@ def test_persist_notification_with_optionals(sample_job, sample_api_key):
     assert persisted_notification.job_id == sample_job.id
     assert persisted_notification.job_row_number == 10
     assert persisted_notification.created_at == created_at
+    mocked_redis.assert_called_once_with(str(sample_job.service_id) + "-2016-01-01-count")
     assert persisted_notification.client_reference == "ref from client"
     assert persisted_notification.reference is None
 
@@ -111,7 +154,7 @@ def test_send_notification_to_queue(notify_db, notify_db_session,
 
 def test_send_notification_to_queue_throws_exception_deletes_notification(sample_notification, mocker):
     mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async', side_effect=Boto3Error("EXPECTED"))
-    with pytest.raises(Boto3Error):
+    with pytest.raises(SendNotificationToQueueError):
         send_notification_to_queue(sample_notification, False)
         mocked.assert_called_once_with([(str(sample_notification.id))], queue='send-sms')
 

tests/app/notifications/test_validators.py

@@ -1,35 +1,146 @@
 import pytest
-from app.notifications.validators import check_service_message_limit, check_template_is_for_notification_type, \
+from freezegun import freeze_time
-    check_template_is_active, service_can_send_to_recipient, check_sms_content_char_count
-from app.v2.errors import BadRequestError, TooManyRequestsError
-from tests.app.conftest import (sample_notification as create_notification,
-                                sample_service as create_service, sample_service_whitelist)
 
-
+import app
-@pytest.mark.parametrize('key_type', ['test', 'team', 'normal'])
+from app.notifications.validators import (
-def test_check_service_message_limit_with_unrestricted_service_passes(key_type,
+    check_service_message_limit,
-                                                                      sample_service,
+    check_template_is_for_notification_type,
-                                                                      sample_notification):
+    check_template_is_active,
-    assert check_service_message_limit(key_type, sample_service) is None
+    service_can_send_to_recipient,
-
+    check_sms_content_char_count
-
+)
-@pytest.mark.parametrize('key_type', ['test', 'team', 'normal'])
+from app.v2.errors import (
-def test_check_service_message_limit_under_message_limit_passes(key_type,
+    BadRequestError,
-                                                                sample_service,
+    TooManyRequestsError
-                                                                sample_notification):
+)
-    assert check_service_message_limit(key_type, sample_service) is None
+from tests.app.conftest import (
+    sample_notification as create_notification,
+    sample_service as create_service,
+    sample_service_whitelist
+)
 
 
 @pytest.mark.parametrize('key_type', ['team', 'normal'])
-def test_check_service_message_limit_over_message_limit_fails(key_type, notify_db, notify_db_session):
+def test_exception_thown_by_redis_store_get_should_not_be_fatal(
+        notify_db,
+        notify_db_session,
+        notify_api,
+        key_type,
+        mocker):
+    mocker.patch('app.notifications.validators.redis_store.redis_store.get', side_effect=Exception("broken redis"))
+    mocker.patch('app.notifications.validators.redis_store.set')
+
     service = create_service(notify_db, notify_db_session, restricted=True, limit=4)
     for x in range(5):
         create_notification(notify_db, notify_db_session, service=service)
+
     with pytest.raises(TooManyRequestsError) as e:
         check_service_message_limit(key_type, service)
     assert e.value.status_code == 429
     assert e.value.message == 'Exceeded send limits (4) for today'
     assert e.value.fields == []
+    app.notifications.validators.redis_store.set.assert_not_called()
+
+
+@pytest.mark.parametrize('key_type', ['test', 'team', 'normal'])
+def test_exception_thown_by_redis_store_set_should_not_be_fatal(
+        key_type,
+        sample_service,
+        mocker):
+    mocker.patch('app.notifications.validators.redis_store.redis_store.set', side_effect=Exception("broken redis"))
+    mocker.patch('app.notifications.validators.redis_store.get', return_value=None)
+    assert not check_service_message_limit(key_type, sample_service)
+
+
+@pytest.mark.parametrize('key_type', ['test', 'team', 'normal'])
+def test_check_service_message_limit_in_cache_with_unrestricted_service_passes(
+        key_type,
+        sample_service,
+        mocker):
+    mocker.patch('app.notifications.validators.redis_store.get', return_value=1)
+    mocker.patch('app.notifications.validators.redis_store.set')
+    mocker.patch('app.notifications.validators.services_dao')
+    assert not check_service_message_limit(key_type, sample_service)
+    app.notifications.validators.redis_store.set.assert_not_called()
+    assert not app.notifications.validators.services_dao.mock_calls
+
+
+@pytest.mark.parametrize('key_type', ['test', 'team', 'normal'])
+def test_check_service_message_limit_in_cache_under_message_limit_passes(
+        key_type,
+        sample_service,
+        mocker):
+    mocker.patch('app.notifications.validators.redis_store.get', return_value=1)
+    mocker.patch('app.notifications.validators.redis_store.set')
+    mocker.patch('app.notifications.validators.services_dao')
+    assert not check_service_message_limit(key_type, sample_service)
+    app.notifications.validators.redis_store.set.assert_not_called()
+    assert not app.notifications.validators.services_dao.mock_calls
+
+
+def test_should_not_interact_with_cache_for_test_key(sample_service, mocker):
+    mocker.patch('app.notifications.validators.redis_store')
+    assert not check_service_message_limit('test', sample_service)
+    assert not app.notifications.validators.redis_store.mock_calls
+
+
+@pytest.mark.parametrize('key_type', ['team', 'normal'])
+def test_should_set_cache_value_as_value_from_database_if_cache_not_set(
+        key_type,
+        notify_db,
+        notify_db_session,
+        sample_service,
+        mocker
+):
+    with freeze_time("2016-01-01 12:00:00.000000"):
+        for x in range(5):
+            create_notification(notify_db, notify_db_session, service=sample_service)
+        mocker.patch('app.notifications.validators.redis_store.get', return_value=None)
+        mocker.patch('app.notifications.validators.redis_store.set')
+        assert not check_service_message_limit(key_type, sample_service)
+        app.notifications.validators.redis_store.set.assert_called_with(
+            str(sample_service.id) + "-2016-01-01-count", 5, ex=3600
+        )
+
+
+@pytest.mark.parametrize('key_type', ['team', 'normal'])
+def test_check_service_message_limit_over_message_limit_fails(key_type, notify_db, notify_db_session, mocker):
+    with freeze_time("2016-01-01 12:00:00.000000"):
+        mocker.patch('app.redis_store.get', return_value=None)
+        mocker.patch('app.notifications.validators.redis_store.set')
+
+        service = create_service(notify_db, notify_db_session, restricted=True, limit=4)
+        for x in range(5):
+            create_notification(notify_db, notify_db_session, service=service)
+        with pytest.raises(TooManyRequestsError) as e:
+            check_service_message_limit(key_type, service)
+        assert e.value.status_code == 429
+        assert e.value.message == 'Exceeded send limits (4) for today'
+        assert e.value.fields == []
+        app.notifications.validators.redis_store.set.assert_called_with(
+            str(service.id) + "-2016-01-01-count", 5, ex=3600
+        )
+
+
+@pytest.mark.parametrize('key_type', ['team', 'normal'])
+def test_check_service_message_limit_in_cache_over_message_limit_fails(
+        notify_db,
+        notify_db_session,
+        key_type,
+        mocker):
+    with freeze_time("2016-01-01 12:00:00.000000"):
+        mocker.patch('app.redis_store.get', return_value=5)
+        mocker.patch('app.notifications.validators.redis_store.set')
+        mocker.patch('app.notifications.validators.services_dao')
+
+        service = create_service(notify_db, notify_db_session, restricted=True, limit=4)
+        with pytest.raises(TooManyRequestsError) as e:
+            check_service_message_limit(key_type, service)
+        assert e.value.status_code == 429
+        assert e.value.message == 'Exceeded send limits (4) for today'
+        assert e.value.fields == []
+        app.notifications.validators.redis_store.set.assert_not_called()
+        assert not app.notifications.validators.services_dao.mock_calls
 
 
 @pytest.mark.parametrize('template_type, notification_type',