Removed/replaced retention redis count with notification count from db call

app/dao/notifications_dao.py

@@ -190,6 +190,10 @@ def dao_get_notification_count_for_job_id(*, job_id):
     return Notification.query.filter_by(job_id=job_id).count()
 
 
+def dao_get_notification_count_for_service(*, service):
+    return Notification.query.filter_by(service_id=service.id).count()
+
+
 def get_notification_with_personalisation(service_id, notification_id, key_type):
     filter_dict = {"service_id": service_id, "id": notification_id}
     if key_type:

app/notifications/process_notifications.py

@@ -2,7 +2,6 @@ import uuid
 from datetime import datetime
 
 from flask import current_app
-from notifications_utils.clients import redis
 from notifications_utils.recipients import (
     format_email_address,
     get_international_phone_info,
@@ -10,7 +9,6 @@ from notifications_utils.recipients import (
 )
 from notifications_utils.template import PlainTextEmailTemplate, SMSMessageTemplate
 
-from app import redis_store
 from app.celery import provider_tasks
 from app.config import QueueNames
 from app.dao.notifications_dao import (
@@ -141,17 +139,7 @@ def persist_notification(
                     service.id
                 )
             )
-            total_key = redis.daily_total_cache_key()
-            if redis_store.get(total_key) is None:
-                current_app.logger.info("Redis daily total cache key does not exist")
-                redis_store.set(total_key, 1, ex=86400)
-                current_app.logger.info("Set redis daily total cache key to 1")
-            else:
-                current_app.logger.info("Redis total limit cache key does exist")
-                redis_store.incr(total_key)
-                current_app.logger.info(
-                    f"Redis total limit cache key has been incremented to {redis_store.get(total_key)}"
-                )
+
         current_app.logger.info(
             "{} {} created at {}".format(
                 notification_type, notification_id, notification_created_at

app/notifications/validators.py

@@ -1,7 +1,6 @@
 from flask import current_app
 from notifications_utils import SMS_CHAR_COUNT_LIMIT
 from notifications_utils.clients.redis import (
-    daily_total_cache_key,
     rate_limit_cache_key,
     total_limit_cache_key,
 )
@@ -13,6 +12,7 @@ from notifications_utils.recipients import (
 from sqlalchemy.orm.exc import NoResultFound
 
 from app import redis_store
+from app.dao.notifications_dao import dao_get_notification_count_for_service
 from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id
 from app.dao.service_sms_sender_dao import dao_get_service_sms_senders_by_id
 from app.models import (
@@ -69,15 +69,10 @@ def check_service_over_total_message_limit(key_type, service):
 def check_application_over_retention_limit(key_type, service):
     if key_type == KEY_TYPE_TEST or not current_app.config["REDIS_ENABLED"]:
         return 0
+    total_stats = dao_get_notification_count_for_service(service=service)
 
-    cache_key = daily_total_cache_key()
     daily_message_limit = current_app.config["DAILY_MESSAGE_LIMIT"]
-    total_stats = redis_store.get(cache_key)
-    if total_stats is None:
-        # first message of the day, set the cache to 0 and the expiry to 24 hours
-        total_stats = 0
-        redis_store.set(cache_key, total_stats, ex=86400)
-        return total_stats
+
     if int(total_stats) >= daily_message_limit:
         current_app.logger.info(
             "while sending for service {}, daily message limit of {} reached".format(

app/service/statistics.py

@@ -1,8 +1,7 @@
 from collections import defaultdict
-from datetime import datetime
 
 from app.dao.date_util import get_months_for_financial_year
-from app.models import NOTIFICATION_STATUS_TYPES, NOTIFICATION_TYPES
+from app.models import NOTIFICATION_TYPES
 
 
 def format_statistics(statistics):
@@ -55,26 +54,6 @@ def create_stats_dict():
     return stats_dict
 
 
-def format_monthly_template_notification_stats(year, rows):
-    stats = {
-        datetime.strftime(date, "%Y-%m"): {}
-        for date in [datetime(year, month, 1) for month in range(4, 13)]
-        + [datetime(year + 1, month, 1) for month in range(1, 4)]
-    }
-
-    for row in rows:
-        formatted_month = row.month.strftime("%Y-%m")
-        if str(row.template_id) not in stats[formatted_month]:
-            stats[formatted_month][str(row.template_id)] = {
-                "name": row.name,
-                "type": row.template_type,
-                "counts": dict.fromkeys(NOTIFICATION_STATUS_TYPES, 0),
-            }
-        stats[formatted_month][str(row.template_id)]["counts"][row.status] += row.count
-
-    return stats
-
-
 def create_zeroed_stats_dicts():
     return {
         template_type: {status: 0 for status in ("requested", "delivered", "failed")}

tests/app/dao/notification_dao/test_notification_dao.py

@@ -13,6 +13,7 @@ from app.dao.notifications_dao import (
     dao_get_last_notification_added_for_job_id,
     dao_get_notification_by_reference,
     dao_get_notification_count_for_job_id,
+    dao_get_notification_count_for_service,
     dao_get_notification_history_by_reference,
     dao_get_notifications_by_recipient_or_reference,
     dao_timeout_notifications,
@@ -595,6 +596,15 @@ def test_dao_get_notification_count_for_job_id(notify_db_session):
     assert dao_get_notification_count_for_job_id(job_id=job.id) == 3
 
 
+def test_dao_get_notification_count_for_service(notify_db_session):
+    service = create_service()
+    template = create_template(service)
+
+    create_notification(template)
+
+    assert dao_get_notification_count_for_service(service=service) == 1
+
+
 def test_dao_get_notification_count_for_job_id_returns_zero_for_no_notifications_for_job(
     notify_db_session,
 ):

tests/app/notifications/test_process_notification.py

@@ -1,7 +1,6 @@
 import datetime
 import uuid
 from collections import namedtuple
-from unittest.mock import call
 
 import pytest
 from boto3.exceptions import Boto3Error
@@ -21,8 +20,7 @@ from app.notifications.process_notifications import (
 )
 from app.serialised_models import SerialisedTemplate
 from app.v2.errors import BadRequestError
-from tests.app.db import create_api_key, create_service, create_template
-from tests.conftest import set_config
+from tests.app.db import create_service, create_template
 
 
 def test_create_content_for_notification_passes(sample_email_template):
@@ -189,103 +187,102 @@ def test_persist_notification_cache_is_not_incremented_on_failure_to_create_noti
     mocked_redis.assert_not_called()
 
 
-def test_persist_notification_does_not_increment_cache_if_test_key(
-    notify_api, sample_template, sample_job, mocker, sample_test_api_key
-):
-    daily_limit_cache = mocker.patch(
-        "app.notifications.process_notifications.redis_store.incr"
-    )
-
-    assert Notification.query.count() == 0
-    assert NotificationHistory.query.count() == 0
-    with set_config(notify_api, "REDIS_ENABLED", True):
-        persist_notification(
-            template_id=sample_template.id,
-            template_version=sample_template.version,
-            recipient="+447111111111",
-            service=sample_template.service,
-            personalisation={},
-            notification_type="sms",
-            api_key_id=sample_test_api_key.id,
-            key_type=sample_test_api_key.key_type,
-            job_id=sample_job.id,
-            job_row_number=100,
-            reference="ref",
-        )
-
-    assert Notification.query.count() == 1
-
-    assert not daily_limit_cache.called
+# def test_persist_notification_does_not_increment_cache_if_test_key(
+#     notify_api, sample_template, sample_job, mocker, sample_test_api_key
+# ):
+#     mocker.patch(
+#         "app.notifications.process_notifications.dao_get_notification_count_for_service",
+#         return_value=1,
+#     )
+#
+#     assert Notification.query.count() == 0
+#     assert NotificationHistory.query.count() == 0
+#     with set_config(notify_api, "REDIS_ENABLED", True):
+#         persist_notification(
+#             template_id=sample_template.id,
+#             template_version=sample_template.version,
+#             recipient="+447111111111",
+#             service=sample_template.service,
+#             personalisation={},
+#             notification_type="sms",
+#             api_key_id=sample_test_api_key.id,
+#             key_type=sample_test_api_key.key_type,
+#             job_id=sample_job.id,
+#             job_row_number=100,
+#             reference="ref",
+#         )
+#
+#     assert Notification.query.count() == 1
 
 
-@pytest.mark.parametrize("restricted_service", [True, False])
-@freeze_time("2016-01-01 11:09:00.061258")
-def test_persist_notification_increments_cache_for_trial_or_live_service(
-    notify_api, notify_db_session, mocker, restricted_service
-):
-    service = create_service(restricted=restricted_service)
-    template = create_template(service=service)
-    api_key = create_api_key(service=service)
-    mocker.patch(
-        "app.notifications.process_notifications.redis_store.get", return_value=1
-    )
-    mock_incr = mocker.patch("app.notifications.process_notifications.redis_store.incr")
-    with set_config(notify_api, "REDIS_ENABLED", True):
-        persist_notification(
-            template_id=template.id,
-            template_version=template.version,
-            recipient="+447111111122",
-            service=template.service,
-            personalisation={},
-            notification_type="sms",
-            api_key_id=api_key.id,
-            key_type=api_key.key_type,
-            reference="ref2",
-        )
-
-        assert mock_incr.call_count == 1
-        mock_incr.assert_has_calls(
-            [
-                # call(str(service.id) + "-2016-01-01-count", ),
-                call(
-                    "2016-01-01-total",
-                )
-            ]
-        )
+# @pytest.mark.parametrize("restricted_service", [True, False])
+# @freeze_time("2016-01-01 11:09:00.061258")
+# def test_persist_notification_increments_cache_for_trial_or_live_service(
+#     notify_api, notify_db_session, mocker, restricted_service
+# ):
+#     service = create_service(restricted=restricted_service)
+#     template = create_template(service=service)
+#     api_key = create_api_key(service=service)
+#     mocker.patch(
+#         "app.notifications.process_notifications.redis_store.get", return_value=1
+#     )
+#     mock_incr = mocker.patch("app.notifications.process_notifications.redis_store.incr")
+#     with set_config(notify_api, "REDIS_ENABLED", True):
+#         persist_notification(
+#             template_id=template.id,
+#             template_version=template.version,
+#             recipient="+447111111122",
+#             service=template.service,
+#             personalisation={},
+#             notification_type="sms",
+#             api_key_id=api_key.id,
+#             key_type=api_key.key_type,
+#             reference="ref2",
+#         )
+#
+#         assert mock_incr.call_count == 1
+#         mock_incr.assert_has_calls(
+#             [
+#                 # call(str(service.id) + "-2016-01-01-count", ),
+#                 call(
+#                     "2016-01-01-total",
+#                 )
+#             ]
+#         )
 
 
-@pytest.mark.parametrize("restricted_service", [True, False])
-@freeze_time("2016-01-01 11:09:00.061258")
-def test_persist_notification_sets_daily_limit_cache_if_one_does_not_exists(
-    notify_api, notify_db_session, mocker, restricted_service
-):
-    service = create_service(restricted=restricted_service)
-    template = create_template(service=service)
-    api_key = create_api_key(service=service)
-    mocker.patch(
-        "app.notifications.process_notifications.redis_store.get", return_value=None
-    )
-    mock_set = mocker.patch("app.notifications.process_notifications.redis_store.set")
-    with set_config(notify_api, "REDIS_ENABLED", True):
-        persist_notification(
-            template_id=template.id,
-            template_version=template.version,
-            recipient="+447111111122",
-            service=template.service,
-            personalisation={},
-            notification_type="sms",
-            api_key_id=api_key.id,
-            key_type=api_key.key_type,
-            reference="ref2",
-        )
-
-        assert mock_set.call_count == 1
-        mock_set.assert_has_calls(
-            [
-                # call(str(service.id) + "-2016-01-01-count", 1, ex=86400),
-                call("2016-01-01-total", 1, ex=86400)
-            ]
-        )
+# @pytest.mark.parametrize("restricted_service", [True, False])
+# @freeze_time("2016-01-01 11:09:00.061258")
+# def test_persist_notification_sets_daily_limit_cache_if_one_does_not_exists(
+#     notify_api, notify_db_session, mocker, restricted_service
+# ):
+#     service = create_service(restricted=restricted_service)
+#     template = create_template(service=service)
+#     api_key = create_api_key(service=service)
+#     mocker.patch(
+#         "app.notifications.process_notifications.redis_store.get", return_value=None
+#     )
+#     mock_set = mocker.patch("app.notifications.process_notifications.redis_store.set")
+#     with set_config(notify_api, "REDIS_ENABLED", True):
+#         persist_notification(
+#             template_id=template.id,
+#             template_version=template.version,
+#             recipient="+447111111122",
+#             service=template.service,
+#             personalisation={},
+#             notification_type="sms",
+#             api_key_id=api_key.id,
+#             key_type=api_key.key_type,
+#             reference="ref2",
+#         )
+#
+#         assert mock_set.call_count == 1
+#         mock_set.assert_has_calls(
+#             [
+#                 # call(str(service.id) + "-2016-01-01-count", 1, ex=86400),
+#                 call("2016-01-01-total", 1, ex=86400)
+#             ]
+#         )
 
 
 @pytest.mark.parametrize(

tests/app/notifications/test_validators.py

@@ -55,11 +55,31 @@ def enable_redis(notify_api):
 
 
 @pytest.mark.parametrize("key_type", ["team", "normal"])
-def test_check_service_message_limit_over_total_limit_fails(
+def test_check_service_over_total_message_limit_fails(
     key_type, mocker, notify_db_session
 ):
     service = create_service()
-    mocker.patch("app.redis_store.get", return_value="5001")
+    mocker.patch(
+        "app.redis_store.get",
+        return_value="250001",
+    )
+
+    with pytest.raises(TotalRequestsError) as e:
+        check_service_over_total_message_limit(key_type, service)
+    assert e.value.status_code == 429
+    assert e.value.message == "Exceeded total application limits (250000) for today"
+    assert e.value.fields == []
+
+
+@pytest.mark.parametrize("key_type", ["team", "normal"])
+def test_check_application_over_retention_limit_fails(
+    key_type, mocker, notify_db_session
+):
+    service = create_service()
+    mocker.patch(
+        "app.notifications.validators.dao_get_notification_count_for_service",
+        return_value="5001",
+    )
 
     with pytest.raises(TotalRequestsError) as e:
         check_application_over_retention_limit(key_type, service)