From 5e76dbb6a7aeb478ff1782793bc206c3504f937f Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Tue, 28 May 2019 16:23:23 +0100
Subject: [PATCH] Cache known domains in Redis
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The domains lookup is a bit slow because it’s serialising all the
organisations in the database. Since we’re putting this in the sign up
flow it should feel snappy, so lets cache just the domain bit of it in
Redis.
---
 app/notify_client/organisations_api_client.py |  4 +-
 .../notify_client/test_organisation_client.py | 94 +++++++++++++++++++
 2 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100644 tests/app/notify_client/test_organisation_client.py

diff --git a/app/notify_client/organisations_api_client.py b/app/notify_client/organisations_api_client.py
index 1d18ebaa8..ecec12359 100644
--- a/app/notify_client/organisations_api_client.py
+++ b/app/notify_client/organisations_api_client.py
@@ -10,8 +10,9 @@ class OrganisationsClient(NotifyAdminAPIClient):
     def get_organisations(self):
         return self.get(url='/organisations')
 
+    @cache.set('domains')
     def get_domains(self):
-        return set(chain.from_iterable(
+        return list(chain.from_iterable(
             organisation['domains']
             for organisation in self.get_organisations()
         ))
@@ -35,6 +36,7 @@ class OrganisationsClient(NotifyAdminAPIClient):
         }
         return self.post(url="/organisations", data=data)
 
+    @cache.delete('domains')
     def update_organisation(self, org_id, **kwargs):
         return self.post(url="/organisations/{}".format(org_id), data=kwargs)
 
diff --git a/tests/app/notify_client/test_organisation_client.py b/tests/app/notify_client/test_organisation_client.py
new file mode 100644
index 000000000..7839cbdad
--- /dev/null
+++ b/tests/app/notify_client/test_organisation_client.py
@@ -0,0 +1,94 @@
+from unittest.mock import call
+
+import pytest
+
+from app import organisations_client
+
+
+@pytest.mark.parametrize(
+    (
+        'expected_cache_get_calls,'
+        'cache_value,'
+        'expected_api_calls,'
+        'expected_cache_set_calls,'
+        'expected_return_value,'
+    ),
+    [
+        (
+            [
+                call('domains')
+            ],
+            b"""
+                [
+                    {"domains": ["a", "b", "c"]},
+                    {"domains": ["c", "d", "e"]}
+                ]
+            """,
+            [],
+            [],
+            ['a', 'b', 'c', 'd', 'e'],
+        ),
+        (
+            [
+                call('domains')
+            ],
+            None,
+            [
+                call(url='/organisations')
+            ],
+            [
+                call(
+                    'domains',
+                    '["x", "y", "z"]',
+                    ex=604800
+                )
+            ],
+            'from api',
+        ),
+    ]
+)
+def test_returns_value_from_cache(
+    app_,
+    mocker,
+    expected_cache_get_calls,
+    cache_value,
+    expected_return_value,
+    expected_api_calls,
+    expected_cache_set_calls,
+):
+
+    mock_redis_get = mocker.patch(
+        'app.extensions.RedisClient.get',
+        return_value=cache_value,
+    )
+    mock_api_get = mocker.patch(
+        'app.notify_client.NotifyAdminAPIClient.get',
+        return_value=[
+            {'domains': ['x', 'y', 'z']}
+        ],
+    )
+    mock_redis_set = mocker.patch(
+        'app.extensions.RedisClient.set',
+    )
+
+    organisations_client.get_domains()
+
+    assert mock_redis_get.call_args_list == expected_cache_get_calls
+    assert mock_api_get.call_args_list == expected_api_calls
+    assert mock_redis_set.call_args_list == expected_cache_set_calls
+
+
+def test_deletes_domain_cache(
+    app_,
+    mock_get_user,
+    mocker,
+    fake_uuid,
+):
+    mocker.patch('app.notify_client.current_user', id='1')
+    mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
+    mock_request = mocker.patch('notifications_python_client.base.BaseAPIClient.request')
+
+    organisations_client.update_organisation(fake_uuid, foo='bar')
+
+    assert call('domains') in mock_redis_delete.call_args_list
+    assert len(mock_request.call_args_list) == 1