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? diff --git a/docs/installation/config.rst b/docs/installation/config.rst index e4476aad..7463f707 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 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 f0d49797..523a9136 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.core.utils import check_objecttype_cached +from objects.utils.client import get_objecttypes_client from .constants import Operators from .utils import merge_patch, string_to_value @@ -40,9 +40,8 @@ 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 @@ -133,14 +132,14 @@ def __call__(self, attrs, serializer): if not geometry: return - client = build_client(object_type.service) - try: - response = client.get(url=object_type.url) - 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.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 76b839f4..a856db35 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 in seconds for cache when retrieving objecttype versions.", + group="Cache", +) # # Additional Django settings diff --git a/src/objects/core/admin.py b/src/objects/core/admin.py index f60a23dc..61890128 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,14 +37,13 @@ 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) - try: - response = client.get(objecttype.versions_url) - versions = list(pagination_helper(client, response.json())) - 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 3ad77620..d111d883 100644 --- a/src/objects/core/models.py +++ b/src/objects/core/models.py @@ -10,11 +10,12 @@ 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 +from .utils import check_objecttype_cached class ObjectType(models.Model): @@ -52,17 +53,13 @@ def clean_fields(self, exclude: Iterable[str] | None = None) -> None: if exclude and "service" in exclude: return - client = build_client(self.service) - - try: - response = client.get(url=self.url) - 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") + 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"] @@ -158,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 eb94f40b..eb86c60d 100644 --- a/src/objects/core/utils.py +++ b/src/objects/core/utils.py @@ -1,37 +1,45 @@ +from django.conf import settings from django.core.exceptions import ValidationError import jsonschema import requests -from zgw_consumers.client import build_client - -def check_objecttype(object_type, version, data): - client = build_client(object_type.service) - objecttype_version_url = f"{object_type.url}/versions/{version}" - - try: - response = client.get(objecttype_version_url) - except requests.RequestException: - msg = "Object type version can not be retrieved." - raise ValidationError(msg) - - try: - response_data = response.json() - except requests.JSONDecodeError: - raise ValidationError("Object type doesn't have retrievable data.") +from objects.core import models +from objects.utils.cache import cache +from objects.utils.client import get_objecttypes_client + + +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(): + 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: - schema = response_data["jsonSchema"] + vesion_data = get_objecttype_version_response() + jsonschema.validate(data, vesion_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) + raise ValidationError( + { + "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(exc.args[0]) from exc + raise ValidationError( + {"non_field_errors": exc.args[0]}, code="invalid_jsonschema" + ) def can_connect_to_objecttypes() -> bool: @@ -40,13 +48,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: - return False - + for service in Service.objects.filter(object_types__isnull=False).distinct(): + with get_objecttypes_client(service) as client: + if not client.can_connect: + return False return True 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..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 @@ -9,7 +13,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 +23,7 @@ @requests_mock.Mocker() -class ObjectTypeValidationTests(TokenAuthMixin, APITestCase): +class ObjectTypeValidationTests(TokenAuthMixin, ClearCachesMixin, APITestCase): @classmethod def setUpTestData(cls): super().setUpTestData() @@ -31,6 +35,78 @@ 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("cache_timeout"): + m.reset_mock() + 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) + self.client.post(url, data, **GEO_WRITE_KWARGS) + # 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) PermissionFactory.create( @@ -102,7 +178,7 @@ def test_create_object_no_version(self, m): data = response.json() self.assertEqual( - data["non_field_errors"], ["Object type doesn't have retrievable data."] + data["non_field_errors"], ["Object type version can not be retrieved."] ) def test_create_object_objecttype_request_error(self, m): @@ -156,7 +232,7 @@ def test_create_object_objecttype_with_no_jsonSchema(self, m): self.assertEqual( data["non_field_errors"], [ - 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." ], ) 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 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 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() 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")