Merge pull request #1461 from alphagov/redis-for-real

don't hit the query to get daily msg stats if redis is disabled

app/notifications/validators.py

@@ -23,7 +23,7 @@ from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id
 
 
 def check_service_over_api_rate_limit(service, api_key):
-    if current_app.config['API_RATE_LIMIT_ENABLED']:
+    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 = current_app.config['API_KEY_LIMITS'][api_key.key_type]['limit']
         interval = current_app.config['API_KEY_LIMITS'][api_key.key_type]['interval']
@@ -33,7 +33,7 @@ def check_service_over_api_rate_limit(service, api_key):
 
 
 def check_service_over_daily_message_limit(key_type, service):
-    if key_type != KEY_TYPE_TEST:
+    if key_type != KEY_TYPE_TEST and current_app.config['REDIS_ENABLED']:
         cache_key = daily_limit_cache_key(service.id)
         service_stats = redis_store.get(cache_key)
         if not service_stats:

tests/app/notifications/rest/test_send_notification.py

@@ -16,7 +16,10 @@ from app.models import (
 from app.dao.templates_dao import dao_get_all_templates_for_service, dao_update_template
 from app.dao.services_dao import dao_update_service
 from app.dao.api_key_dao import save_model_api_key
-from app.v2.errors import RateLimitError
+from app.errors import InvalidRequest
+from app.models import Template
+from app.v2.errors import RateLimitError, TooManyRequestsError
+
 from tests import create_authorization_header
 from tests.app.conftest import (
     sample_notification as create_sample_notification,
@@ -28,9 +31,6 @@ from tests.app.conftest import (
     sample_service,
     sample_template_without_sms_permission,
     sample_template_without_email_permission)
-
-from app.models import Template
-from app.errors import InvalidRequest
 from tests.app.db import create_service, create_reply_to_email
 
 
@@ -414,6 +414,10 @@ def test_should_block_api_call_if_over_day_limit_for_live_service(
         mocker):
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
+            mocker.patch(
+                'app.notifications.validators.check_service_over_daily_message_limit',
+                side_effect=TooManyRequestsError(1)
+            )
             mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
 
             service = create_sample_service(notify_db, notify_db_session, limit=1, restricted=False)
@@ -446,6 +450,10 @@ def test_should_block_api_call_if_over_day_limit_for_restricted_service(
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
             mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
+            mocker.patch(
+                'app.notifications.validators.check_service_over_daily_message_limit',
+                side_effect=TooManyRequestsError(1)
+            )
 
             service = create_sample_service(notify_db, notify_db_session, limit=1, restricted=True)
             email_template = create_sample_email_template(notify_db, notify_db_session, service=service)

tests/app/notifications/test_validators.py

@@ -1,6 +1,7 @@
 import pytest
 from freezegun import freeze_time
 from flask import current_app
+
 import app
 from app.models import INTERNATIONAL_SMS_TYPE, SMS_TYPE, EMAIL_TYPE
 from app.notifications.validators import (
@@ -18,6 +19,8 @@ from app.v2.errors import (
     BadRequestError,
     TooManyRequestsError,
     RateLimitError)
+
+from tests.conftest import set_config
 from tests.app.conftest import (
     sample_notification as create_notification,
     sample_service as create_service,
@@ -26,6 +29,13 @@ from tests.app.conftest import (
 from tests.app.db import create_reply_to_email, create_service_sms_sender
 
 
+# all of these tests should have redis enabled (except where we specifically disable it)
+@pytest.fixture(scope='module', autouse=True)
+def enable_redis(notify_api):
+    with set_config(notify_api, 'REDIS_ENABLED', True):
+        yield
+
+
 @pytest.mark.parametrize('key_type', ['test', 'team', 'normal'])
 def test_check_service_message_limit_in_cache_with_unrestricted_service_is_allowed(
         key_type,
@@ -78,6 +88,15 @@ def test_should_set_cache_value_as_value_from_database_if_cache_not_set(
         )
 
 
+def test_should_not_access_database_if_redis_disabled(notify_api, sample_service, mocker):
+    with set_config(notify_api, 'REDIS_ENABLED', False):
+        db_mock = mocker.patch('app.notifications.validators.services_dao')
+
+        check_service_over_daily_message_limit('normal', sample_service)
+
+        assert db_mock.method_calls == []
+
+
 @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"):

tests/app/service/test_rest.py

@@ -2163,18 +2163,17 @@ def test_search_for_notification_by_to_field_returns_content(
     assert notifications[0]['template']['content'] == 'Hello (( Name))\nYour thing is due soon'
 
 
-def test_send_one_off_notification(admin_request, mocker):
-    service = create_service()
-    template = create_template(service=service)
+def test_send_one_off_notification(sample_service, admin_request, mocker):
+    template = create_template(service=sample_service)
     mocker.patch('app.service.send_notification.send_notification_to_queue')
 
     response = admin_request.post(
         'service.create_one_off_notification',
-        service_id=service.id,
+        service_id=sample_service.id,
         _data={
             'template_id': str(template.id),
             'to': '07700900001',
-            'created_by': str(service.created_by_id)
+            'created_by': str(sample_service.created_by_id)
         },
         _expected_status=201
     )

tests/app/service/test_send_one_off_notification.py

@@ -151,9 +151,13 @@ def test_send_one_off_notification_raises_if_cant_send_to_recipient(notify_db_se
     assert 'service is in trial mode' in e.value.message
 
 
-def test_send_one_off_notification_raises_if_over_limit(notify_db_session):
+def test_send_one_off_notification_raises_if_over_limit(notify_db_session, mocker):
     service = create_service(message_limit=0)
     template = create_template(service=service)
+    mocker.patch(
+        'app.service.send_notification.check_service_over_daily_message_limit',
+        side_effect=TooManyRequestsError(1)
+    )
 
     post_data = {
         'template_id': str(template.id),