From ec5bed9d1782a1f917d1836ceeb8e3ec23800556 Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Thu, 17 Apr 2025 11:05:58 +0200 Subject: [PATCH 1/5] :sparkles: [#550] Add cache to retrieve versions of objecttypes --- docs/installation/config.rst | 6 +++++ requirements/base.txt | 2 +- requirements/ci.txt | 4 +-- requirements/dev.txt | 4 +-- src/objects/api/validators.py | 1 - src/objects/conf/base.py | 10 +++++++ src/objects/core/utils.py | 49 +++++++++++++++++++++-------------- src/objects/utils/cache.py | 20 ++++++++++++++ 8 files changed, 71 insertions(+), 25 deletions(-) create mode 100644 src/objects/utils/cache.py diff --git a/docs/installation/config.rst b/docs/installation/config.rst index e4476aad..03f4fce0 100644 --- a/docs/installation/config.rst +++ b/docs/installation/config.rst @@ -70,6 +70,12 @@ Content Security Policy * ``CSP_OBJECT_SRC``: ``object-src`` urls. Defaults to: ``['"\'none\'"']``. +Cache +----- + +* ``OBJECTTYPE_VERSION_CACHE_TIMEOUT``: Timeout for cache when retrieving objecttype versions. Defaults to: ``300``. + + Optional -------- diff --git a/requirements/base.txt b/requirements/base.txt index f39b4e35..e5818a0c 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -16,7 +16,7 @@ asgiref==3.7.2 # django-cors-headers asn1crypto==1.5.1 # via webauthn -attrs==20.3.0 +attrs==25.3.0 # via # glom # jsonschema diff --git a/requirements/ci.txt b/requirements/ci.txt index f6067b59..eb41a5ee 100644 --- a/requirements/ci.txt +++ b/requirements/ci.txt @@ -31,7 +31,7 @@ asn1crypto==1.5.1 # -c requirements/base.txt # -r requirements/base.txt # webauthn -attrs==20.3.0 +attrs==25.3.0 # via # -c requirements/base.txt # -r requirements/base.txt @@ -510,7 +510,7 @@ phonenumberslite==8.13.30 # -c requirements/base.txt # -r requirements/base.txt # django-two-factor-auth -platformdirs==4.3.3 +platformdirs==4.3.7 # via black pluggy==1.5.0 # via pytest diff --git a/requirements/dev.txt b/requirements/dev.txt index 86d03e33..ff8fdef8 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -36,7 +36,7 @@ asn1crypto==1.5.1 # -c requirements/ci.txt # -r requirements/ci.txt # webauthn -attrs==20.3.0 +attrs==25.3.0 # via # -c requirements/ci.txt # -r requirements/ci.txt @@ -609,7 +609,7 @@ pip==24.3.1 # via pip-tools pip-tools==7.4.1 # via -r requirements/dev.in -platformdirs==4.3.3 +platformdirs==4.3.7 # via # -c requirements/ci.txt # -r requirements/ci.txt diff --git a/src/objects/api/validators.py b/src/objects/api/validators.py index f0d49797..04356793 100644 --- a/src/objects/api/validators.py +++ b/src/objects/api/validators.py @@ -40,7 +40,6 @@ def __call__(self, attrs, serializer): if not object_type or not version: return - try: check_objecttype(object_type, version, data) except ValidationError as exc: diff --git a/src/objects/conf/base.py b/src/objects/conf/base.py index 76b839f4..c71fb232 100644 --- a/src/objects/conf/base.py +++ b/src/objects/conf/base.py @@ -34,6 +34,16 @@ # FIXME should this be UTC? TIME_ZONE = "Europe/Amsterdam" +# +# Caches +# + +OBJECTTYPE_VERSION_CACHE_TIMEOUT = config( + "OBJECTTYPE_VERSION_CACHE_TIMEOUT", + default=5 * 60, # 300 seconds + help_text="Timeout for cache when retrieving objecttype versions.", + group="Cache", +) # # Additional Django settings diff --git a/src/objects/core/utils.py b/src/objects/core/utils.py index eb94f40b..e056c87f 100644 --- a/src/objects/core/utils.py +++ b/src/objects/core/utils.py @@ -1,37 +1,48 @@ +from django.conf import settings from django.core.exceptions import ValidationError import jsonschema import requests from zgw_consumers.client import build_client +from objects.utils.cache import cache + def check_objecttype(object_type, version, data): - client = build_client(object_type.service) - objecttype_version_url = f"{object_type.url}/versions/{version}" + @cache( + f"objecttypen-{object_type.uuid}:versions-{version}", + timeout=settings.OBJECTTYPE_VERSION_CACHE_TIMEOUT, + ) + def get_objecttype_version_response(): + client = build_client(object_type.service) + try: + return client.get(f"{object_type.versions_url}/{version}") + except requests.RequestException: + raise ValidationError( + {"type": "Object type version can not be retrieved."}, + code="invalid", + ) - try: - response = client.get(objecttype_version_url) - except requests.RequestException: - msg = "Object type version can not be retrieved." - raise ValidationError(msg) + response = get_objecttype_version_response() try: response_data = response.json() - except requests.JSONDecodeError: - raise ValidationError("Object type doesn't have retrievable data.") - - try: schema = response_data["jsonSchema"] - except KeyError: - msg = f"{objecttype_version_url} does not appear to be a valid objecttype." - raise ValidationError(msg) - - # TODO: Set warning header if objecttype is not published. - - try: jsonschema.validate(data, schema) + except requests.JSONDecodeError: + raise ValidationError( + {"type": "Object type doesn't have retrievable data."}, + code="invalid_json", + ) + except KeyError: + raise ValidationError( + { + "type": f"{object_type.versions_url} does not appear to be a valid objecttype." + }, + code="invalid_key", + ) except jsonschema.exceptions.ValidationError as exc: - raise ValidationError(exc.args[0]) from exc + raise ValidationError({"data": exc.args[0]}, code="invalid_jsonschema") def can_connect_to_objecttypes() -> bool: diff --git a/src/objects/utils/cache.py b/src/objects/utils/cache.py new file mode 100644 index 00000000..6a457744 --- /dev/null +++ b/src/objects/utils/cache.py @@ -0,0 +1,20 @@ +from functools import wraps + +from django.core.cache import caches + + +def cache(key: str, alias: str = "default", **set_options): + def decorator(func: callable): + @wraps(func) + def wrapped(*args, **kwargs): + _cache = caches[alias] + result = _cache.get(key) + if result is not None: + return result + result = func(*args, **kwargs) + _cache.set(key, result, **set_options) + return result + + return wrapped + + return decorator From 3814da7b51329ccc3833e662b31ae9746f04a168 Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Tue, 22 Apr 2025 12:01:27 +0200 Subject: [PATCH 2/5] :white_check_mark: [#550] Fix tests --- requirements/base.txt | 2 +- requirements/ci.txt | 4 ++-- requirements/dev.txt | 4 ++-- src/objects/tests/v2/test_jsonschema.py | 4 ++-- src/objects/tests/v2/test_validation.py | 20 +++++++------------- src/objects/utils/test.py | 13 +++++++++++++ 6 files changed, 27 insertions(+), 20 deletions(-) diff --git a/requirements/base.txt b/requirements/base.txt index e5818a0c..f39b4e35 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -16,7 +16,7 @@ asgiref==3.7.2 # django-cors-headers asn1crypto==1.5.1 # via webauthn -attrs==25.3.0 +attrs==20.3.0 # via # glom # jsonschema diff --git a/requirements/ci.txt b/requirements/ci.txt index eb41a5ee..f6067b59 100644 --- a/requirements/ci.txt +++ b/requirements/ci.txt @@ -31,7 +31,7 @@ asn1crypto==1.5.1 # -c requirements/base.txt # -r requirements/base.txt # webauthn -attrs==25.3.0 +attrs==20.3.0 # via # -c requirements/base.txt # -r requirements/base.txt @@ -510,7 +510,7 @@ phonenumberslite==8.13.30 # -c requirements/base.txt # -r requirements/base.txt # django-two-factor-auth -platformdirs==4.3.7 +platformdirs==4.3.3 # via black pluggy==1.5.0 # via pytest diff --git a/requirements/dev.txt b/requirements/dev.txt index ff8fdef8..86d03e33 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -36,7 +36,7 @@ asn1crypto==1.5.1 # -c requirements/ci.txt # -r requirements/ci.txt # webauthn -attrs==25.3.0 +attrs==20.3.0 # via # -c requirements/ci.txt # -r requirements/ci.txt @@ -609,7 +609,7 @@ pip==24.3.1 # via pip-tools pip-tools==7.4.1 # via -r requirements/dev.in -platformdirs==4.3.7 +platformdirs==4.3.3 # via # -c requirements/ci.txt # -r requirements/ci.txt diff --git a/src/objects/tests/v2/test_jsonschema.py b/src/objects/tests/v2/test_jsonschema.py index f8068493..bae51e6e 100644 --- a/src/objects/tests/v2/test_jsonschema.py +++ b/src/objects/tests/v2/test_jsonschema.py @@ -8,7 +8,7 @@ from objects.core.tests.factories import ObjectTypeFactory from objects.token.constants import PermissionModes from objects.token.tests.factories import PermissionFactory -from objects.utils.test import TokenAuthMixin +from objects.utils.test import ClearCachesMixin, TokenAuthMixin from ..constants import GEO_WRITE_KWARGS from ..utils import mock_objecttype, mock_objecttype_version, mock_service_oas_get @@ -18,7 +18,7 @@ @requests_mock.Mocker() -class JsonSchemaTests(TokenAuthMixin, APITestCase): +class JsonSchemaTests(TokenAuthMixin, ClearCachesMixin, APITestCase): """GH issue - https://github.com/maykinmedia/objects-api/issues/330""" @classmethod diff --git a/src/objects/tests/v2/test_validation.py b/src/objects/tests/v2/test_validation.py index 06bdcb85..9d9e2e55 100644 --- a/src/objects/tests/v2/test_validation.py +++ b/src/objects/tests/v2/test_validation.py @@ -9,7 +9,7 @@ from objects.core.tests.factories import ObjectRecordFactory, ObjectTypeFactory from objects.token.constants import PermissionModes from objects.token.tests.factories import PermissionFactory -from objects.utils.test import TokenAuthMixin +from objects.utils.test import ClearCachesMixin, TokenAuthMixin from ..constants import GEO_WRITE_KWARGS from ..utils import mock_objecttype, mock_objecttype_version, mock_service_oas_get @@ -19,7 +19,7 @@ @requests_mock.Mocker() -class ObjectTypeValidationTests(TokenAuthMixin, APITestCase): +class ObjectTypeValidationTests(TokenAuthMixin, ClearCachesMixin, APITestCase): @classmethod def setUpTestData(cls): super().setUpTestData() @@ -101,9 +101,7 @@ def test_create_object_no_version(self, m): self.assertEqual(Object.objects.count(), 0) data = response.json() - self.assertEqual( - data["non_field_errors"], ["Object type doesn't have retrievable data."] - ) + self.assertEqual(data["type"], ["Object type doesn't have retrievable data."]) def test_create_object_objecttype_request_error(self, m): mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") @@ -125,9 +123,7 @@ def test_create_object_objecttype_request_error(self, m): self.assertEqual(Object.objects.count(), 0) data = response.json() - self.assertEqual( - data["non_field_errors"], ["Object type version can not be retrieved."] - ) + self.assertEqual(data["type"], ["Object type version can not be retrieved."]) def test_create_object_objecttype_with_no_jsonSchema(self, m): mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") @@ -154,9 +150,9 @@ def test_create_object_objecttype_with_no_jsonSchema(self, m): data = response.json() self.assertEqual( - data["non_field_errors"], + data["type"], [ - f"{self.object_type.url}/versions/10 does not appear to be a valid objecttype." + f"{self.object_type.versions_url} does not appear to be a valid objecttype." ], ) @@ -183,9 +179,7 @@ def test_create_object_schema_invalid(self, m): self.assertEqual(Object.objects.count(), 0) data = response.json() - self.assertEqual( - data["non_field_errors"], ["'diameter' is a required property"] - ) + self.assertEqual(data["data"], ["'diameter' is a required property"]) def test_create_object_without_record_invalid(self, m): mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") diff --git a/src/objects/utils/test.py b/src/objects/utils/test.py index f7fd6fab..ab51f2f6 100644 --- a/src/objects/utils/test.py +++ b/src/objects/utils/test.py @@ -1,3 +1,5 @@ +from django.core.cache import caches + from objects.token.tests.factories import TokenAuthFactory @@ -14,3 +16,14 @@ def setUp(self): super().setUp() self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token_auth.token}") + + +class ClearCachesMixin: + def setUp(self): + super().setUp() + self._clear_caches() + self.addCleanup(self._clear_caches) + + def _clear_caches(self): + for cache in caches.all(): + cache.clear() From d53c0b4a14d3aa869819e9747ad3f40dea3be9bc Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Tue, 29 Apr 2025 16:08:08 +0200 Subject: [PATCH 3/5] :white_check_mark: [#550] Configured ObjecttypesClient --- docs/installation/config.rst | 2 +- src/objects/api/validators.py | 8 +- src/objects/conf/base.py | 2 +- src/objects/core/admin.py | 9 +- src/objects/core/models.py | 10 +-- src/objects/core/utils.py | 32 +++---- src/objects/tests/v2/test_validation.py | 57 ++++++++++++- src/objects/utils/client.py | 86 +++++++++++++++++++ src/objects/utils/test.py | 107 +++++++++++++++++++++++- 9 files changed, 272 insertions(+), 41 deletions(-) create mode 100644 src/objects/utils/client.py diff --git a/docs/installation/config.rst b/docs/installation/config.rst index 03f4fce0..7463f707 100644 --- a/docs/installation/config.rst +++ b/docs/installation/config.rst @@ -73,7 +73,7 @@ Content Security Policy Cache ----- -* ``OBJECTTYPE_VERSION_CACHE_TIMEOUT``: Timeout for cache when retrieving objecttype versions. Defaults to: ``300``. +* ``OBJECTTYPE_VERSION_CACHE_TIMEOUT``: Timeout in seconds for cache when retrieving objecttype versions. Defaults to: ``300``. Optional diff --git a/src/objects/api/validators.py b/src/objects/api/validators.py index 04356793..07927208 100644 --- a/src/objects/api/validators.py +++ b/src/objects/api/validators.py @@ -4,9 +4,9 @@ import requests from rest_framework import serializers from rest_framework.fields import get_attribute -from zgw_consumers.client import build_client from objects.core.utils import check_objecttype +from objects.utils.client import get_objecttypes_client from .constants import Operators from .utils import merge_patch, string_to_value @@ -131,15 +131,15 @@ def __call__(self, attrs, serializer): if not geometry: return + client = get_objecttypes_client(object_type.service) - client = build_client(object_type.service) try: - response = client.get(url=object_type.url) + response_data = client.get_objecttype(object_type.uuid) except requests.RequestException as exc: msg = f"Object type can not be retrieved: {exc.args[0]}" raise ValidationError(msg) - allow_geometry = response.json().get("allowGeometry", True) + allow_geometry = response_data.get("allowGeometry", True) if geometry and not allow_geometry: raise serializers.ValidationError(self.message, code=self.code) diff --git a/src/objects/conf/base.py b/src/objects/conf/base.py index c71fb232..a856db35 100644 --- a/src/objects/conf/base.py +++ b/src/objects/conf/base.py @@ -41,7 +41,7 @@ OBJECTTYPE_VERSION_CACHE_TIMEOUT = config( "OBJECTTYPE_VERSION_CACHE_TIMEOUT", default=5 * 60, # 300 seconds - help_text="Timeout for cache when retrieving objecttype versions.", + help_text="Timeout in seconds for cache when retrieving objecttype versions.", group="Cache", ) diff --git a/src/objects/core/admin.py b/src/objects/core/admin.py index f60a23dc..6cd3d020 100644 --- a/src/objects/core/admin.py +++ b/src/objects/core/admin.py @@ -7,8 +7,8 @@ from django.urls import path import requests -from zgw_consumers.client import build_client -from zgw_consumers.service import pagination_helper + +from objects.utils.client import get_objecttypes_client from .models import Object, ObjectRecord, ObjectType @@ -37,10 +37,9 @@ def get_urls(self): def versions_view(self, request, objecttype_id): versions = [] if objecttype := self.get_object(request, objecttype_id): - client = build_client(objecttype.service) + client = get_objecttypes_client(objecttype.service) try: - response = client.get(objecttype.versions_url) - versions = list(pagination_helper(client, response.json())) + versions = client.list_objecttype_versions(objecttype.uuid) except (requests.RequestException, requests.JSONDecodeError): logger.exception( "Something went wrong while fetching objecttype versions" diff --git a/src/objects/core/models.py b/src/objects/core/models.py index 3ad77620..e7d88715 100644 --- a/src/objects/core/models.py +++ b/src/objects/core/models.py @@ -10,9 +10,10 @@ import requests from requests.exceptions import ConnectionError -from zgw_consumers.client import build_client from zgw_consumers.models import Service +from objects.utils.client import get_objecttypes_client + from .query import ObjectQuerySet, ObjectRecordQuerySet, ObjectTypeQuerySet from .utils import check_objecttype @@ -52,15 +53,12 @@ def clean_fields(self, exclude: Iterable[str] | None = None) -> None: if exclude and "service" in exclude: return - client = build_client(self.service) + client = get_objecttypes_client(self.service) try: - response = client.get(url=self.url) + object_type_data = client.get_objecttype(self.uuid) except (requests.RequestException, ConnectionError, ValueError) as exc: raise ValidationError(f"Objecttype can't be requested: {exc}") - - try: - object_type_data = response.json() except requests.exceptions.JSONDecodeError: raise ValidationError("Object type version didn't have any data") diff --git a/src/objects/core/utils.py b/src/objects/core/utils.py index e056c87f..e0988be9 100644 --- a/src/objects/core/utils.py +++ b/src/objects/core/utils.py @@ -3,9 +3,9 @@ import jsonschema import requests -from zgw_consumers.client import build_client from objects.utils.cache import cache +from objects.utils.client import get_objecttypes_client def check_objecttype(object_type, version, data): @@ -14,26 +14,19 @@ def check_objecttype(object_type, version, data): timeout=settings.OBJECTTYPE_VERSION_CACHE_TIMEOUT, ) def get_objecttype_version_response(): - client = build_client(object_type.service) + client = get_objecttypes_client(object_type.service) try: - return client.get(f"{object_type.versions_url}/{version}") - except requests.RequestException: + return client.get_objecttype_version(object_type.uuid, version) + except (requests.RequestException, requests.JSONDecodeError): raise ValidationError( {"type": "Object type version can not be retrieved."}, code="invalid", ) - response = get_objecttype_version_response() - try: - response_data = response.json() - schema = response_data["jsonSchema"] - jsonschema.validate(data, schema) - except requests.JSONDecodeError: - raise ValidationError( - {"type": "Object type doesn't have retrievable data."}, - code="invalid_json", - ) + vesion_data = get_objecttype_version_response() + + jsonschema.validate(data, vesion_data["jsonSchema"]) except KeyError: raise ValidationError( { @@ -51,13 +44,8 @@ def can_connect_to_objecttypes() -> bool: """ from zgw_consumers.models import Service - objecttypes_services = Service.objects.filter(object_types__isnull=False).distinct() - for service in objecttypes_services: - client = build_client(service) - - try: - client.get("objecttypes") - except requests.RequestException: + for service in Service.objects.filter(object_types__isnull=False).distinct(): + client = get_objecttypes_client(service) + if not client.can_connect: return False - return True diff --git a/src/objects/tests/v2/test_validation.py b/src/objects/tests/v2/test_validation.py index 9d9e2e55..943744e9 100644 --- a/src/objects/tests/v2/test_validation.py +++ b/src/objects/tests/v2/test_validation.py @@ -31,6 +31,61 @@ def setUpTestData(cls): token_auth=cls.token_auth, ) + def test_valid_create_object_check_cache(self, m): + mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") + m.get( + f"{self.object_type.url}/versions/1", + json=mock_objecttype_version(self.object_type.url), + ) + m.get( + f"{self.object_type.url}/versions/2", + json=mock_objecttype_version(self.object_type.url), + ) + + url = reverse("object-list") + data = { + "type": self.object_type.url, + "record": { + "typeVersion": 1, + "data": {"plantDate": "2020-04-12", "diameter": 30}, + "startAt": "2020-01-01", + }, + } + with self.subTest("ok_cache"): + self.assertEqual(m.call_count, 0) + self.assertEqual(Object.objects.count(), 0) + for n in range(5): + self.client.post(url, data, **GEO_WRITE_KWARGS) + # just one request should run — the first one + self.assertEqual(m.call_count, 1) + self.assertEqual(Object.objects.count(), 5) + + with self.subTest("clear_cache"): + m.reset_mock() + self.assertEqual(m.call_count, 0) + for n in range(5): + self._clear_caches() + self.client.post(url, data, **GEO_WRITE_KWARGS) + self.assertEqual(m.call_count, 5) + self.assertEqual(Object.objects.count(), 10) + + with self.subTest("change_version"): + self._clear_caches() + m.reset_mock() + self.assertEqual(m.call_count, 0) + for n in range(5): + self.client.post(url, data, **GEO_WRITE_KWARGS) + # one request version 1 + self.assertEqual(m.call_count, 1) + self.assertEqual(Object.objects.count(), 15) + + data["record"]["typeVersion"] = 2 + for n in range(5): + self.client.post(url, data, **GEO_WRITE_KWARGS) + # one request for version 1 and one for version 2 + self.assertEqual(m.call_count, 2) + self.assertEqual(Object.objects.count(), 20) + def test_create_object_with_not_found_objecttype_url(self, m): object_type_invalid = ObjectTypeFactory(service=self.object_type.service) PermissionFactory.create( @@ -101,7 +156,7 @@ def test_create_object_no_version(self, m): self.assertEqual(Object.objects.count(), 0) data = response.json() - self.assertEqual(data["type"], ["Object type doesn't have retrievable data."]) + self.assertEqual(data["type"], ["Object type version can not be retrieved."]) def test_create_object_objecttype_request_error(self, m): mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") diff --git a/src/objects/utils/client.py b/src/objects/utils/client.py new file mode 100644 index 00000000..e7989a57 --- /dev/null +++ b/src/objects/utils/client.py @@ -0,0 +1,86 @@ +from typing import Any +from uuid import UUID + +import requests +from zgw_consumers.client import build_client +from zgw_consumers.nlx import NLXClient +from zgw_consumers.service import pagination_helper +from zgw_consumers.utils import PaginatedResponseData + + +class ObjecttypesClient(NLXClient): + def _get_paginated( + self, + endpoint: str, + page: int | None = None, + page_size: int | None = None, + query_params: dict[Any, Any] | None = None, + ): + query_params = query_params or {} + if page is None and page_size is None: + response = self.get(endpoint, params=query_params) + response.raise_for_status() + data: PaginatedResponseData[dict[str, Any]] = response.json() + all_data = pagination_helper(self, data) + return list(all_data) + + if page is not None: + query_params["page"] = page + if page_size is not None: + query_params["pageSize"] = page_size + + response = self.get(endpoint, params=query_params) + response.raise_for_status() + return response.json()["results"] + + @property + def can_connect(self) -> bool: + try: + response = self.get("objecttypes") + response.raise_for_status() + return response.status_code == 200 + except requests.RequestException: + return False + + def list_objecttypes( + self, + page: int | None = None, + page_size: int | None = None, + ) -> list[dict[str, Any]]: + return self._get_paginated( + "objecttypes", + page=page, + page_size=page_size, + ) + + def get_objecttype( + self, + objecttype_uuid: str | UUID, + ) -> dict[str, Any]: + response = self.get(f"objecttypes/{objecttype_uuid}") + response.raise_for_status() + return response.json() + + def list_objecttype_versions( + self, + objecttype_uuid: str | UUID, + page: int | None = None, + page_size: int | None = None, + ) -> list[dict[str, Any]]: + return self._get_paginated( + f"objecttypes/{objecttype_uuid}/versions", page=page, page_size=page_size + ) + + def get_objecttype_version( + self, + objecttype_uuid: str | UUID, + version: int, + ) -> dict[str, Any]: + response = self.get(f"objecttypes/{objecttype_uuid}/versions/{version}") + response.raise_for_status() + return response.json() + + +def get_objecttypes_client(service) -> ObjecttypesClient: + assert service is not None + return build_client(service, client_factory=ObjecttypesClient) diff --git a/src/objects/utils/test.py b/src/objects/utils/test.py index ab51f2f6..15fe4ea6 100644 --- a/src/objects/utils/test.py +++ b/src/objects/utils/test.py @@ -1,6 +1,20 @@ from django.core.cache import caches -from objects.token.tests.factories import TokenAuthFactory +import requests_mock +from rest_framework.test import APITestCase + +from objects.core.models import ObjectType +from objects.core.tests.factories import ObjectTypeFactory +from objects.tests.utils import ( + mock_objecttype, + mock_objecttype_version, + mock_service_oas_get, +) +from objects.token.constants import PermissionModes +from objects.token.tests.factories import PermissionFactory, TokenAuthFactory +from objects.utils.client import get_objecttypes_client + +OBJECT_TYPES_API = "https://example.com/objecttypes/v1/" class TokenAuthMixin: @@ -27,3 +41,94 @@ def setUp(self): def _clear_caches(self): for cache in caches.all(): cache.clear() + + +@requests_mock.Mocker() +class ObjecttypesClientTest(TokenAuthMixin, APITestCase): + @classmethod + def setUpTestData(cls): + super().setUpTestData() + + cls.object_type = ObjectTypeFactory(service__api_root=OBJECT_TYPES_API) + PermissionFactory.create( + object_type=cls.object_type, + mode=PermissionModes.read_and_write, + token_auth=cls.token_auth, + ) + + def test_list_objecttypes(self, m): + object_type = ObjectType.objects.first() + mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") + object_type_mock = mock_objecttype(object_type.url) + m.get( + f"{OBJECT_TYPES_API}objecttypes", + json={ + "count": 1, + "next": None, + "previous": None, + "results": [object_type_mock], + }, + ) + client = get_objecttypes_client(object_type.service) + self.assertTrue(client.can_connect) + + data = client.list_objecttypes() + self.assertEqual(data, [object_type_mock]) + self.assertEqual(len(data), 1) + + def test_get_objecttype(self, m): + object_type = ObjectType.objects.first() + mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") + m.get(object_type.url, json=mock_objecttype(object_type.url)) + + client = get_objecttypes_client(object_type.service) + data = client.get_objecttype(object_type.uuid) + self.assertTrue(data["url"], str(object_type.url)) + + def test_list_objecttype_versions(self, m): + object_type = ObjectType.objects.first() + mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") + object_type_mock = mock_objecttype(object_type.url) + m.get( + f"{OBJECT_TYPES_API}objecttypes", + json={ + "count": 1, + "next": None, + "previous": None, + "results": [object_type_mock], + }, + ) + version_mock = mock_objecttype_version(object_type.url) + m.get( + f"{object_type.url}/versions", + json={ + "count": 1, + "next": None, + "previous": None, + "results": [version_mock], + }, + ) + + client = get_objecttypes_client(object_type.service) + self.assertTrue(client.can_connect) + + data = client.list_objecttypes() + self.assertEqual(data, [object_type_mock]) + self.assertEqual(len(data), 1) + + data = client.list_objecttype_versions(object_type.uuid) + self.assertEqual(data, [version_mock]) + self.assertEqual(len(data), 1) + self.assertEqual(data[0]["version"], 1) + + def test_get_objecttype_version(self, m): + object_type = ObjectType.objects.first() + mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") + m.get( + f"{self.object_type.url}/versions/1", + json=mock_objecttype_version(self.object_type.url), + ) + + client = get_objecttypes_client(object_type.service) + data = client.get_objecttype_version(object_type.uuid, 1) + self.assertEqual(data["url"], f"{self.object_type.url}/versions/1") From eaf7f3bfcc9c7227e23927901f759bac67b47af9 Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Tue, 29 Apr 2025 16:51:41 +0200 Subject: [PATCH 4/5] :wrench: [#550] Add www.utrecht.nl in linkcheck_ignore --- docs/conf.py | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/conf.py b/docs/conf.py index 4828a1e9..a4791065 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -99,6 +99,7 @@ r"https?://.*\.gemeente.nl", r"http://localhost:\d+/", r"https://.*sentry.*", + "https://www.utrecht.nl/", "https://github.com/maykinmedia/objects-api-performance", "https://objects.municipality.nl/admin/", "https://sparxsystems.com/products/ea/trial/request.html", # this raises 403 for crawlers probably? From 0a9967f303f1f0dc81a4701ee289e6729b3d5ca4 Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Thu, 1 May 2025 12:22:13 +0200 Subject: [PATCH 5/5] :construction: [#550] Improvements tests, client --- src/objects/api/validators.py | 16 ++-- src/objects/core/admin.py | 14 ++-- src/objects/core/models.py | 19 ++--- src/objects/core/utils.py | 34 ++++---- src/objects/tests/v2/test_validation.py | 61 ++++++++++---- src/objects/utils/test.py | 107 +----------------------- src/objects/utils/tests/test_client.py | 105 +++++++++++++++++++++++ 7 files changed, 193 insertions(+), 163 deletions(-) create mode 100644 src/objects/utils/tests/test_client.py diff --git a/src/objects/api/validators.py b/src/objects/api/validators.py index 07927208..523a9136 100644 --- a/src/objects/api/validators.py +++ b/src/objects/api/validators.py @@ -5,7 +5,7 @@ from rest_framework import serializers from rest_framework.fields import get_attribute -from objects.core.utils import check_objecttype +from objects.core.utils import check_objecttype_cached from objects.utils.client import get_objecttypes_client from .constants import Operators @@ -41,7 +41,7 @@ def __call__(self, attrs, serializer): if not object_type or not version: return try: - check_objecttype(object_type, version, data) + check_objecttype_cached(object_type, version, data) except ValidationError as exc: raise serializers.ValidationError(exc.args[0], code=self.code) from exc @@ -131,13 +131,13 @@ def __call__(self, attrs, serializer): if not geometry: return - client = get_objecttypes_client(object_type.service) - try: - response_data = client.get_objecttype(object_type.uuid) - except requests.RequestException as exc: - msg = f"Object type can not be retrieved: {exc.args[0]}" - raise ValidationError(msg) + with get_objecttypes_client(object_type.service) as client: + try: + response_data = client.get_objecttype(object_type.uuid) + except requests.RequestException as exc: + msg = f"Object type can not be retrieved: {exc.args[0]}" + raise ValidationError(msg) allow_geometry = response_data.get("allowGeometry", True) diff --git a/src/objects/core/admin.py b/src/objects/core/admin.py index 6cd3d020..61890128 100644 --- a/src/objects/core/admin.py +++ b/src/objects/core/admin.py @@ -37,13 +37,13 @@ def get_urls(self): def versions_view(self, request, objecttype_id): versions = [] if objecttype := self.get_object(request, objecttype_id): - client = get_objecttypes_client(objecttype.service) - try: - versions = client.list_objecttype_versions(objecttype.uuid) - except (requests.RequestException, requests.JSONDecodeError): - logger.exception( - "Something went wrong while fetching objecttype versions" - ) + with get_objecttypes_client(objecttype.service) as client: + try: + versions = client.list_objecttype_versions(objecttype.uuid) + except (requests.RequestException, requests.JSONDecodeError): + logger.exception( + "Something went wrong while fetching objecttype versions" + ) return JsonResponse(versions, safe=False) diff --git a/src/objects/core/models.py b/src/objects/core/models.py index e7d88715..d111d883 100644 --- a/src/objects/core/models.py +++ b/src/objects/core/models.py @@ -15,7 +15,7 @@ from objects.utils.client import get_objecttypes_client from .query import ObjectQuerySet, ObjectRecordQuerySet, ObjectTypeQuerySet -from .utils import check_objecttype +from .utils import check_objecttype_cached class ObjectType(models.Model): @@ -53,14 +53,13 @@ def clean_fields(self, exclude: Iterable[str] | None = None) -> None: if exclude and "service" in exclude: return - client = get_objecttypes_client(self.service) - - try: - object_type_data = client.get_objecttype(self.uuid) - except (requests.RequestException, ConnectionError, ValueError) as exc: - raise ValidationError(f"Objecttype can't be requested: {exc}") - except requests.exceptions.JSONDecodeError: - raise ValidationError("Object type version didn't have any data") + with get_objecttypes_client(self.service) as client: + try: + object_type_data = client.get_objecttype(self.uuid) + except (requests.RequestException, ConnectionError, ValueError) as exc: + raise ValidationError(f"Objecttype can't be requested: {exc}") + except requests.exceptions.JSONDecodeError: + raise ValidationError("Object type version didn't have any data") if not self._name: self._name = object_type_data["name"] @@ -156,7 +155,7 @@ def clean(self): super().clean() if hasattr(self.object, "object_type") and self.version and self.data: - check_objecttype(self.object.object_type, self.version, self.data) + check_objecttype_cached(self.object.object_type, self.version, self.data) def save(self, *args, **kwargs): if not self.id and self.object.last_record: diff --git a/src/objects/core/utils.py b/src/objects/core/utils.py index e0988be9..eb86c60d 100644 --- a/src/objects/core/utils.py +++ b/src/objects/core/utils.py @@ -4,38 +4,42 @@ import jsonschema import requests +from objects.core import models from objects.utils.cache import cache from objects.utils.client import get_objecttypes_client -def check_objecttype(object_type, version, data): +def check_objecttype_cached( + object_type: "models.ObjectType", version: int, data: dict +) -> None: @cache( f"objecttypen-{object_type.uuid}:versions-{version}", timeout=settings.OBJECTTYPE_VERSION_CACHE_TIMEOUT, ) def get_objecttype_version_response(): - client = get_objecttypes_client(object_type.service) - try: - return client.get_objecttype_version(object_type.uuid, version) - except (requests.RequestException, requests.JSONDecodeError): - raise ValidationError( - {"type": "Object type version can not be retrieved."}, - code="invalid", - ) + with get_objecttypes_client(object_type.service) as client: + try: + return client.get_objecttype_version(object_type.uuid, version) + except (requests.RequestException, requests.JSONDecodeError): + raise ValidationError( + {"non_field_errors": "Object type version can not be retrieved."}, + code="invalid", + ) try: vesion_data = get_objecttype_version_response() - jsonschema.validate(data, vesion_data["jsonSchema"]) except KeyError: raise ValidationError( { - "type": f"{object_type.versions_url} does not appear to be a valid objecttype." + "non_field_errors": f"{object_type.versions_url} does not appear to be a valid objecttype." }, code="invalid_key", ) except jsonschema.exceptions.ValidationError as exc: - raise ValidationError({"data": exc.args[0]}, code="invalid_jsonschema") + raise ValidationError( + {"non_field_errors": exc.args[0]}, code="invalid_jsonschema" + ) def can_connect_to_objecttypes() -> bool: @@ -45,7 +49,7 @@ def can_connect_to_objecttypes() -> bool: from zgw_consumers.models import Service for service in Service.objects.filter(object_types__isnull=False).distinct(): - client = get_objecttypes_client(service) - if not client.can_connect: - return False + with get_objecttypes_client(service) as client: + if not client.can_connect: + return False return True diff --git a/src/objects/tests/v2/test_validation.py b/src/objects/tests/v2/test_validation.py index 943744e9..27042b47 100644 --- a/src/objects/tests/v2/test_validation.py +++ b/src/objects/tests/v2/test_validation.py @@ -1,7 +1,11 @@ +import datetime import uuid +from django.conf import settings + import requests import requests_mock +from freezegun import freeze_time from rest_framework import status from rest_framework.test import APITestCase @@ -69,22 +73,39 @@ def test_valid_create_object_check_cache(self, m): self.assertEqual(m.call_count, 5) self.assertEqual(Object.objects.count(), 10) - with self.subTest("change_version"): - self._clear_caches() + with self.subTest("cache_timeout"): m.reset_mock() - self.assertEqual(m.call_count, 0) - for n in range(5): + old_datetime = datetime.datetime(2025, 5, 1, 12, 0) + with freeze_time(old_datetime.isoformat()): + self.assertEqual(m.call_count, 0) self.client.post(url, data, **GEO_WRITE_KWARGS) - # one request version 1 - self.assertEqual(m.call_count, 1) - self.assertEqual(Object.objects.count(), 15) - - data["record"]["typeVersion"] = 2 - for n in range(5): self.client.post(url, data, **GEO_WRITE_KWARGS) - # one request for version 1 and one for version 2 - self.assertEqual(m.call_count, 2) - self.assertEqual(Object.objects.count(), 20) + # only one request for two post + self.assertEqual(m.call_count, 1) + + # cache_timeout is still ok + cache_timeout = settings.OBJECTTYPE_VERSION_CACHE_TIMEOUT + new_datetime = old_datetime + datetime.timedelta( + seconds=(cache_timeout - 60) + ) + with freeze_time(new_datetime.isoformat()): + # same request as before + self.assertEqual(m.call_count, 1) + self.client.post(url, data, **GEO_WRITE_KWARGS) + # same request as before + self.assertEqual(m.call_count, 1) + + # cache_timeout is expired + cache_timeout = settings.OBJECTTYPE_VERSION_CACHE_TIMEOUT + new_datetime = old_datetime + datetime.timedelta( + seconds=(cache_timeout + 60) + ) + with freeze_time(new_datetime.isoformat()): + # same request as before + self.assertEqual(m.call_count, 1) + self.client.post(url, data, **GEO_WRITE_KWARGS) + # new request + self.assertEqual(m.call_count, 2) def test_create_object_with_not_found_objecttype_url(self, m): object_type_invalid = ObjectTypeFactory(service=self.object_type.service) @@ -156,7 +177,9 @@ def test_create_object_no_version(self, m): self.assertEqual(Object.objects.count(), 0) data = response.json() - self.assertEqual(data["type"], ["Object type version can not be retrieved."]) + self.assertEqual( + data["non_field_errors"], ["Object type version can not be retrieved."] + ) def test_create_object_objecttype_request_error(self, m): mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") @@ -178,7 +201,9 @@ def test_create_object_objecttype_request_error(self, m): self.assertEqual(Object.objects.count(), 0) data = response.json() - self.assertEqual(data["type"], ["Object type version can not be retrieved."]) + self.assertEqual( + data["non_field_errors"], ["Object type version can not be retrieved."] + ) def test_create_object_objecttype_with_no_jsonSchema(self, m): mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") @@ -205,7 +230,7 @@ def test_create_object_objecttype_with_no_jsonSchema(self, m): data = response.json() self.assertEqual( - data["type"], + data["non_field_errors"], [ f"{self.object_type.versions_url} does not appear to be a valid objecttype." ], @@ -234,7 +259,9 @@ def test_create_object_schema_invalid(self, m): self.assertEqual(Object.objects.count(), 0) data = response.json() - self.assertEqual(data["data"], ["'diameter' is a required property"]) + self.assertEqual( + data["non_field_errors"], ["'diameter' is a required property"] + ) def test_create_object_without_record_invalid(self, m): mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") diff --git a/src/objects/utils/test.py b/src/objects/utils/test.py index 15fe4ea6..ab51f2f6 100644 --- a/src/objects/utils/test.py +++ b/src/objects/utils/test.py @@ -1,20 +1,6 @@ from django.core.cache import caches -import requests_mock -from rest_framework.test import APITestCase - -from objects.core.models import ObjectType -from objects.core.tests.factories import ObjectTypeFactory -from objects.tests.utils import ( - mock_objecttype, - mock_objecttype_version, - mock_service_oas_get, -) -from objects.token.constants import PermissionModes -from objects.token.tests.factories import PermissionFactory, TokenAuthFactory -from objects.utils.client import get_objecttypes_client - -OBJECT_TYPES_API = "https://example.com/objecttypes/v1/" +from objects.token.tests.factories import TokenAuthFactory class TokenAuthMixin: @@ -41,94 +27,3 @@ def setUp(self): def _clear_caches(self): for cache in caches.all(): cache.clear() - - -@requests_mock.Mocker() -class ObjecttypesClientTest(TokenAuthMixin, APITestCase): - @classmethod - def setUpTestData(cls): - super().setUpTestData() - - cls.object_type = ObjectTypeFactory(service__api_root=OBJECT_TYPES_API) - PermissionFactory.create( - object_type=cls.object_type, - mode=PermissionModes.read_and_write, - token_auth=cls.token_auth, - ) - - def test_list_objecttypes(self, m): - object_type = ObjectType.objects.first() - mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") - object_type_mock = mock_objecttype(object_type.url) - m.get( - f"{OBJECT_TYPES_API}objecttypes", - json={ - "count": 1, - "next": None, - "previous": None, - "results": [object_type_mock], - }, - ) - client = get_objecttypes_client(object_type.service) - self.assertTrue(client.can_connect) - - data = client.list_objecttypes() - self.assertEqual(data, [object_type_mock]) - self.assertEqual(len(data), 1) - - def test_get_objecttype(self, m): - object_type = ObjectType.objects.first() - mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") - m.get(object_type.url, json=mock_objecttype(object_type.url)) - - client = get_objecttypes_client(object_type.service) - data = client.get_objecttype(object_type.uuid) - self.assertTrue(data["url"], str(object_type.url)) - - def test_list_objecttype_versions(self, m): - object_type = ObjectType.objects.first() - mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") - object_type_mock = mock_objecttype(object_type.url) - m.get( - f"{OBJECT_TYPES_API}objecttypes", - json={ - "count": 1, - "next": None, - "previous": None, - "results": [object_type_mock], - }, - ) - version_mock = mock_objecttype_version(object_type.url) - m.get( - f"{object_type.url}/versions", - json={ - "count": 1, - "next": None, - "previous": None, - "results": [version_mock], - }, - ) - - client = get_objecttypes_client(object_type.service) - self.assertTrue(client.can_connect) - - data = client.list_objecttypes() - self.assertEqual(data, [object_type_mock]) - self.assertEqual(len(data), 1) - - data = client.list_objecttype_versions(object_type.uuid) - self.assertEqual(data, [version_mock]) - self.assertEqual(len(data), 1) - self.assertEqual(data[0]["version"], 1) - - def test_get_objecttype_version(self, m): - object_type = ObjectType.objects.first() - mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") - m.get( - f"{self.object_type.url}/versions/1", - json=mock_objecttype_version(self.object_type.url), - ) - - client = get_objecttypes_client(object_type.service) - data = client.get_objecttype_version(object_type.uuid, 1) - self.assertEqual(data["url"], f"{self.object_type.url}/versions/1") diff --git a/src/objects/utils/tests/test_client.py b/src/objects/utils/tests/test_client.py new file mode 100644 index 00000000..c7604460 --- /dev/null +++ b/src/objects/utils/tests/test_client.py @@ -0,0 +1,105 @@ +import requests_mock +from rest_framework.test import APITestCase + +from objects.core.models import ObjectType +from objects.core.tests.factories import ObjectTypeFactory +from objects.tests.utils import ( + mock_objecttype, + mock_objecttype_version, + mock_service_oas_get, +) +from objects.token.constants import PermissionModes +from objects.token.tests.factories import PermissionFactory +from objects.utils.client import get_objecttypes_client + +from ..test import TokenAuthMixin + +OBJECT_TYPES_API = "https://example.com/objecttypes/v1/" + + +@requests_mock.Mocker() +class ObjecttypesClientTest(TokenAuthMixin, APITestCase): + @classmethod + def setUpTestData(cls): + super().setUpTestData() + + cls.object_type = ObjectTypeFactory(service__api_root=OBJECT_TYPES_API) + PermissionFactory.create( + object_type=cls.object_type, + mode=PermissionModes.read_and_write, + token_auth=cls.token_auth, + ) + + def test_list_objecttypes(self, m): + object_type = ObjectType.objects.first() + mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") + object_type_mock = mock_objecttype(object_type.url) + m.get( + f"{OBJECT_TYPES_API}objecttypes", + json={ + "count": 1, + "next": None, + "previous": None, + "results": [object_type_mock], + }, + ) + with get_objecttypes_client(object_type.service) as client: + self.assertTrue(client.can_connect) + data = client.list_objecttypes() + self.assertEqual(data, [object_type_mock]) + self.assertEqual(len(data), 1) + + def test_get_objecttype(self, m): + object_type = ObjectType.objects.first() + mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") + m.get(object_type.url, json=mock_objecttype(object_type.url)) + + with get_objecttypes_client(object_type.service) as client: + data = client.get_objecttype(object_type.uuid) + self.assertTrue(data["url"], str(object_type.url)) + + def test_list_objecttype_versions(self, m): + object_type = ObjectType.objects.first() + mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") + object_type_mock = mock_objecttype(object_type.url) + m.get( + f"{OBJECT_TYPES_API}objecttypes", + json={ + "count": 1, + "next": None, + "previous": None, + "results": [object_type_mock], + }, + ) + version_mock = mock_objecttype_version(object_type.url) + m.get( + f"{object_type.url}/versions", + json={ + "count": 1, + "next": None, + "previous": None, + "results": [version_mock], + }, + ) + + with get_objecttypes_client(object_type.service) as client: + self.assertTrue(client.can_connect) + data = client.list_objecttypes() + self.assertEqual(data, [object_type_mock]) + self.assertEqual(len(data), 1) + data = client.list_objecttype_versions(object_type.uuid) + self.assertEqual(data, [version_mock]) + self.assertEqual(len(data), 1) + self.assertEqual(data[0]["version"], 1) + + def test_get_objecttype_version(self, m): + object_type = ObjectType.objects.first() + mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") + m.get( + f"{self.object_type.url}/versions/1", + json=mock_objecttype_version(self.object_type.url), + ) + + with get_objecttypes_client(object_type.service) as client: + data = client.get_objecttype_version(object_type.uuid, 1) + self.assertEqual(data["url"], f"{self.object_type.url}/versions/1")