Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zgw-consumers refactor #38

Merged
merged 39 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
c55cae7
:alien: Update NotificationsConfig client usage
stevenbal Oct 24, 2024
dffe1e6
Update project requirements
Oct 24, 2024
1c8c547
Add ape-pie & zgw-consumers requirements
Oct 24, 2024
d1f6a09
Use to do external API calls
Oct 24, 2024
f24ee8f
Remove APICredentials model
Oct 24, 2024
e131840
Remove zds_client usage
Oct 24, 2024
15e7d3b
Run formatting
Oct 24, 2024
4a628a1
Update tox configuration
Oct 24, 2024
d201498
Update documentation
Oct 24, 2024
ab92b11
Add reverse migration path
Oct 24, 2024
b93738e
Update docs
Oct 24, 2024
c79f786
Update CI configuration
Oct 24, 2024
8cd3bea
Describe `APICredentials` to `Service` model migration
SonnyBA Oct 25, 2024
fd16c37
Add missing field for Service's
SonnyBA Oct 25, 2024
4933a19
Try to determine `api_type` from `api_root`
SonnyBA Oct 25, 2024
82786bf
Reuse `client.auth` to retrieve token
SonnyBA Oct 25, 2024
726b28a
Remove oas related validators
SonnyBA Oct 25, 2024
0649f9c
Fix `build_client` usage
SonnyBA Oct 25, 2024
4794536
Remove `request_object_attribute`
SonnyBA Oct 25, 2024
9e3498a
Fix import error
SonnyBA Oct 25, 2024
9760dbb
Remove `notifications-api-common` related definitions
SonnyBA Oct 25, 2024
e35bfbd
Remove notifications-api-common related models & update
SonnyBA Oct 25, 2024
417ab78
Fix migrations, use simple `get_client` function, migrate
SonnyBA Oct 25, 2024
91744ee
Update JWTAuth error handling
SonnyBA Oct 25, 2024
f8857cc
Fix import errors
SonnyBA Oct 25, 2024
475f5f5
Revert "Remove oas related validators"
stevenbal Oct 31, 2024
f66f868
:memo: Re-add OAS utils docs
stevenbal Nov 8, 2024
6c5af46
:bug: Fix publishvalidator by adding missing imports
stevenbal Nov 8, 2024
8de8a6c
:recycle: Replace get_auth_headers with generate_jwt util
stevenbal Nov 8, 2024
f3fe4ee
:white_check_mark: Add migration tests for service migrations
stevenbal Nov 8, 2024
120062f
Add zgw-consumers-oas to requirements
SonnyBA Nov 12, 2024
44a4032
call `raise_for_status` when testing AC & NRC connections
SonnyBA Nov 13, 2024
4517d90
:ok_hand: [maykinmedia/open-api-framework#66] PR feedback
stevenbal Nov 14, 2024
5d331a6
:heavy_plus_sign: Add missing requests-mock dependency
stevenbal Nov 14, 2024
df70763
Handle duplicate service slugs
SonnyBA Nov 15, 2024
d83ca95
Apply formatting
SonnyBA Nov 18, 2024
b420445
Move `zgw-consumers-oas` to test requirements
SonnyBA Nov 19, 2024
e7ad23a
Merge pull request #40 from maykinmedia/slug-migrations
stevenbal Nov 22, 2024
d73f24c
:bug: Check for correct installed notifs app in nrc config check
stevenbal Nov 22, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
strategy:
matrix:
python: ['3.10', '3.11']
django: ['3.2', '4.2']
django: ['4.2']
services:
postgres:
image: postgres:14
Expand Down Expand Up @@ -62,7 +62,7 @@ jobs:
- uses: actions/checkout@v3
- uses: actions/setup-python@v4
with:
python-version: '3.9'
python-version: '3.10'

- name: Build sdist and wheel
run: |
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/code_quality.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-python@v2
with:
python-version: '3.9'
python-version: '3.10'
- name: Install dependencies
run: pip install tox tox-gh-actions
- run: tox
Expand Down
29 changes: 7 additions & 22 deletions docs/ref/client.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,12 @@
Obtaining a client
==================

Internally, `zds-client`_ is used to resolve remote API objects where the API is
documented using OpenAPI 3 specifications.
Internally, the `APIClient`_ client is used to resolve remote API objects. To
allow the `APIClient`_ to correctly, e.g use correct credentials for authentication
, an `Service`_ instance is required with a matching `api_root`. This replaces
the previous mechanism to use `APICredentials` for this purpose. A data migration
will be performed to migrate `APICredentials` to the `Service`_ model.
stevenbal marked this conversation as resolved.
Show resolved Hide resolved

Subclasses of the base ``Client`` class can also be used, in a pluggable fashion. By
default, the base class is used in combination with
:class:`vng_api_common.models.APICredential`.


Public API
==========

Settings
--------

The setting ``CUSTOM_CLIENT_FETCHER`` is a string with the dotted path to a callable
taking a single ``url`` string argument. The callable should return a ready-to-use
client instance for that particular URL, or ``None`` if no suitable client can be
determined.

.. automodule:: vng_api_common.client
:members:


.. _zds-client: https://pypi.org/project/gemma-zds-client/
.. _APIClient: https://ape-pie.readthedocs.io/en/stable/reference.html#apiclient-class
.. _Service: https://zgw-consumers.readthedocs.io/en/latest/models.html#zgw_consumers.models.Service
6 changes: 2 additions & 4 deletions docs/ref/oas.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@
OpenAPI Specification handling
==============================

The processing of external OAS is limited to OAS 3.x. Most of the heavy lifting
to interact with remote APIs is done through the `zds-client`_.
The processing of external OAS is limited to OAS 3.x. This module provides utilities to
perform operations on OAS.

.. automodule:: vng_api_common.oas
:members:

.. _zds-client: https://pypi.org/project/gemma-zds-client/
16 changes: 8 additions & 8 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ keywords = openapi, swagger, django
classifiers =
Development Status :: 5 - Production/Stable
Framework :: Django
Framework :: Django :: 3.2
Framework :: Django :: 4.2
Intended Audience :: Developers
Operating System :: Unix
Expand All @@ -34,22 +33,23 @@ scripts =
bin/patch_content_types
bin/use_external_components
install_requires =
django>=3.2.0
django-filter>=2.0
django>=4.2.0
django-filter>=23.1,<=25.1
django-solo
djangorestframework>=3.11.0
djangorestframework>=3.15.0
djangorestframework_camel_case>=1.2.0
django-rest-framework-condition
drf-yasg>=1.20.0
drf-nested-routers>=0.93.3
gemma-zds-client>=0.14.0
drf-yasg>=1.20.0 # TODO: remove usage?
stevenbal marked this conversation as resolved.
Show resolved Hide resolved
drf-nested-routers>=0.94.1
iso-639
isodate
notifications-api-common>=0.2.2
notifications-api-common>=0.3.0
zgw-consumers>=0.35.1
oyaml
PyJWT>=2.0.0
requests
coreapi
ape-pie
tests_require =
pytest
pytest-django
Expand Down
13 changes: 0 additions & 13 deletions tests/test_config.py

This file was deleted.

8 changes: 3 additions & 5 deletions tests/test_jwtsecrets.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
from rest_framework import status
from rest_framework.reverse import reverse
from rest_framework.test import APIClient
from zds_client.auth import ClientAuth

from vng_api_common.authorizations.models import Applicatie, Autorisatie
from vng_api_common.authorizations.utils import generate_jwt
from vng_api_common.constants import ComponentTypes
from vng_api_common.models import JWTSecret

Expand All @@ -31,10 +31,8 @@ def test_authorized_jwtsecret_create_ok():
component=ComponentTypes.ac,
scopes=["autorisaties.credentials-registreren"],
)
auth = ClientAuth(client_id="pytest", secret="sekrit").credentials()[
"Authorization"
]
client.credentials(HTTP_AUTHORIZATION=auth)
token = generate_jwt("pytest", "sekrit", "pytest", "pytest")
client.credentials(HTTP_AUTHORIZATION=token)

response = client.post(url, {"identifier": "foo", "secret": "bar"})

Expand Down
176 changes: 176 additions & 0 deletions tests/test_migrations.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
from django.apps import apps as global_apps
from django.db import connection
from django.db.migrations.executor import MigrationExecutor
from django.test import override_settings

import pytest
from zgw_consumers.constants import APITypes, AuthTypes


class BaseMigrationTest:
app = None
migrate_from = None # The migration before the one we want to test
migrate_to = None # The migration we want to test
setting_overrides = {}

@pytest.fixture(autouse=True)
def setup_migration(self, db):
"""
Setup the migration test by reversing to `migrate_from` state,
then applying the `migrate_to` state.
"""
assert self.app is not None, "You must define the `app` attribute"
assert self.migrate_from is not None, "You must define `migrate_from`"
assert self.migrate_to is not None, "You must define `migrate_to`"

# Step 1: Set up the MigrationExecutor
executor = MigrationExecutor(connection)

# Step 2: Reverse to the starting migration state
migrate_from = [(self.app, self.migrate_from)]
migrate_to = [(self.app, self.migrate_to)]
old_migrate_state = executor.migrate(migrate_from)
old_apps = old_migrate_state.apps

# Step 3: Run the `setup_before_migration` hook with old state models
self.setup_before_migration(old_apps)

# Step 4: Apply the migration we want to test with any settings overrides
with override_settings(**self.setting_overrides):
executor = MigrationExecutor(connection)
executor.loader.build_graph() # reload the graph in case of dependency changes
executor.migrate(migrate_to)

# Set `self.apps` to the post-migration state
self.apps = executor.loader.project_state(migrate_to).apps

def setup_before_migration(self, apps):
"""
Hook to set up the state before the migration. Subclasses can
override this to create data or perform other setup actions.
"""
pass


class TestMigrateAPICredentialToService(BaseMigrationTest):
app = "vng_api_common"
migrate_from = "0005_auto_20190614_1346"
migrate_to = "0006_delete_apicredential"

def setup_before_migration(self, apps):
Service = apps.get_model("zgw_consumers", "Service")
APICredential = apps.get_model("vng_api_common", "APICredential")

APICredential.objects.create(
api_root="https://example.com/zaken/api/v1/",
label="Zaken API",
client_id="user",
secret="secret",
user_id="user",
user_representation="User",
)

APICredential.objects.create(
api_root="https://example.com/api/v1/",
label="Selectielijst API",
client_id="user2",
secret="secret2",
user_id="user2",
user_representation="User2",
)

APICredential.objects.create(
api_root="https://example.com/documenten/api/v1/",
label="Documenten API",
client_id="user3",
secret="secret3",
user_id="user3",
user_representation="User3",
)
Service.objects.create(
api_root="https://example.com/documenten/api/v1/",
label="Documenten",
slug="already-exists",
api_type=APITypes.drc,
auth_type=AuthTypes.zgw,
client_id="user4",
secret="secret4",
user_id="user4",
user_representation="User4",
)

def test_migrate_api_credentials_to_services(self):
Service = global_apps.get_model("zgw_consumers", "Service")

zaken_service = Service.objects.get(slug="zaken-api")

assert zaken_service.api_root == "https://example.com/zaken/api/v1/"
assert zaken_service.label == "Zaken API"
assert zaken_service.api_type == APITypes.zrc
assert zaken_service.auth_type == AuthTypes.zgw
assert zaken_service.client_id == "user"
assert zaken_service.secret == "secret"
assert zaken_service.user_id == "user"
assert zaken_service.user_representation == "User"

selectielijst_service = Service.objects.get(slug="selectielijst-api")

assert selectielijst_service.api_root == "https://example.com/api/v1/"
assert selectielijst_service.label == "Selectielijst API"
assert selectielijst_service.api_type == APITypes.orc
assert selectielijst_service.auth_type == AuthTypes.zgw
assert selectielijst_service.client_id == "user2"
assert selectielijst_service.secret == "secret2"
assert selectielijst_service.user_id == "user2"
assert selectielijst_service.user_representation == "User2"

documenten_service = Service.objects.get(slug="already-exists")

# Because there already was a service for this API root, that data is used instead
assert documenten_service.api_root == "https://example.com/documenten/api/v1/"
assert documenten_service.label == "Documenten"
assert documenten_service.api_type == APITypes.drc
assert documenten_service.auth_type == AuthTypes.zgw
assert documenten_service.client_id == "user4"
assert documenten_service.secret == "secret4"
assert documenten_service.user_id == "user4"
assert documenten_service.user_representation == "User4"


class TestMigrateAuthorizationsConfigAPIRootToService(BaseMigrationTest):
app = "authorizations"
migrate_from = "0015_auto_20220318_1608"
migrate_to = "0016_remove_authorizationsconfig_api_root_and_more"

def setup_before_migration(self, apps):
Service = apps.get_model("zgw_consumers", "Service")
AuthorizationsConfig = apps.get_model("authorizations", "AuthorizationsConfig")

config, _ = AuthorizationsConfig.objects.get_or_create()
config.api_root = "https://example.com/autorisaties/api/v1/"
config.save()

self.service = Service.objects.create(
api_root="https://example.com/autorisaties/api/v1/",
label="Autorisaties",
slug="autorisaties",
api_type=APITypes.ac,
auth_type=AuthTypes.zgw,
client_id="user",
secret="secret",
user_id="user",
user_representation="User",
)

def test_migrate_api_root_to_service(self):
AuthorizationsConfig = global_apps.get_model(
"authorizations", "AuthorizationsConfig"
)

config, _ = AuthorizationsConfig.objects.get_or_create()

assert config.authorizations_api_service.pk == self.service.pk
assert (
config.authorizations_api_service.api_root
== "https://example.com/autorisaties/api/v1/"
)
File renamed without changes.
11 changes: 8 additions & 3 deletions tests/test_views.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,23 @@
from django.test import override_settings
from unittest.mock import patch

from django.urls import reverse

import pytest
from notifications_api_common.models import NotificationsConfig
from rest_framework import status


@pytest.mark.django_db
@override_settings(CUSTOM_CLIENT_FETCHER="testapp.utils.get_client_with_auth")
def test_config_view(api_client):
"""
regression test for https://github.com/open-zaak/open-notificaties/issues/119
"""
path = reverse("view-config")
notifications_config = NotificationsConfig.get_solo()
notifications_config.notifications_api_service = None

notifications_config.save()

path = reverse("view-config")
response = api_client.get(path)

assert response.status_code == status.HTTP_200_OK
4 changes: 1 addition & 3 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tox]
envlist =
py{310,311}-django{32,42}
py{310,311}-django{42}
isort
docs
black
Expand All @@ -13,15 +13,13 @@ python =

[gh-actions:env]
DJANGO =
3.2: django32
4.2: django42

[testenv]
extras =
tests
coverage
deps =
django32: Django~=3.2.0
django42: Django~=4.2.0
setenv =
PYTHONPATH = {toxinidir}
Expand Down
Loading
Loading