From 47f06ad928e3493720531508875eaab9021ad378 Mon Sep 17 00:00:00 2001 From: Ryo Chijiiwa Date: Fri, 9 Aug 2019 22:24:17 -0700 Subject: [PATCH 1/8] more serializer optimizations --- dynamic_rest/prefetch.py | 2 ++ dynamic_rest/serializers.py | 40 +++++++++++++++++++++++++++---------- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/dynamic_rest/prefetch.py b/dynamic_rest/prefetch.py index 460475ee..6a3b7d93 100644 --- a/dynamic_rest/prefetch.py +++ b/dynamic_rest/prefetch.py @@ -15,6 +15,8 @@ class FastObject(dict): + IS_FAST = True + def __init__(self, *args, **kwargs): self.pk_field = kwargs.pop('pk_field', 'id') return super(FastObject, self).__init__(*args) diff --git a/dynamic_rest/serializers.py b/dynamic_rest/serializers.py index e3e5dd04..35fdb63b 100644 --- a/dynamic_rest/serializers.py +++ b/dynamic_rest/serializers.py @@ -21,8 +21,9 @@ ) from dynamic_rest.conf import settings from dynamic_rest.fields import ( - DynamicRelationField, DynamicGenericRelationField, + DynamicMethodField, + DynamicRelationField, ) from dynamic_rest.links import merge_link_object from dynamic_rest.meta import get_model_table @@ -548,7 +549,7 @@ def _readable_fields(self): if not field.write_only ] - @cached_property + @resettable_cached_property def _readable_id_fields(self): fields = self._readable_fields return { @@ -564,6 +565,21 @@ def _readable_id_fields(self): ) } + @resettable_cached_property + def _readable_static_fields(self): + return { + field for field in self._readable_fields + if not isinstance(field, ( + DynamicGenericRelationField, + DynamicMethodField, + DynamicRelationField + )) + } | self._readable_id_fields + + @resettable_cached_property + def _simple_fields(self): + return set(getattr(self.Meta, 'simple_fields', [])) + def _faster_to_representation(self, instance): """Modified to_representation with optimizations. @@ -581,21 +597,22 @@ def _faster_to_representation(self, instance): ret = {} fields = self._readable_fields - is_fast = isinstance(instance, prefetch.FastObject) + is_fast = getattr(instance, 'IS_FAST', False) id_fields = self._readable_id_fields + # static fields are non-Dynamic fields + static_fields = self._readable_static_fields + + # fields declared as being "simple" (i.e. doesn't require + # field.to_representation() to be serializable) + simple_field_names = self._simple_fields + for field in fields: attribute = None # we exclude dynamic fields here because the proper fastquery # dereferencing happens in the `get_attribute` method now - if ( - is_fast and - not isinstance( - field, - (DynamicGenericRelationField, DynamicRelationField) - ) - ): + if (is_fast and field in static_fields): if field in id_fields and field.source not in instance: # TODO - make better. attribute = instance.get(field.source + '_id') @@ -628,6 +645,9 @@ def _faster_to_representation(self, instance): # We skip `to_representation` for `None` values so that # fields do not have to explicitly deal with that case. ret[field.field_name] = None + elif field.field_name in simple_field_names: + ret[field.field_name] = attribute + continue else: ret[field.field_name] = field.to_representation(attribute) From 942d273701cf5da22ee3f0afd804e23eb3cdb3e1 Mon Sep 17 00:00:00 2001 From: Ryo Chijiiwa Date: Sun, 11 Aug 2019 18:40:14 -0700 Subject: [PATCH 2/8] more efficient links generation --- dynamic_rest/conf.py | 1 - dynamic_rest/links.py | 5 ++++- dynamic_rest/routers.py | 14 +++++++++----- dynamic_rest/serializers.py | 8 +++++++- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/dynamic_rest/conf.py b/dynamic_rest/conf.py index 39cad400..12f49c67 100644 --- a/dynamic_rest/conf.py +++ b/dynamic_rest/conf.py @@ -88,7 +88,6 @@ def __init__(self, name, defaults, settings, class_attrs=None): self.defaults = defaults self.keys = set(defaults.keys()) self.class_attrs = class_attrs - self._cache = {} self._reload(getattr(settings, self.name, {})) diff --git a/dynamic_rest/links.py b/dynamic_rest/links.py index c0743e8b..36b32884 100644 --- a/dynamic_rest/links.py +++ b/dynamic_rest/links.py @@ -2,7 +2,6 @@ from django.utils import six from dynamic_rest.conf import settings -from dynamic_rest.routers import DynamicRouter def merge_link_object(serializer, data, instance): @@ -32,10 +31,14 @@ def merge_link_object(serializer, data, instance): if settings.ENABLE_HOST_RELATIVE_LINKS: # if the resource isn't registered, this will default back to # using resource-relative urls for links. + base_url = serializer.canonical_base_path or '' + base_url += "/%s/" % instance.pk + ''' base_url = DynamicRouter.get_canonical_path( serializer.get_resource_key(), instance.pk ) or '' + ''' link = '%s%s/' % (base_url, name) # Default to DREST-generated relation endpoints. elif callable(link): diff --git a/dynamic_rest/routers.py b/dynamic_rest/routers.py index f9992b15..c89127a4 100644 --- a/dynamic_rest/routers.py +++ b/dynamic_rest/routers.py @@ -242,6 +242,14 @@ def register_resource(self, viewset, namespace=None): # map the resource name to the resource key for easier lookup resource_name_map[resource_name] = resource_key + @staticmethod + def get_canonical_base_path(resource_key): + if resource_key not in resource_map: + # Note: Maybe raise? + return None + + return get_script_prefix() + resource_map[resource_key]['path'] + @staticmethod def get_canonical_path(resource_key, pk=None): """ @@ -254,11 +262,7 @@ def get_canonical_path(resource_key, pk=None): Returns: Absolute URL as string. """ - if resource_key not in resource_map: - # Note: Maybe raise? - return None - - base_path = get_script_prefix() + resource_map[resource_key]['path'] + base_path = DynamicRouter.get_canonical_base_path() if pk: return '%s/%s/' % (base_path, pk) else: diff --git a/dynamic_rest/serializers.py b/dynamic_rest/serializers.py index 35fdb63b..89cfbc34 100644 --- a/dynamic_rest/serializers.py +++ b/dynamic_rest/serializers.py @@ -13,13 +13,13 @@ from rest_framework.fields import SkipField from rest_framework.utils.serializer_helpers import ReturnDict, ReturnList -from dynamic_rest import prefetch from dynamic_rest.bases import ( CacheableFieldMixin, DynamicSerializerBase, resettable_cached_property ) from dynamic_rest.conf import settings +from dynamic_rest.routers import DynamicRouter from dynamic_rest.fields import ( DynamicGenericRelationField, DynamicMethodField, @@ -517,6 +517,12 @@ def is_field_sideloaded(self, field_name): def get_link_fields(self): return self._link_fields + @cached_property + def canonical_base_path(self): + return DynamicRouter.get_canonical_base_path( + self.get_resource_key() + ) + @resettable_cached_property def _link_fields(self): """Construct dict of name:field for linkable fields.""" From ea7798ea9574e545c572cca40139e5eccd948086 Mon Sep 17 00:00:00 2001 From: Ryo Chijiiwa Date: Wed, 2 Oct 2019 12:28:37 -0700 Subject: [PATCH 3/8] fix bug --- dynamic_rest/routers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dynamic_rest/routers.py b/dynamic_rest/routers.py index c89127a4..f4f3f1f6 100644 --- a/dynamic_rest/routers.py +++ b/dynamic_rest/routers.py @@ -262,7 +262,7 @@ def get_canonical_path(resource_key, pk=None): Returns: Absolute URL as string. """ - base_path = DynamicRouter.get_canonical_base_path() + base_path = DynamicRouter.get_canonical_base_path(resource_key) if pk: return '%s/%s/' % (base_path, pk) else: From 2717ce90d59aa835f0dcb9d2341c76412b7ea0d3 Mon Sep 17 00:00:00 2001 From: Ryo Chijiiwa Date: Thu, 5 Nov 2020 12:40:23 -0800 Subject: [PATCH 4/8] auto-infer simple fields --- Makefile | 2 +- dynamic_rest/serializers.py | 15 ++++++++++++++- tests/test_serializers.py | 5 +++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 4933755f..b4309605 100644 --- a/Makefile +++ b/Makefile @@ -65,7 +65,7 @@ clean: clean_working_directory @rm -rf $(INSTALL_DIR) # Run tests -test: install lint +test: install $(call header,"Running unit tests") @$(INSTALL_DIR)/bin/py.test --cov=$(APP_NAME) tests/$(TEST) diff --git a/dynamic_rest/serializers.py b/dynamic_rest/serializers.py index 89cfbc34..c9a87e85 100644 --- a/dynamic_rest/serializers.py +++ b/dynamic_rest/serializers.py @@ -584,7 +584,20 @@ def _readable_static_fields(self): @resettable_cached_property def _simple_fields(self): - return set(getattr(self.Meta, 'simple_fields', [])) + if hasattr(self.Meta, 'simple_fields'): + return set(getattr(self.Meta, 'simple_fields', [])) + + if not hasattr(self, '_declared_fields'): + # The `_declared_fields` attr should be set by DRF, but since + # it's a private attribute, we'll be safe. + return [] + + simple_fields = set() + for name, field in six.iteritems(self.get_all_fields()): + if field not in self._declared_fields: + simple_fields.add(name) + + return simple_fields def _faster_to_representation(self, instance): """Modified to_representation with optimizations. diff --git a/tests/test_serializers.py b/tests/test_serializers.py index 057d4d36..3ffe5609 100644 --- a/tests/test_serializers.py +++ b/tests/test_serializers.py @@ -506,6 +506,11 @@ def test_serializer_propagation_consistency(self): self.assertEqual(r1, r2) self.assertEqual(r2, r3) + def test_simple_fields(self): + s = LocationSerializer() + simple_fields = s._simple_fields() + import pdb;pdb.set_trace() + @patch.dict('dynamic_rest.processors.POST_PROCESSORS', {}) def test_post_processors(self): From b47d5aba47f7e925d0c76554cf9f974ddc8f371a Mon Sep 17 00:00:00 2001 From: Ryo Chijiiwa Date: Thu, 5 Nov 2020 15:02:13 -0800 Subject: [PATCH 5/8] infer simple fields --- dynamic_rest/serializers.py | 12 ++++++++++-- tests/test_serializers.py | 10 +++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/dynamic_rest/serializers.py b/dynamic_rest/serializers.py index c9a87e85..65822500 100644 --- a/dynamic_rest/serializers.py +++ b/dynamic_rest/serializers.py @@ -582,8 +582,13 @@ def _readable_static_fields(self): )) } | self._readable_id_fields - @resettable_cached_property + @cached_property def _simple_fields(self): + """ + Simple fields are fields that return serializable values straight + from the DB and therefore don't require logic on the model or Field + object to extract and serialize. + """ if hasattr(self.Meta, 'simple_fields'): return set(getattr(self.Meta, 'simple_fields', [])) @@ -594,7 +599,7 @@ def _simple_fields(self): simple_fields = set() for name, field in six.iteritems(self.get_all_fields()): - if field not in self._declared_fields: + if name not in self._declared_fields: simple_fields.add(name) return simple_fields @@ -606,6 +611,8 @@ def _faster_to_representation(self, instance): (Constructing ordered dict is ~100x slower than `{}`.) 2) Ensure we use a cached list of fields (this optimization exists in DRF 3.2 but not 3.1) + 3) Bypass DRF whenever possible, use simple dict lookup + if FastQuery is enabled (which returns dicts). Arguments: instance: a model instance or data object @@ -655,6 +662,7 @@ def _faster_to_representation(self, instance): ) ) else: + # Non-optimized standard DRF approach... try: attribute = field.get_attribute(instance) except SkipField: diff --git a/tests/test_serializers.py b/tests/test_serializers.py index 3ffe5609..164d7fa2 100644 --- a/tests/test_serializers.py +++ b/tests/test_serializers.py @@ -507,9 +507,13 @@ def test_serializer_propagation_consistency(self): self.assertEqual(r2, r3) def test_simple_fields(self): - s = LocationSerializer() - simple_fields = s._simple_fields() - import pdb;pdb.set_trace() + # Should return non-declared non-dynamic fields + # See WithDynamicSerializerMixin._simple_fields for more. + szr = LocationSerializer() + self.assertEqual( + set(['id', 'name']), + szr._simple_fields + ) @patch.dict('dynamic_rest.processors.POST_PROCESSORS', {}) def test_post_processors(self): From cc0f8a22f08995c82f609a0aba85c018070acfb2 Mon Sep 17 00:00:00 2001 From: Ryo Chijiiwa Date: Thu, 5 Nov 2020 17:36:05 -0800 Subject: [PATCH 6/8] make inflection requirement more open --- install_requires.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/install_requires.txt b/install_requires.txt index 3c1bf9f9..2bd72c4e 100644 --- a/install_requires.txt +++ b/install_requires.txt @@ -1,4 +1,4 @@ Django>=1.11,<3.0.0 djangorestframework>=3.8.0,<3.12.0 -inflection==0.4.0 +inflection>=0.3.0,<0.5.0 requests From de2a608a4ce25ed1373926ffd8ab311a182b9941 Mon Sep 17 00:00:00 2001 From: Ryo Chijiiwa Date: Fri, 6 Nov 2020 13:16:15 -0800 Subject: [PATCH 7/8] check field types to consider 'simple' --- dynamic_rest/serializers.py | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/dynamic_rest/serializers.py b/dynamic_rest/serializers.py index 65822500..7e1d60f9 100644 --- a/dynamic_rest/serializers.py +++ b/dynamic_rest/serializers.py @@ -10,7 +10,16 @@ from rest_framework import __version__ as drf_version from rest_framework import exceptions, fields, serializers from rest_framework.relations import RelatedField -from rest_framework.fields import SkipField +from rest_framework.fields import ( + BooleanField, + CharField, + DateField, + DateTimeField, + FloatField, + IntegerField, + SkipField, + UUIDField, +) from rest_framework.utils.serializer_helpers import ReturnDict, ReturnList from dynamic_rest.bases import ( @@ -597,9 +606,28 @@ def _simple_fields(self): # it's a private attribute, we'll be safe. return [] + # We assume inferred fields of these types to be "simple" + simple_field_classes = ( + BooleanField, # also the base class for NullBooleanField + CharField, # also the base class for others, like SlugField + DateField, + DateTimeField, + FloatField, + IntegerField, + UUIDField, + ) + + # This meta attr can explicitly opt fields out, e.g. if it's a + # compatible field type but actually needs to go thru DRF Field + complex_fields = set(getattr(self.Meta, 'complex_fields', [])) + simple_fields = set() for name, field in six.iteritems(self.get_all_fields()): - if name not in self._declared_fields: + if name in self._declared_fields: + continue + if name in complex_fields: + continue + if isinstance(field, simple_field_classes): simple_fields.add(name) return simple_fields @@ -674,7 +702,6 @@ def _faster_to_representation(self, instance): ret[field.field_name] = None elif field.field_name in simple_field_names: ret[field.field_name] = attribute - continue else: ret[field.field_name] = field.to_representation(attribute) From 9720922504f78e2befcb4929d8ea760ca84b61e3 Mon Sep 17 00:00:00 2001 From: Ryo Chijiiwa Date: Fri, 6 Nov 2020 22:15:37 -0800 Subject: [PATCH 8/8] improve handling of id-only fields --- dynamic_rest/serializers.py | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/dynamic_rest/serializers.py b/dynamic_rest/serializers.py index 7e1d60f9..75e91e57 100644 --- a/dynamic_rest/serializers.py +++ b/dynamic_rest/serializers.py @@ -591,7 +591,7 @@ def _readable_static_fields(self): )) } | self._readable_id_fields - @cached_property + @resettable_cached_property def _simple_fields(self): """ Simple fields are fields that return serializable values straight @@ -610,8 +610,8 @@ def _simple_fields(self): simple_field_classes = ( BooleanField, # also the base class for NullBooleanField CharField, # also the base class for others, like SlugField - DateField, - DateTimeField, + # DateField, + # DateTimeField, FloatField, IntegerField, UUIDField, @@ -667,11 +667,25 @@ def _faster_to_representation(self, instance): # we exclude dynamic fields here because the proper fastquery # dereferencing happens in the `get_attribute` method now if (is_fast and field in static_fields): - if field in id_fields and field.source not in instance: - # TODO - make better. - attribute = instance.get(field.source + '_id') - ret[field.field_name] = attribute - continue + if field in id_fields: + if field.source not in instance: + # TODO - make better. + attribute = instance.get(field.source + '_id') + ret[field.field_name] = attribute + continue + elif not instance[field.source]: + ret[field.field_name] = None + continue + elif 'id' in instance[field.source]: + # reverse of o2o field + print("Beep beep! s=%s f=%s" % ( + self.__class__, + field.field_name + )) + ret[field.field_name] = instance[field.source]['id'] + continue + else: + attribute = field.get_attribute(instance) else: try: attribute = instance[field.source]