From 34ec6a3f05f7a49b261b9e13b9c56c5c94f0048a Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Mon, 27 Jun 2022 17:37:27 +0200 Subject: [PATCH 1/7] :pushpin: [open-zaak/open-zaak#1205] Pin vng-api-common to commit --- requirements/base.in | 2 +- requirements/base.txt | 17 ++++++++++++++--- requirements/ci.txt | 19 +++++++++++++++++-- requirements/dev.txt | 24 ++++++++++++++++++++++-- 4 files changed, 54 insertions(+), 8 deletions(-) diff --git a/requirements/base.in b/requirements/base.in index 85db64e9..6c7b8954 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -27,7 +27,7 @@ mozilla-django-oidc-db # API libraries djangorestframework gemma-zds-client<1.0 -git+https://github.com/VNG-Realisatie/vng-api-common.git@43779a141cfba3ecd5f90959c5d6b98c80386281#egg=vng_api_common +git+https://github.com/VNG-Realisatie/vng-api-common.git@45303c6c79198c0bd6f39a2dcc5de9012bfcb06c#egg=vng_api_common # WSGI servers & monitoring - production oriented uwsgi diff --git a/requirements/base.txt b/requirements/base.txt index c5b6e8f6..31033d09 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with python 3.9 +# This file is autogenerated by pip-compile with python 3.7 # To update, run: # # ./bin/compile_dependencies.sh @@ -22,6 +22,8 @@ boltons==21.0.0 # via # face # glom +cached-property==1.5.2 + # via kombu celery==5.2.6 # via # -r requirements/base.in @@ -149,7 +151,12 @@ imagesize==1.1.0 # via sphinx importlib-metadata==4.11.3 # via + # celery + # click + # humanize + # kombu # markdown + # redis # sphinx inflection==0.5.1 # via drf-yasg @@ -271,7 +278,11 @@ sqlparse==0.4.2 tornado==6.1 # via flower typing-extensions==4.1.1 - # via redis + # via + # asgiref + # async-timeout + # importlib-metadata + # redis uritemplate==3.0.0 # via # coreapi @@ -287,7 +298,7 @@ vine==5.0.0 # amqp # celery # kombu -vng-api-common @ git+https://github.com/VNG-Realisatie/vng-api-common.git@43779a141cfba3ecd5f90959c5d6b98c80386281 +vng_api_common @ git+https://github.com/VNG-Realisatie/vng-api-common.git@45303c6c79198c0bd6f39a2dcc5de9012bfcb06c # via -r requirements/base.in wcwidth==0.2.5 # via prompt-toolkit diff --git a/requirements/ci.txt b/requirements/ci.txt index 1f2d1e3c..8fcf3672 100644 --- a/requirements/ci.txt +++ b/requirements/ci.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with python 3.9 +# This file is autogenerated by pip-compile with python 3.7 # To update, run: # # ./bin/compile_dependencies.sh @@ -41,6 +41,10 @@ boltons==21.0.0 # -r requirements/base.txt # face # glom +cached-property==1.5.2 + # via + # -r requirements/base.txt + # kombu celery==5.2.6 # via # -r requirements/base.txt @@ -230,7 +234,13 @@ imagesize==1.1.0 importlib-metadata==4.11.3 # via # -r requirements/base.txt + # celery + # click + # flake8 + # humanize + # kombu # markdown + # redis # sphinx inflection==0.5.1 # via @@ -444,10 +454,15 @@ tornado==6.1 # via # -r requirements/base.txt # flower +typed-ast==1.5.4 + # via black typing-extensions==4.1.1 # via # -r requirements/base.txt + # asgiref + # async-timeout # black + # importlib-metadata # redis uritemplate==3.0.0 # via @@ -467,7 +482,7 @@ vine==5.0.0 # amqp # celery # kombu -vng-api-common @ git+https://github.com/VNG-Realisatie/vng-api-common.git@43779a141cfba3ecd5f90959c5d6b98c80386281 +vng_api_common @ git+https://github.com/VNG-Realisatie/vng-api-common.git@45303c6c79198c0bd6f39a2dcc5de9012bfcb06c # via -r requirements/base.txt waitress==2.1.1 # via webtest diff --git a/requirements/dev.txt b/requirements/dev.txt index 70e23d59..9cdda3de 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with python 3.9 +# This file is autogenerated by pip-compile with python 3.7 # To update, run: # # ./bin/compile_dependencies.sh @@ -45,6 +45,10 @@ boltons==21.0.0 # glom bump2version==1.0.1 # via -r requirements/dev.in +cached-property==1.5.2 + # via + # -r requirements/ci.txt + # kombu celery==5.2.6 # via # -r requirements/ci.txt @@ -250,7 +254,14 @@ imagesize==1.1.0 importlib-metadata==4.11.3 # via # -r requirements/ci.txt + # celery + # click + # flake8 + # humanize + # kombu # markdown + # pep517 + # redis # sphinx inflection==0.5.1 # via @@ -497,10 +508,18 @@ tornado==6.1 # via # -r requirements/ci.txt # flower +typed-ast==1.5.4 + # via + # -r requirements/ci.txt + # black typing-extensions==4.1.1 # via # -r requirements/ci.txt + # asgiref + # async-timeout # black + # gitpython + # importlib-metadata # redis uritemplate==3.0.0 # via @@ -520,7 +539,7 @@ vine==5.0.0 # amqp # celery # kombu -vng-api-common @ git+https://github.com/VNG-Realisatie/vng-api-common.git@43779a141cfba3ecd5f90959c5d6b98c80386281 +vng_api_common @ git+https://github.com/VNG-Realisatie/vng-api-common.git@45303c6c79198c0bd6f39a2dcc5de9012bfcb06c # via -r requirements/ci.txt waitress==2.1.1 # via @@ -548,6 +567,7 @@ zipp==3.8.0 # via # -r requirements/ci.txt # importlib-metadata + # pep517 # The following packages are considered to be unsafe in a requirements file: # pip From febd36fd35472cf3349ee01a419e8bf2d3345ff8 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Mon, 27 Jun 2022 15:57:56 +0200 Subject: [PATCH 2/7] :arrow_up: [open-zaak/open-zaak#1203] Pin vng-api-common and add git to docker image, to let CI pass for now --- requirements/base.in | 2 +- requirements/base.txt | 2 +- requirements/ci.txt | 2 +- requirements/dev.txt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/requirements/base.in b/requirements/base.in index 6c7b8954..c8cfdfce 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -27,7 +27,7 @@ mozilla-django-oidc-db # API libraries djangorestframework gemma-zds-client<1.0 -git+https://github.com/VNG-Realisatie/vng-api-common.git@45303c6c79198c0bd6f39a2dcc5de9012bfcb06c#egg=vng_api_common +git+https://github.com/VNG-Realisatie/vng-api-common.git@b2434918b1deb577263dfa2468a4bdfc440fa82a#egg=vng_api_common # WSGI servers & monitoring - production oriented uwsgi diff --git a/requirements/base.txt b/requirements/base.txt index 31033d09..50743d87 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -298,7 +298,7 @@ vine==5.0.0 # amqp # celery # kombu -vng_api_common @ git+https://github.com/VNG-Realisatie/vng-api-common.git@45303c6c79198c0bd6f39a2dcc5de9012bfcb06c +vng_api_common @ git+https://github.com/VNG-Realisatie/vng-api-common.git@b2434918b1deb577263dfa2468a4bdfc440fa82a # via -r requirements/base.in wcwidth==0.2.5 # via prompt-toolkit diff --git a/requirements/ci.txt b/requirements/ci.txt index 8fcf3672..a09242a0 100644 --- a/requirements/ci.txt +++ b/requirements/ci.txt @@ -482,7 +482,7 @@ vine==5.0.0 # amqp # celery # kombu -vng_api_common @ git+https://github.com/VNG-Realisatie/vng-api-common.git@45303c6c79198c0bd6f39a2dcc5de9012bfcb06c +vng_api_common @ git+https://github.com/VNG-Realisatie/vng-api-common.git@b2434918b1deb577263dfa2468a4bdfc440fa82a # via -r requirements/base.txt waitress==2.1.1 # via webtest diff --git a/requirements/dev.txt b/requirements/dev.txt index 9cdda3de..894b31e5 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -539,7 +539,7 @@ vine==5.0.0 # amqp # celery # kombu -vng_api_common @ git+https://github.com/VNG-Realisatie/vng-api-common.git@45303c6c79198c0bd6f39a2dcc5de9012bfcb06c +vng_api_common @ git+https://github.com/VNG-Realisatie/vng-api-common.git@b2434918b1deb577263dfa2468a4bdfc440fa82a # via -r requirements/ci.txt waitress==2.1.1 # via From cb05e202a43bbb38cb9bac7747dffa6f6a49515b Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Tue, 28 Jun 2022 16:54:29 +0200 Subject: [PATCH 3/7] :sparkles: [open-zaak/open-zaak#1206] Send email if notifs fail --- src/nrc/api/tasks.py | 60 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/src/nrc/api/tasks.py b/src/nrc/api/tasks.py index 7edf6ac7..e4fa3ba0 100644 --- a/src/nrc/api/tasks.py +++ b/src/nrc/api/tasks.py @@ -1,7 +1,13 @@ +import inspect import json import logging +from urllib.parse import urlparse +from uuid import UUID +from django.conf import settings +from django.core.mail import send_mail from django.core.serializers.json import DjangoJSONEncoder +from django.urls import reverse from django.utils.translation import gettext_lazy as _ import requests @@ -29,7 +35,7 @@ def add_autoretry_behaviour(task, **options): if autoretry_for and not hasattr(task, "_orig_run"): @wraps(task.run) - def run(*args, **kwargs): + def run(sub_id: int, msg: dict, **kwargs): config = NotificationsConfig.get_solo() max_retries = config.notification_delivery_max_retries retry_backoff = config.notification_delivery_retry_backoff @@ -38,7 +44,7 @@ def run(*args, **kwargs): task.max_retries = max_retries try: - return task._orig_run(*args, **kwargs) + return task._orig_run(sub_id, msg, **kwargs) except Ignore: # If Ignore signal occurs task shouldn't be retried, # even if it suits autoretry_for list @@ -59,11 +65,17 @@ def run(*args, **kwargs): task, "override_max_retries", max_retries ) - ret = task.retry(exc=exc, **retry_kwargs) - # Stop propagation - if hasattr(task, "override_max_retries"): - delattr(task, "override_max_retries") - raise ret + try: + ret = task.retry(exc=exc, **retry_kwargs) + # Stop propagation + if hasattr(task, "override_max_retries"): + delattr(task, "override_max_retries") + raise ret + except autoretry_for: + # Final retry failed, send email + send_email_to_admins.delay(sub_id, msg["uuid"]) + except Exception: + raise task._orig_run, task.run = task.run, run @@ -72,6 +84,38 @@ class NotificationException(Exception): pass +@app.task +def send_email_to_admins(sub_id: int, notificatie_uuid: UUID) -> None: + subscription = Abonnement.objects.get(pk=sub_id) + config = NotificationsConfig.get_solo() + + if not config.failed_notification_admin_recipients: + return + + parsed = urlparse(config.api_root) + notifications_changelist = reverse("admin:datamodel_notificatie_changelist") + + body = inspect.cleandoc( + """ + Notification {notificatie_uuid} to subscriber {sub_uuid} failed. + + See {admin_url} for more info + """.format( + notificatie_uuid=notificatie_uuid, + sub_uuid=subscription.uuid, + admin_url=f"{parsed.scheme}://{parsed.netloc}{notifications_changelist}", + ) + ) + + send_mail( + f"Failed notification - {notificatie_uuid}", + body, + settings.DEFAULT_FROM_EMAIL, + config.failed_notification_admin_recipients, + fail_silently=False, + ) + + @app.task def deliver_message(sub_id: int, msg: dict, **kwargs) -> None: """ @@ -117,7 +161,7 @@ def deliver_message(sub_id: int, msg: dict, **kwargs) -> None: notificatie_id=notificatie_id, abonnement=sub, attempt=kwargs.get("attempt", 1), - **response_init_kwargs + **response_init_kwargs, ) From b93460455757478274a2394be16fbe820106c0ee Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Tue, 28 Jun 2022 17:00:06 +0200 Subject: [PATCH 4/7] :pushpin: [open-zaak/open-zaak#1206] Pin vng-api-common --- requirements/base.in | 2 +- requirements/base.txt | 17 +++-------------- requirements/ci.txt | 19 ++----------------- requirements/dev.txt | 24 ++---------------------- 4 files changed, 8 insertions(+), 54 deletions(-) diff --git a/requirements/base.in b/requirements/base.in index c8cfdfce..fb379347 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -27,7 +27,7 @@ mozilla-django-oidc-db # API libraries djangorestframework gemma-zds-client<1.0 -git+https://github.com/VNG-Realisatie/vng-api-common.git@b2434918b1deb577263dfa2468a4bdfc440fa82a#egg=vng_api_common +git+https://github.com/VNG-Realisatie/vng-api-common.git@c870aec8ccabb8a7556426262f3c2f55800b08a8#egg=vng_api_common # WSGI servers & monitoring - production oriented uwsgi diff --git a/requirements/base.txt b/requirements/base.txt index 50743d87..0a530e5f 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with python 3.7 +# This file is autogenerated by pip-compile with python 3.9 # To update, run: # # ./bin/compile_dependencies.sh @@ -22,8 +22,6 @@ boltons==21.0.0 # via # face # glom -cached-property==1.5.2 - # via kombu celery==5.2.6 # via # -r requirements/base.in @@ -151,12 +149,7 @@ imagesize==1.1.0 # via sphinx importlib-metadata==4.11.3 # via - # celery - # click - # humanize - # kombu # markdown - # redis # sphinx inflection==0.5.1 # via drf-yasg @@ -278,11 +271,7 @@ sqlparse==0.4.2 tornado==6.1 # via flower typing-extensions==4.1.1 - # via - # asgiref - # async-timeout - # importlib-metadata - # redis + # via redis uritemplate==3.0.0 # via # coreapi @@ -298,7 +287,7 @@ vine==5.0.0 # amqp # celery # kombu -vng_api_common @ git+https://github.com/VNG-Realisatie/vng-api-common.git@b2434918b1deb577263dfa2468a4bdfc440fa82a +vng-api-common @ git+https://github.com/VNG-Realisatie/vng-api-common.git@c870aec8ccabb8a7556426262f3c2f55800b08a8 # via -r requirements/base.in wcwidth==0.2.5 # via prompt-toolkit diff --git a/requirements/ci.txt b/requirements/ci.txt index a09242a0..5886ecf7 100644 --- a/requirements/ci.txt +++ b/requirements/ci.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with python 3.7 +# This file is autogenerated by pip-compile with python 3.9 # To update, run: # # ./bin/compile_dependencies.sh @@ -41,10 +41,6 @@ boltons==21.0.0 # -r requirements/base.txt # face # glom -cached-property==1.5.2 - # via - # -r requirements/base.txt - # kombu celery==5.2.6 # via # -r requirements/base.txt @@ -234,13 +230,7 @@ imagesize==1.1.0 importlib-metadata==4.11.3 # via # -r requirements/base.txt - # celery - # click - # flake8 - # humanize - # kombu # markdown - # redis # sphinx inflection==0.5.1 # via @@ -454,15 +444,10 @@ tornado==6.1 # via # -r requirements/base.txt # flower -typed-ast==1.5.4 - # via black typing-extensions==4.1.1 # via # -r requirements/base.txt - # asgiref - # async-timeout # black - # importlib-metadata # redis uritemplate==3.0.0 # via @@ -482,7 +467,7 @@ vine==5.0.0 # amqp # celery # kombu -vng_api_common @ git+https://github.com/VNG-Realisatie/vng-api-common.git@b2434918b1deb577263dfa2468a4bdfc440fa82a +vng-api-common @ git+https://github.com/VNG-Realisatie/vng-api-common.git@c870aec8ccabb8a7556426262f3c2f55800b08a8 # via -r requirements/base.txt waitress==2.1.1 # via webtest diff --git a/requirements/dev.txt b/requirements/dev.txt index 894b31e5..40bedb5b 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with python 3.7 +# This file is autogenerated by pip-compile with python 3.9 # To update, run: # # ./bin/compile_dependencies.sh @@ -45,10 +45,6 @@ boltons==21.0.0 # glom bump2version==1.0.1 # via -r requirements/dev.in -cached-property==1.5.2 - # via - # -r requirements/ci.txt - # kombu celery==5.2.6 # via # -r requirements/ci.txt @@ -254,14 +250,7 @@ imagesize==1.1.0 importlib-metadata==4.11.3 # via # -r requirements/ci.txt - # celery - # click - # flake8 - # humanize - # kombu # markdown - # pep517 - # redis # sphinx inflection==0.5.1 # via @@ -508,18 +497,10 @@ tornado==6.1 # via # -r requirements/ci.txt # flower -typed-ast==1.5.4 - # via - # -r requirements/ci.txt - # black typing-extensions==4.1.1 # via # -r requirements/ci.txt - # asgiref - # async-timeout # black - # gitpython - # importlib-metadata # redis uritemplate==3.0.0 # via @@ -539,7 +520,7 @@ vine==5.0.0 # amqp # celery # kombu -vng_api_common @ git+https://github.com/VNG-Realisatie/vng-api-common.git@b2434918b1deb577263dfa2468a4bdfc440fa82a +vng-api-common @ git+https://github.com/VNG-Realisatie/vng-api-common.git@c870aec8ccabb8a7556426262f3c2f55800b08a8 # via -r requirements/ci.txt waitress==2.1.1 # via @@ -567,7 +548,6 @@ zipp==3.8.0 # via # -r requirements/ci.txt # importlib-metadata - # pep517 # The following packages are considered to be unsafe in a requirements file: # pip From bccd0b34787983ec3fd0f388f7fc80faa08d0f15 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Mon, 4 Jul 2022 10:32:29 +0200 Subject: [PATCH 5/7] :white_check_mark: [open-zaak/open-zaak#1206] Add test for failed notification email --- src/nrc/api/tests/test_notificatie.py | 78 ++++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/src/nrc/api/tests/test_notificatie.py b/src/nrc/api/tests/test_notificatie.py index 6a7472a2..30d72db4 100644 --- a/src/nrc/api/tests/test_notificatie.py +++ b/src/nrc/api/tests/test_notificatie.py @@ -1,5 +1,7 @@ +import inspect from unittest.mock import patch +from django.core import mail from django.test import TestCase, override_settings from django.utils.timezone import now @@ -12,7 +14,7 @@ from vng_api_common.notifications.models import NotificationsConfig from vng_api_common.tests import JWTAuthMixin -from nrc.api.tasks import deliver_message +from nrc.api.tasks import NotificationException, deliver_message, send_email_to_admins from nrc.datamodel.models import Notificatie from nrc.datamodel.tests.factories import ( AbonnementFactory, @@ -210,3 +212,77 @@ def test_notificatie_retry_use_global_config( full_jitter=False, ) self.assertEqual(deliver_message.max_retries, 4) + + @patch("nrc.api.tasks.send_email_to_admins.delay", side_effect=send_email_to_admins) + def test_notificatie_retry_email( + self, + mock_email, + mock_retry, + mock_config, + mock_get_exponential_backoff, + ): + """ + Verify that an email is sent after all retries are done + """ + mock_config.return_value = NotificationsConfig( + api_root="https://nrc.com/api/v1/", + notification_delivery_max_retries=4, + notification_delivery_retry_backoff=4, + notification_delivery_retry_backoff_max=28, + failed_notification_admin_recipients=["foo@bar.nl", "bar@baz.nl"], + ) + kanaal = KanaalFactory.create( + naam="zaken", filters=["bron", "zaaktype", "vertrouwelijkheidaanduiding"] + ) + abon = AbonnementFactory.create(callback_url="https://example.com/callback") + filter_group = FilterGroupFactory.create(kanaal=kanaal, abonnement=abon) + FilterFactory.create( + filter_group=filter_group, key="bron", value="082096752011" + ) + msg = { + "uuid": "920fc3b4-622c-45c9-b656-dee6cd463627", + "kanaal": "zaken", + "hoofdObject": "https://ref.tst.vng.cloud/zrc/api/v1/zaken/d7a22", + "resource": "status", + "resourceUrl": "https://ref.tst.vng.cloud/zrc/api/v1/statussen/d7a22/721c9", + "actie": "create", + "aanmaakdatum": now(), + "kenmerken": { + "bron": "082096752011", + "zaaktype": "example.com/api/v1/zaaktypen/5aa5c", + "vertrouwelijkheidaanduiding": "openbaar", + }, + } + + # Mock that max retries have been exceeded + mock_retry.side_effect = NotificationException() + with requests_mock.Mocker() as m: + m.post(abon.callback_url, status_code=404) + deliver_message(abon.id, msg) + + mock_email.assert_called_once_with( + abon.pk, "920fc3b4-622c-45c9-b656-dee6cd463627" + ) + + self.assertEqual(len(mail.outbox), 1) + + outbound_mail = mail.outbox[0] + notifications_changelist = reverse("admin:datamodel_notificatie_changelist") + admin_url = f"https://nrc.com{notifications_changelist}" + + self.assertEqual( + outbound_mail.subject, + "Failed notification - 920fc3b4-622c-45c9-b656-dee6cd463627", + ) + self.assertEqual( + outbound_mail.body, + inspect.cleandoc( + f""" + Notification 920fc3b4-622c-45c9-b656-dee6cd463627 to subscriber {abon.uuid} failed. + + See {admin_url} for more info + """ + ), + ) + self.assertEqual(outbound_mail.from_email, "opennotificaties@example.com") + self.assertEqual(outbound_mail.to, ["foo@bar.nl", "bar@baz.nl"]) From fbb307752cc8596450abc1dc37f1418a528ead08 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Mon, 4 Jul 2022 12:46:19 +0200 Subject: [PATCH 6/7] :sparkles: [open-zaak/open-zaak#1206] Cooldown period for failed notification emails --- src/nrc/api/tasks.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/nrc/api/tasks.py b/src/nrc/api/tasks.py index e4fa3ba0..e1989fbd 100644 --- a/src/nrc/api/tasks.py +++ b/src/nrc/api/tasks.py @@ -1,13 +1,16 @@ import inspect import json import logging +from datetime import datetime from urllib.parse import urlparse from uuid import UUID from django.conf import settings +from django.core.cache import cache from django.core.mail import send_mail from django.core.serializers.json import DjangoJSONEncoder from django.urls import reverse +from django.utils import timezone from django.utils.translation import gettext_lazy as _ import requests @@ -22,6 +25,9 @@ logger = logging.getLogger(__name__) +LATEST_EMAIL_CACHE_PREFIX = "latest_notification_email" + + def add_autoretry_behaviour(task, **options): """ Adapted from celery to use admin configurable autoretry settings @@ -84,7 +90,29 @@ class NotificationException(Exception): pass +def cooldown_period(func): + def inner(sub_id: int, notificatie_uuid: UUID): + subscription = Abonnement.objects.get(pk=sub_id) + cache_key = f"{LATEST_EMAIL_CACHE_PREFIX}:{subscription.uuid}" + current_time = timezone.now() + latest_timestamp = cache.get(cache_key) + if ( + not latest_timestamp + or ( + current_time + - timezone.make_aware(datetime.fromtimestamp(latest_timestamp)) + ).seconds + / 3600 + >= 24 + ): + cache.set(cache_key, current_time.timestamp()) + return func(sub_id, notificatie_uuid) + + return inner + + @app.task +@cooldown_period def send_email_to_admins(sub_id: int, notificatie_uuid: UUID) -> None: subscription = Abonnement.objects.get(pk=sub_id) config = NotificationsConfig.get_solo() From c52cb8cd46460c9028d08513d943f2346d7d2553 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Mon, 4 Jul 2022 12:47:08 +0200 Subject: [PATCH 7/7] :white_check_mark: [open-zaak/open-zaak#1206] Add test for failed notif email cooldown --- src/nrc/api/tests/test_notificatie.py | 79 ++++++++++++++++++++++++++- 1 file changed, 78 insertions(+), 1 deletion(-) diff --git a/src/nrc/api/tests/test_notificatie.py b/src/nrc/api/tests/test_notificatie.py index 30d72db4..f37d234f 100644 --- a/src/nrc/api/tests/test_notificatie.py +++ b/src/nrc/api/tests/test_notificatie.py @@ -1,12 +1,15 @@ import inspect +from datetime import datetime from unittest.mock import patch from django.core import mail +from django.core.cache import cache from django.test import TestCase, override_settings from django.utils.timezone import now import requests_mock from celery.exceptions import Retry +from freezegun import freeze_time from rest_framework import status from rest_framework.reverse import reverse from rest_framework.test import APITestCase @@ -14,7 +17,12 @@ from vng_api_common.notifications.models import NotificationsConfig from vng_api_common.tests import JWTAuthMixin -from nrc.api.tasks import NotificationException, deliver_message, send_email_to_admins +from nrc.api.tasks import ( + LATEST_EMAIL_CACHE_PREFIX, + NotificationException, + deliver_message, + send_email_to_admins, +) from nrc.datamodel.models import Notificatie from nrc.datamodel.tests.factories import ( AbonnementFactory, @@ -161,6 +169,7 @@ def test_notificatie_send_empty_kenmerk_value(self, mock_task): ) +@freeze_time("2022-01-01T12:00:00Z") @patch("nrc.api.tasks.get_exponential_backoff_interval") @patch("nrc.api.tasks.NotificationsConfig.get_solo") @patch("nrc.api.serializers.deliver_message.retry") @@ -264,6 +273,10 @@ def test_notificatie_retry_email( abon.pk, "920fc3b4-622c-45c9-b656-dee6cd463627" ) + self.assertEqual( + cache.get(f"{LATEST_EMAIL_CACHE_PREFIX}:{abon.uuid}"), + datetime(2022, 1, 1, 12, 0, 0).timestamp(), + ) self.assertEqual(len(mail.outbox), 1) outbound_mail = mail.outbox[0] @@ -286,3 +299,67 @@ def test_notificatie_retry_email( ) self.assertEqual(outbound_mail.from_email, "opennotificaties@example.com") self.assertEqual(outbound_mail.to, ["foo@bar.nl", "bar@baz.nl"]) + + @patch("nrc.api.tasks.send_email_to_admins.delay", side_effect=send_email_to_admins) + def test_notificatie_retry_email_cooldown_period( + self, + mock_email, + mock_retry, + mock_config, + mock_get_exponential_backoff, + ): + """ + Verify that an email is sent after all retries are done + """ + mock_config.return_value = NotificationsConfig( + api_root="https://nrc.com/api/v1/", + notification_delivery_max_retries=4, + notification_delivery_retry_backoff=4, + notification_delivery_retry_backoff_max=28, + failed_notification_admin_recipients=["foo@bar.nl", "bar@baz.nl"], + ) + kanaal = KanaalFactory.create( + naam="zaken", filters=["bron", "zaaktype", "vertrouwelijkheidaanduiding"] + ) + abon = AbonnementFactory.create(callback_url="https://example.com/callback") + filter_group = FilterGroupFactory.create(kanaal=kanaal, abonnement=abon) + FilterFactory.create( + filter_group=filter_group, key="bron", value="082096752011" + ) + msg = { + "uuid": "920fc3b4-622c-45c9-b656-dee6cd463627", + "kanaal": "zaken", + "hoofdObject": "https://ref.tst.vng.cloud/zrc/api/v1/zaken/d7a22", + "resource": "status", + "resourceUrl": "https://ref.tst.vng.cloud/zrc/api/v1/statussen/d7a22/721c9", + "actie": "create", + "aanmaakdatum": now(), + "kenmerken": { + "bron": "082096752011", + "zaaktype": "example.com/api/v1/zaaktypen/5aa5c", + "vertrouwelijkheidaanduiding": "openbaar", + }, + } + + cache.set( + f"{LATEST_EMAIL_CACHE_PREFIX}:{abon.uuid}", + datetime(2022, 1, 1, 6, 0, 0).timestamp(), + ) + + # Mock that max retries have been exceeded + mock_retry.side_effect = NotificationException() + with requests_mock.Mocker() as m: + m.post(abon.callback_url, status_code=404) + deliver_message(abon.id, msg) + + mock_email.assert_called_once_with( + abon.pk, "920fc3b4-622c-45c9-b656-dee6cd463627" + ) + + # Latest notification remains the same + self.assertEqual( + cache.get(f"{LATEST_EMAIL_CACHE_PREFIX}:{abon.uuid}"), + datetime(2022, 1, 1, 6, 0, 0).timestamp(), + ) + # No emails should be sent, since the cooldown period is active + self.assertEqual(len(mail.outbox), 0)