diff --git a/src/openapi.yaml b/src/openapi.yaml index 623905d067..ec8e536fe0 100644 --- a/src/openapi.yaml +++ b/src/openapi.yaml @@ -2902,6 +2902,46 @@ paths: $ref: '#/components/headers/X-Is-Form-Designer' Content-Language: $ref: '#/components/headers/Content-Language' + /api/v2/geo/address-autocomplete: + get: + operationId: geo_address_autocomplete_retrieve + description: |- + Get the street name and city for a given postal code and house number. + + **NOTE** the `/api/v2/location/get-street-name-and-city/` endpoint will be removed in v3. Use `/api/v2/geo/address-autocomplete/` instead. + summary: Get a street name and city + parameters: + - in: query + name: house_number + schema: + type: number + description: House number of the address + required: true + - in: query + name: postcode + schema: + type: string + description: Postal code of the address + required: true + tags: + - geo + deprecated: true + responses: + '200': + content: + application/json: + schema: + $ref: '#/components/schemas/GetStreetNameAndCityViewResult' + description: '' + headers: + X-Session-Expires-In: + $ref: '#/components/headers/X-Session-Expires-In' + X-CSRFToken: + $ref: '#/components/headers/X-CSRFToken' + X-Is-Form-Designer: + $ref: '#/components/headers/X-Is-Form-Designer' + Content-Language: + $ref: '#/components/headers/Content-Language' /api/v2/geo/address-search: get: operationId: geo_address_search_list @@ -3208,11 +3248,11 @@ paths: $ref: '#/components/headers/Content-Language' /api/v2/location/get-street-name-and-city: get: - operationId: get_street_name_and_city_list + operationId: location_get_street_name_and_city_retrieve description: |- Get the street name and city for a given postal code and house number. - **NOTE** this endpoint will redirect to `/api/v3/geo/address-autocomplete/` in v3. + **NOTE** the `/api/v2/location/get-street-name-and-city/` endpoint will be removed in v3. Use `/api/v2/geo/address-autocomplete/` instead. summary: Get a street name and city parameters: - in: query diff --git a/src/openforms/api/urls.py b/src/openforms/api/urls.py index 5e5ce0b73b..56b03a9186 100644 --- a/src/openforms/api/urls.py +++ b/src/openforms/api/urls.py @@ -10,6 +10,7 @@ from rest_framework import routers from rest_framework_nested.routers import NestedSimpleRouter +from openforms.contrib.kadaster.api.views import AddressAutocompleteView from openforms.forms.api.viewsets import ( CategoryViewSet, FormDefinitionViewSet, @@ -97,7 +98,13 @@ path("forms-import", FormsImportAPIView.as_view(), name="forms-import"), path("prefill/", include("openforms.prefill.api.urls")), path("validation/", include("openforms.validations.api.urls")), - path("location/", include("openforms.locations.api.urls")), + # TODO: in Open Forms v3, this must become a simple RedirectView to the + # endpoint for openforms.contrib.kadaster.api.views.AddressAutocomplete + path( + "location/get-street-name-and-city", + AddressAutocompleteView.as_view(), + name="get-street-name-and-city-list", + ), path( "logic/description", GenerateLogicDescriptionView.as_view(), diff --git a/src/openforms/contrib/kadaster/api/serializers.py b/src/openforms/contrib/kadaster/api/serializers.py index c00bb5934b..bba3100fdc 100644 --- a/src/openforms/contrib/kadaster/api/serializers.py +++ b/src/openforms/contrib/kadaster/api/serializers.py @@ -4,6 +4,22 @@ from rest_framework import serializers +class GetStreetNameAndCityViewInputSerializer(serializers.Serializer): + postcode = serializers.CharField( + label=_("postal code"), help_text=_("Postal code to use in search") + ) + house_number = serializers.CharField( + label=_("house number"), help_text=_("House number to use in search") + ) + + +class GetStreetNameAndCityViewResultSerializer(serializers.Serializer): + street_name = serializers.CharField( + label=_("street name"), help_text=_("Found street name") + ) + city = serializers.CharField(label=_("city"), help_text=_("Found city")) + + class LatitudeLongitudeSerializer(serializers.Serializer): lat = serializers.FloatField(label=_("Latitude")) lng = serializers.FloatField(label=_("Longitude")) @@ -15,7 +31,7 @@ class RijksDriehoekSerializer(serializers.Serializer): class AddressSearchResultSerializer(serializers.Serializer): - label = serializers.CharField(label=_("Location name")) + label = serializers.CharField(label=_("Location name")) # type: ignore lat_lng = LatitudeLongitudeSerializer( label=_("Latitude/longitude"), help_text=_("Latitude and longitude in the WGS 84 coordinate system."), @@ -47,4 +63,4 @@ class LatLngSearchInputSerializer(serializers.Serializer): class LatLngSearchResultSerializer(serializers.Serializer): - label = serializers.CharField(label=_("Closest address")) + label = serializers.CharField(label=_("Closest address")) # type: ignore diff --git a/src/openforms/contrib/kadaster/api/urls.py b/src/openforms/contrib/kadaster/api/urls.py index c35463df0f..b3ccf466e2 100644 --- a/src/openforms/contrib/kadaster/api/urls.py +++ b/src/openforms/contrib/kadaster/api/urls.py @@ -1,10 +1,15 @@ from django.urls import path -from .views import AddressSearchView, LatLngSearchView +from .views import AddressAutocompleteView, AddressSearchView, LatLngSearchView app_name = "geo" urlpatterns = [ + path( + "address-autocomplete", + AddressAutocompleteView.as_view(), + name="address-autocomplete", + ), path( "address-search", AddressSearchView.as_view(), diff --git a/src/openforms/contrib/kadaster/api/views.py b/src/openforms/contrib/kadaster/api/views.py index 16aa5f0cd5..06c0e9a6f5 100644 --- a/src/openforms/contrib/kadaster/api/views.py +++ b/src/openforms/contrib/kadaster/api/views.py @@ -1,5 +1,7 @@ import logging +from functools import partial +from django.core.cache import cache from django.utils.translation import gettext_lazy as _ from drf_spectacular.types import OpenApiTypes @@ -11,11 +13,14 @@ from openforms.api.serializers import ExceptionSerializer, ValidationErrorSerializer from openforms.api.views.mixins import ListMixin +from openforms.contrib.kadaster.clients.bag import AddressResult from openforms.submissions.api.permissions import AnyActiveSubmissionPermission -from ..clients import get_locatieserver_client +from ..clients import get_bag_client, get_locatieserver_client from .serializers import ( AddressSearchResultSerializer, + GetStreetNameAndCityViewInputSerializer, + GetStreetNameAndCityViewResultSerializer, LatLngSearchInputSerializer, LatLngSearchResultSerializer, ) @@ -23,11 +28,76 @@ logger = logging.getLogger(__name__) +ADDRESS_AUTOCOMPLETE_CACHE_TIMEOUT = ( + 60 * 60 * 24 +) # 24 hours - address data does NOT update frequently + + +def lookup_address(postcode: str, number: str) -> AddressResult | None: + with get_bag_client() as client: + return client.get_address(postcode, number) + + +class AddressAutocompleteView(APIView): + """ + Get the street name and city when given a postcode and house number. + """ + + authentication_classes = () + permission_classes = [AnyActiveSubmissionPermission] + + @extend_schema( + summary=_("Get a street name and city"), # type: ignore + description=_( + "Get the street name and city for a given postal code and house number.\n\n" + "**NOTE** the `/api/v2/location/get-street-name-and-city/` endpoint will " + "be removed in v3. Use `/api/v2/geo/address-autocomplete/` instead." + ), # type: ignore + responses=GetStreetNameAndCityViewResultSerializer, + parameters=[ + OpenApiParameter( + "postcode", + OpenApiTypes.STR, + OpenApiParameter.QUERY, + description=_("Postal code of the address"), # type: ignore + required=True, + ), + OpenApiParameter( + "house_number", + OpenApiTypes.NUMBER, + OpenApiParameter.QUERY, + description=_("House number of the address"), # type: ignore + required=True, + ), + ], + deprecated=True, + ) + def get(self, request, *args, **kwargs): + serializer = GetStreetNameAndCityViewInputSerializer(data=request.query_params) + serializer.is_valid(raise_exception=True) + data = serializer.validated_data + postcode, number = data["postcode"], data["house_number"] + + # check the cache so we avoid hitting the remote API too often (and risk + # of being throttled, see #1832) + address_data = cache.get_or_set( + key=f"BAG|get_address|{postcode}|{number}", + default=partial(lookup_address, postcode, number), + timeout=ADDRESS_AUTOCOMPLETE_CACHE_TIMEOUT, + ) + + if not address_data: + # If address is not found just return an empty response + return Response({}) + + return Response(GetStreetNameAndCityViewResultSerializer(address_data).data) + + @extend_schema( - summary=_("Get an adress based on coordinates"), + summary=_("Get an adress based on coordinates"), # type: ignore description=_( "Get the closest address name based on the given longitude and latitude." - ), + ), # type: ignore parameters=[ OpenApiParameter( "lat", @@ -77,13 +147,13 @@ def get(self, request: Request): @extend_schema( - summary=_("List address suggestions with coordinates."), + summary=_("List address suggestions with coordinates."), # type: ignore description=_( "Get a list of addresses, ordered by relevance/match score of the input " "query. Note that only results having latitude/longitude data are returned.\n\n" "The results are retrieved from the configured geo search service, defaulting " "to the Kadaster location server." - ), + ), # type: ignore parameters=[ OpenApiParameter( "q", diff --git a/src/openforms/locations/api/tests/test_api.py b/src/openforms/contrib/kadaster/tests/test_api_address_autocomplete.py similarity index 94% rename from src/openforms/locations/api/tests/test_api.py rename to src/openforms/contrib/kadaster/tests/test_api_address_autocomplete.py index e68acab470..e78544c98b 100644 --- a/src/openforms/locations/api/tests/test_api.py +++ b/src/openforms/contrib/kadaster/tests/test_api_address_autocomplete.py @@ -8,12 +8,13 @@ import requests_mock -from openforms.contrib.kadaster.clients.bag import AddressResult -from openforms.contrib.kadaster.models import KadasterApiConfig from openforms.submissions.tests.factories import SubmissionFactory from openforms.submissions.tests.mixins import SubmissionsMixin from zgw_consumers_ext.tests.factories import ServiceFactory +from ..clients.bag import AddressResult +from ..models import KadasterApiConfig + CACHES = settings.CACHES.copy() CACHES["default"] = {"BACKEND": "django.core.cache.backends.locmem.LocMemCache"} @@ -31,7 +32,7 @@ def setUp(self): self.addCleanup(caches["default"].clear) self._add_submission_to_session(self.submission) - @patch("openforms.locations.api.views.lookup_address") + @patch("openforms.contrib.kadaster.api.views.lookup_address") def test_getting_street_name_and_city(self, m_lookup_address): m_lookup_address.return_value = AddressResult( street_name="Keizersgracht", city="Amsterdam" @@ -111,7 +112,7 @@ def test_getting_street_name_and_city_without_house_number_returns_error( }, ) - @patch("openforms.locations.api.views.lookup_address") + @patch("openforms.contrib.kadaster.api.views.lookup_address") def test_getting_street_name_and_city_with_extra_query_params_ignores_extra_param( self, m_lookup_address ): @@ -129,7 +130,7 @@ def test_getting_street_name_and_city_with_extra_query_params_ignores_extra_para self.assertEqual(response.json()["streetName"], "Keizersgracht") self.assertEqual(response.json()["city"], "Amsterdam") - @patch("openforms.locations.api.views.lookup_address") + @patch("openforms.contrib.kadaster.api.views.lookup_address") def test_address_not_found_returns_empty_200_response(self, m_lookup_address): m_lookup_address.return_value = None @@ -142,7 +143,7 @@ def test_address_not_found_returns_empty_200_response(self, m_lookup_address): self.assertEqual(response.json(), {}) @tag("gh-1832") - @patch("openforms.locations.api.views.lookup_address") + @patch("openforms.contrib.kadaster.api.views.lookup_address") def test_endpoint_uses_caching(self, m_lookup_address): m_lookup_address.return_value = AddressResult( street_name="Keizersgracht", city="Amsterdam" diff --git a/src/openforms/contrib/microsoft/client.py b/src/openforms/contrib/microsoft/client.py index 53ff539ade..ece87f4441 100644 --- a/src/openforms/contrib/microsoft/client.py +++ b/src/openforms/contrib/microsoft/client.py @@ -1,3 +1,12 @@ +""" +Microsoft (graph) API client. + +This client implementation wraps around the API client/integration implemented in the +O365 package. While it uses requests-oauth2client under the hood, we opt to *not* +use our own :module:`api_client` implementation here, as the typical Dutch API/service +requirements such as mTLS are not relevant. The service model definition also does not +allow configuring any of those aspects. +""" import json import os from io import BytesIO @@ -6,9 +15,9 @@ from O365 import Account -from openforms.contrib.microsoft.constants import ConflictHandling -from openforms.contrib.microsoft.exceptions import MSAuthenticationError -from openforms.contrib.microsoft.models import MSGraphService +from .constants import ConflictHandling +from .exceptions import MSAuthenticationError +from .models import MSGraphService class MSGraphClient: diff --git a/src/openforms/contrib/microsoft/tests/factories.py b/src/openforms/contrib/microsoft/tests/factories.py index 664e616815..d808c7c237 100644 --- a/src/openforms/contrib/microsoft/tests/factories.py +++ b/src/openforms/contrib/microsoft/tests/factories.py @@ -1,6 +1,6 @@ import factory -from openforms.contrib.microsoft.models import MSGraphService +from ..models import MSGraphService class MSGraphServiceFactory(factory.django.DjangoModelFactory): diff --git a/src/openforms/formio/components/np_family_members/stuf_bg.py b/src/openforms/formio/components/np_family_members/stuf_bg.py index 97b45a79ec..4d14b969d6 100644 --- a/src/openforms/formio/components/np_family_members/stuf_bg.py +++ b/src/openforms/formio/components/np_family_members/stuf_bg.py @@ -1,18 +1,11 @@ from typing import List, Tuple -from stuf.stuf_bg.models import StufBGConfig +from stuf.stuf_bg.client import get_client def get_np_children_stuf_bg(bsn: str) -> List[Tuple[str, str]]: - config = StufBGConfig.get_solo() - assert isinstance(config, StufBGConfig) - client = config.get_client() - - attributes = [ - "inp.heeftAlsKinderen", - ] - - data = client.get_values(bsn, attributes) + with get_client() as client: + data = client.get_values(bsn, ["inp.heeftAlsKinderen"]) # Kids child_choices = [] diff --git a/src/openforms/formio/components/np_family_members/tests/test_family_members.py b/src/openforms/formio/components/np_family_members/tests/test_family_members.py index 2827140358..34c84196b4 100644 --- a/src/openforms/formio/components/np_family_members/tests/test_family_members.py +++ b/src/openforms/formio/components/np_family_members/tests/test_family_members.py @@ -107,9 +107,7 @@ def test_get_children_haal_centraal(self, mock_brp_config_get_solo): self.assertEqual(("456789123", "Bolly van Doe"), kids_choices[0]) self.assertEqual(("789123456", "Billy van Doe"), kids_choices[1]) - @patch( - "openforms.formio.components.np_family_members.stuf_bg.StufBGConfig.get_solo" - ) + @patch("stuf.stuf_bg.client.StufBGConfig.get_solo") def test_get_children_stuf_bg(self, mock_stufbg_config_get_solo): stuf_bg_service = StufServiceFactory.build() mock_stufbg_config_get_solo.return_value = StufBGConfig(service=stuf_bg_service) diff --git a/src/openforms/locations/__init__.py b/src/openforms/locations/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/src/openforms/locations/api/__init__.py b/src/openforms/locations/api/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/src/openforms/locations/api/serializers.py b/src/openforms/locations/api/serializers.py deleted file mode 100644 index 7a345580d3..0000000000 --- a/src/openforms/locations/api/serializers.py +++ /dev/null @@ -1,19 +0,0 @@ -from django.utils.translation import gettext_lazy as _ - -from rest_framework import serializers - - -class GetStreetNameAndCityViewInputSerializer(serializers.Serializer): - postcode = serializers.CharField( - label=_("postal code"), help_text=_("Postal code to use in search") - ) - house_number = serializers.CharField( - label=_("house number"), help_text=_("House number to use in search") - ) - - -class GetStreetNameAndCityViewResultSerializer(serializers.Serializer): - street_name = serializers.CharField( - label=_("street name"), help_text=_("Found street name") - ) - city = serializers.CharField(label=_("city"), help_text=_("Found city")) diff --git a/src/openforms/locations/api/tests/__init__.py b/src/openforms/locations/api/tests/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/src/openforms/locations/api/urls.py b/src/openforms/locations/api/urls.py deleted file mode 100644 index 62d5c778f2..0000000000 --- a/src/openforms/locations/api/urls.py +++ /dev/null @@ -1,11 +0,0 @@ -from django.urls import path - -from .views import GetStreetNameAndCityView - -urlpatterns = [ - path( - "get-street-name-and-city", - GetStreetNameAndCityView.as_view(), - name="get-street-name-and-city-list", - ), -] diff --git a/src/openforms/locations/api/views.py b/src/openforms/locations/api/views.py deleted file mode 100644 index 932eb26151..0000000000 --- a/src/openforms/locations/api/views.py +++ /dev/null @@ -1,79 +0,0 @@ -from functools import partial - -from django.core.cache import caches -from django.utils.translation import ugettext_lazy as _ - -from drf_spectacular.types import OpenApiTypes -from drf_spectacular.utils import OpenApiParameter, extend_schema -from rest_framework.response import Response -from rest_framework.views import APIView - -from openforms.contrib.kadaster.clients import get_bag_client -from openforms.locations.api.serializers import ( - GetStreetNameAndCityViewInputSerializer, - GetStreetNameAndCityViewResultSerializer, -) -from openforms.submissions.api.permissions import AnyActiveSubmissionPermission - -CACHE_TIMEOUT = 60 * 60 * 24 # 24 hours - address data does NOT update frequently - - -def lookup_address(postcode: str, number: str): - with get_bag_client() as client: - return client.get_address(postcode, number) - - -class GetStreetNameAndCityView(APIView): - """ - Get the street name and city when given a postcode and house number - """ - - authentication_classes = () - permission_classes = [AnyActiveSubmissionPermission] - - @extend_schema( - operation_id="get_street_name_and_city_list", - summary=_("Get a street name and city"), - description=_( - "Get the street name and city for a given postal code and house number.\n\n" - "**NOTE** this endpoint will redirect to `/api/v3/geo/address-autocomplete/`" - " in v3." - ), - responses=GetStreetNameAndCityViewResultSerializer, - parameters=[ - OpenApiParameter( - "postcode", - OpenApiTypes.STR, - OpenApiParameter.QUERY, - description=_("Postal code of the address"), - required=True, - ), - OpenApiParameter( - "house_number", - OpenApiTypes.NUMBER, - OpenApiParameter.QUERY, - description=_("House number of the address"), - required=True, - ), - ], - deprecated=True, - ) - def get(self, request, *args, **kwargs): - serializer = GetStreetNameAndCityViewInputSerializer(data=request.query_params) - serializer.is_valid(raise_exception=True) - data = serializer.validated_data - postcode, number = data["postcode"], data["house_number"] - - # check the cache so we avoid hitting the remote API too often (and risk - # of being throttled, see #1832) - address_data = caches["default"].get_or_set( - key=f"BAG|get_address|{postcode}|{number}", - default=partial(lookup_address, postcode, number), - timeout=CACHE_TIMEOUT, - ) - - if not address_data: - # If address is not found just return an empty response - return Response({}) - - return Response(GetStreetNameAndCityViewResultSerializer(address_data).data) diff --git a/src/openforms/pre_requests/base.py b/src/openforms/pre_requests/base.py index 5888b72f51..47de457712 100644 --- a/src/openforms/pre_requests/base.py +++ b/src/openforms/pre_requests/base.py @@ -19,7 +19,7 @@ def __call__( self, method: str, url: str, - kwargs: dict, + kwargs: dict | None, context: PreRequestClientContext | None = None, ) -> None: raise NotImplementedError() # pragma: nocover diff --git a/src/openforms/pre_requests/clients.py b/src/openforms/pre_requests/clients.py index 27bf04e9ee..e966fb7cfa 100644 --- a/src/openforms/pre_requests/clients.py +++ b/src/openforms/pre_requests/clients.py @@ -1,4 +1,6 @@ -from typing import TYPE_CHECKING, Any, Optional, TypedDict +from __future__ import annotations + +from typing import TYPE_CHECKING, Any, TypedDict from requests.models import PreparedRequest, Request from zgw_consumers.client import ZGWClient @@ -10,7 +12,7 @@ class PreRequestClientContext(TypedDict): - submission: Optional["Submission"] + submission: Submission | None class PreRequestZGWClient(ZGWClient): @@ -18,12 +20,13 @@ class PreRequestZGWClient(ZGWClient): A :class:`zgw_consumers.client.ZGWClient` with pre-requests support. .. note:: this client is being deprecated in favour of a pure requests approach, - using :class:`api_client.APIClient` + using :class:`api_client.APIClient` and + :class:`pre_requests.clients.PreRequestMixin`. """ _context = None - def pre_request(self, method: str, url: str, kwargs: Optional[dict] = None) -> Any: + def pre_request(self, method: str, url: str, kwargs: dict | None = None) -> Any: result = super().pre_request(method, url, kwargs) for pre_request in registry: @@ -32,7 +35,7 @@ def pre_request(self, method: str, url: str, kwargs: Optional[dict] = None) -> A return result @property - def context(self) -> Optional[PreRequestClientContext]: + def context(self) -> PreRequestClientContext | None: return self._context @context.setter @@ -80,4 +83,4 @@ def prepare_request(self, request: Request) -> PreparedRequest: for key, value in kwargs.items(): setattr(request, key, value) - return super().prepare_request(request) + return super().prepare_request(request) # type: ignore diff --git a/src/openforms/prefill/contrib/stufbg/plugin.py b/src/openforms/prefill/contrib/stufbg/plugin.py index 9e16b96b74..705755e65c 100644 --- a/src/openforms/prefill/contrib/stufbg/plugin.py +++ b/src/openforms/prefill/contrib/stufbg/plugin.py @@ -1,17 +1,18 @@ import logging -from typing import Any, Dict, Iterable, List, Tuple +from typing import Any from django.urls import reverse from django.utils.translation import gettext_lazy as _ +import requests from glom import T as Target, glom from lxml import etree -from requests import HTTPError, RequestException from openforms.authentication.constants import AuthAttribute from openforms.plugins.exceptions import InvalidPluginConfiguration from openforms.submissions.models import Submission from openforms.utils.xml import fromstring +from stuf.stuf_bg.client import NoServiceConfigured, get_client from stuf.stuf_bg.constants import FieldChoices from stuf.stuf_bg.models import StufBGConfig @@ -95,15 +96,14 @@ class StufBgPrefill(BasePlugin): verbose_name = _("StUF-BG") requires_auth = AuthAttribute.bsn - def get_available_attributes(self) -> Iterable[Tuple[str, str]]: + def get_available_attributes(self) -> list[tuple[str, str]]: return FieldChoices.choices def _get_values_for_bsn( - self, bsn: str, attributes: Iterable[str] - ) -> Dict[str, Any]: - config = StufBGConfig.get_solo() - with config.get_client() as client: - data = client.get_values(bsn, attributes) + self, bsn: str, attributes: list[FieldChoices] + ) -> dict[str, Any]: + with get_client() as client: + data = client.get_values(bsn, [str(attr) for attr in attributes]) response_dict = {} for attribute in attributes: @@ -146,9 +146,9 @@ def get_identifier_value( def get_prefill_values( self, submission: Submission, - attributes: List[str], + attributes: list[FieldChoices], identifier_role: str = IdentifierRoles.main, - ) -> Dict[str, Any]: + ) -> dict[str, Any]: if not (bsn_value := self.get_identifier_value(submission, identifier_role)): # If there is no bsn we can't prefill any values so just return logger.info("No BSN associated with submission, cannot prefill.") @@ -156,7 +156,7 @@ def get_prefill_values( return self._get_values_for_bsn(bsn_value, attributes) - def get_co_sign_values(self, identifier: str) -> Tuple[Dict[str, Any], str]: + def get_co_sign_values(self, identifier: str) -> tuple[dict[str, Any], str]: """ Given an identifier, fetch the co-sign specific values. @@ -170,11 +170,11 @@ def get_co_sign_values(self, identifier: str) -> Tuple[Dict[str, Any], str]: """ values = self._get_values_for_bsn( identifier, - ( + [ FieldChoices.voornamen, FieldChoices.voorvoegselGeslachtsnaam, FieldChoices.geslachtsnaam, - ), + ], ) representation_bits = [ " ".join( @@ -190,41 +190,36 @@ def get_co_sign_values(self, identifier: str) -> Tuple[Dict[str, Any], str]: def check_config(self): check_bsn = "111222333" + try: + with get_client() as client: + response = client.templated_request( + "npsLv01", + template="stuf_bg/StufBgRequest.xml", + context={"bsn": check_bsn}, + ) + response.raise_for_status() + except NoServiceConfigured as exc: + raise InvalidPluginConfiguration(_("Service not selected")) from exc + except requests.RequestException as exc: + raise InvalidPluginConfiguration( + _("Client error: {exception}").format(exception=exc) + ) from exc - config = StufBGConfig.get_solo() - if not config.service: - raise InvalidPluginConfiguration(_("Service not selected")) - - client = config.get_client() try: - response = client.templated_request( - "npsLv01", - template="stuf_bg/StufBgRequest.xml", - context={"bsn": check_bsn}, - ) - response.raise_for_status() - except (RequestException, HTTPError) as e: + xml = fromstring(response.content) + except etree.XMLSyntaxError as exc: raise InvalidPluginConfiguration( - _("Client error: {exception}").format(exception=e) - ) - else: - try: - xml = fromstring(response.content) - except etree.XMLSyntaxError as e: - raise InvalidPluginConfiguration( - _("SyntaxError in response: {exception}").format(exception=e) + _("SyntaxError in response: {exception}").format(exception=exc) + ) from exc + + # we expect a valid 'object not found' response, + # but also accept an empty response (for 3rd party backend implementation reasons) + if not is_object_not_found_response(xml) and not is_empty_wrapped_response(xml): + raise InvalidPluginConfiguration( + _("Unexpected response: expected '{message}' SOAP response").format( + message="Object niet gevonden" ) - else: - # we expect a valid 'object not found' response, - # but also accept an empty response (for 3rd party backend implementation reasons) - if not is_object_not_found_response( - xml - ) and not is_empty_wrapped_response(xml): - raise InvalidPluginConfiguration( - _( - "Unexpected response: expected '{message}' SOAP response" - ).format(message="Object niet gevonden") - ) + ) def get_config_actions(self): return [ diff --git a/src/openforms/prefill/contrib/stufbg/tests/test_plugin.py b/src/openforms/prefill/contrib/stufbg/tests/test_plugin.py index 889cbcaee3..4ccc1fdc26 100644 --- a/src/openforms/prefill/contrib/stufbg/tests/test_plugin.py +++ b/src/openforms/prefill/contrib/stufbg/tests/test_plugin.py @@ -53,6 +53,18 @@ def setUp(self) -> None: stufbg_config_patcher.start() self.addCleanup(stufbg_config_patcher.stop) + def test_getting_available_attributes(self): + attributes = self.plugin.get_available_attributes() + self.assertIsInstance(attributes, list) + + for entry in attributes: + with self.subTest(entry=entry): + self.assertIsInstance(entry, tuple) + self.assertEqual(len(entry), 2) + value, label = entry + self.assertEqual(value, str(value)) + self.assertEqual(label, str(label)) + def test_get_available_attributes_returns_correct_attributes(self): client_patcher = mock_stufbg_client("StufBgResponse.xml") self.addCleanup(client_patcher.stop) @@ -241,13 +253,20 @@ def setUp(self) -> None: self.plugin = StufBgPrefill("test-plugin") # mock out django-solo interface (we don't have to deal with caches then) + self.config = StufBGConfig(service=self.stuf_bg_service) stufbg_config_patcher = patch( "openforms.prefill.contrib.stufbg.plugin.StufBGConfig.get_solo", - return_value=StufBGConfig(service=self.stuf_bg_service), + return_value=self.config, ) stufbg_config_patcher.start() self.addCleanup(stufbg_config_patcher.stop) + def test_no_service_configured(self): + self.config.service = None + + with self.assertRaises(InvalidPluginConfiguration): + self.plugin.check_config() + def test_check_config_exception(self): with patch( "stuf.stuf_bg.client.Client.get_values_for_attributes", @@ -256,6 +275,13 @@ def test_check_config_exception(self): with self.assertRaises(InvalidPluginConfiguration): self.plugin.check_config() + @requests_mock.Mocker() + def test_check_config_invalid_xml_returned(self, m): + m.register_uri(requests_mock.ANY, requests_mock.ANY, json={"not": "xml"}) + + with self.assertRaises(InvalidPluginConfiguration): + self.plugin.check_config() + def test_check_config_ok_not_found(self): try: with mock_stufbg_make_request("StufBgNotFoundResponse.xml"): diff --git a/src/openforms/registrations/contrib/stuf_zds/plugin.py b/src/openforms/registrations/contrib/stuf_zds/plugin.py index bb6fca89e4..ef41bb9b1c 100644 --- a/src/openforms/registrations/contrib/stuf_zds/plugin.py +++ b/src/openforms/registrations/contrib/stuf_zds/plugin.py @@ -23,6 +23,7 @@ ) from openforms.submissions.models import Submission, SubmissionReport from openforms.utils.mixins import JsonSchemaSerializerMixin +from stuf.stuf_zds.client import NoServiceConfigured, ZaakOptions, get_client from stuf.stuf_zds.constants import VertrouwelijkheidsAanduidingen from stuf.stuf_zds.models import StufZDSConfig @@ -32,6 +33,8 @@ logger = logging.getLogger(__name__) +PLUGIN_IDENTIFIER = "stuf-zds-create-zaak" + class ZaakOptionsSerializer(JsonSchemaSerializerMixin, serializers.Serializer): gemeentecode = serializers.CharField( @@ -154,7 +157,7 @@ def _gender_choices(value): return value -@register("stuf-zds-create-zaak") +@register(PLUGIN_IDENTIFIER) class StufZDSRegistration(BasePlugin): verbose_name = _("StUF-ZDS") configuration_options = ZaakOptionsSerializer @@ -195,11 +198,10 @@ class StufZDSRegistration(BasePlugin): ), } - def pre_register_submission(self, submission: "Submission", options: dict) -> None: - config = StufZDSConfig.get_solo() - config.apply_defaults_to(options) - - with config.get_client(options) as client: + def pre_register_submission( + self, submission: "Submission", options: ZaakOptions + ) -> None: + with get_client(options=options) as client: # obtain a zaaknummer & save it - first, check if we have an intermediate result # from earlier attempts. if we do, do not generate a new number zaak_id = execute_unless_result_exists( @@ -213,8 +215,8 @@ def pre_register_submission(self, submission: "Submission", options: dict) -> No submission.save() def register_submission( - self, submission: Submission, options: dict - ) -> Optional[dict]: + self, submission: Submission, options: ZaakOptions + ) -> dict | None: """ Register the submission by creating a ZAAK. @@ -225,12 +227,8 @@ def register_submission( prevents Open Forms from reserving case numbers over and over again (for example). See #1183 for a reported issue about this. """ - config = StufZDSConfig.get_solo() - config.apply_defaults_to(options) - options["omschrijving"] = submission.form.admin_name - - with config.get_client(options) as client: + with get_client(options=options) as client: # Zaak ID reserved during the pre-registration phase zaak_id = submission.public_registration_reference @@ -339,10 +337,8 @@ def get_reference_from_result(self, result: Dict[str, str]) -> str: """ return result["zaak"] - def update_payment_status(self, submission: "Submission", options: dict): - config = StufZDSConfig.get_solo() - config.apply_defaults_to(options) - with config.get_client(options) as client: + def update_payment_status(self, submission: "Submission", options: ZaakOptions): + with get_client(options) as client: client.set_zaak_payment( submission.registration_result["zaak"], ) @@ -350,23 +346,26 @@ def update_payment_status(self, submission: "Submission", options: dict): def check_config(self): config = StufZDSConfig.get_solo() assert isinstance(config, StufZDSConfig) - if not config.service_id: - raise InvalidPluginConfiguration(_("StufService not selected")) + if not config.gemeentecode: raise InvalidPluginConfiguration( _("StufService missing setting '{name}'").format(name="gemeentecode") ) - options = { + options: ZaakOptions = { "omschrijving": "MyForm", "zds_zaaktype_code": "test", "zds_zaaktype_omschrijving": "test", "zds_zaaktype_status_code": "test", "zds_zaaktype_status_omschrijving": "test", "zds_documenttype_omschrijving_inzending": "test", - } - config.apply_defaults_to(options) - with config.get_client(options) as client: + } # type: ignore + try: + client = get_client(options) + except NoServiceConfigured: + raise InvalidPluginConfiguration(_("StufService not selected")) + + with client: try: client.check_config() except Exception as e: diff --git a/src/openforms/registrations/contrib/stuf_zds/tests/test_config_check.py b/src/openforms/registrations/contrib/stuf_zds/tests/test_config_check.py new file mode 100644 index 0000000000..777f170437 --- /dev/null +++ b/src/openforms/registrations/contrib/stuf_zds/tests/test_config_check.py @@ -0,0 +1,50 @@ +from unittest.mock import patch + +from django.test import SimpleTestCase + +import requests +import requests_mock + +from openforms.plugins.exceptions import InvalidPluginConfiguration +from stuf.stuf_zds.models import StufZDSConfig +from stuf.tests.factories import StufServiceFactory + +from ..plugin import PLUGIN_IDENTIFIER, StufZDSRegistration + + +class ConfigCheckTests(SimpleTestCase): + def setUp(self): + super().setUp() + + self.config = StufZDSConfig( + service=StufServiceFactory.build(), + gemeentecode="foo", + ) + patcher = patch( + "stuf.stuf_zds.client.StufZDSConfig.get_solo", + return_value=self.config, + ) + patcher.start() + self.addCleanup(patcher.stop) + + def test_no_service_configured(self): + self.config.service = None + plugin = StufZDSRegistration(PLUGIN_IDENTIFIER) + + with self.assertRaises(InvalidPluginConfiguration): + plugin.check_config() + + def test_missing_gemeentecode(self): + self.config.gemeentecode = "" + plugin = StufZDSRegistration(PLUGIN_IDENTIFIER) + + with self.assertRaises(InvalidPluginConfiguration): + plugin.check_config() + + @requests_mock.Mocker() + def test_failing_request(self, m): + m.post(requests_mock.ANY, exc=requests.ConnectionError("nope")) + plugin = StufZDSRegistration(PLUGIN_IDENTIFIER) + + with self.assertRaises(InvalidPluginConfiguration): + plugin.check_config() diff --git a/src/openforms/utils/xml.py b/src/openforms/utils/xml.py index fc9b944492..184d0b49c3 100644 --- a/src/openforms/utils/xml.py +++ b/src/openforms/utils/xml.py @@ -7,7 +7,7 @@ from lxml.etree import XMLParser, fromstring as _fromstring -def fromstring(content: str): +def fromstring(content: str | bytes): """ Create an LXML etree from the string content without resolving entities. diff --git a/src/stuf/stuf_bg/client.py b/src/stuf/stuf_bg/client.py index 6859c32aaa..23de2eeadd 100644 --- a/src/stuf/stuf_bg/client.py +++ b/src/stuf/stuf_bg/client.py @@ -12,10 +12,23 @@ from ..models import StufService from ..service_client_factory import ServiceClientFactory, get_client_init_kwargs from .constants import NAMESPACE_REPLACEMENTS, STUF_BG_EXPIRY_MINUTES +from .models import StufBGConfig logger = logging.getLogger(__name__) +class NoServiceConfigured(RuntimeError): + pass + + +def get_client() -> "Client": + config = StufBGConfig.get_solo() + assert isinstance(config, StufBGConfig) + if not (service := config.service): + raise NoServiceConfigured("You must configure a service!") + return StufBGClient(service) + + def StufBGClient(service: StufService) -> "Client": """ Client instance factory, given a service configured in the database. diff --git a/src/stuf/stuf_bg/models.py b/src/stuf/stuf_bg/models.py index 1faf1fd379..a2aaf785f4 100644 --- a/src/stuf/stuf_bg/models.py +++ b/src/stuf/stuf_bg/models.py @@ -20,13 +20,5 @@ class StufBGConfig(SingletonModel): objects = ConfigManager() - def get_client(self): - from .client import StufBGClient - - if not self.service: - raise RuntimeError("You must configure a service!") - - return StufBGClient(self.service) - class Meta: verbose_name = _("StUF-BG configuration") diff --git a/src/stuf/stuf_bg/tests/test_client.py b/src/stuf/stuf_bg/tests/test_client.py index 89b8af7f18..592b7389a6 100644 --- a/src/stuf/stuf_bg/tests/test_client.py +++ b/src/stuf/stuf_bg/tests/test_client.py @@ -1,10 +1,11 @@ from functools import lru_cache from pathlib import Path from random import shuffle +from unittest.mock import patch from django.conf import settings from django.template import loader -from django.test import TestCase, tag +from django.test import SimpleTestCase, TestCase, tag from django.utils import timezone import requests_mock @@ -16,13 +17,14 @@ from openforms.prefill.contrib.stufbg.plugin import ATTRIBUTES_TO_STUF_BG_MAPPING from soap.constants import SOAP_VERSION_CONTENT_TYPES, SOAPVersion from stuf.models import StufService -from stuf.stuf_bg.client import StufBGClient -from stuf.stuf_bg.constants import NAMESPACE_REPLACEMENTS, FieldChoices -from stuf.stuf_bg.models import StufBGConfig from stuf.stuf_zds.client import nsmap from stuf.tests.factories import StufServiceFactory from stuf.xml import fromstring +from ..client import NoServiceConfigured, StufBGClient, get_client +from ..constants import NAMESPACE_REPLACEMENTS, FieldChoices +from ..models import StufBGConfig + PATH_XSDS = (Path(settings.BASE_DIR) / "src" / "stuf" / "stuf_bg" / "xsd").resolve() STUF_BG_XSD = PATH_XSDS / "bg0310" / "vraagAntwoord" / "bg0310_namespace.xsd" @@ -70,21 +72,23 @@ def _stuf_bg_response(service: StufService): return response_body -class StufBGConfigTests(TestCase): - def test_client_requires_a_service(self): - config = StufBGConfig(service=None) - - with self.assertRaises(RuntimeError): - config.get_client() +class StufBGConfigTests(SimpleTestCase): + @patch( + "stuf.stuf_bg.client.StufBGConfig.get_solo", + return_value=StufBGConfig(service=None), + ) + def test_client_requires_a_service(self, m_get_solo): + with self.assertRaises(NoServiceConfigured): + get_client() def test_all_prefill_attributes_are_mapped(self): - available_attributes = FieldChoices.values - for attribute in available_attributes: + for attribute in FieldChoices: with self.subTest(attribute=attribute): glom_target = ATTRIBUTES_TO_STUF_BG_MAPPING.get(attribute) self.assert_(glom_target, f"unmapped attribute: {attribute}") +# not SimpleTestCase because of openforms.logging.logevent usage class StufBGClientTests(TestCase): def setUp(self): super().setUp() diff --git a/src/stuf/stuf_zds/client.py b/src/stuf/stuf_zds/client.py index f7ef063ac8..0abb4f16bb 100644 --- a/src/stuf/stuf_zds/client.py +++ b/src/stuf/stuf_zds/client.py @@ -25,6 +25,7 @@ from ..service_client_factory import ServiceClientFactory, get_client_init_kwargs from ..xml import fromstring from .constants import STUF_ZDS_EXPIRY_MINUTES +from .models import StufZDSConfig logger = logging.getLogger(__name__) @@ -96,6 +97,19 @@ class ZaakOptions(TypedDict): referentienummer: str +class NoServiceConfigured(RuntimeError): + pass + + +def get_client(options: ZaakOptions) -> "Client": + config = StufZDSConfig.get_solo() + assert isinstance(config, StufZDSConfig) + config.apply_defaults_to(options) + if not (service := config.service): + raise NoServiceConfigured("You must configure a service!") + return StufZDSClient(service, options) + + def StufZDSClient(service: StufService, options: ZaakOptions) -> "Client": factory = ServiceClientFactory(service) init_kwargs = get_client_init_kwargs( diff --git a/src/stuf/stuf_zds/models.py b/src/stuf/stuf_zds/models.py index 86230b2efe..eb9f6c0825 100644 --- a/src/stuf/stuf_zds/models.py +++ b/src/stuf/stuf_zds/models.py @@ -36,10 +36,5 @@ class StufZDSConfig(SingletonModel): def apply_defaults_to(self, options): options.setdefault("gemeentecode", self.gemeentecode) - def get_client(self, options): - from .client import StufZDSClient - - return StufZDSClient(self.service, options) - class Meta: verbose_name = _("StUF-ZDS configuration")