From 51b71927508eb3e4766a16c8755688fb2dc67165 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 23 Dec 2020 15:33:27 +0000 Subject: [PATCH] Cache provider lookups for 2 seconds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For every email or text message we send we have to work out which provider to send it with. Every time we do this we go and load the list of providers from the database. For emails, the result will always be the same. For text messages the result is randomly chosen to balance the load between the providers. For international text messages the result is always the same (we only have one international text message provider). This commit adds an in-memory cache with a 2 second TTL so that we’re not fetching the providers from the database every time, which should speed things up a bit. This does mean that, for text messages, the random choice will ‘stick’ for two seconds on each instance, before being re-chosen. I think this is OK because it will even out to the same distribution over time. I really don’t like having to clear the cache in the tests, so would welcome suggestions on a better way of doing this… --- app/delivery/send_to_providers.py | 6 +++++- tests/app/delivery/test_send_to_providers.py | 21 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index b22855e08..1c780e801 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -1,7 +1,7 @@ import random from urllib import parse from datetime import datetime, timedelta - +from cachetools import TTLCache, cached from flask import current_app from notifications_utils.recipients import ( validate_and_format_phone_number, @@ -148,6 +148,10 @@ def update_notification_to_sending(notification, provider): dao_update_notification(notification) +provider_cache = TTLCache(maxsize=8, ttl=2) + + +@cached(cache=provider_cache) def provider_to_use(notification_type, international=False): active_providers = [ p for p in get_provider_details_by_notification_type(notification_type, international) if p.active diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 6cf1013a4..ffde4d39b 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -47,6 +47,23 @@ def test_provider_to_use_should_return_random_provider(mocker, notify_db_session assert ret.get_name() == 'mmg' +def test_provider_to_use_should_cache_repeated_calls(mocker, notify_db_session): + mock_choices = mocker.patch( + 'app.delivery.send_to_providers.random.choices', + wraps=send_to_providers.random.choices, + ) + + send_to_providers.provider_cache.clear() + + results = [ + send_to_providers.provider_to_use('sms', international=False) + for _ in range(10) + ] + + assert all(result == results[0] for result in results) + assert len(mock_choices.call_args_list) == 1 + + def test_provider_to_use_should_only_return_mmg_for_international(mocker, notify_db_session): mmg = get_provider_details_by_identifier('mmg') mock_choices = mocker.patch('app.delivery.send_to_providers.random.choices', return_value=[mmg]) @@ -73,6 +90,8 @@ def test_provider_to_use_raises_if_no_active_providers(mocker, restore_provider_ mmg = get_provider_details_by_identifier('mmg') mmg.active = False + send_to_providers.provider_cache.clear() + with pytest.raises(Exception): send_to_providers.provider_to_use('sms', international=True) @@ -624,6 +643,7 @@ def test_should_send_sms_to_international_providers( international=True, reply_to_text=sample_template.service.get_default_sms_sender() ) + send_to_providers.provider_cache.clear() send_to_providers.send_sms_to_provider( notification_uk ) @@ -674,6 +694,7 @@ def test_should_handle_sms_sender_and_prefix_message( template = create_template(service, content='bar') notification = create_notification(template, reply_to_text=sms_sender) + send_to_providers.provider_cache.clear() send_to_providers.send_sms_to_provider(notification) mmg_client.send_sms.assert_called_once_with(