diff --git a/CHANGELOG.rst b/CHANGELOG.rst index cfd84a0..9805ab1 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,6 +2,11 @@ Changelog ========= +0.3.1 (2024-10-27) +------------------ + +* Fixed kanalen not being registered. This regression was introduced in `0.3.0`. + 0.3.0 (2024-10-24) ------------------ diff --git a/notifications_api_common/management/commands/register_kanalen.py b/notifications_api_common/management/commands/register_kanalen.py index 4b65e36..b766f4f 100644 --- a/notifications_api_common/management/commands/register_kanalen.py +++ b/notifications_api_common/management/commands/register_kanalen.py @@ -1,9 +1,14 @@ import logging +from typing import Optional from django.contrib.sites.models import Site from django.core.management.base import BaseCommand from django.urls import reverse +from requests import Response +from requests.exceptions import JSONDecodeError, RequestException +from zgw_consumers.models import Service + from ...kanalen import KANAAL_REGISTRY from ...models import NotificationsConfig from ...settings import get_setting @@ -11,11 +16,39 @@ logger = logging.getLogger(__name__) -class KanaalExists(Exception): - pass +class KanaalException(Exception): + kanaal: str + data: dict | list + service: Service + + def __init__( + self, kanaal: str, service: Service, data: Optional[dict | list] = None + ): + super().__init__() + + self.kanaal = kanaal + self.service = service + self.data = data or {} + + +class KanaalRequestException(KanaalException): + def __str__(self) -> str: + return ( + f"Unable to retrieve kanaal {self.kanaal} from {self.service}: {self.data}" + ) + + +class KanaalCreateException(KanaalException): + def __str__(self) -> str: + return f"Unable to create kanaal {self.kanaal} at {self.service}: {self.data}" -def create_kanaal(kanaal: str) -> None: +class KanaalExistsException(KanaalException): + def __str__(self) -> str: + return f"Kanaal '{self.kanaal}' already exists within {self.service}" + + +def create_kanaal(kanaal: str, service: Service) -> None: """ Create a kanaal, if it doesn't exist yet. """ @@ -26,9 +59,19 @@ def create_kanaal(kanaal: str) -> None: # look up the exchange in the registry _kanaal = next(k for k in KANAAL_REGISTRY if k.label == kanaal) - kanalen = client.get("kanaal", params={"naam": kanaal}) + response_data = [] + + try: + response: Response = client.get("kanaal", params={"naam": kanaal}) + kanalen: list[dict] = response.json() or [] + response.raise_for_status() + except (RequestException, JSONDecodeError) as exception: + raise KanaalRequestException( + kanaal=kanaal, service=service, data=response_data + ) from exception + if kanalen: - raise KanaalExists() + raise KanaalExistsException(kanaal=kanaal, service=service, data=response_data) # build up own documentation URL domain = Site.objects.get_current().domain @@ -37,14 +80,22 @@ def create_kanaal(kanaal: str) -> None: f"{protocol}://{domain}{reverse('notifications:kanalen')}#{kanaal}" ) - client.post( - "kanaal", - json={ - "naam": kanaal, - "documentatieLink": documentation_url, - "filters": list(_kanaal.kenmerken), - }, - ) + try: + response: Response = client.post( + "kanaal", + json={ + "naam": kanaal, + "documentatieLink": documentation_url, + "filters": list(_kanaal.kenmerken), + }, + ) + + response_data: dict = response.json() or {} + response.raise_for_status() + except (RequestException, JSONDecodeError) as exception: + raise KanaalCreateException( + kanaal=kanaal, service=service, data=response_data + ) from exception class Command(BaseCommand): @@ -69,7 +120,7 @@ def handle(self, **options): "`notifications_api_service` configured" ) - api_root = config.notifications_api_service.api_root + service = config.notifications_api_service # use CLI arg or fall back to setting kanalen = options["kanalen"] or sorted( @@ -78,7 +129,9 @@ def handle(self, **options): for kanaal in kanalen: try: - create_kanaal(kanaal) - self.stdout.write(f"Registered kanaal '{kanaal}' with {api_root}") - except KanaalExists: - self.stderr.write(f"Kanaal '{kanaal}' already exists within {api_root}") + create_kanaal(kanaal, service) + self.stdout.write( + f"Registered kanaal '{kanaal}' with {service.api_root}" + ) + except (KanaalException,) as exception: + self.stderr.write(f"{str(exception)} . Skipping..") diff --git a/testapp/settings.py b/testapp/settings.py index cad3247..1daf370 100644 --- a/testapp/settings.py +++ b/testapp/settings.py @@ -25,6 +25,7 @@ "django.contrib.contenttypes", "django.contrib.auth", "django.contrib.sessions", + "django.contrib.sites", "django.contrib.admin", "django.contrib.messages", "django.contrib.staticfiles", @@ -64,3 +65,5 @@ ] ROOT_URLCONF = "testapp.urls" + +SITE_ID = 1 diff --git a/tests/test_register_kanalen.py b/tests/test_register_kanalen.py new file mode 100644 index 0000000..8a38f5e --- /dev/null +++ b/tests/test_register_kanalen.py @@ -0,0 +1,191 @@ +from io import StringIO +from unittest.mock import Mock, patch +from urllib.parse import urlencode + +from django.test.testcases import call_command + +import pytest + +from notifications_api_common.kanalen import KANAAL_REGISTRY, Kanaal + +kanalen = set( + ( + Kanaal(label="foobar", main_resource=Mock()), + Kanaal(label="boofar", main_resource=Mock()), + ) +) + +KANAAL_REGISTRY.clear() +KANAAL_REGISTRY.update(kanalen) + + +@pytest.mark.django_db +def test_register_kanalen_success(notifications_config, requests_mock): + kanaal_url = f"{notifications_config.notifications_api_service.api_root}kanaal" + params = urlencode(dict(naam="foobar")) + + requests_mock.get(f"{kanaal_url}?{params}", json=[]) + + requests_mock.post( + kanaal_url, + json={ + "url": "http://example.com", + "naam": "string", + "documentatieLink": "http://example.com", + "filters": ["string"], + }, + status_code=201, + ) + + reverse_patch = ( + "notifications_api_common.management.commands.register_kanalen.reverse" + ) + + with patch(reverse_patch) as mocked_reverse: + mocked_reverse.return_value = "/notifications/kanalen" + + call_command("register_kanalen", kanalen=["foobar"]) + + assert len(requests_mock.request_history) == 2 + + get_request = requests_mock.request_history[0] + + assert get_request._request.url == f"{kanaal_url}?{params}" + + post_request = requests_mock.request_history[1] + + assert post_request._request.url == kanaal_url + + +@pytest.mark.django_db +def test_register_kanalen_from_registry_success(notifications_config, requests_mock): + kanaal_url = f"{notifications_config.notifications_api_service.api_root}kanaal" + + url_mapping = { + kanaal.label: f"{kanaal_url}?{urlencode(dict(naam=kanaal.label))}" + for kanaal in kanalen + } + + for kanaal in kanalen: + requests_mock.get(url_mapping[kanaal.label], json=[]) + + requests_mock.post( + kanaal_url, + json={ + "url": "http://example.com", + "naam": kanaal.label, + "documentatieLink": "http://example.com", + "filters": ["string"], + }, + status_code=201, + ) + + reverse_patch = ( + "notifications_api_common.management.commands.register_kanalen.reverse" + ) + + with patch(reverse_patch) as mocked_reverse: + mocked_reverse.return_value = "/notifications/kanalen" + + call_command("register_kanalen") + + assert len(requests_mock.request_history) == 4 + + # kanalen are sorted by label + first_get_request = requests_mock.request_history[0] + assert first_get_request._request.url == url_mapping["boofar"] + + first_post_request = requests_mock.request_history[1] + assert first_post_request._request.url == kanaal_url + + second_get_request = requests_mock.request_history[2] + assert second_get_request._request.url == url_mapping["foobar"] + + second_post_request = requests_mock.request_history[3] + assert second_post_request._request.url == kanaal_url + + +@pytest.mark.django_db +def test_register_kanalen_existing_kanalen(notifications_config, requests_mock): + """ + Test that already registered kanalen does not cause issues + """ + kanaal_url = f"{notifications_config.notifications_api_service.api_root}kanaal" + params = urlencode(dict(naam="foobar")) + + requests_mock.get( + f"{kanaal_url}?{params}", + json=[ + { + "url": "http://example.com", + "naam": "foobar", + "documentatieLink": "http://example.com", + "filters": ["string"], + } + ], + ) + + call_command("register_kanalen", kanalen=["foobar"]) + + assert len(requests_mock.request_history) == 1 + + request = requests_mock.request_history[0] + + assert request._request.url == f"{kanaal_url}?{params}" + + +@pytest.mark.django_db +def test_register_kanalen_unknown_url(notifications_config, requests_mock): + kanaal_url = f"{notifications_config.notifications_api_service.api_root}kanaal" + params = urlencode(dict(naam="foobar")) + + requests_mock.get(f"{kanaal_url}?{params}", status_code=404) + + stderr = StringIO() + + call_command("register_kanalen", kanalen=["foobar"], stderr=stderr) + + partial_failure_message = "Unable to retrieve kanaal foobar" + + assert partial_failure_message in stderr.getvalue() + + assert len(requests_mock.request_history) == 1 + + request = requests_mock.request_history[0] + + assert request._request.url == f"{kanaal_url}?{params}" + + +@pytest.mark.django_db +def test_register_kanalen_incorrect_post(notifications_config, requests_mock): + kanaal_url = f"{notifications_config.notifications_api_service.api_root}kanaal" + params = urlencode(dict(naam="foobar")) + + requests_mock.get(f"{kanaal_url}?{params}", json=[]) + + requests_mock.post(kanaal_url, json={"error": "foo"}, status_code=400) + + stderr = StringIO() + + reverse_patch = ( + "notifications_api_common.management.commands.register_kanalen.reverse" + ) + + with patch(reverse_patch) as mocked_reverse: + mocked_reverse.return_value = "/notifications/kanalen" + + call_command("register_kanalen", kanalen=["foobar"], stderr=stderr) + + partial_failure_message = "Unable to create kanaal foobar" + + assert partial_failure_message in stderr.getvalue() + + assert len(requests_mock.request_history) == 2 + + get_request = requests_mock.request_history[0] + + assert get_request._request.url == f"{kanaal_url}?{params}" + + post_request = requests_mock.request_history[1] + + assert post_request._request.url == kanaal_url diff --git a/tests/test_register_webhook.py b/tests/test_register_webhook.py index 346ac43..2b651cb 100644 --- a/tests/test_register_webhook.py +++ b/tests/test_register_webhook.py @@ -4,7 +4,6 @@ from django.utils.translation import gettext as _ import pytest -from requests import Response from requests.exceptions import HTTPError, RequestException from notifications_api_common.admin import register_webhook