set 0'd dict in redis if no data exists in redis

to get the data for a day can be reasonably slow (a few hundred
milliseconds), and if someone's viewing a service with no activity we
don't want to do that query seven times every two seconds. So if there
is no data in redis, when we get the data out of the database, we
should put it in redis so we can just grab it from there next time.

This'll happen in two cases:
* redis data is deleted
* the service sent no messages that day

additionally, make sure that we convert nicely from redis' return
values (ascii strings) to unicode keys and integer counts.

app/dao/notifications_dao.py

@@ -81,10 +81,12 @@ def dao_get_template_usage(service_id, *, limit_days=None, day=None):
         Template.name,
         Template.template_type,
         Template.is_precompiled_letter,
-        notifications_aggregate_query.c.count
+        func.coalesce(notifications_aggregate_query.c.count, 0).label('count')
-    ).join(
+    ).outerjoin(
         notifications_aggregate_query,
         notifications_aggregate_query.c.template_id == Template.id
+    ).filter(
+        Template.service_id == service_id
     ).order_by(Template.name)
 
     return query.all()

app/template_statistics/rest.py

@@ -1,7 +1,8 @@
 from flask import (
     Blueprint,
     jsonify,
-    request
+    request,
+    current_app
 )
 
 from app import redis_store
@@ -61,15 +62,28 @@ def get_template_statistics_for_last_n_days(service_id, limit_days):
     template_stats_by_id = Counter()
 
     for day in last_n_days(limit_days):
+        print('\n')
         # "{SERVICE_ID}-template-usage-{YYYY-MM-DD}"
         key = cache_key_for_service_template_usage_per_day(service_id, day)
         stats = redis_store.get_all_from_hash(key)
-        if not stats:
+        if stats:
+            stats = {
+                k.decode('utf-8'): int(v) for k, v in stats.items()
+            }
+        else:
             # key didn't exist (or redis was down) - lets populate from DB.
             stats = {
                 str(row.id): row.count for row in dao_get_template_usage(service_id, day=day)
             }
-
+            # if there is data in db, but not in redis - lets put it in redis so we don't have to do
+            # this calc again next time. If there isn't any data, we can't put it in redis.
+            # Zero length hashes aren't a thing in redis. (There'll only be no data if the service has no templates)
+            if stats:
+                redis_store.set_hash_and_expire(
+                    key,
+                    stats,
+                    current_app.config['EXPIRE_CACHE_EIGHT_DAYS']
+                )
         template_stats_by_id += Counter(stats)
 
     # attach count from stats to name/type/etc from database
@@ -83,4 +97,7 @@ def get_template_statistics_for_last_n_days(service_id, limit_days):
             'is_precompiled_letter': template.is_precompiled_letter
         }
         for template in template_details
+        # we don't want to return templates with no count to the front-end,
+        # but they're returned from the DB and might be put in redis like that (if there was no data that day)
+        if template_stats_by_id[str(template.id)] != 0
     ]

tests/app/dao/notification_dao/test_notification_dao_template_usage.py

@@ -114,23 +114,28 @@ def test_template_usage_should_filter_by_service(notify_db_session):
 
     template_1 = create_template(service_1)
     template_2 = create_template(service_2)  # noqa
-    template_3 = create_template(service_3)
+    template_3a = create_template(service_3)
+    template_3b = create_template(service_3)  # noqa
 
     # two for service_1, one for service_3
     create_notification(template_1)
     create_notification(template_1)
 
-    create_notification(template_3)
+    create_notification(template_3a)
 
     res1 = dao_get_template_usage(service_1.id, limit_days=1)
     res2 = dao_get_template_usage(service_2.id, limit_days=1)
     res3 = dao_get_template_usage(service_3.id, limit_days=1)
 
     assert len(res1) == 1
-    assert len(res2) == 0
-    assert len(res3) == 1
     assert res1[0].count == 2
+
+    assert len(res2) == 1
+    assert res2[0].count == 0
+
+    assert len(res3) == 2
     assert res3[0].count == 1
+    assert res3[1].count == 0
 
 
 def test_template_usage_should_by_able_to_get_zero_count_from_notifications_history_if_no_rows(sample_service):

tests/app/template_statistics/test_rest.py

@@ -3,6 +3,7 @@ from datetime import datetime
 from unittest.mock import Mock, call, ANY
 
 import pytest
+from flask import current_app
 from freezegun import freeze_time
 
 from tests.app.db import (
@@ -11,8 +12,24 @@ from tests.app.db import (
 )
 
 
+def set_up_get_all_from_hash(mock_redis, side_effect):
+    """
+    redis returns binary strings for both keys and values - so given a list of side effects (return values),
+    make sure
+    """
+    assert type(side_effect) == list
+    side_effects = []
+    for ret_val in side_effect:
+        if ret_val is None:
+            side_effects.append(None)
+        else:
+            side_effects += [{str(k).encode('utf-8'): str(v).encode('utf-8') for k, v in ret_val.items()}]
+
+    mock_redis.get_all_from_hash.side_effect = side_effects
+
 # get_template_statistics_for_service_by_day
 
+
 @pytest.mark.parametrize('query_string', [
     {},
     {'limit_days': 0},
@@ -54,9 +71,9 @@ def test_get_template_statistics_for_service_by_day_gets_out_of_redis_if_availab
     sample_template
 ):
     mock_redis = mocker.patch('app.template_statistics.rest.redis_store')
-    mock_redis.get_all_from_hash.return_value = {
+    set_up_get_all_from_hash(mock_redis, [
-        str(sample_template.id): 3
+        {sample_template.id: 3}
-    }
+    ])
 
     json_resp = admin_request.get(
         'template_statistics.get_template_statistics_for_service_by_day',
@@ -68,7 +85,7 @@ def test_get_template_statistics_for_service_by_day_gets_out_of_redis_if_availab
     assert json_resp['data'][0]['count'] == 3
     assert json_resp['data'][0]['template_id'] == str(sample_template.id)
     mock_redis.get_all_from_hash.assert_called_once_with(
-        "service-{}-template-usage-{}".format(sample_template.service_id, '2018-01-01')
+        'service-{}-template-usage-{}'.format(sample_template.service_id, '2018-01-01')
     )
 
 
@@ -81,10 +98,10 @@ def test_get_template_statistics_for_service_by_day_goes_to_db_if_not_in_redis(
     mock_redis = mocker.patch('app.template_statistics.rest.redis_store')
 
     # first time it is called redis returns data, second time returns none
-    mock_redis.get_all_from_hash.side_effect = [
+    set_up_get_all_from_hash(mock_redis, [
-        {str(sample_template.id): 2},
+        {sample_template.id: 2},
         None
-    ]
+    ])
     mock_dao = mocker.patch(
         'app.template_statistics.rest.dao_get_template_usage',
         return_value=[
@@ -102,18 +119,20 @@ def test_get_template_statistics_for_service_by_day_goes_to_db_if_not_in_redis(
     assert json_resp['data'][0]['count'] == 5
     assert json_resp['data'][0]['template_id'] == str(sample_template.id)
     # first redis call
-    assert mock_redis.mock_calls == [
+    assert mock_redis.get_all_from_hash.mock_calls == [
-        call.get_all_from_hash(
+        call('service-{}-template-usage-{}'.format(sample_template.service_id, '2018-01-01')),
-            "service-{}-template-usage-{}".format(sample_template.service_id, '2018-01-01')
+        call('service-{}-template-usage-{}'.format(sample_template.service_id, '2018-01-02'))
-        ),
-        call.get_all_from_hash(
-            "service-{}-template-usage-{}".format(sample_template.service_id, '2018-01-02')
-        )
     ]
     # dao only called for 2nd, since redis returned values for first call
     mock_dao.assert_called_once_with(
         str(sample_template.service_id), day=datetime(2018, 1, 2)
     )
+    mock_redis.set_hash_and_expire.assert_called_once_with(
+        'service-{}-template-usage-{}'.format(sample_template.service_id, '2018-01-02'),
+        # sets the data that the dao returned
+        {str(sample_template.id): 3},
+        current_app.config['EXPIRE_CACHE_EIGHT_DAYS']
+    )
 
 
 def test_get_template_statistics_for_service_by_day_combines_templates_correctly(
@@ -127,11 +146,11 @@ def test_get_template_statistics_for_service_by_day_combines_templates_correctly
     mock_redis = mocker.patch('app.template_statistics.rest.redis_store')
 
     # first time it is called redis returns data, second time returns none
-    mock_redis.get_all_from_hash.side_effect = [
+    set_up_get_all_from_hash(mock_redis, [
-        {str(t1.id): 2},
+        {t1.id: 2},
         None,
-        {str(t1.id): 1, str(t2.id): 4},
+        {t1.id: 1, t2.id: 4},
-    ]
+    ])
     mock_dao = mocker.patch(
         'app.template_statistics.rest.dao_get_template_usage',
         return_value=[
@@ -165,15 +184,15 @@ def test_get_template_statistics_for_service_by_day_gets_stats_for_correct_days(
     mock_redis = mocker.patch('app.template_statistics.rest.redis_store')
 
     # first time it is called redis returns data, second time returns none
-    mock_redis.get_all_from_hash.side_effect = [
+    set_up_get_all_from_hash(mock_redis, [
-        {str(sample_template.id): 1},
+        {sample_template.id: 1},
         None,
-        {str(sample_template.id): 1},
+        {sample_template.id: 1},
-        {str(sample_template.id): 1},
+        {sample_template.id: 1},
-        {str(sample_template.id): 1},
+        {sample_template.id: 1},
         None,
         None,
-    ]
+    ])
     mock_dao = mocker.patch(
         'app.template_statistics.rest.dao_get_template_usage',
         return_value=[
@@ -213,6 +232,8 @@ def test_get_template_statistics_for_service_by_day_returns_empty_list_if_no_tem
     mocker,
     sample_service
 ):
+    mock_redis = mocker.patch('app.template_statistics.rest.redis_store')
+
     json_resp = admin_request.get(
         'template_statistics.get_template_statistics_for_service_by_day',
         service_id=sample_service.id,
@@ -220,6 +241,9 @@ def test_get_template_statistics_for_service_by_day_returns_empty_list_if_no_tem
     )
 
     assert len(json_resp['data']) == 0
+    assert mock_redis.get_all_from_hash.call_count == 7
+    # make sure we don't try and set any empty hashes in redis
+    assert mock_redis.set_hash_and_expire.call_count == 0
 
 # get_template_statistics_for_template