From 35e905de46c314bd3db7793e71c37ffd00312d06 Mon Sep 17 00:00:00 2001 From: Diederik van der Boor Date: Mon, 17 Jun 2024 16:36:11 +0200 Subject: [PATCH 1/2] docs: improve some docstrings for serializer levels --- src/dso_api/dynamic_api/serializers/base.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/dso_api/dynamic_api/serializers/base.py b/src/dso_api/dynamic_api/serializers/base.py index 7887a9c13..a9a164c35 100644 --- a/src/dso_api/dynamic_api/serializers/base.py +++ b/src/dso_api/dynamic_api/serializers/base.py @@ -70,7 +70,9 @@ class DynamicListSerializer(DSOModelListSerializer): - """This serializer class is used internally when :class:`DynamicSerializer` + """The serializer for a list of objects. + + This serializer class is used internally when :class:`DynamicSerializer` is initialized with the ``many=True`` init kwarg to process a list of objects. """ @@ -210,7 +212,7 @@ def expanded_fields(self) -> list[EmbeddedFieldMatch]: ) -class FieldAccessMixin(serializers.ModelSerializer): +class FieldAccessMixin(DSOModelSerializerBase): """Mixin for serializers to remove fields the user doesn't have access to.""" fields_always_included = {"_links"} @@ -420,9 +422,9 @@ def build_property_field(self, field_name, model_class): class DynamicBodySerializer(DynamicSerializer): - """This subclass of the dynamic serializer only exposes the non-relational fields. + """The serializer for the *main* object. - This serializer base class is used for the main object fields. + This subclass of the dynamic serializer only exposes the non-relational fields. """ def get_fields(self): @@ -496,15 +498,19 @@ def to_representation(self, instance): ret = OrderedDict() for field in self._readable_fields: + # Extra check: remove fields that we don't have access to. snaked_field_name = to_snake_case(field.field_name) if snaked_field_name in schema_fields_lookup and not has_field_access( user_scopes, schema_fields_lookup[snaked_field_name] ): continue + + # This is the base class logic: try: attribute = field.get_attribute(instance) except SkipField: continue + check_for_none = attribute.pk if isinstance(attribute, PKOnlyObject) else attribute if check_for_none is None: ret[field.field_name] = None From 7936e279291e15e45a0503bfbc5214ebd1f35a29 Mon Sep 17 00:00:00 2001 From: Diederik van der Boor Date: Mon, 17 Jun 2024 16:41:02 +0200 Subject: [PATCH 2/2] Fix massive DSO-API performance issue, due to deepcopy() DRF performs a deepcopy() of the serializer._declared_fields before it will handle the request. This is analogous to how Django forms work, and allows updating the fields with request state and 'parent' references. Since the deepcopy() also touched dataset definitions of Amsterdam Schema, it basically took 14sec to copy all those dicts as well. This can be avoided, bringing to performance of DSO-API back to reasonable levels. --- src/dso_api/dynamic_api/serializers/factories.py | 4 +--- src/dso_api/dynamic_api/serializers/fields.py | 10 ++++++++++ src/rest_framework_dso/fields.py | 11 +++++++++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/dso_api/dynamic_api/serializers/factories.py b/src/dso_api/dynamic_api/serializers/factories.py index 7f42d0738..e04e51443 100644 --- a/src/dso_api/dynamic_api/serializers/factories.py +++ b/src/dso_api/dynamic_api/serializers/factories.py @@ -534,11 +534,9 @@ def _build_serializer_embedded_field( embedded_field = EmbeddedFieldClass( serializer_class=cast(type[base.DynamicSerializer], serializer_class), - # serializer_class=serializer_class, + field_schema=field_schema, source=model_field.name, ) - # Attach the field schema so access rules can be applied here. - embedded_field.field_schema = field_schema # The field name is still generated from the model_field, in case this is a reverse field. serializer_part.add_embedded_field(toCamelCase(model_field.name), embedded_field) diff --git a/src/dso_api/dynamic_api/serializers/fields.py b/src/dso_api/dynamic_api/serializers/fields.py index 2967173b0..d26f87be7 100644 --- a/src/dso_api/dynamic_api/serializers/fields.py +++ b/src/dso_api/dynamic_api/serializers/fields.py @@ -63,8 +63,18 @@ def __init__(self, table_schema: DatasetTableSchema, *args, **kwargs): # Init adds temporal definitions at construction, removing runtime model lookups. # It also allows the PK optimization to be used. super().__init__(*args, **kwargs) + + # Link the table to perform temporal query filtering self.table_schema = table_schema + def __deepcopy__(self, memo): + # Fix a massive performance hit when DRF performs a deepcopy() of all fields + result = self.__class__.__new__(self.__class__) + memo[id(result)] = self + result.__dict__.update({k: v for k, v in self.__dict__.items() if k != "table_schema"}) + result.table_schema = self.table_schema + return result + def use_pk_only_optimization(self): return True # only need to have an "id" here. diff --git a/src/rest_framework_dso/fields.py b/src/rest_framework_dso/fields.py index 2151019ed..6fe4efb29 100644 --- a/src/rest_framework_dso/fields.py +++ b/src/rest_framework_dso/fields.py @@ -15,6 +15,7 @@ from rest_framework.exceptions import ValidationError from rest_framework_gis.fields import GeometryField from schematools.contrib.django.models import LooseRelationField +from schematools.types import DatasetFieldSchema from rest_framework_dso.utils import group_dotted_names, unlazy_object @@ -289,14 +290,24 @@ def __init__( self, serializer_class: type[serializers.Serializer], *, + field_schema: DatasetFieldSchema | None = None, source=None, ): self.serializer_class = serializer_class self.source = source self.field_name = None + self.field_schema = field_schema self.parent_serializer_class = None + def __deepcopy__(self, memo): + # Fix a massive performance hit when DRF performs a deepcopy() of all fields + result = self.__class__.__new__(self.__class__) + memo[id(result)] = self + result.__dict__.update({k: v for k, v in self.__dict__.items() if k != "field_schema"}) + result.field_schema = self.field_schema + return result + def __repr__(self): try: parent_serializer = self.parent_serializer_class.__name__