darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2026-08-11 09:27:56 -04:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #2986 from alphagov/cache-on-tasks

Browse Source 
Use cache to improve performance of CSV processing
This commit is contained in:
Chris Hill-Scott
2021-01-19 11:15:18 +00:00
committed by GitHub
parent 9bd7607016 4eb4ea1772
commit a496ef0a97
4 changed files with 148 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
app/celery/tasks.py
View File

@@ -42,7 +42,7 @@ from app.dao.returned_letters_dao import insert_or_update_returned_letters
from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id
from app.dao.service_inbound_api_dao import get_service_inbound_api_for_service
from app.dao.service_sms_sender_dao import dao_get_service_sms_senders_by_id
from app.dao.services_dao import dao_fetch_service_by_id, fetch_todays_total_message_count
from app.dao.services_dao import fetch_todays_total_message_count
from app.dao.templates_dao import dao_get_template_by_id
from app.exceptions import DVLAException, NotificationTechnicalFailureException
from app.models import (
@@ -65,6 +65,7 @@ from app.models import (
)
from app.notifications.process_notifications import persist_notification
from app.service.utils import service_allowed_to_send_to
from app.serialised_models import SerialisedService, SerialisedTemplate
from app.utils import DATETIME_FORMAT
@@ -190,13 +191,17 @@ def save_sms(self,
encrypted_notification,
sender_id=None):
notification = encryption.decrypt(encrypted_notification)
service = dao_fetch_service_by_id(service_id)
template = dao_get_template_by_id(notification['template'], version=notification['template_version'])
service = SerialisedService.from_id(service_id)
template = SerialisedTemplate.from_id_and_service_id(
notification['template'],
service_id=service.id,
version=notification['template_version'],
)
if sender_id:
reply_to_text = dao_get_service_sms_senders_by_id(service_id, sender_id).sms_sender
else:
reply_to_text = template.get_reply_to_text()
reply_to_text = template.reply_to_text
if not service_allowed_to_send_to(notification['to'], service, KEY_TYPE_NORMAL):
current_app.logger.debug(
@@ -246,13 +251,17 @@ def save_email(self,
sender_id=None):
notification = encryption.decrypt(encrypted_notification)
service = dao_fetch_service_by_id(service_id)
template = dao_get_template_by_id(notification['template'], version=notification['template_version'])
service = SerialisedService.from_id(service_id)
template = SerialisedTemplate.from_id_and_service_id(
notification['template'],
service_id=service.id,
version=notification['template_version'],
)
if sender_id:
reply_to_text = dao_get_reply_to_by_id(service_id, sender_id).email_address
else:
reply_to_text = template.get_reply_to_text()
reply_to_text = template.reply_to_text
if not service_allowed_to_send_to(notification['to'], service, KEY_TYPE_NORMAL):
current_app.logger.info("Email {} failed as restricted service".format(notification_id))
@@ -300,7 +309,7 @@ def save_api_sms(self, encrypted_notification):
def save_api_email_or_sms(self, encrypted_notification):
notification = encryption.decrypt(encrypted_notification)
service = dao_fetch_service_by_id(notification['service_id'])
service = SerialisedService.from_id(notification['service_id'])
q = QueueNames.SEND_EMAIL if notification['notification_type'] == EMAIL_TYPE else QueueNames.SEND_SMS
provider_task = provider_tasks.deliver_email if notification['notification_type'] == EMAIL_TYPE \
else provider_tasks.deliver_sms
@@ -356,8 +365,12 @@ def save_letter(
Columns(notification['personalisation'])
)
service = dao_fetch_service_by_id(service_id)
template = dao_get_template_by_id(notification['template'], version=notification['template_version'])
service = SerialisedService.from_id(service_id)
template = SerialisedTemplate.from_id_and_service_id(
notification['template'],
service_id=service.id,
version=notification['template_version'],
)
try:
# if we don't want to actually send the letter, then start it off in SENDING so we don't pick it up
@@ -378,7 +391,7 @@ def save_letter(
job_row_number=notification['row_number'],
notification_id=notification_id,
reference=create_random_identifier(),
reply_to_text=template.get_reply_to_text(),
reply_to_text=template.reply_to_text,
status=status
)

11
app/serialised_models.py
View File

@@ -51,18 +51,19 @@ class SerialisedTemplate(SerialisedModel):
@classmethod
@memory_cache
def from_id_and_service_id(cls, template_id, service_id):
return cls(cls.get_dict(template_id, service_id)['data'])
def from_id_and_service_id(cls, template_id, service_id, version=None):
return cls(cls.get_dict(template_id, service_id, version)['data'])
@staticmethod
@redis_cache.set('service-{service_id}-template-{template_id}-version-None')
def get_dict(template_id, service_id):
@redis_cache.set('service-{service_id}-template-{template_id}-version-{version}')
def get_dict(template_id, service_id, version):
from app.dao import templates_dao
from app.schemas import template_schema
fetched_template = templates_dao.dao_get_template_by_id_and_service_id(
template_id=template_id,
service_id=service_id
service_id=service_id,
version=version,
)
template_dict = template_schema.dump(fetched_template).data

117
tests/app/celery/test_tasks.py
View File

@@ -49,6 +49,7 @@ from app.models import (
SMS_TYPE,
ReturnedLetter,
NOTIFICATION_CREATED)
from app.serialised_models import SerialisedService, SerialisedTemplate
from app.utils import DATETIME_FORMAT
from tests.app import load_example_csv
@@ -1888,3 +1889,119 @@ def test_save_api_email_dont_retry_if_notification_already_exists(sample_service
assert notifications[0].created_at == datetime(2020, 3, 25, 14, 30)
# should only have sent the notification once.
mock_provider_task.assert_called_once_with([data['id']], queue=expected_queue)
@pytest.mark.parametrize('task_function, delivery_mock, recipient, template_args', (
(
save_email,
'app.celery.provider_tasks.deliver_email.apply_async',
'test@example.com',
{'template_type': 'email', 'subject': 'Hello'},
), (
save_sms,
'app.celery.provider_tasks.deliver_sms.apply_async',
'07700 900890',
{'template_type': 'sms'}
), (
save_letter,
'app.celery.letters_pdf_tasks.get_pdf_for_templated_letter.apply_async',
'123 Example Street\nCity of Town\nXM4 5HQ',
{'template_type': 'letter', 'subject': 'Hello'}
),
))
def test_save_tasks_use_cached_service_and_template(
notify_db_session,
mocker,
task_function,
delivery_mock,
recipient,
template_args,
):
service = create_service()
template = create_template(service=service, **template_args)
notification = _notification_json(template, to=recipient)
delivery_mock = mocker.patch(delivery_mock)
service_dict_mock = mocker.patch(
'app.serialised_models.SerialisedService.get_dict',
wraps=SerialisedService.get_dict,
)
template_dict_mock = mocker.patch(
'app.serialised_models.SerialisedTemplate.get_dict',
wraps=SerialisedTemplate.get_dict,
)
for _ in range(3):
task_function(
service.id,
uuid.uuid4(),
encryption.encrypt(notification),
)
# We talk to the database once for the service and once for the
# template; subsequent calls are caught by the in memory cache
assert service_dict_mock.call_args_list == [
call(service.id),
]
assert template_dict_mock.call_args_list == [
call(str(template.id), str(service.id), 1),
]
# But we save 3 notifications and enqueue 3 tasks
assert len(Notification.query.all()) == 3
assert len(delivery_mock.call_args_list) == 3
@freeze_time('2020-03-25 14:30')
@pytest.mark.parametrize('notification_type, task_function, expected_queue, recipient', (
('sms', save_api_sms, QueueNames.SEND_SMS, '+447700900855'),
('email', save_api_email, QueueNames.SEND_EMAIL, 'jane.citizen@example.com'),
))
def test_save_api_tasks_use_cache(
sample_service,
mocker,
notification_type,
task_function,
expected_queue,
recipient,
):
mock_provider_task = mocker.patch(
f'app.celery.provider_tasks.deliver_{notification_type}.apply_async'
)
service_dict_mock = mocker.patch(
'app.serialised_models.SerialisedService.get_dict',
wraps=SerialisedService.get_dict,
)
template = create_template(sample_service, template_type=notification_type)
api_key = create_api_key(service=template.service)
def create_encrypted_notification():
return encryption.encrypt({
"to": recipient,
"id": str(uuid.uuid4()),
"template_id": str(template.id),
"template_version": template.version,
"service_id": str(template.service_id),
"personalisation": None,
"notification_type": template.template_type,
"api_key_id": str(api_key.id),
"key_type": api_key.key_type,
"client_reference": 'our email',
"reply_to_text": "our.email@gov.uk",
"document_download_count": 0,
"status": NOTIFICATION_CREATED,
"created_at": datetime.utcnow().strftime(DATETIME_FORMAT),
})
assert len(Notification.query.all()) == 0
for _ in range(3):
task_function(encrypted_notification=create_encrypted_notification())
assert service_dict_mock.call_args_list == [
call(str(template.service_id))
]
assert len(Notification.query.all()) == 3
assert len(mock_provider_task.call_args_list) == 3

2
tests/app/v2/notifications/test_post_notifications.py
View File

@@ -233,7 +233,7 @@ def test_should_cache_template_lookups_in_memory(mocker, client, sample_template
assert mock_get_template.call_count == 1
assert mock_get_template.call_args_list == [
call(service_id=str(sample_template.service_id), template_id=str(sample_template.id))
call(service_id=str(sample_template.service_id), template_id=str(sample_template.id), version=None)
]
assert Notification.query.count() == 5