From d347466bd542d2363746883d53c43b3557f6398a Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Mon, 2 Oct 2023 17:00:24 +0200 Subject: [PATCH 1/5] :pencil: [#3489] Document API client usage in Open Forms --- docs/developers/backend/api-clients.rst | 172 ++++++++++++++++++++++++ docs/developers/backend/index.rst | 1 + src/zgw_consumers_ext/api_client.py | 26 ++++ 3 files changed, 199 insertions(+) create mode 100644 docs/developers/backend/api-clients.rst diff --git a/docs/developers/backend/api-clients.rst b/docs/developers/backend/api-clients.rst new file mode 100644 index 0000000000..8f3b0d2462 --- /dev/null +++ b/docs/developers/backend/api-clients.rst @@ -0,0 +1,172 @@ +.. _developers_backend_api_clients: + +=========== +API clients +=========== + +Background +========== + +Open Forms interfaces over HTTP with a bunch of third party APIs/services, for example +to: + +* fetch :ref:`prefill data ` +* :ref:`register ` submission data +* perform :ref:`service fetch ` + +There are different flavours of interaction - JSON services (including REST), but also +SOAP (XML), StUF (SOAP/XML-based standard) and unknowns that the future may bring. + +In the Dutch (local) government landscape, there are some common patterns in how you +connect with these services: + +* mutual TLS (mTLS) +* basic auth credentials (username/password) +* API key +* Oauth2-based flows +* IP allowlists + +Combinations of these patterns are possible too! + +Open Forms uses an abstraction that accounts for these variations while allowing +developers to focus on the actual consuming of the service. + +API client base +=============== + +We have opted to implement a base API client class around the :class:`requests.Session`, +more specifically - a subclass of ``Session``. + +Import it from: + +.. code-block:: python + + from api_client import APIClient + + +It's a small extension of the requests ``Session``, allowing you to specify some session +defaults (such as mTLS and auth parameters) while requiring you to set a base URL. The +base URL prevents accidental sending of credentials to other URLs than the base, while +allowing you to implement clients using relative paths/URLs. + +Because it extends the core :mod:`requests` API, usage should feel familiar. + +You are encouraged to define your own service-specific subclasses to modify behaviour +where needed. + +Recommended usage +----------------- + +Our extension allows for configuring such an instance from "configuration objects", +whatever those may be: + +.. code-block:: python + + from api_client import APIClient + + from .factories import my_factory + + client = APIClient.configure_from(my_factory) + + with client: + # ⚡️ context manager -> uses connection pooling and is recommended! + response1 = client.get("some-relative-path", params={"foo": ["bar"]}) + response2 = client.post("other-path", json={...}) + +The ``my_factory`` is a "special" +:ref:`configuration source `, which feeds the +relevant initialization parameters to the :class:`api_client.client.APIClient` instance. + +.. note:: You can (and should) use the client/session in a context manager to benefit + from connection pooling and thus better performance when multiple requests are made. + +Low level usage +--------------- + +The low level usage is essentially what is called by the factory usage. The biggest risk +is forgetting to apply certain connection configuration, like mTLS parameters, therefor +we recommend setting up a configuration factory instead. + +.. code-block:: + + from api_client import APIClient + from requests.auth import + + # You can pass most attributes available on requests.Session, like auth/verify/cert... + client = APIClient( + "https://example.com/api/v1/", + auth=HTTPBasicAuth("superuser", "letmein"), + verify="/path/to/custom/ca-bundle.pem", + ) + + with client: + # ⚡️ context manager -> uses connection pooling and is recommended! + response1 = client.get("some-relative-path", params={"foo": ["bar"]}) + response2 = client.post("other-path", json={...}) + + +.. _developers_backend_api_clients_factories: + +Configuration factories +======================= + +Configuration factories are a small abstraction that allow you to instantiate clients +with the appropriate configuration/presets from sources holding the configuration +details - for example database records. + +Such a factory must implemented the ``APIClientFactory`` protocol: + +.. autoclass:: api_client.typing.APIClientFactory + :members: + + +Some examples that can serve as a reference: + +* :class:`zgw_consumers_ext.api_client.ServiceClientFactory` +* :class:`soap.client.session_factory.SessionFactory` +* :class:`stuf.service_client_factory.ServiceClientFactory` + + +Reference +========= + +ZGW-consumers (JSON-based/RESTful services) +------------------------------------------- + +.. automodule:: zgw_consumers_ext.api_client + :members: + +Zeep (SOAP client) +------------------ + +Zeep supports a ``session`` keyword argument for its transport, which is plug and play +with our base client. + +.. automodule:: soap.client + :members: + +StUF (template based SOAP/XML) +------------------------------ + +.. automodule:: stuf.client + :members: + + +Design constraints +------------------ + +The client implementation was set up with some design constraints: + +- Must support the :class:`requests.Session` API +- Must be compatible with :class:`zgw_consumers.models.Service`, + :class:`stuf.models.StUFService` and :class:`soap.models.SOAPService` +- Should encourage best practices (closing resources after use) +- Should not create problems when used with other libraries, e.g. ``requests-oauth2client`` + +Eventually we'd like to explore using ``httpx`` rather than ``requests`` - it has a +similar API, but it's also ``async`` / ``await`` capable. The abstraction of the +underlying driver (now requests, later httpx) should not matter and most importantly, +not be leaky. + +.. note:: Not being leaky here means that you can use the requests API (in the future: + httpx) like you would normally do without this library getting in the way. diff --git a/docs/developers/backend/index.rst b/docs/developers/backend/index.rst index a0c5c3f3d8..0329a85c06 100644 --- a/docs/developers/backend/index.rst +++ b/docs/developers/backend/index.rst @@ -65,6 +65,7 @@ General purpose functionality .. toctree:: :maxdepth: 1 + api-clients core/utils core/tokens core/testing-tools diff --git a/src/zgw_consumers_ext/api_client.py b/src/zgw_consumers_ext/api_client.py index 0c3bfad0d5..ee2ed117c8 100644 --- a/src/zgw_consumers_ext/api_client.py +++ b/src/zgw_consumers_ext/api_client.py @@ -19,6 +19,11 @@ def build_client(service: Service, client_factory: type[T] = NLXClient, **kwargs) -> T: """ Build a client for a given :class:`zgw_consumers.models.Service`. + + :arg service: The model instance holding the service configuration. + :arg client_factory: Particular client (sub)class for more specialized clients. + + Any additional keyword arguments are forwarded to the client initialization. """ factory = ServiceClientFactory(service) return client_factory.configure_from(factory, nlx_base_url=service.nlx, **kwargs) @@ -26,12 +31,25 @@ def build_client(service: Service, client_factory: type[T] = NLXClient, **kwargs @dataclass class ServiceClientFactory: + """ + Map zgw_consumers.Service model configuration to API client parameters. + """ + service: Service def get_client_base_url(self) -> str: + """ + Use the configured API root as client base URL. + """ return self.service.api_root def get_client_session_kwargs(self) -> dict[str, Any]: + """ + Pass connection parameters from the configured service. + + * mTLS configuration from uploaded/configured certificates + * authentication type and credentials from DB configuration + """ kwargs = {} # mTLS: verify server certificate if configured @@ -62,6 +80,10 @@ def get_client_session_kwargs(self) -> dict[str, Any]: @dataclass class APIKeyAuth(AuthBase): + """ + Requests authentication class for API key-based auth (in the request headers). + """ + header: str key: str @@ -72,6 +94,10 @@ def __call__(self, request: PreparedRequest): @dataclass class ZGWAuth(AuthBase): + """ + Requests authentication class for JWT-based ZGW auth. + """ + service: Service auth: ClientAuth = field(init=False) From 5cca83e1493bb903a19c0c76367dddfcfd680c0e Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Mon, 2 Oct 2023 17:19:10 +0200 Subject: [PATCH 2/5] :pencil: [#3489] Update BAG configuration documentation * OAS is no longer required * Change updated labels --- docs/configuration/prefill/bag.rst | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/configuration/prefill/bag.rst b/docs/configuration/prefill/bag.rst index 8aed49e8c5..e413029d75 100644 --- a/docs/configuration/prefill/bag.rst +++ b/docs/configuration/prefill/bag.rst @@ -25,15 +25,16 @@ Configuration * **Authorization type**: API key * **Header key**: X-Api-Key * **Header value**: *The BAG API key obtained in step 1* - * **OAS**: ``https://raw.githubusercontent.com/lvbag/BAG-API/4ab6ea5f3c2749f62d1079e2fe7539d4526c04e7/Technische%20specificatie/Archief/Yaml's/BAG%20API%20Individuele%20Bevragingen/resolved/individuelebevragingen/v2/adressen.yaml`` 4. Click **Save** -5. Navigate to **Configuration** > **Overview**. In the **Address search plugin** group, click on **Configuration** for the **BAG** line. +5. Navigate to **Configuration** > **Overview**. In the **Address lookup plugins** + group, click on **Configuration** for the **Kadaster API: BAG** line. 6. Select for the **BAG service**, the **[ORC (Overige)] BAG (Kadaster)** option, that we just created in step 3. 7. Click **Save** -The BAG configuration is now completed. +The BAG configuration is now complete. You can refresh the configuration overview page +to do a connection check. .. _`BAG API`: https://api.bag.kadaster.nl/lvbag/individuelebevragingen/v2/openapi.yaml .. _`BAG API key`: https://www.kadaster.nl/zakelijk/producten/adressen-en-gebouwen/bag-api-individuele-bevragingen From 3c581d49dc295b3d600425c0f84020ee1cb1a294 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Mon, 2 Oct 2023 17:22:42 +0200 Subject: [PATCH 3/5] :pencil: [#3489] Update HC: BRP bevragen configuration documentation * OAS is no longer required * Change updated labels * document v2.0 is supported --- docs/configuration/prefill/haal_centraal.rst | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/docs/configuration/prefill/haal_centraal.rst b/docs/configuration/prefill/haal_centraal.rst index dd4a3cf794..cfab42a30b 100644 --- a/docs/configuration/prefill/haal_centraal.rst +++ b/docs/configuration/prefill/haal_centraal.rst @@ -36,15 +36,18 @@ Configuration * **Authorization type**: API key *(but can differ per supplier)* * **Header key**: Authorization * **Header value**: *The API key from step 1* - * **OAS**: *URL to the Open API-specification provided by supplier or use the Github raw URL* 4. Click **Save** -5. Navigate to **Configuration** > **Overview**. In the **Prefill plugin** group, click on **Configuration** for the **Haal Centraal** line. -6. Select for the **Haal Centraal API**, the **[ORC (Overige)] My BRP API** +5. Navigate to **Configuration** > **Configuration overview**. In the **Prefill plugins** + group, click on **Configuration** for the **Haal Centraal: BRP Personen Bevragen** + line. +6. Select for the **BRP Personen Bevragen API**, the **[ORC (Overige)] My BRP API** option, that we just created in step 3. -7. Click **Save** +7. Select the correct version for **BRP Personen Bevragen API version** - new + installations likely use ``v2.0``. +8. Click **Save** -The Haal Centraal configuration is now completed. +The Haal Centraal configuration is now complete. Technical @@ -53,5 +56,6 @@ Technical ================ =================== API Supported versions ================ =================== -BRP bevragen API 1.0 +BRP bevragen API 2.0 +BRP bevragen API 1.3 ================ =================== From cb5fbfc4808b5ba5c64cd36ff57c5bff5ba5e081 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Mon, 2 Oct 2023 17:24:54 +0200 Subject: [PATCH 4/5] :speech_balloon: [#3489] Update labels for kadaster services --- ..._alter_kadasterapiconfig_search_service.py | 30 +++++++++++++++++++ src/openforms/contrib/kadaster/models.py | 3 +- 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 src/openforms/contrib/kadaster/migrations/0004_alter_kadasterapiconfig_search_service.py diff --git a/src/openforms/contrib/kadaster/migrations/0004_alter_kadasterapiconfig_search_service.py b/src/openforms/contrib/kadaster/migrations/0004_alter_kadasterapiconfig_search_service.py new file mode 100644 index 0000000000..4a5ec6f930 --- /dev/null +++ b/src/openforms/contrib/kadaster/migrations/0004_alter_kadasterapiconfig_search_service.py @@ -0,0 +1,30 @@ +# Generated by Django 3.2.21 on 2023-10-02 15:24 + +import django.db.models.deletion +from django.db import migrations, models + +import openforms.contrib.kadaster.models + + +class Migration(migrations.Migration): + + dependencies = [ + ("zgw_consumers", "0019_alter_service_uuid"), + ("kadaster", "0003_move_bag_service_config"), + ] + + operations = [ + migrations.AlterField( + model_name="kadasterapiconfig", + name="search_service", + field=models.OneToOneField( + default=openforms.contrib.kadaster.models.get_default_search_service, + help_text="Service for geo search queries.", + limit_choices_to={"api_type": "orc"}, + on_delete=django.db.models.deletion.PROTECT, + related_name="+", + to="zgw_consumers.service", + verbose_name="Locatieserver API", + ), + ), + ] diff --git a/src/openforms/contrib/kadaster/models.py b/src/openforms/contrib/kadaster/models.py index 1806e0a1de..14cb1225c9 100644 --- a/src/openforms/contrib/kadaster/models.py +++ b/src/openforms/contrib/kadaster/models.py @@ -39,7 +39,8 @@ class KadasterApiConfig(SingletonModel): search_service = models.OneToOneField( "zgw_consumers.Service", - verbose_name=_("Kadaster API"), + verbose_name=_("Locatieserver API"), + help_text=_("Service for geo search queries."), on_delete=models.PROTECT, limit_choices_to={"api_type": APITypes.orc}, related_name="+", From 629213f04804fc358fda2d76e5f057883e609af7 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Fri, 6 Oct 2023 15:54:56 +0200 Subject: [PATCH 5/5] :heavy_plus_sign: [#3489] use PyPI dependency instead of vendored api client The implementation was moved to a standalone package. --- docs/conf.py | 16 ++ docs/developers/backend/api-clients.rst | 100 +-------- requirements/base.in | 1 + requirements/base.txt | 4 + requirements/ci.txt | 6 + requirements/dev.txt | 6 + requirements/extensions.txt | 6 + src/api_client/README.md | 75 ------- src/api_client/__init__.py | 4 - src/api_client/client.py | 125 ----------- src/api_client/exceptions.py | 5 - src/api_client/tests/__init__.py | 0 src/api_client/tests/test_client_api.py | 209 ------------------ src/api_client/tests/test_helpers.py | 76 ------- src/api_client/typing.py | 28 --- .../appointments/contrib/qmatic/client.py | 2 +- .../contrib/haal_centraal/clients/brp.py | 2 +- src/openforms/contrib/hal_client.py | 2 +- .../contrib/kadaster/clients/locatieserver.py | 3 +- src/openforms/contrib/zgw/clients/utils.py | 3 +- .../zgw/tests/test_pagination_helper.py | 3 +- src/soap/client.py | 3 +- src/soap/tests/test_client.py | 2 +- src/stuf/client.py | 4 +- src/stuf/tests/test_client.py | 2 +- src/zgw_consumers_ext/nlx.py | 3 +- .../tests/test_client_factory.py | 2 +- 27 files changed, 56 insertions(+), 636 deletions(-) delete mode 100644 src/api_client/README.md delete mode 100644 src/api_client/__init__.py delete mode 100644 src/api_client/client.py delete mode 100644 src/api_client/exceptions.py delete mode 100644 src/api_client/tests/__init__.py delete mode 100644 src/api_client/tests/test_client_api.py delete mode 100644 src/api_client/tests/test_helpers.py delete mode 100644 src/api_client/typing.py diff --git a/docs/conf.py b/docs/conf.py index f674007650..fa2f4a2351 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -38,6 +38,7 @@ extensions = [ "sphinx.ext.autodoc", "sphinx.ext.todo", + "sphinx.ext.intersphinx", "sphinx_tabs.tabs", "recommonmark", # "sphinx_markdown_tables", @@ -60,6 +61,21 @@ source_suffix = [".rst", ".md"] +intersphinx_mapping = { + "requests": ( + "https://docs.python-requests.org/en/latest/", + None, + ), + "ape_pie": ( + "https://ape-pie.readthedocs.io/en/latest/", + None, + ), + "django": ( + "http://docs.djangoproject.com/en/3.2/", + "http://docs.djangoproject.com/en/3.2/_objects/", + ), +} + # -- Options for HTML output ------------------------------------------------- # The theme to use for HTML and HTML Help pages. See the documentation for diff --git a/docs/developers/backend/api-clients.rst b/docs/developers/backend/api-clients.rst index 8f3b0d2462..e9daff934b 100644 --- a/docs/developers/backend/api-clients.rst +++ b/docs/developers/backend/api-clients.rst @@ -29,82 +29,14 @@ connect with these services: Combinations of these patterns are possible too! Open Forms uses an abstraction that accounts for these variations while allowing -developers to focus on the actual consuming of the service. - -API client base -=============== - -We have opted to implement a base API client class around the :class:`requests.Session`, -more specifically - a subclass of ``Session``. - -Import it from: - -.. code-block:: python - - from api_client import APIClient - - -It's a small extension of the requests ``Session``, allowing you to specify some session -defaults (such as mTLS and auth parameters) while requiring you to set a base URL. The -base URL prevents accidental sending of credentials to other URLs than the base, while -allowing you to implement clients using relative paths/URLs. +developers to focus on the actual consuming of the service, packaged into the library +`ape-pie `_. Because it extends the core :mod:`requests` API, usage should feel familiar. You are encouraged to define your own service-specific subclasses to modify behaviour where needed. -Recommended usage ------------------ - -Our extension allows for configuring such an instance from "configuration objects", -whatever those may be: - -.. code-block:: python - - from api_client import APIClient - - from .factories import my_factory - - client = APIClient.configure_from(my_factory) - - with client: - # ⚡️ context manager -> uses connection pooling and is recommended! - response1 = client.get("some-relative-path", params={"foo": ["bar"]}) - response2 = client.post("other-path", json={...}) - -The ``my_factory`` is a "special" -:ref:`configuration source `, which feeds the -relevant initialization parameters to the :class:`api_client.client.APIClient` instance. - -.. note:: You can (and should) use the client/session in a context manager to benefit - from connection pooling and thus better performance when multiple requests are made. - -Low level usage ---------------- - -The low level usage is essentially what is called by the factory usage. The biggest risk -is forgetting to apply certain connection configuration, like mTLS parameters, therefor -we recommend setting up a configuration factory instead. - -.. code-block:: - - from api_client import APIClient - from requests.auth import - - # You can pass most attributes available on requests.Session, like auth/verify/cert... - client = APIClient( - "https://example.com/api/v1/", - auth=HTTPBasicAuth("superuser", "letmein"), - verify="/path/to/custom/ca-bundle.pem", - ) - - with client: - # ⚡️ context manager -> uses connection pooling and is recommended! - response1 = client.get("some-relative-path", params={"foo": ["bar"]}) - response2 = client.post("other-path", json={...}) - - .. _developers_backend_api_clients_factories: Configuration factories @@ -114,15 +46,11 @@ Configuration factories are a small abstraction that allow you to instantiate cl with the appropriate configuration/presets from sources holding the configuration details - for example database records. -Such a factory must implemented the ``APIClientFactory`` protocol: - -.. autoclass:: api_client.typing.APIClientFactory - :members: - +Such a factory must implemented the :class:`ape_pie.ConfigAdapter` protocol. Some examples that can serve as a reference: -* :class:`zgw_consumers_ext.api_client.ServiceClientFactory` +* :class:`zgw_consumers_ext.ape_pie.ServiceClientFactory` * :class:`soap.client.session_factory.SessionFactory` * :class:`stuf.service_client_factory.ServiceClientFactory` @@ -150,23 +78,3 @@ StUF (template based SOAP/XML) .. automodule:: stuf.client :members: - - -Design constraints ------------------- - -The client implementation was set up with some design constraints: - -- Must support the :class:`requests.Session` API -- Must be compatible with :class:`zgw_consumers.models.Service`, - :class:`stuf.models.StUFService` and :class:`soap.models.SOAPService` -- Should encourage best practices (closing resources after use) -- Should not create problems when used with other libraries, e.g. ``requests-oauth2client`` - -Eventually we'd like to explore using ``httpx`` rather than ``requests`` - it has a -similar API, but it's also ``async`` / ``await`` capable. The abstraction of the -underlying driver (now requests, later httpx) should not matter and most importantly, -not be leaky. - -.. note:: Not being leaky here means that you can use the requests API (in the future: - httpx) like you would normally do without this library getting in the way. diff --git a/requirements/base.in b/requirements/base.in index f8007ecd65..cf642792ad 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -1,4 +1,5 @@ # Core python libraries +ape-pie bleach[css] >= 5 celery ~= 5.0 celery-once diff --git a/requirements/base.txt b/requirements/base.txt index ce4cdf5ccb..70357cace6 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -8,6 +8,8 @@ amqp==5.0.9 # via kombu +ape-pie==0.1.0 + # via -r requirements/base.in asgiref==3.5.0 # via django async-timeout==4.0.2 @@ -247,6 +249,7 @@ frozendict==2.3.4 furl==2.1.2 # via # -r requirements/base.in + # ape-pie # django-digid-eherkenning gemma-zds-client==2.0.0 # via zgw-consumers @@ -400,6 +403,7 @@ redis==4.5.4 # portalocker requests==2.31.0 # via + # ape-pie # django-camunda # django-log-outgoing-requests # gemma-zds-client diff --git a/requirements/ci.txt b/requirements/ci.txt index 378cc5bff9..4f2cae0813 100644 --- a/requirements/ci.txt +++ b/requirements/ci.txt @@ -13,6 +13,10 @@ amqp==5.0.9 # -c requirements/base.txt # -r requirements/base.txt # kombu +ape-pie==0.1.0 + # via + # -c requirements/base.txt + # -r requirements/base.txt asgiref==3.5.0 # via # -c requirements/base.txt @@ -443,6 +447,7 @@ furl==2.1.2 # via # -c requirements/base.txt # -r requirements/base.txt + # ape-pie # django-digid-eherkenning gemma-zds-client==2.0.0 # via @@ -774,6 +779,7 @@ requests==2.31.0 # via # -c requirements/base.txt # -r requirements/base.txt + # ape-pie # django-camunda # django-log-outgoing-requests # gemma-zds-client diff --git a/requirements/dev.txt b/requirements/dev.txt index 65f89a4089..3cb480148b 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -16,6 +16,10 @@ amqp==5.0.9 # -c requirements/ci.txt # -r requirements/ci.txt # kombu +ape-pie==0.1.0 + # via + # -c requirements/ci.txt + # -r requirements/ci.txt asgiref==3.5.0 # via # -c requirements/ci.txt @@ -494,6 +498,7 @@ furl==2.1.2 # via # -c requirements/ci.txt # -r requirements/ci.txt + # ape-pie # django-digid-eherkenning gemma-zds-client==2.0.0 # via @@ -900,6 +905,7 @@ requests==2.31.0 # via # -c requirements/ci.txt # -r requirements/ci.txt + # ape-pie # ddt-api-calls # django-camunda # django-log-outgoing-requests diff --git a/requirements/extensions.txt b/requirements/extensions.txt index c4bb02844e..7dd4158717 100644 --- a/requirements/extensions.txt +++ b/requirements/extensions.txt @@ -10,6 +10,10 @@ amqp==5.0.9 # via # -r requirements/base.txt # kombu +ape-pie==0.1.0 + # via + # -c requirements/base.in + # -r requirements/base.txt asgiref==3.5.0 # via # -r requirements/base.txt @@ -376,6 +380,7 @@ furl==2.1.2 # via # -c requirements/base.in # -r requirements/base.txt + # ape-pie # django-digid-eherkenning # open-forms-ext-token-exchange gemma-zds-client==2.0.0 @@ -624,6 +629,7 @@ redis==4.5.4 requests==2.31.0 # via # -r requirements/base.txt + # ape-pie # django-camunda # django-log-outgoing-requests # gemma-zds-client diff --git a/src/api_client/README.md b/src/api_client/README.md deleted file mode 100644 index 2aeb7c2c97..0000000000 --- a/src/api_client/README.md +++ /dev/null @@ -1,75 +0,0 @@ -# API Client - -Implements a generic/base API client as a wrapper around `requests` API. - -The client is a general purpose HTTP client, meaning it's not limited to RESTful services but also -suitable for SOAP (for example). - -## Usage - -The client is a thin wrapper around `requests.Session`, with some guard rails in place: - -- specify a base URL and check that absolute URL requests fit within the base URL -- manage resources by closing the session even for one-off requests - -There are two ways to instantiate a client. - -- from a factory (preferred) -- manually - -The factory approach is preferred as it provides the most robust way to honour authentication -configuration such as credentials and mutual TLS parameters. - -### From a factory - -Factories must implement the `api_client.typing.APIClientFactory` protocol, which provides the -client instance with the base URL and any session-level keyword arguments. This could come from a -Django model instance, or some class taking a YAML configuration file. - -```py -from api_client import APIClient - -from .factories import my_factory - -client = APIClient.configure_from(my_factory) - -with client: - # ⚡️ context manager -> uses connection pooling and is recommended! - response1 = client.get("some-relative-path", params={"foo": ["bar"]}) - response2 = client.post("other-path", json={...}) -``` - -### Manually - -```py -from api_client import APIClient -from requests.auth import - -client = APIClient( - "https://example.com/api/v1/", - auth=HTTPBasicAuth("superuser", "letmein"), - verify="/path/to/custom/ca-bundle.pem", -) - -with client: - # ⚡️ context manager -> uses connection pooling and is recommended! - response1 = client.get("some-relative-path", params={"foo": ["bar"]}) - response2 = client.post("other-path", json={...}) -``` - -## Design constraints - -- Must support the `requests.Session` API -- Must be compatible with `zgw_consumers.Service`, `stuf.StUFService` and `soap.SOAPService` -- Should encourage best practices (closing resources after use) -- Should not create problems when used with other libraries, e.g. `requests-oauth2client` - -The client is "simply" a subclass of `requests.Session` which allows us to achieve many of the above -constraints. - -Eventually we'd like to jump ship to use `httpx` rather than `requests` - it has a similar API, but -it's also `async` / `await` capable. The abstraction of the underlying driver (now requests, later -httpx) should not matter and most importantly, not be leaky. - -NOTE: not being leaky here means that you can use the requests API (in the future: httpx) like you -would normally do without this library getting in the way. diff --git a/src/api_client/__init__.py b/src/api_client/__init__.py deleted file mode 100644 index 75d823f1f6..0000000000 --- a/src/api_client/__init__.py +++ /dev/null @@ -1,4 +0,0 @@ -from .client import APIClient -from .exceptions import InvalidURLError - -__all__ = ["APIClient", "InvalidURLError"] diff --git a/src/api_client/client.py b/src/api_client/client.py deleted file mode 100644 index 59d7ff83b1..0000000000 --- a/src/api_client/client.py +++ /dev/null @@ -1,125 +0,0 @@ -""" -Implements an API client class as a :class:`requests.Session` subclass. - -Some inspiration was taken from https://github.com/guillp/requests_oauth2client/, -notably: - -* Implementing the client as a ``Session`` subclass -* Providing a base_url and making this absolute -""" -from contextlib import contextmanager -from typing import Any - -from furl import furl -from requests import Session - -from .exceptions import InvalidURLError -from .typing import APIClientFactory - -sentinel = object() - - -def is_base_url(url: str | furl) -> bool: - """ - Check if a URL is not a relative path/URL. - - A URL is considered a base URL if it has: - - * a scheme - * a netloc - - Protocol relative URLs like //example.com cannot be properly handled by requests, - as there is no default adapter available. - """ - if not isinstance(url, furl): - url = furl(url) - return bool(url.scheme and url.netloc) - - -class APIClient(Session): - base_url: str - _request_kwargs: dict[str, Any] - _in_context_manager: bool = False - - def __init__( - self, - base_url: str, - request_kwargs: dict[str, Any] | None = None, - **kwargs, # subclasses may require additional configuration - ): - # base class does not take any kwargs - super().__init__() - # normalize to dict - request_kwargs = request_kwargs or {} - - self.base_url = base_url - - # set the attributes that requests.Session supports directly, but only if an - # actual value was provided. - for attr in self.__attrs__: - val = request_kwargs.pop(attr, sentinel) - if val is sentinel: - continue - setattr(self, attr, val) - - # store the remainder so we can inject it in the ``request`` method. - self._request_kwargs = request_kwargs - - def __enter__(self): - self._in_context_manager = True - return super().__enter__() - - def __exit__(self, *args): - self._in_context_manager = False - return super().__exit__(*args) - - @classmethod - def configure_from(cls, factory: APIClientFactory, **kwargs): - base_url = factory.get_client_base_url() - session_kwargs = factory.get_client_session_kwargs() - return cls(base_url, session_kwargs, **kwargs) - - def request(self, method, url, *args, **kwargs): - for attr, val in self._request_kwargs.items(): - kwargs.setdefault(attr, val) - url = self.to_absolute_url(url) - with self._maybe_close_session(): - return super().request(method, url, *args, **kwargs) - - @contextmanager - def _maybe_close_session(self): - """ - Clean up resources to avoid leaking them. - - A requests session uses connection pooling when used in a context manager, and - the __exit__ method will properly clean up this connection pool when the block - exists. However, it's also possible to instantiate and use a client outside a - context block which potentially does not clean up any resources. - - We detect these situations and close the session if needed. - """ - _should_close = not self._in_context_manager - try: - yield - finally: - if _should_close: - self.close() - - def to_absolute_url(self, maybe_relative_url: str) -> str: - base_furl = furl(self.base_url) - # absolute here should be interpreted as "fully qualified url", with a protocol - # and netloc - is_absolute = is_base_url(maybe_relative_url) - if is_absolute: - # we established the target URL is absolute, so ensure that it's contained - # within the self.base_url domain, otherwise you risk sending credentials - # intended for the base URL to some other domain. - has_same_base = maybe_relative_url.startswith(self.base_url) - if not has_same_base: - raise InvalidURLError( - f"Target URL {maybe_relative_url} has a different base URL than the " - f"client ({self.base_url})." - ) - return maybe_relative_url - fully_qualified = base_furl / maybe_relative_url - return str(fully_qualified) diff --git a/src/api_client/exceptions.py b/src/api_client/exceptions.py deleted file mode 100644 index c3dde94a26..0000000000 --- a/src/api_client/exceptions.py +++ /dev/null @@ -1,5 +0,0 @@ -import requests - - -class InvalidURLError(requests.RequestException): - pass diff --git a/src/api_client/tests/__init__.py b/src/api_client/tests/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/src/api_client/tests/test_client_api.py b/src/api_client/tests/test_client_api.py deleted file mode 100644 index 6a93b98658..0000000000 --- a/src/api_client/tests/test_client_api.py +++ /dev/null @@ -1,209 +0,0 @@ -from unittest import TestCase -from unittest.mock import patch - -import requests_mock -from hypothesis import given, strategies as st -from requests import Session -from requests.auth import HTTPBasicAuth - -from api_client.exceptions import InvalidURLError - -from ..client import APIClient - -http_methods = st.sampled_from( - ["GET", "OPTIONS", "HEAD", "POST", "PUT", "PATCH", "DELETE"] -) - - -class DirectInstantiationTests(TestCase): - def test_defaults_from_requests_session(self): - vanilla_session = Session() - - client = APIClient("https://example.com") - - for attr in Session.__attrs__: - if attr == "adapters": - continue - with self.subTest(attr=attr): - vanilla_value = getattr(vanilla_session, attr) - client_value = getattr(client, attr) - - self.assertEqual(client_value, vanilla_value) - - def test_can_override_defaults(self): - vanilla_session = Session() - overrides = [ - ("auth", HTTPBasicAuth("superuser", "letmein")), - ("verify", False), - ("cert", ("/tmp/cert.pem", "/tmp/key.pem")), - ] - - for attr, value in overrides: - # sanity check for test itself - assert attr in Session.__attrs__ - - with self.subTest(attr=attr, value=value): - client = APIClient("https://example.com", {attr: value}) - - vanilla_value = getattr(vanilla_session, attr) - client_value = getattr(client, attr) - - self.assertNotEqual(client_value, vanilla_value) - self.assertEqual(client_value, value) - - -class TestFactory: - @staticmethod - def get_client_base_url(): - return "https://from-factory.example.com" - - @staticmethod - def get_client_session_kwargs(): - return { - "verify": False, - "timeout": 20, - "auth": HTTPBasicAuth("superuser", "letmein"), - } - - -class FromFactoryTests(TestCase): - def test_factory_can_configure_session(self): - factory = TestFactory() - - client = APIClient.configure_from(factory) - - self.assertFalse(client.verify) - self.assertIsNotNone(client.auth) - self.assertFalse(hasattr(client, "timeout")) - - -class RequestTests(TestCase): - @requests_mock.Mocker() - def test_runtime_request_kwargs(self, m): - m.get(requests_mock.ANY, text="ok") - factory = TestFactory() - - with APIClient.configure_from(factory) as client: - client.get("https://from-factory.example.com/foo") - - self.assertEqual(m.last_request.url, "https://from-factory.example.com/foo") - headers = m.last_request.headers - self.assertIn("Authorization", headers) - self.assertTrue(headers["Authorization"].startswith("Basic ")) - self.assertFalse(m.last_request.verify) - self.assertEqual(m.last_request.timeout, 20.0) - - @requests_mock.Mocker() - def test_request_kwargs_overrule_defaults(self, m): - m.get(requests_mock.ANY, text="ok") - factory = TestFactory() - - with APIClient.configure_from(factory) as client: - client.get( - "https://from-factory.example.com/foo", - timeout=5, - verify=True, - ) - - self.assertEqual(m.last_request.url, "https://from-factory.example.com/foo") - headers = m.last_request.headers - self.assertIn("Authorization", headers) - self.assertTrue(headers["Authorization"].startswith("Basic ")) - self.assertTrue(m.last_request.verify) - self.assertEqual(m.last_request.timeout, 5.0) - - @given(http_methods) - def test_applies_to_any_http_method(self, method): - factory = TestFactory() - - with ( - requests_mock.Mocker() as m, - APIClient.configure_from(factory) as client, - ): - m.register_uri(requests_mock.ANY, requests_mock.ANY) - - client.request(method, "https://from-factory.example.com/foo") - - self.assertEqual(len(m.request_history), 1) - self.assertEqual(m.last_request.url, "https://from-factory.example.com/foo") - self.assertEqual(m.last_request.method, method) - headers = m.last_request.headers - self.assertIn("Authorization", headers) - self.assertTrue(headers["Authorization"].startswith("Basic ")) - self.assertFalse(m.last_request.verify) - self.assertEqual(m.last_request.timeout, 20.0) - - @given(http_methods) - def test_relative_urls_are_made_absolute(self, method): - factory = TestFactory() - client = APIClient.configure_from(factory) - - with ( - requests_mock.Mocker() as m, - client, - ): - m.register_uri(requests_mock.ANY, requests_mock.ANY) - - client.request(method, "foo") - - self.assertEqual(len(m.request_history), 1) - self.assertEqual(m.last_request.url, "https://from-factory.example.com/foo") - - @given(http_methods) - def test_absolute_urls_must_match_base_url(self, method): - factory = TestFactory() - client = APIClient.configure_from(factory) - - with self.assertRaises(InvalidURLError): - client.request(method, "https://example.com/bar") - - @given(http_methods) - def test_absolute_urls_must_match_base_url_happy_flow(self, method): - factory = TestFactory() - client = APIClient.configure_from(factory) - - with ( - requests_mock.Mocker() as m, - client, - ): - m.register_uri(requests_mock.ANY, requests_mock.ANY) - - client.request(method, "https://from-factory.example.com/foo/bar") - - self.assertEqual(len(m.request_history), 1) - self.assertEqual(m.last_request.url, "https://from-factory.example.com/foo/bar") - - @given(http_methods) - def test_discouraged_usage_without_context(self, method): - client = APIClient("https://example.com") - - with ( - requests_mock.Mocker() as m, - patch.object(client, "close", wraps=client.close) as mock_close, - ): - m.register_uri(requests_mock.ANY, requests_mock.ANY) - - client.request(method, "foo") - - self.assertEqual(len(m.request_history), 1) - mock_close.assert_called_once() - - @given(http_methods) - def test_encouraged_usage_with_context_do_not_close_prematurely(self, method): - client = APIClient("https://example.com") - - with ( - patch.object(client, "close", wraps=client.close) as mock_close, - requests_mock.Mocker() as m, - client, - ): - m.register_uri(requests_mock.ANY, requests_mock.ANY) - - client.request(method, "foo") - - # may not be called inside context block - mock_close.assert_not_called() - - self.assertEqual(len(m.request_history), 1) - # must be called outside context block - mock_close.assert_called_once() diff --git a/src/api_client/tests/test_helpers.py b/src/api_client/tests/test_helpers.py deleted file mode 100644 index ef719d7cc2..0000000000 --- a/src/api_client/tests/test_helpers.py +++ /dev/null @@ -1,76 +0,0 @@ -import string -from urllib.parse import quote_plus, urlsplit - -from django.test import SimpleTestCase - -from furl import furl -from hypothesis import assume, example, given, strategies as st -from hypothesis.provisional import domains, urls - -from ..client import is_base_url - -printable_text = st.text(string.printable) - - -class IsBaseUrlTests(SimpleTestCase): - @given(domains()) - def test_domain_without_protocol(self, item: str): - assume(not item.startswith("http://")) - assume(not item.startswith("https://")) - - self.assertFalse(is_base_url(item)) - - @given(st.text(string.printable)) - @example("/some/absolute/path") - def test_random_text_without_protocol(self, item: str): - assume(not item.startswith("http://")) - assume(not item.startswith("https://")) - - try: - is_base = is_base_url(item) - except ValueError: - # furl got something that it can't parse as a URL, and we do want to bubble - # this error up to the caller - pass - else: - self.assertFalse(is_base) - - @given( - st.sampled_from(["https", "http", "ftp", "file"]), - st.lists(printable_text.map(quote_plus)).map("/".join), - ) - def test_protocol_but_no_netloc(self, protocol, path): - url = f"{protocol}:///{path}" - - self.assertFalse(is_base_url(url)) - - @given(urls()) - def test_rfc_3986_url(self, url): - assert url.startswith("http://") or url.startswith("https://") - bits = urlsplit(url) - # not allowed for communication between hosts - it's a way to request a dynamically - # allocated port number. - assume(bits.port != 0) - - self.assertTrue(is_base_url(url)) - - @given( - st.sampled_from(["ftp", "file", "madeupthing"]), - domains(), - st.lists(printable_text.map(quote_plus)).map("/".join), - ) - def test_non_http_protocol(self, protocol, domain, path): - url = f"{protocol}://{domain}/{path}" - - # we don't really care about the actual protocol, you *could* install a requests - # adapter for non-http(s) - self.assertTrue(is_base_url(url)) - - def test_handle_str_or_furl_instance(self): - example = "https://example.com/foo" - - with self.subTest("raw string"): - self.assertTrue(is_base_url(example)) - - with self.subTest("furl instance string"): - self.assertTrue(is_base_url(furl(example))) diff --git a/src/api_client/typing.py b/src/api_client/typing.py deleted file mode 100644 index 583e444f55..0000000000 --- a/src/api_client/typing.py +++ /dev/null @@ -1,28 +0,0 @@ -from typing import Any, Protocol - - -class APIClientFactory(Protocol): - def get_client_base_url(self) -> str: # pragma: no cover - """ - Return the API root/base URL to which relative URLs are made. - """ - ... - - def get_client_session_kwargs(self) -> dict[str, Any]: # pragma: no cover - """ - Return kwargs to feed to :class:`requests.Session` when using the client. - - Provide a dict of possible :class:`requests.Session` attributes which will - (typically) be used as defaults for each request sent from the session, such as - ``auth`` for authentication or ``cert`` and/or ``verify`` for mutual TLS - purposes. Other examples would be a ``timeout`` that's service-specific (and - potentially different from the global default). - - Note that many of these kwargs can still be overridden at call time, e.g.: - - .. code-block:: python - - with APIClient.configure_from(some_service) as client: - response = client.get("some/relative/path", timeout=10) - """ - ... diff --git a/src/openforms/appointments/contrib/qmatic/client.py b/src/openforms/appointments/contrib/qmatic/client.py index 25d04e7c47..b9c0ba85f7 100644 --- a/src/openforms/appointments/contrib/qmatic/client.py +++ b/src/openforms/appointments/contrib/qmatic/client.py @@ -2,10 +2,10 @@ from typing import TypedDict import pytz +from ape_pie.client import APIClient from dateutil.parser import isoparse from zgw_consumers.models import Service -from api_client.client import APIClient from zgw_consumers_ext.api_client import ServiceClientFactory from .exceptions import QmaticException diff --git a/src/openforms/contrib/haal_centraal/clients/brp.py b/src/openforms/contrib/haal_centraal/clients/brp.py index 894496851e..edff74ab70 100644 --- a/src/openforms/contrib/haal_centraal/clients/brp.py +++ b/src/openforms/contrib/haal_centraal/clients/brp.py @@ -11,8 +11,8 @@ from dataclasses import dataclass import requests +from ape_pie import APIClient -from api_client import APIClient from openforms.contrib.hal_client import HALMixin from openforms.pre_requests.clients import PreRequestMixin from openforms.typing import JSONObject diff --git a/src/openforms/contrib/hal_client.py b/src/openforms/contrib/hal_client.py index 7368049a22..79b15aa6b1 100644 --- a/src/openforms/contrib/hal_client.py +++ b/src/openforms/contrib/hal_client.py @@ -1,4 +1,4 @@ -from api_client import APIClient +from ape_pie import APIClient HAL_CONTENT_TYPE = "application/hal+json" diff --git a/src/openforms/contrib/kadaster/clients/locatieserver.py b/src/openforms/contrib/kadaster/clients/locatieserver.py index efe3b9eb67..4d31a14f9a 100644 --- a/src/openforms/contrib/kadaster/clients/locatieserver.py +++ b/src/openforms/contrib/kadaster/clients/locatieserver.py @@ -4,8 +4,7 @@ from django.contrib.gis.geos import fromstr import requests - -from api_client import APIClient +from ape_pie import APIClient logger = logging.getLogger(__name__) diff --git a/src/openforms/contrib/zgw/clients/utils.py b/src/openforms/contrib/zgw/clients/utils.py index 8ed681d8f3..a42377b347 100644 --- a/src/openforms/contrib/zgw/clients/utils.py +++ b/src/openforms/contrib/zgw/clients/utils.py @@ -3,8 +3,7 @@ from django.utils import timezone import pytz - -from api_client import APIClient +from ape_pie import APIClient TIMEZONE_AMS = pytz.timezone("Europe/Amsterdam") diff --git a/src/openforms/contrib/zgw/tests/test_pagination_helper.py b/src/openforms/contrib/zgw/tests/test_pagination_helper.py index 0a43388ba2..c730d19a7a 100644 --- a/src/openforms/contrib/zgw/tests/test_pagination_helper.py +++ b/src/openforms/contrib/zgw/tests/test_pagination_helper.py @@ -1,8 +1,7 @@ from unittest import TestCase import requests_mock - -from api_client import APIClient +from ape_pie import APIClient from ..clients.utils import pagination_helper diff --git a/src/soap/client.py b/src/soap/client.py index 6c2dc3e0f4..68c7be0d77 100644 --- a/src/soap/client.py +++ b/src/soap/client.py @@ -1,8 +1,7 @@ +from ape_pie.client import APIClient as SessionBase, is_base_url from zeep.client import Client from zeep.transports import Transport -from api_client.client import APIClient as SessionBase, is_base_url - from .models import SoapService from .session_factory import SessionFactory diff --git a/src/soap/tests/test_client.py b/src/soap/tests/test_client.py index 373e7c9883..9345bdb925 100644 --- a/src/soap/tests/test_client.py +++ b/src/soap/tests/test_client.py @@ -6,8 +6,8 @@ from django.test import TestCase import requests_mock +from ape_pie import InvalidURLError -from api_client import InvalidURLError from simple_certmanager_ext.tests.factories import CertificateFactory from ..client import SOAPSession, build_client diff --git a/src/stuf/client.py b/src/stuf/client.py index d0791cf063..c21eeda9d5 100644 --- a/src/stuf/client.py +++ b/src/stuf/client.py @@ -17,10 +17,10 @@ from django.template import loader +from ape_pie import APIClient, InvalidURLError +from ape_pie.client import is_base_url from requests.models import Response -from api_client import APIClient, InvalidURLError -from api_client.client import is_base_url from soap.constants import SOAP_VERSION_CONTENT_TYPES, SOAPVersion from .constants import EndpointType diff --git a/src/stuf/tests/test_client.py b/src/stuf/tests/test_client.py index 32ec37cb6c..996d873173 100644 --- a/src/stuf/tests/test_client.py +++ b/src/stuf/tests/test_client.py @@ -11,9 +11,9 @@ from django.test import SimpleTestCase import requests_mock +from ape_pie import InvalidURLError from privates.test import temp_private_root -from api_client import InvalidURLError from soap.constants import EndpointSecurity from ..client import BaseClient diff --git a/src/zgw_consumers_ext/nlx.py b/src/zgw_consumers_ext/nlx.py index 30f0ac52ed..7d0c2c48ec 100644 --- a/src/zgw_consumers_ext/nlx.py +++ b/src/zgw_consumers_ext/nlx.py @@ -1,13 +1,12 @@ import json import logging +from ape_pie import APIClient from requests import JSONDecodeError from requests.models import PreparedRequest, Request, Response from requests.utils import guess_json_utf from zgw_consumers.nlx import Rewriter -from api_client import APIClient - logger = logging.getLogger(__name__) diff --git a/src/zgw_consumers_ext/tests/test_client_factory.py b/src/zgw_consumers_ext/tests/test_client_factory.py index d075b1ceb6..e2bd2f906e 100644 --- a/src/zgw_consumers_ext/tests/test_client_factory.py +++ b/src/zgw_consumers_ext/tests/test_client_factory.py @@ -1,11 +1,11 @@ from django.test import TestCase import requests_mock +from ape_pie import APIClient from privates.test import temp_private_root from simple_certmanager.constants import CertificateTypes from zgw_consumers.constants import AuthTypes -from api_client import APIClient from simple_certmanager_ext.tests.factories import CertificateFactory from ..api_client import ServiceClientFactory