Skip to content

Commit

Permalink
👽 Replace ZGWClient with ape_pie.APIClient
Browse files Browse the repository at this point in the history
  • Loading branch information
stevenbal committed Sep 27, 2024
1 parent 26f59a2 commit 410e968
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 43 deletions.
3 changes: 1 addition & 2 deletions notifications_api_common/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

from requests.exceptions import RequestException
from solo.admin import SingletonModelAdmin
from zds_client.client import ClientError

from .models import NotificationsConfig, Subscription

Expand All @@ -20,7 +19,7 @@ def register_webhook(modeladmin, request, queryset):

try:
sub.register()
except (ClientError, RequestException) as e:
except RequestException as e:
messages.error(
request,
_(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@ def create_kanaal(kanaal: str) -> None:
"""
client = NotificationsConfig.get_client()

assert client

# look up the exchange in the registry
_kanaal = next(k for k in KANAAL_REGISTRY if k.label == kanaal)

kanalen = client.list("kanaal", query_params={"naam": kanaal})
kanalen = client.get("kanaal", params={"naam": kanaal})
if kanalen:
raise KanaalExists()

Expand All @@ -35,9 +37,9 @@ def create_kanaal(kanaal: str) -> None:
f"{protocol}://{domain}{reverse('notifications:kanalen')}#{kanaal}"
)

client.create(
client.post(
"kanaal",
{
json={
"naam": kanaal,
"documentatieLink": documentation_url,
"filters": list(_kanaal.kenmerken),
Expand Down
24 changes: 12 additions & 12 deletions notifications_api_common/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
from django.db import models
from django.utils.translation import gettext_lazy as _

from ape_pie.client import APIClient
from solo.models import SingletonModel
from zds_client import Client, ClientAuth
from zgw_consumers.client import ZGWAuth, build_client
from zgw_consumers.constants import APITypes
from zgw_consumers.models import Service

Expand Down Expand Up @@ -56,13 +57,15 @@ def __str__(self):
)

@classmethod
def get_client(cls) -> Optional[Client]:
def get_client(cls) -> Optional[APIClient]:
"""
Construct a client, prepared with the required auth.
"""
config = cls.get_solo()
if config.notifications_api_service:
return config.notifications_api_service.build_client()
return build_client(
config.notifications_api_service, client_factory=APIClient
)
return None


Expand Down Expand Up @@ -110,23 +113,20 @@ def register(self) -> None:
"""
Registers the webhook with the notification component.
"""
assert (
NotificationsConfig.get_solo().notifications_api_service
), "No service for Notifications API configured"
service = NotificationsConfig.get_solo().notifications_api_service
assert service, "No service for Notifications API configured"

client = NotificationsConfig.get_client()
assert client

# This authentication is for the NC to call us. Thus, it's *not* for
# calling the NC to create a subscription.
# TODO should be replaced with `TokenAuth`
# see: maykinmedia/notifications-api-common/pull/1#discussion_r941450384
self_auth = ClientAuth(
client_id=self.client_id,
secret=self.secret,
)
self_auth = ZGWAuth(service)
data = {
"callbackUrl": self.callback_url,
"auth": self_auth.credentials()["Authorization"],
"auth": f"Bearer {self_auth._token}",
"kanalen": [
{
"naam": channel,
Expand All @@ -138,7 +138,7 @@ def register(self) -> None:
}

# register the subscriber
subscriber = client.create("abonnement", data=data)
subscriber = client.post("abonnement", json=data).json()

self._subscription = subscriber["url"]
self.save(update_fields=["_subscription"])
10 changes: 5 additions & 5 deletions notifications_api_common/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import requests
from celery import shared_task
from zds_client import ClientError

from .autoretry import add_autoretry_behaviour
from .models import NotificationsConfig
Expand All @@ -27,13 +26,14 @@ def send_notification(self, message: dict) -> None:
return

try:
client.create("notificaties", message)
response = client.post("notificaties", json=message)
response.raise_for_status()
# any unexpected errors should show up in error-monitoring, so we only
# catch ClientError exceptions
except (ClientError, requests.HTTPError) as exc:
# catch HTTPError exceptions
except requests.HTTPError as exc:
logger.warning(
"Could not deliver message to %s",
client.api_root,
client.base_url,
exc_info=exc,
extra={
"notification_msg": message,
Expand Down
1 change: 1 addition & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ install_requires =
djangorestframework_camel_case>=1.2.0
gemma-zds-client>=0.15.0
zgw-consumers[testutils]
ape-pie
tests_require =
psycopg2
pytest
Expand Down
2 changes: 1 addition & 1 deletion testapp/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,4 @@

ROOT_URLCONF = "testapp.urls"

ZGW_CONSUMERS_CLIENT_CLASS = "zgw_consumers.legacy.client.ZGWClient"
ZGW_CONSUMERS_CLIENT_CLASS = "ape_pie.APIClient"
26 changes: 16 additions & 10 deletions tests/test_register_webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,24 @@
from django.utils.translation import gettext as _

import pytest
from requests.exceptions import RequestException
from zds_client.client import ClientError
from requests import Response
from requests.exceptions import HTTPError, RequestException

from notifications_api_common.admin import register_webhook
from notifications_api_common.models import Subscription


class MockResponse:
def __init__(self, data):
self.data = data

def json(self):
return self.data


@patch(
"zds_client.client.Client.create",
return_value={"url": "https://example.com/api/v1/abonnementen/1"},
"ape_pie.APIClient.post",
return_value=MockResponse({"url": "https://example.com/api/v1/abonnementen/1"}),
)
@pytest.mark.django_db
def test_register_webhook_success(
Expand Down Expand Up @@ -47,9 +55,7 @@ def test_register_webhook_request_exception(
channels=["zaken"],
)

with patch(
"zds_client.client.Client.create", side_effect=RequestException("exception")
):
with patch("ape_pie.APIClient.post", side_effect=RequestException("exception")):
register_webhook(object, request_with_middleware, Subscription.objects.all())

messages = list(get_messages(request_with_middleware))
Expand All @@ -61,20 +67,20 @@ def test_register_webhook_request_exception(


@pytest.mark.django_db
def test_register_webhook_client_error(request_with_middleware, notifications_config):
def test_register_webhook_http_error(request_with_middleware, notifications_config):
Subscription.objects.create(
callback_url="https://example.com/callback",
client_id="client_id",
secret="secret",
channels=["zaken"],
)

with patch("zds_client.client.Client.create", side_effect=ClientError("exception")):
with patch("ape_pie.APIClient.post", side_effect=HTTPError("400")):
register_webhook(object, request_with_middleware, Subscription.objects.all())

messages = list(get_messages(request_with_middleware))

assert len(messages) == 1
assert messages[0].message == _(
"Something went wrong while registering subscription for {callback_url}: {e}"
).format(callback_url="https://example.com/callback", e="exception")
).format(callback_url="https://example.com/callback", e="400")
13 changes: 3 additions & 10 deletions tests/test_send_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import pytest
from celery.exceptions import Retry
from freezegun import freeze_time
from zgw_consumers.test import mock_service_oas_get

from notifications_api_common.tasks import NotificationException, send_notification
from testapp.models import Person
Expand Down Expand Up @@ -52,8 +51,6 @@ def test_api_create_person(api_client, notifications_config):
def test_task_send_notification_success(notifications_config, requests_mock, settings):
msg = {"foo": "bar"}
# add mocks
settings.ZGW_CONSUMERS_TEST_SCHEMA_DIRS = [TESTS_DIR / "schemas"]
mock_service_oas_get(requests_mock, NOTIFICATIONS_API_ROOT, "nrc")
requests_mock.post(f"{NOTIFICATIONS_API_ROOT}notificaties", status_code=201)

send_notification(msg)
Expand All @@ -71,8 +68,6 @@ def test_task_send_notification_with_retry(
msg = {"foo": "bar"}

# add NRC mocks
settings.ZGW_CONSUMERS_TEST_SCHEMA_DIRS = [TESTS_DIR / "schemas"]
mock_service_oas_get(requests_mock, NOTIFICATIONS_API_ROOT, "nrc")
exc = NotificationException()
requests_mock.post(f"{NOTIFICATIONS_API_ROOT}notificaties", exc=exc)

Expand All @@ -94,8 +89,6 @@ def test_task_send_notification_catch_404(
which can be handled by the lib users
"""
# add NRC mocks
settings.ZGW_CONSUMERS_TEST_SCHEMA_DIRS = [TESTS_DIR / "schemas"]
mock_service_oas_get(requests_mock, NOTIFICATIONS_API_ROOT, "nrc")
requests_mock.post(f"{NOTIFICATIONS_API_ROOT}notificaties", status_code=404)

with pytest.raises(NotificationException):
Expand All @@ -111,8 +104,6 @@ def test_task_send_notification_catch_500(
which can be handled by the lib users
"""
# add NRC mocks
settings.ZGW_CONSUMERS_TEST_SCHEMA_DIRS = [TESTS_DIR / "schemas"]
mock_service_oas_get(requests_mock, NOTIFICATIONS_API_ROOT, "nrc")
requests_mock.post(f"{NOTIFICATIONS_API_ROOT}notificaties", status_code=500)

with pytest.raises(NotificationException):
Expand All @@ -136,7 +127,9 @@ def test_api_create_person_unconfigured(api_client, notifications_config):
@freeze_time("2022-01-01")
@pytest.mark.django_db(transaction=True)
@override_settings(NOTIFICATIONS_GUARANTEE_DELIVERY=False)
def test_api_create_person_unconfigured(api_client, notifications_config):
def test_api_create_person_unconfigured_notifications_guarantee_delivery_false(
api_client, notifications_config
):
notifications_config.delete()
url = reverse("person-list")
data = {"name": "John", "address_street": "Grotestraat", "address_number": "1"}
Expand Down

0 comments on commit 410e968

Please sign in to comment.