Remove now unused methods that populated template usage redis cache

app/commands.py

@@ -1,4 +1,3 @@
-import sys
 import functools
 import uuid
 from datetime import datetime, timedelta
@@ -9,10 +8,9 @@ import flask
 from click_datetime import Datetime as click_dt
 from flask import current_app, json
 from sqlalchemy.orm.exc import NoResultFound
-from sqlalchemy import func
 from notifications_utils.statsd_decorators import statsd
 
-from app import db, DATETIME_FORMAT, encryption, redis_store
+from app import db, DATETIME_FORMAT, encryption
 from app.celery.scheduled_tasks import send_total_sent_notifications_to_performance_platform
 from app.celery.service_callback_tasks import send_delivery_status_to_service
 from app.celery.letters_pdf_tasks import create_letters_pdf
@@ -34,11 +32,7 @@ from app.dao.services_dao import (
 from app.dao.users_dao import delete_model_user, delete_user_verify_codes
 from app.models import PROVIDERS, User, Notification
 from app.performance_platform.processing_time import send_processing_time_for_start_and_end
-from app.utils import (
-    cache_key_for_service_template_usage_per_day,
-    get_london_midnight_in_utc,
-    get_midnight_for_day_before,
-)
+from app.utils import get_london_midnight_in_utc, get_midnight_for_day_before
 
 
 @click.group(name='command', help='Additional commands')
@@ -430,50 +424,6 @@ def migrate_data_to_ft_billing(start_date, end_date):
     current_app.logger.info('Total inserted/updated records = {}'.format(total_updated))
 
 
-@notify_command()
-@click.option('-s', '--service_id', required=True, type=click.UUID)
-@click.option('-d', '--day', required=True, type=click_dt(format='%Y-%m-%d'))
-def populate_redis_template_usage(service_id, day):
-    """
-    Recalculate and replace the stats in redis for a day.
-    To be used if redis data is lost for some reason.
-    """
-    if not current_app.config['REDIS_ENABLED']:
-        current_app.logger.error('Cannot populate redis template usage - redis not enabled')
-        sys.exit(1)
-
-    # the day variable is set by click to be midnight of that day
-    start_time = get_london_midnight_in_utc(day)
-    end_time = get_london_midnight_in_utc(day + timedelta(days=1))
-
-    usage = {
-        str(row.template_id): row.count
-        for row in db.session.query(
-            Notification.template_id,
-            func.count().label('count')
-        ).filter(
-            Notification.service_id == service_id,
-            Notification.created_at >= start_time,
-            Notification.created_at < end_time
-        ).group_by(
-            Notification.template_id
-        )
-    }
-    current_app.logger.info('Populating usage dict for service {} day {}: {}'.format(
-        service_id,
-        day,
-        usage.items())
-    )
-    if usage:
-        key = cache_key_for_service_template_usage_per_day(service_id, day)
-        redis_store.set_hash_and_expire(
-            key,
-            usage,
-            current_app.config['EXPIRE_CACHE_EIGHT_DAYS'],
-            raise_exception=True
-        )
-
-
 @notify_command(name='rebuild-ft-billing-for-day')
 @click.option('-s', '--service_id', required=False, type=click.UUID)
 @click.option('-d', '--day', help="The date to recalculate, as YYYY-MM-DD", required=True,

app/notifications/process_notifications.py

@@ -9,7 +9,7 @@ from notifications_utils.recipients import (
     validate_and_format_phone_number,
     format_email_address
 )
-from notifications_utils.timezones import convert_bst_to_utc, convert_utc_to_bst
+from notifications_utils.timezones import convert_bst_to_utc
 
 from app import redis_store
 from app.celery import provider_tasks
@@ -35,11 +35,7 @@ from app.dao.notifications_dao import (
 from app.dao.templates_dao import dao_get_template_by_id
 
 from app.v2.errors import BadRequestError
-from app.utils import (
-    cache_key_for_service_template_counter,
-    cache_key_for_service_template_usage_per_day,
-    get_template_instance,
-)
+from app.utils import cache_key_for_service_template_counter, get_template_instance
 
 
 def create_content_for_notification(template, personalisation):
@@ -127,23 +123,12 @@ def persist_notification(
             if redis_store.get_all_from_hash(cache_key_for_service_template_counter(service.id)):
                 redis_store.increment_hash_value(cache_key_for_service_template_counter(service.id), template_id)
 
-            increment_template_usage_cache(service.id, template_id, notification_created_at)
-
         current_app.logger.info(
             "{} {} created at {}".format(notification_type, notification_id, notification_created_at)
         )
     return notification
 
 
-def increment_template_usage_cache(service_id, template_id, created_at):
-    key = cache_key_for_service_template_usage_per_day(service_id, convert_utc_to_bst(created_at))
-    redis_store.increment_hash_value(key, template_id)
-    # set key to expire in eight days - we don't know if we've just created the key or not, so must assume that we
-    # have and reset the expiry. Eight days is longer than any notification is in the notifications table, so we'll
-    # always capture the full week's numbers
-    redis_store.expire(key, current_app.config['EXPIRE_CACHE_EIGHT_DAYS'])
-
-
 def send_notification_to_queue(notification, research_mode, queue=None):
     if research_mode or notification.key_type == KEY_TYPE_TEST:
         queue = QueueNames.RESEARCH_MODE

app/utils.py

@@ -72,13 +72,6 @@ def cache_key_for_service_template_counter(service_id, limit_days=7):
     return "{}-template-counter-limit-{}-days".format(service_id, limit_days)
 
 
-def cache_key_for_service_template_usage_per_day(service_id, datetime):
-    """
-    You should pass a BST datetime into this function
-    """
-    return "service-{}-template-usage-{}".format(service_id, datetime.date().isoformat())
-
-
 def get_public_notify_type_text(notify_type, plural=False):
     from app.models import (SMS_TYPE, UPLOAD_DOCUMENT, PRECOMPILED_LETTER)
     notify_type_text = notify_type

tests/app/commands/test_populate_redis.py

@@ -1,73 +0,0 @@
-from datetime import datetime
-
-from freezegun import freeze_time
-import pytest
-
-from app.commands import populate_redis_template_usage
-
-from tests.conftest import set_config
-from tests.app.db import create_notification, create_template, create_service
-
-
-def test_populate_redis_template_usage_does_nothing_if_redis_disabled(mocker, notify_api, sample_service):
-    mock_redis = mocker.patch('app.commands.redis_store')
-    with set_config(notify_api, 'REDIS_ENABLED', False):
-        with pytest.raises(SystemExit) as exit_signal:
-            populate_redis_template_usage.callback.__wrapped__(sample_service.id, datetime.utcnow())
-
-    assert mock_redis.mock_calls == []
-    # sys.exit with nonzero exit code
-    assert exit_signal.value.code != 0
-
-
-def test_populate_redis_template_usage_does_nothing_if_no_data(mocker, notify_api, sample_service):
-    mock_redis = mocker.patch('app.commands.redis_store')
-    with set_config(notify_api, 'REDIS_ENABLED', True):
-        populate_redis_template_usage.callback.__wrapped__(sample_service.id, datetime.utcnow())
-
-    assert mock_redis.mock_calls == []
-
-
-@freeze_time('2017-06-12')
-def test_populate_redis_template_usage_only_populates_for_today(mocker, notify_api, sample_template):
-    mock_redis = mocker.patch('app.commands.redis_store')
-    # created at in utc
-    create_notification(sample_template, created_at=datetime(2017, 6, 9, 23, 0, 0))
-    create_notification(sample_template, created_at=datetime(2017, 6, 9, 23, 0, 0))
-    create_notification(sample_template, created_at=datetime(2017, 6, 10, 0, 0, 0))
-    create_notification(sample_template, created_at=datetime(2017, 6, 10, 23, 0, 0))  # actually on 11th BST
-
-    with set_config(notify_api, 'REDIS_ENABLED', True):
-        populate_redis_template_usage.callback.__wrapped__(sample_template.service_id, datetime(2017, 6, 10))
-
-    mock_redis.set_hash_and_expire.assert_called_once_with(
-        'service-{}-template-usage-2017-06-10'.format(sample_template.service_id),
-        {str(sample_template.id): 3},
-        notify_api.config['EXPIRE_CACHE_EIGHT_DAYS'],
-        raise_exception=True
-    )
-
-
-@freeze_time('2017-06-12')
-def test_populate_redis_template_usage_only_populates_for_given_service(mocker, notify_api, notify_db_session):
-    mock_redis = mocker.patch('app.commands.redis_store')
-    # created at in utc
-    s1 = create_service(service_name='a')
-    s2 = create_service(service_name='b')
-    t1 = create_template(s1)
-    t2 = create_template(s2)
-
-    create_notification(t1, created_at=datetime(2017, 6, 10))
-    create_notification(t1, created_at=datetime(2017, 6, 10))
-
-    create_notification(t2, created_at=datetime(2017, 6, 10))
-
-    with set_config(notify_api, 'REDIS_ENABLED', True):
-        populate_redis_template_usage.callback.__wrapped__(s1.id, datetime(2017, 6, 10))
-
-    mock_redis.set_hash_and_expire.assert_called_once_with(
-        'service-{}-template-usage-2017-06-10'.format(s1.id),
-        {str(t1.id): 2},
-        notify_api.config['EXPIRE_CACHE_EIGHT_DAYS'],
-        raise_exception=True
-    )

tests/app/notifications/test_process_notification.py

@@ -213,7 +213,6 @@ def test_persist_notification_with_optionals(sample_job, sample_api_key, mocker)
 @freeze_time("2016-01-01 11:09:00.061258")
 def test_persist_notification_doesnt_touch_cache_for_old_keys_that_dont_exist(sample_template, sample_api_key, mocker):
     mock_incr = mocker.patch('app.notifications.process_notifications.redis_store.incr')
-    mock_incr_hash_value = mocker.patch('app.notifications.process_notifications.redis_store.increment_hash_value')
     mocker.patch('app.notifications.process_notifications.redis_store.get', return_value=None)
     mocker.patch('app.notifications.process_notifications.redis_store.get_all_from_hash', return_value=None)
 
@@ -229,16 +228,11 @@ def test_persist_notification_doesnt_touch_cache_for_old_keys_that_dont_exist(sa
         reference="ref"
     )
     mock_incr.assert_not_called()
-    mock_incr_hash_value.assert_called_once_with(
-        "service-{}-template-usage-2016-01-01".format(sample_template.service_id),
-        sample_template.id
-    )
 
 
 @freeze_time("2016-01-01 11:09:00.061258")
 def test_persist_notification_increments_cache_if_key_exists(sample_template, sample_api_key, mocker):
     mock_incr = mocker.patch('app.notifications.process_notifications.redis_store.incr')
-    mock_incr_hash_value = mocker.patch('app.notifications.process_notifications.redis_store.increment_hash_value')
     mocker.patch('app.notifications.process_notifications.redis_store.get', return_value=1)
     mocker.patch('app.notifications.process_notifications.redis_store.get_all_from_hash',
                  return_value={sample_template.id, 1})
@@ -255,10 +249,6 @@ def test_persist_notification_increments_cache_if_key_exists(sample_template, sa
         reference="ref2")
 
     mock_incr.assert_called_once_with(str(sample_template.service_id) + "-2016-01-01-count", )
-    assert mock_incr_hash_value.mock_calls == [
-        call("{}-template-counter-limit-7-days".format(sample_template.service_id), sample_template.id),
-        call("service-{}-template-usage-2016-01-01".format(sample_template.service_id), sample_template.id),
-    ]
 
 
 @pytest.mark.parametrize((
@@ -516,44 +506,6 @@ def test_persist_letter_notification_finds_correct_postage(
     assert persisted_notification.postage == expected_postage
 
 
-@pytest.mark.parametrize('utc_time, day_in_key', [
-    ('2016-01-01 23:00:00', '2016-01-01'),
-    ('2016-06-01 22:59:00', '2016-06-01'),
-    ('2016-06-01 23:00:00', '2016-06-02'),
-])
-def test_persist_notification_increments_and_expires_redis_template_usage(
-    utc_time,
-    day_in_key,
-    sample_template,
-    sample_api_key,
-    mocker
-):
-    mock_incr_hash_value = mocker.patch('app.notifications.process_notifications.redis_store.increment_hash_value')
-    mock_expire = mocker.patch('app.notifications.process_notifications.redis_store.expire')
-    mocker.patch('app.notifications.process_notifications.redis_store.get', return_value=None)
-    mocker.patch('app.notifications.process_notifications.redis_store.get_all_from_hash', return_value=None)
-
-    with freeze_time(utc_time):
-        persist_notification(
-            template_id=sample_template.id,
-            template_version=sample_template.version,
-            recipient='+447111111122',
-            service=sample_template.service,
-            personalisation={},
-            notification_type='sms',
-            api_key_id=sample_api_key.id,
-            key_type=sample_api_key.key_type,
-        )
-    mock_incr_hash_value.assert_called_once_with(
-        'service-{}-template-usage-{}'.format(str(sample_template.service_id), day_in_key),
-        sample_template.id
-    )
-    mock_expire.assert_called_once_with(
-        'service-{}-template-usage-{}'.format(str(sample_template.service_id), day_in_key),
-        current_app.config['EXPIRE_CACHE_EIGHT_DAYS']
-    )
-
-
 def test_persist_notification_with_billable_units_stores_correct_info(
     sample_template,
 ):