Cache organisations, not just domains in Redis

This should make the ‘All organisations’ page load a lil’ bit quicker.
Still worth caching the domains separately so the response is smaller
when we only care about domains. This is because the code that uses the
domains is part of the sign up flow, so it’s really important that it’s
snappy.
This commit is contained in:
Chris Hill-Scott
2019-06-14 11:10:57 +01:00
parent c7325576d6
commit 8f9ade7a8b
4 changed files with 87 additions and 15 deletions

View File

@@ -417,7 +417,12 @@ def clear_cache():
('letter_branding', [
'letter_branding',
'letter_branding-????????-????-????-????-????????????',
])
]),
('organisation', [
'organisations',
'domains',
'live-service-and-organisation-counts',
]),
])
form = ClearCacheForm()

View File

@@ -7,6 +7,7 @@ from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache
class OrganisationsClient(NotifyAdminAPIClient):
@cache.set('organisations')
def get_organisations(self):
return self.get(url='/organisations')
@@ -30,6 +31,7 @@ class OrganisationsClient(NotifyAdminAPIClient):
return None
raise error
@cache.delete('organisations')
def create_organisation(self, name):
data = {
"name": name
@@ -37,6 +39,7 @@ class OrganisationsClient(NotifyAdminAPIClient):
return self.post(url="/organisations", data=data)
@cache.delete('domains')
@cache.delete('organisations')
def update_organisation(self, org_id, **kwargs):
return self.post(url="/organisations/{}".format(org_id), data=kwargs)

View File

@@ -900,24 +900,42 @@ def test_clear_cache_shows_form(client_request, platform_admin_user, mocker):
assert page.select('input[type=radio]')[0]['value'] == 'user'
assert page.select('input[type=radio]')[1]['value'] == 'service'
assert page.select('input[type=radio]')[2]['value'] == 'template'
assert page.select('input[type=radio]')[3]['value'] == 'email_branding'
assert page.select('input[type=radio]')[4]['value'] == 'letter_branding'
assert page.select('input[type=radio]')[5]['value'] == 'organisation'
assert not redis.delete_cache_keys_by_pattern.called
def test_clear_cache_submits_and_tells_you_how_many_things_were_deleted(client_request, platform_admin_user, mocker):
@pytest.mark.parametrize('model_type, expected_calls, expected_confirmation', (
('template', [
call('service-????????-????-????-????-????????????-templates'),
call('template-????????-????-????-????-????????????-version-*'),
call('template-????????-????-????-????-????????????-versions'),
], 'Removed 3 template objects from redis'),
('organisation', [
call('organisations'),
call('domains'),
call('live-service-and-organisation-counts'),
], 'Removed 3 organisation objects from redis'),
))
def test_clear_cache_submits_and_tells_you_how_many_things_were_deleted(
client_request,
platform_admin_user,
mocker,
model_type,
expected_calls,
expected_confirmation,
):
redis = mocker.patch('app.main.views.platform_admin.redis_client')
redis.delete_cache_keys_by_pattern.side_effect = [0, 3, 1]
client_request.login(platform_admin_user)
page = client_request.post('main.clear_cache', _data={'model_type': 'template'}, _expected_status=200)
page = client_request.post('main.clear_cache', _data={'model_type': model_type}, _expected_status=200)
assert redis.delete_cache_keys_by_pattern.call_args_list == [
call('service-????????-????-????-????-????????????-templates'),
call('template-????????-????-????-????-????????????-version-*'),
call('template-????????-????-????-????-????????????-versions'),
]
assert redis.delete_cache_keys_by_pattern.call_args_list == expected_calls
flash_banner = page.find('div', class_='banner-default')
assert flash_banner.text.strip() == 'Removed 3 template objects from redis'
assert flash_banner.text.strip() == expected_confirmation
def test_clear_cache_requires_option(client_request, platform_admin_user, mocker):

View File

@@ -7,6 +7,7 @@ from app import organisations_client
@pytest.mark.parametrize(
(
'client_method,'
'expected_cache_get_calls,'
'cache_value,'
'expected_api_calls,'
@@ -15,13 +16,14 @@ from app import organisations_client
),
[
(
'get_domains',
[
call('domains')
call('domains'),
],
b"""
[
{"domains": ["a", "b", "c"]},
{"domains": ["c", "d", "e"]}
{"name": "org 1", "domains": ["a", "b", "c"]},
{"name": "org 2", "domains": ["c", "d", "e"]}
]
""",
[],
@@ -29,19 +31,62 @@ from app import organisations_client
['a', 'b', 'c', 'd', 'e'],
),
(
'get_domains',
[
call('domains')
call('domains'),
call('organisations'),
],
None,
[
call(url='/organisations')
],
[
call(
'organisations',
'[{"domains": ["x", "y", "z"]}]',
ex=604800,
),
call(
'domains',
'["x", "y", "z"]',
ex=604800
)
),
],
'from api',
),
(
'get_organisations',
[
call('organisations'),
],
b"""
[
{"name": "org 1", "domains": ["a", "b", "c"]},
{"name": "org 2", "domains": ["c", "d", "e"]}
]
""",
[],
[],
[
{"name": "org 1", "domains": ["a", "b", "c"]},
{"name": "org 2", "domains": ["c", "d", "e"]}
],
),
(
'get_organisations',
[
call('organisations'),
],
None,
[
call(url='/organisations')
],
[
call(
'organisations',
'[{"domains": ["x", "y", "z"]}]',
ex=604800,
),
],
'from api',
),
@@ -50,6 +95,7 @@ from app import organisations_client
def test_returns_value_from_cache(
app_,
mocker,
client_method,
expected_cache_get_calls,
cache_value,
expected_return_value,
@@ -71,7 +117,7 @@ def test_returns_value_from_cache(
'app.extensions.RedisClient.set',
)
organisations_client.get_domains()
getattr(organisations_client, client_method)()
assert mock_redis_get.call_args_list == expected_cache_get_calls
assert mock_api_get.call_args_list == expected_api_calls