Merge pull request #17 from open-formulieren/feature/3489-update-for-…

…new-api-clients

Use new client approach in refactored Open Forms

.github/workflows/ci.yaml

@@ -39,6 +39,7 @@ jobs:
         with:
           repository: open-formulieren/open-forms
           path: open-forms
+          ref: refactor/3489-zgw-and-objects-api
 
       - name: Checkout Haal Centraal HR extension
         uses: actions/checkout@v3

prefill_haalcentraalhr/client.py

@@ -0,0 +1,23 @@
+# Shipped in Open Forms
+from openforms.contrib.hal_client import HALClient
+from openforms.pre_requests.clients import PreRequestMixin
+from zgw_consumers_ext.api_client import ServiceClientFactory
+
+from .models import HaalCentraalHRConfig
+
+
+class NoServiceConfigured(RuntimeError):
+    pass
+
+
+def get_client(**kwargs) -> "Client":
+    config = HaalCentraalHRConfig.get_solo()
+    assert isinstance(config, HaalCentraalHRConfig)
+    if not (service := config.service):
+        raise NoServiceConfigured("No service configured!")
+    service_client_factory = ServiceClientFactory(service)
+    return Client.configure_from(service_client_factory, **kwargs)
+
+
+class Client(PreRequestMixin, HALClient):
+    pass

prefill_haalcentraalhr/models.py

@@ -3,8 +3,6 @@
 from django.db import models
 from django.utils.translation import gettext_lazy as _
 
-from openforms.pre_requests.clients import PreRequestClientContext, PreRequestZGWClient
-from openforms.submissions.models import Submission
 from solo.models import SingletonModel
 from zgw_consumers.constants import APITypes
 
@@ -13,7 +11,12 @@
 
 class HaalCentraalHRConfigManager(models.Manager):
     def get_queryset(self):
-        return super().get_queryset().select_related("service")
+        qs = super().get_queryset()
+        return qs.select_related(
+            "service",
+            "service__server_certificate",
+            "service__client_certificate",
+        )
 
 
 class HaalCentraalHRConfig(SingletonModel):
@@ -32,16 +35,3 @@ class HaalCentraalHRConfig(SingletonModel):
 
     class Meta:
         verbose_name = _("Haal Centraal HR configuration")
-
-    def build_client(
-        self, submission: Submission | None = None
-    ) -> PreRequestZGWClient | None:
-        if not self.service:
-            logger.info("No service configured for Haal Centraal HR.")
-            return
-
-        client = self.service.build_client()
-        if submission:
-            client.context = PreRequestClientContext(submission=submission)
-
-        return client

prefill_haalcentraalhr/plugin.py

@@ -2,26 +2,24 @@
 
 from django.utils.translation import gettext_lazy as _
 
+import requests
 from glom import GlomError, glom
 from openforms.authentication.constants import AuthAttribute
 from openforms.plugins.exceptions import InvalidPluginConfiguration
+from openforms.pre_requests.clients import PreRequestClientContext
 from openforms.prefill.base import BasePlugin
 from openforms.prefill.constants import IdentifierRoles
 from openforms.prefill.registry import register
 from openforms.submissions.models import Submission
 from openforms.typing import JSONObject
 from zds_client import ClientError
 
+from .client import NoServiceConfigured, get_client
 from .constants import Attributes
-from .models import HaalCentraalHRConfig
 
 logger = logging.getLogger(__name__)
 
 
-class HaalCentraalHRZGWClientError(Exception):
-    pass
-
-
 @register("haalcentraal_hr")
 class HaalCentraalHRPrefill(BasePlugin):
     verbose_name = _("Haal Centraal HR")
@@ -71,53 +69,39 @@ def get_prefill_values(
         if not (kvk_value := self.get_identifier_value(submission, identifier_role)):
             return {}
 
-        config = HaalCentraalHRConfig.get_solo()
-
-        haal_centraal_hr_client = config.build_client(submission=submission)
-        if haal_centraal_hr_client is None:
+        context = PreRequestClientContext(submission=submission)
+        try:
+            haal_centraal_hr_client = get_client(context=context)
+        except NoServiceConfigured:
             logger.exception("Haal Centraal HR service not configured.")
             return {}
 
         try:
-            data = haal_centraal_hr_client.retrieve(
-                "RaadpleegMaatschappelijkeActiviteitOpKvKnummer",
-                url=f"maatschappelijkeactiviteiten/{kvk_value}",
-                request_kwargs={
-                    "headers": {
-                        "Accept": "application/hal+json",
-                    },
-                },
-            )
-        except HaalCentraalHRZGWClientError as e:
+            with haal_centraal_hr_client:
+                response = haal_centraal_hr_client.get(
+                    f"maatschappelijkeactiviteiten/{kvk_value}"
+                )
+                response.raise_for_status()
+        except requests.RequestException as exc:
             logger.exception(
-                "Exception while making request to Haal Centraal HR", exc_info=e
+                "Exception while making request to Haal Centraal HR", exc_info=exc
             )
             return {}
 
+        data = response.json()
         return self.extract_requested_attributes(attributes, data)
 
     def check_config(self) -> None:
-        config = HaalCentraalHRConfig.get_solo()
-
-        if not config.service:
-            raise InvalidPluginConfiguration(_("Service not selected"))
-
-        haal_centraal_hr_client = config.build_client()
-
         kvk_value = "TEST"
         try:
-            haal_centraal_hr_client.retrieve(
-                "RaadpleegMaatschappelijkeActiviteitOpKvKnummer",
-                url=f"maatschappelijkeactiviteiten/{kvk_value}",
-                request_kwargs={
-                    "headers": {
-                        "Accept": "application/hal+json",
-                    },
-                },
-            )
-        except ClientError as exc:
-            if exc.args[0].get("status") == 400:
+            with get_client() as client:
+                response = client.get(f"maatschappelijkeactiviteiten/{kvk_value}")
+                response.raise_for_status()
+        except NoServiceConfigured as exc:
+            raise InvalidPluginConfiguration(_("Service not selected")) from exc
+        except requests.RequestException as exc:
+            if (response := exc.response) is not None and response.status_code == 400:
                 return
             raise InvalidPluginConfiguration(
                 _("Client error: {exception}").format(exception=exc)
-            )
+            ) from exc

prefill_haalcentraalhr/tests/test_plugin.py

@@ -6,11 +6,10 @@
 
 from openforms.authentication.constants import AuthAttribute
 from openforms.plugins.exceptions import InvalidPluginConfiguration
-from openforms.registrations.contrib.zgw_apis.tests.factories import ServiceFactory
 from openforms.submissions.tests.factories import SubmissionFactory
 from requests_mock import Mocker
 from zgw_consumers.constants import APITypes
-from zgw_consumers.test import mock_service_oas_get
+from zgw_consumers_ext.tests.factories import ServiceFactory
 
 from ..models import HaalCentraalHRConfig
 from ..plugin import HaalCentraalHRPrefill
@@ -43,7 +42,7 @@ def test_no_service_configured(self):
         )
 
         with patch(
-            "prefill_haalcentraalhr.plugin.HaalCentraalHRConfig.get_solo",
+            "prefill_haalcentraalhr.client.HaalCentraalHRConfig.get_solo",
             return_value=HaalCentraalHRConfig(),
         ):
             data = plugin.get_prefill_values(
@@ -56,13 +55,6 @@ def test_no_service_configured(self):
     @Mocker()
     @override_settings(ZGW_CONSUMERS_TEST_SCHEMA_DIRS=[FILES_DIR])
     def test_happy_flow(self, m):
-        mock_service_oas_get(
-            m,
-            url="http://haalcentraal-hr.nl/api/",
-            service="haalcentraal-hr-oas",
-            oas_url="https://haalcentraal-hr.nl/api/schema/openapi.yaml",
-        )
-
         with open(FILES_DIR / "maatschappelijkeactiviteiten-response.json", "rb") as f:
             m.get(
                 "http://haalcentraal-hr.nl/api/maatschappelijkeactiviteiten/111222333",
@@ -80,7 +72,7 @@ def test_happy_flow(self, m):
         )
 
         with patch(
-            "prefill_haalcentraalhr.plugin.HaalCentraalHRConfig.get_solo",
+            "prefill_haalcentraalhr.client.HaalCentraalHRConfig.get_solo",
             return_value=HaalCentraalHRConfig(service=service),
         ):
             data = plugin.get_prefill_values(
@@ -97,13 +89,6 @@ def test_happy_flow(self, m):
     @Mocker()
     @override_settings(ZGW_CONSUMERS_TEST_SCHEMA_DIRS=[FILES_DIR])
     def test_check_config_happy_flow(self, m):
-        mock_service_oas_get(
-            m,
-            url="http://haalcentraal-hr.nl/api/",
-            service="haalcentraal-hr-oas",
-            oas_url="https://haalcentraal-hr.nl/api/schema/openapi.yaml",
-        )
-
         m.get(
             "http://haalcentraal-hr.nl/api/maatschappelijkeactiviteiten/TEST",
             status_code=400,
@@ -123,21 +108,14 @@ def test_check_config_happy_flow(self, m):
         )
 
         with patch(
-            "prefill_haalcentraalhr.plugin.HaalCentraalHRConfig.get_solo",
+            "prefill_haalcentraalhr.client.HaalCentraalHRConfig.get_solo",
             return_value=HaalCentraalHRConfig(service=service),
         ):
             plugin.check_config()
 
     @Mocker()
     @override_settings(ZGW_CONSUMERS_TEST_SCHEMA_DIRS=[FILES_DIR])
     def test_check_config_wrong_response(self, m):
-        mock_service_oas_get(
-            m,
-            url="http://haalcentraal-hr.nl/api/",
-            service="haalcentraal-hr-oas",
-            oas_url="https://haalcentraal-hr.nl/api/schema/openapi.yaml",
-        )
-
         m.get(
             "http://haalcentraal-hr.nl/api/maatschappelijkeactiviteiten/TEST",
             status_code=403,
@@ -152,7 +130,7 @@ def test_check_config_wrong_response(self, m):
         )
 
         with patch(
-            "prefill_haalcentraalhr.plugin.HaalCentraalHRConfig.get_solo",
+            "prefill_haalcentraalhr.client.HaalCentraalHRConfig.get_solo",
             return_value=HaalCentraalHRConfig(service=service),
         ):
             with self.assertRaises(
@@ -165,7 +143,7 @@ def test_check_config_no_service_configured(self):
         plugin = HaalCentraalHRPrefill("haalcentraal_hr")
 
         with patch(
-            "prefill_haalcentraalhr.plugin.HaalCentraalHRConfig.get_solo",
+            "prefill_haalcentraalhr.client.HaalCentraalHRConfig.get_solo",
             return_value=HaalCentraalHRConfig(),
         ):
             with self.assertRaises(

prefill_haalcentraalhr/tests/test_plugin_with_token_exchange.py

@@ -9,13 +9,12 @@
 from openforms.authentication.constants import AuthAttribute
 from openforms.pre_requests.base import PreRequestHookBase
 from openforms.pre_requests.registry import Registry
-from openforms.registrations.contrib.zgw_apis.tests.factories import ServiceFactory
 from openforms.submissions.models import Submission
 from openforms.submissions.tests.factories import SubmissionFactory
 from requests.auth import AuthBase
 from requests_mock import Mocker
 from zgw_consumers.constants import APITypes
-from zgw_consumers.test import mock_service_oas_get
+from zgw_consumers_ext.tests.factories import ServiceFactory
 
 from ..models import HaalCentraalHRConfig
 from ..plugin import HaalCentraalHRPrefill
@@ -51,13 +50,6 @@ def __call__(
 @override_settings(ZGW_CONSUMERS_TEST_SCHEMA_DIRS=[FILES_DIR])
 class HaalCentraalHRPluginWithTokenExchangeTests(TestCase):
     def test_token_exchange(self, m):
-        mock_service_oas_get(
-            m,
-            url="http://haalcentraal-hr.nl/api/",
-            service="haalcentraal-hr-oas",
-            oas_url="https://haalcentraal-hr.nl/api/schema/openapi.yaml",
-        )
-
         with open(FILES_DIR / "maatschappelijkeactiviteiten-response.json", "rb") as f:
             m.get(
                 "http://haalcentraal-hr.nl/api/maatschappelijkeactiviteiten/111222333",
@@ -82,7 +74,7 @@ def test_token_exchange(self, m):
         )
 
         with patch(
-            "prefill_haalcentraalhr.plugin.HaalCentraalHRConfig.get_solo",
+            "prefill_haalcentraalhr.client.HaalCentraalHRConfig.get_solo",
             return_value=HaalCentraalHRConfig(service=service),
         ), patch(
             "openforms.pre_requests.clients.registry",