Moved the cache key to the utils module.

Renamed the dao method.

app/__init__.py

@@ -176,7 +176,3 @@ def process_user_agent(user_agent_string):
         return "non-notify-user-agent"
     else:
         return "unknown"
-
-
-def cache_key_for_service_template_counter(service_id, limit_days=7):
-    return "{}-template-counter-limit-{}-days".format(service_id, limit_days)

app/dao/templates_dao.py

@@ -1,7 +1,7 @@
 import uuid
 
-import sqlalchemy
 from sqlalchemy import desc
+from sqlalchemy.sql.expression import bindparam
 
 from app import db
 from app.models import (Template, TemplateHistory)
@@ -59,16 +59,16 @@ def dao_get_template_versions(service_id, template_id):
     ).all()
 
 
-def dao_get_templates_by_for_cache(cache):
+def dao_get_templates_for_cache(cache):
     if not cache or len(cache) == 0:
         return []
 
     # First create a subquery that is a union select of the cache values
     # Then join templates to the subquery
     cache_queries = [
-        db.session.query(sqlalchemy.sql.expression.bindparam("template_id" + str(i),
+        db.session.query(bindparam("template_id" + str(i),
-                                                             uuid.UUID(template_id.decode())).label('template_id'),
+                                   uuid.UUID(template_id.decode())).label('template_id'),
-                         sqlalchemy.sql.expression.bindparam("count" + str(i), int(count.decode())).label('count'))
+                         bindparam("count" + str(i), int(count.decode())).label('count'))
         for i, (template_id, count) in enumerate(cache)]
     cache_subq = cache_queries[0].union(*cache_queries[1:]).subquery()
     query = db.session.query(Template.id.label('template_id'),

app/notifications/process_notifications.py

@@ -2,13 +2,13 @@ from datetime import datetime
 
 from flask import current_app
 
-from app import redis_store, cache_key_for_service_template_counter
+from app import redis_store
 from app.celery import provider_tasks
 from notifications_utils.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.v2.errors import BadRequestError, SendNotificationToQueueError
-from app.utils import get_template_instance
+from app.utils import get_template_instance, cache_key_for_service_template_counter
 
 
 def create_content_for_notification(template, personalisation):

app/template_statistics/rest.py

@@ -8,9 +8,10 @@ from app import redis_store
 from app.dao.notifications_dao import (
     dao_get_template_usage,
     dao_get_last_template_usage)
-from app.dao.templates_dao import dao_get_templates_by_for_cache
+from app.dao.templates_dao import dao_get_templates_for_cache
 
 from app.schemas import notification_with_template_schema
+from app.utils import cache_key_for_service_template_counter
 
 template_statistics = Blueprint('template-statistics',
                                 __name__,
@@ -61,12 +62,12 @@ def get_template_statistics_for_template_id(service_id, template_id):
 
 
 def get_template_statistics_for_7_days(limit_days, service_id):
-    cache_key = "{}-template-counter-limit-7-days".format(service_id)
+    cache_key = cache_key_for_service_template_counter(service_id)
     template_stats_by_id = redis_store.get_all_from_hash(cache_key)
     if not template_stats_by_id:
         stats = dao_get_template_usage(service_id, limit_days=limit_days)
         cache_values = dict([(x.template_id, x.count) for x in stats])
         redis_store.set_hash_and_expire(cache_key, cache_values, current_app.config.get('EXPIRE_CACHE_IN_SECONDS', 600))
     else:
-        stats = dao_get_templates_by_for_cache(template_stats_by_id.items())
+        stats = dao_get_templates_for_cache(template_stats_by_id.items())
     return stats

app/utils.py

@@ -63,3 +63,7 @@ def get_london_month_from_utc_column(column):
         "month",
         func.timezone("Europe/London", func.timezone("UTC", column))
     )
+
+
+def cache_key_for_service_template_counter(service_id, limit_days=7):
+    return "{}-template-counter-limit-{}-days".format(service_id, limit_days)

tests/app/dao/test_templates_dao.py

@@ -5,7 +5,7 @@ from app.dao.templates_dao import (
     dao_get_all_templates_for_service,
     dao_update_template,
     dao_get_template_versions,
-    dao_get_templates_by_for_cache)
+    dao_get_templates_for_cache)
 from tests.app.conftest import sample_template as create_sample_template
 from app.models import Template, TemplateHistory
 import pytest
@@ -293,7 +293,7 @@ def test_get_templates_by_ids_successful(notify_db, notify_db_session):
     sample_cache_dict = {str.encode(str(template_1.id)): str.encode('2'),
                          str.encode(str(template_2.id)): str.encode('3')}
     cache = [[k, v] for k, v in sample_cache_dict.items()]
-    templates = dao_get_templates_by_for_cache(cache)
+    templates = dao_get_templates_for_cache(cache)
     assert len(templates) == 2
     assert [(template_1.id, template_1.template_type, template_1.name, 2),
             (template_2.id, template_2.template_type, template_2.name, 3)] == templates
@@ -309,11 +309,11 @@ def test_get_templates_by_ids_successful_for_one_cache_item(notify_db, notify_db
     )
     sample_cache_dict = {str.encode(str(template_1.id)): str.encode('2')}
     cache = [[k, v] for k, v in sample_cache_dict.items()]
-    templates = dao_get_templates_by_for_cache(cache)
+    templates = dao_get_templates_for_cache(cache)
     assert len(templates) == 1
     assert [(template_1.id, template_1.template_type, template_1.name, 2)] == templates
 
 
 def test_get_templates_by_ids_returns_empty_list():
-        assert dao_get_templates_by_for_cache({}) == []
+        assert dao_get_templates_for_cache({}) == []
-        assert dao_get_templates_by_for_cache(None) == []
+        assert dao_get_templates_for_cache(None) == []

tests/app/notifications/test_process_notification.py

@@ -7,13 +7,13 @@ from sqlalchemy.exc import SQLAlchemyError
 from freezegun import freeze_time
 from collections import namedtuple
 
-from app import cache_key_for_service_template_counter
 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,
                                                      simulated_recipient)
+from app.utils import cache_key_for_service_template_counter
 from app.v2.errors import BadRequestError
 
 
@@ -116,7 +116,7 @@ def test_exception_thown_by_redis_store_get_should_not_be_fatal(sample_template,
 
 def test_cache_is_not_incremented_on_failure_to_persist_notification(sample_api_key, mocker):
     mocked_redis = mocker.patch('app.redis_store.incr')
-    mocked_redis_hash = mocker.patch('app.redis_store.increment_hash_value')
+    mock_service_template_cache = mocker.patch('app.redis_store.increment_hash_value')
     with pytest.raises(SQLAlchemyError):
         persist_notification(template_id=None,
                              template_version=None,
@@ -127,7 +127,7 @@ def test_cache_is_not_incremented_on_failure_to_persist_notification(sample_api_
                              api_key_id=sample_api_key.id,
                              key_type=sample_api_key.key_type)
     mocked_redis.assert_not_called()
-    mocked_redis_hash.assert_not_called()
+    mock_service_template_cache.assert_not_called()
 
 
 @freeze_time("2016-01-01 11:09:00.061258")
@@ -135,7 +135,8 @@ 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')
-    mocked_redis_hash = mocker.patch('app.notifications.process_notifications.redis_store.increment_hash_value')
+    mock_service_template_cache = mocker.patch(
+        'app.notifications.process_notifications.redis_store.increment_hash_value')
     n_id = uuid.uuid4()
     created_at = datetime.datetime(2016, 11, 11, 16, 8, 18)
     persist_notification(template_id=sample_job.template.id,
@@ -158,8 +159,8 @@ def test_persist_notification_with_optionals(sample_job, sample_api_key, mocker)
     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")
-    mocked_redis_hash.assert_called_once_with(cache_key_for_service_template_counter(sample_job.service_id),
+    mock_service_template_cache.assert_called_once_with(cache_key_for_service_template_counter(sample_job.service_id),
-                                              sample_job.template.id)
+                                                        sample_job.template.id)
     assert persisted_notification.client_reference == "ref from client"
     assert persisted_notification.reference is None