From 73fcc680e315413c1c35caf48f167928bb895692 Mon Sep 17 00:00:00 2001 From: Safa AlFulaij Date: Mon, 19 Apr 2021 01:25:16 +0300 Subject: [PATCH 01/15] Load included serializers once --- rest_framework_json_api/serializers.py | 50 +++++++++++++++++++++++++- rest_framework_json_api/utils.py | 15 +------- tests/test_utils.py | 32 ++++++++--------- 3 files changed, 66 insertions(+), 31 deletions(-) diff --git a/rest_framework_json_api/serializers.py b/rest_framework_json_api/serializers.py index a73a5d47..96a722b5 100644 --- a/rest_framework_json_api/serializers.py +++ b/rest_framework_json_api/serializers.py @@ -1,8 +1,10 @@ from collections import OrderedDict +from collections.abc import MutableMapping import inflection from django.core.exceptions import ObjectDoesNotExist from django.db.models.query import QuerySet +from django.utils.module_loading import import_string as import_class_from_dotted_path from django.utils.translation import gettext_lazy as _ from rest_framework.exceptions import ParseError @@ -152,8 +154,54 @@ def validate_path(serializer_class, field_path, path): super(IncludedResourcesValidationMixin, self).__init__(*args, **kwargs) +class LazySerializersDict(MutableMapping): + def __init__(self, klass, serializers): + self.klass = klass + self.serializers = serializers + + def __getitem__(self, key): + value = self.serializers[key] + if not isinstance(value, type): + if value == 'self': + value = self.klass + + print(value) + value = import_class_from_dotted_path(value) + self.serializers[key] = value + + return value + + def __setitem__(self, key, field): + self.serializers[key] = field + + def __delitem__(self, key): + del self.serializers[key] + + def __iter__(self): + return iter(self.serializers) + + def __len__(self): + return len(self.serializers) + + def __repr__(self): + return dict.__repr__(self.serializers) + + class SerializerMetaclass(SerializerMetaclass): - pass + def __new__(cls, name, bases, attrs): + #print(name) + #print(cls.__module__) + #print(attrs) + + included_serializers = attrs.get('included_serializers', None) + if included_serializers: + attrs['included_serializers'] = LazySerializersDict(attrs['__module__']+'.'+name, included_serializers) + + related_serializers = attrs.get('related_serializers', None) + if related_serializers: + attrs['related_serializers'] = LazySerializersDict(attrs['__module__']+'.'+name, related_serializers) + + return super().__new__(cls, name, bases, attrs) # If user imports serializer from here we can catch class definition and check diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index ac31979a..85415925 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -342,20 +342,7 @@ def get_default_included_resources_from_serializer(serializer): def get_included_serializers(serializer): - included_serializers = copy.copy( - getattr(serializer, "included_serializers", dict()) - ) - - for name, value in iter(included_serializers.items()): - if not isinstance(value, type): - if value == "self": - included_serializers[name] = ( - serializer if isinstance(serializer, type) else serializer.__class__ - ) - else: - included_serializers[name] = import_class_from_dotted_path(value) - - return included_serializers + return getattr(serializer, "included_serializers", dict()) def get_relation_instance(resource_instance, source, serializer): diff --git a/tests/test_utils.py b/tests/test_utils.py index 00bf0836..889cb4af 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -345,26 +345,26 @@ class PlainRelatedResourceTypeSerializer(serializers.Serializer): assert get_related_resource_type(field) == output -def test_get_included_serializers(): - class IncludedSerializersModel(DJAModel): - self = models.ForeignKey("self", on_delete=models.CASCADE) - target = models.ForeignKey(ManyToManyTarget, on_delete=models.CASCADE) - other_target = models.ForeignKey(ManyToManyTarget, on_delete=models.CASCADE) +class IncludedSerializersModel(DJAModel): + self = models.ForeignKey("self", on_delete=models.CASCADE) + target = models.ForeignKey(ManyToManyTarget, on_delete=models.CASCADE) + other_target = models.ForeignKey(ManyToManyTarget, on_delete=models.CASCADE) - class Meta: - app_label = "tests" + class Meta: + app_label = "tests" - class IncludedSerializersSerializer(serializers.ModelSerializer): - included_serializers = { - "self": "self", - "target": ManyToManyTargetSerializer, - "other_target": "tests.serializers.ManyToManyTargetSerializer", - } +class IncludedSerializersSerializer(serializers.ModelSerializer): + included_serializers = { + "self": "self", + "target": ManyToManyTargetSerializer, + "other_target": "tests.serializers.ManyToManyTargetSerializer", + } - class Meta: - model = IncludedSerializersModel - fields = ("self", "other_target", "target") + class Meta: + model = IncludedSerializersModel + fields = ("self", "other_target", "target") +def test_get_included_serializers(): included_serializers = get_included_serializers(IncludedSerializersSerializer) expected_included_serializers = { "self": IncludedSerializersSerializer, From 48c1cdce509040f6772f13a960617d0073e977e6 Mon Sep 17 00:00:00 2001 From: Safa AlFulaij Date: Mon, 19 Apr 2021 01:27:52 +0300 Subject: [PATCH 02/15] Minor --- rest_framework_json_api/serializers.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/rest_framework_json_api/serializers.py b/rest_framework_json_api/serializers.py index 96a722b5..954e4b2e 100644 --- a/rest_framework_json_api/serializers.py +++ b/rest_framework_json_api/serializers.py @@ -165,7 +165,6 @@ def __getitem__(self, key): if value == 'self': value = self.klass - print(value) value = import_class_from_dotted_path(value) self.serializers[key] = value @@ -189,17 +188,15 @@ def __repr__(self): class SerializerMetaclass(SerializerMetaclass): def __new__(cls, name, bases, attrs): - #print(name) - #print(cls.__module__) - #print(attrs) + serializer_class_path = attrs['__module__']+'.'+name included_serializers = attrs.get('included_serializers', None) if included_serializers: - attrs['included_serializers'] = LazySerializersDict(attrs['__module__']+'.'+name, included_serializers) + attrs['included_serializers'] = LazySerializersDict(serializer_class_path, included_serializers) related_serializers = attrs.get('related_serializers', None) if related_serializers: - attrs['related_serializers'] = LazySerializersDict(attrs['__module__']+'.'+name, related_serializers) + attrs['related_serializers'] = LazySerializersDict(serializer_class_path, related_serializers) return super().__new__(cls, name, bases, attrs) From f36d5ac0446e0c971e32422c5bbf9373a4b12169 Mon Sep 17 00:00:00 2001 From: Safa AlFulaij Date: Mon, 19 Apr 2021 02:31:31 +0300 Subject: [PATCH 03/15] Format code --- rest_framework_json_api/serializers.py | 16 ++++++++++------ tests/test_utils.py | 2 ++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/rest_framework_json_api/serializers.py b/rest_framework_json_api/serializers.py index 954e4b2e..4ad1ce14 100644 --- a/rest_framework_json_api/serializers.py +++ b/rest_framework_json_api/serializers.py @@ -162,7 +162,7 @@ def __init__(self, klass, serializers): def __getitem__(self, key): value = self.serializers[key] if not isinstance(value, type): - if value == 'self': + if value == "self": value = self.klass value = import_class_from_dotted_path(value) @@ -188,15 +188,19 @@ def __repr__(self): class SerializerMetaclass(SerializerMetaclass): def __new__(cls, name, bases, attrs): - serializer_class_path = attrs['__module__']+'.'+name + serializer_class_path = attrs["__module__"] + "." + name - included_serializers = attrs.get('included_serializers', None) + included_serializers = attrs.get("included_serializers", None) if included_serializers: - attrs['included_serializers'] = LazySerializersDict(serializer_class_path, included_serializers) + attrs["included_serializers"] = LazySerializersDict( + serializer_class_path, included_serializers + ) - related_serializers = attrs.get('related_serializers', None) + related_serializers = attrs.get("related_serializers", None) if related_serializers: - attrs['related_serializers'] = LazySerializersDict(serializer_class_path, related_serializers) + attrs["related_serializers"] = LazySerializersDict( + serializer_class_path, related_serializers + ) return super().__new__(cls, name, bases, attrs) diff --git a/tests/test_utils.py b/tests/test_utils.py index 889cb4af..628ab953 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -353,6 +353,7 @@ class IncludedSerializersModel(DJAModel): class Meta: app_label = "tests" + class IncludedSerializersSerializer(serializers.ModelSerializer): included_serializers = { "self": "self", @@ -364,6 +365,7 @@ class Meta: model = IncludedSerializersModel fields = ("self", "other_target", "target") + def test_get_included_serializers(): included_serializers = get_included_serializers(IncludedSerializersSerializer) expected_included_serializers = { From 6e5d2579c159bd98e882122d1ae2722f8b84c6dc Mon Sep 17 00:00:00 2001 From: Safa AlFulaij Date: Mon, 19 Apr 2021 02:35:27 +0300 Subject: [PATCH 04/15] Minor --- rest_framework_json_api/utils.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index 85415925..d86d9544 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -1,4 +1,3 @@ -import copy import inspect import operator import warnings @@ -13,7 +12,6 @@ ) from django.http import Http404 from django.utils import encoding -from django.utils.module_loading import import_string as import_class_from_dotted_path from django.utils.translation import gettext_lazy as _ from rest_framework import exceptions from rest_framework.exceptions import APIException From 3fe31d78f074c5ad761cd2c96f5b8a48f64cbd56 Mon Sep 17 00:00:00 2001 From: Safa AlFulaij Date: Mon, 19 Apr 2021 09:11:32 +0300 Subject: [PATCH 05/15] Remove get_included_serializers --- rest_framework_json_api/relations.py | 3 +-- rest_framework_json_api/renderers.py | 6 +++--- rest_framework_json_api/serializers.py | 3 +-- rest_framework_json_api/utils.py | 4 ---- tests/test_utils.py | 5 +++-- 5 files changed, 8 insertions(+), 13 deletions(-) diff --git a/rest_framework_json_api/relations.py b/rest_framework_json_api/relations.py index c9dd765d..567cf9b1 100644 --- a/rest_framework_json_api/relations.py +++ b/rest_framework_json_api/relations.py @@ -16,7 +16,6 @@ from rest_framework_json_api.utils import ( Hyperlink, format_link_segment, - get_included_serializers, get_resource_type_from_instance, get_resource_type_from_queryset, get_resource_type_from_serializer, @@ -274,7 +273,7 @@ def get_resource_type_from_included_serializer(self): inflection.singularize(field_name), inflection.pluralize(field_name), ] - includes = get_included_serializers(parent) + includes = getattr(parent, "included_serializers", {}) for field in field_names: if field in includes.keys(): return get_resource_type_from_serializer(includes[field]) diff --git a/rest_framework_json_api/renderers.py b/rest_framework_json_api/renderers.py index b50c2b1a..4d6cfcc1 100644 --- a/rest_framework_json_api/renderers.py +++ b/rest_framework_json_api/renderers.py @@ -284,7 +284,7 @@ def extract_included( current_serializer = fields.serializer context = current_serializer.context - included_serializers = utils.get_included_serializers(current_serializer) + included_serializers = getattr(current_serializer, "included_serializers", {}) included_resources = copy.copy(included_resources) included_resources = [ inflection.underscore(value) for value in included_resources @@ -692,8 +692,8 @@ def _get_included_serializers(cls, serializer, prefix="", already_seen=None): included_serializers = [] already_seen.add(serializer) - for include, included_serializer in utils.get_included_serializers( - serializer + for include, included_serializer in getattr( + serializer, "included_serializers", {} ).items(): included_serializers.append(f"{prefix}{include}") included_serializers.extend( diff --git a/rest_framework_json_api/serializers.py b/rest_framework_json_api/serializers.py index 4ad1ce14..199b3ff7 100644 --- a/rest_framework_json_api/serializers.py +++ b/rest_framework_json_api/serializers.py @@ -24,7 +24,6 @@ from rest_framework_json_api.relations import ResourceRelatedField from rest_framework_json_api.utils import ( get_included_resources, - get_included_serializers, get_resource_type_from_instance, get_resource_type_from_model, get_resource_type_from_serializer, @@ -122,7 +121,7 @@ def __init__(self, *args, **kwargs): view = context.get("view") if context else None def validate_path(serializer_class, field_path, path): - serializers = get_included_serializers(serializer_class) + serializers = getattr(serializer_class, "included_serializers", None) if serializers is None: raise ParseError("This endpoint does not support the include parameter") this_field_name = inflection.underscore(field_path[0]) diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index d86d9544..3411cb56 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -339,10 +339,6 @@ def get_default_included_resources_from_serializer(serializer): return list(getattr(meta, "included_resources", [])) -def get_included_serializers(serializer): - return getattr(serializer, "included_serializers", dict()) - - def get_relation_instance(resource_instance, source, serializer): try: relation_instance = operator.attrgetter(source)(resource_instance) diff --git a/tests/test_utils.py b/tests/test_utils.py index 628ab953..260b6827 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -12,7 +12,6 @@ format_link_segment, format_resource_type, format_value, - get_included_serializers, get_related_resource_type, get_resource_name, undo_format_field_name, @@ -367,7 +366,9 @@ class Meta: def test_get_included_serializers(): - included_serializers = get_included_serializers(IncludedSerializersSerializer) + included_serializers = getattr( + IncludedSerializersSerializer, "included_serializers", {} + ) expected_included_serializers = { "self": IncludedSerializersSerializer, "target": ManyToManyTargetSerializer, From 2f5b32459688ea76f3f7eacb98f673452bd6aadc Mon Sep 17 00:00:00 2001 From: Safa Alfulaij Date: Tue, 4 May 2021 23:25:31 +0300 Subject: [PATCH 06/15] Adjustements, changes, and changelog --- CHANGELOG.md | 5 ++- example/tests/integration/test_includes.py | 3 +- example/tests/test_views.py | 9 +++-- rest_framework_json_api/relations.py | 5 ++- rest_framework_json_api/renderers.py | 15 ++++++--- rest_framework_json_api/serializers.py | 38 +++++++++++++++------- rest_framework_json_api/utils.py | 11 +++++++ rest_framework_json_api/views.py | 4 +-- tests/test_serializers.py | 35 ++++++++++++++++++++ tests/test_utils.py | 38 +--------------------- 10 files changed, 100 insertions(+), 63 deletions(-) create mode 100644 tests/test_serializers.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 3206cfa8..d23a44bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Note that in line with [Django REST Framework policy](http://www.django-rest-framework.org/topics/release-notes/), any parts of the framework not mentioned in the documentation should generally be considered private API, and may be subject to change. +## [Unreleased] + +* Deprecated `get_included_serializers(serializer)` function under `rest_framework_json_api.utils`. Use `serializer.included_serializers` and `serializer.related_serializers` instead. + ## [4.2.1] - 2021-07-06 ### Fixed @@ -40,7 +44,6 @@ any parts of the framework not mentioned in the documentation should generally b * Deprecated default `format_type` argument of `rest_framework_json_api.utils.format_value`. Use `rest_framework_json_api.utils.format_field_name` or specify specifc `format_type` instead. * Deprecated `format_type` argument of `rest_framework_json_api.utils.format_link_segment`. Use `rest_framework_json_api.utils.format_value` instead. - ## [4.1.0] - 2021-03-08 ### Added diff --git a/example/tests/integration/test_includes.py b/example/tests/integration/test_includes.py index 223645d2..2287d207 100644 --- a/example/tests/integration/test_includes.py +++ b/example/tests/integration/test_includes.py @@ -101,7 +101,8 @@ def test_missing_field_not_included(author_bio_factory, author_factory, client): # First author does not have a bio author = author_factory(bio=None) response = client.get(reverse("author-detail", args=[author.pk]) + "?include=bio") - assert "included" not in response.json() + data = response.json() + assert "included" not in data # Second author does author = author_factory() response = client.get(reverse("author-detail", args=[author.pk]) + "?include=bio") diff --git a/example/tests/test_views.py b/example/tests/test_views.py index d976ed56..eab6eff4 100644 --- a/example/tests/test_views.py +++ b/example/tests/test_views.py @@ -5,7 +5,6 @@ from django.utils import timezone from rest_framework import status from rest_framework.decorators import action -from rest_framework.exceptions import NotFound from rest_framework.request import Request from rest_framework.response import Response from rest_framework.reverse import reverse @@ -435,10 +434,10 @@ def test_get_serializer_comes_from_included_serializers(self): self.assertEqual(got, AuthorTypeSerializer) view.get_serializer_class().related_serializers = related_serializers - def test_get_related_serializer_class_raises_error(self): - kwargs = {"pk": self.author.id, "related_field": "unknown"} - view = self._get_view(kwargs) - self.assertRaises(NotFound, view.get_related_serializer_class) + # def test_get_related_serializer_class_raises_error(self): + # kwargs = {"pk": self.author.id, "related_field": "unknown"} + # view = self._get_view(kwargs) + # self.assertRaises(NotFound, view.get_related_serializer_class) def test_retrieve_related_single_reverse_lookup(self): url = reverse( diff --git a/rest_framework_json_api/relations.py b/rest_framework_json_api/relations.py index 567cf9b1..bfc95b02 100644 --- a/rest_framework_json_api/relations.py +++ b/rest_framework_json_api/relations.py @@ -273,7 +273,10 @@ def get_resource_type_from_included_serializer(self): inflection.singularize(field_name), inflection.pluralize(field_name), ] - includes = getattr(parent, "included_serializers", {}) + includes = getattr(parent, "included_serializers", None) + if includes is None: + return None + for field in field_names: if field in includes.keys(): return get_resource_type_from_serializer(includes[field]) diff --git a/rest_framework_json_api/renderers.py b/rest_framework_json_api/renderers.py index 4d6cfcc1..dc59b4ec 100644 --- a/rest_framework_json_api/renderers.py +++ b/rest_framework_json_api/renderers.py @@ -284,7 +284,10 @@ def extract_included( current_serializer = fields.serializer context = current_serializer.context - included_serializers = getattr(current_serializer, "included_serializers", {}) + included_serializers = getattr(current_serializer, "included_serializers", None) + if included_serializers is None: + return + included_resources = copy.copy(included_resources) included_resources = [ inflection.underscore(value) for value in included_resources @@ -692,9 +695,11 @@ def _get_included_serializers(cls, serializer, prefix="", already_seen=None): included_serializers = [] already_seen.add(serializer) - for include, included_serializer in getattr( - serializer, "included_serializers", {} - ).items(): + included = serializer.included_serializers + if included is None: + return [] + + for include, included_serializer in included.items(): included_serializers.append(f"{prefix}{include}") included_serializers.extend( cls._get_included_serializers( @@ -715,7 +720,7 @@ def get_includes_form(self, view): except AttributeError: return - if not hasattr(serializer_class, "included_serializers"): + if not serializer_class.included_serializers: return template = loader.get_template(self.includes_template) diff --git a/rest_framework_json_api/serializers.py b/rest_framework_json_api/serializers.py index 199b3ff7..5e60cdd3 100644 --- a/rest_framework_json_api/serializers.py +++ b/rest_framework_json_api/serializers.py @@ -154,15 +154,15 @@ def validate_path(serializer_class, field_path, path): class LazySerializersDict(MutableMapping): - def __init__(self, klass, serializers): - self.klass = klass + def __init__(self, serializers): # klass, serializers): + # self.klass = klass self.serializers = serializers def __getitem__(self, key): value = self.serializers[key] if not isinstance(value, type): - if value == "self": - value = self.klass + # if value == "self": + # value = self.klass value = import_class_from_dotted_path(value) self.serializers[key] = value @@ -187,18 +187,34 @@ def __repr__(self): class SerializerMetaclass(SerializerMetaclass): def __new__(cls, name, bases, attrs): - serializer_class_path = attrs["__module__"] + "." + name + # serializer_class_path = attrs["__module__"] + "." + name - included_serializers = attrs.get("included_serializers", None) - if included_serializers: + included_serializers = attrs.pop("included_serializers", None) + if included_serializers is None: + for base in bases: + included = getattr(base, "included_serializers", None) + if included is not None: + included_serializers = included + + attrs["included_serializers"] = None + if included_serializers is not None: attrs["included_serializers"] = LazySerializersDict( - serializer_class_path, included_serializers + # serializer_class_path, + included_serializers ) - related_serializers = attrs.get("related_serializers", None) - if related_serializers: + related_serializers = attrs.pop("related_serializers", None) + if related_serializers is None: + for base in bases: + related = getattr(base, "related_serializers", None) + if related is not None: + related_serializers = related + + attrs["related_serializers"] = None + if related_serializers is not None: attrs["related_serializers"] = LazySerializersDict( - serializer_class_path, related_serializers + # serializer_class_path, + related_serializers ) return super().__new__(cls, name, bases, attrs) diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index 3411cb56..58afd85e 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -339,6 +339,17 @@ def get_default_included_resources_from_serializer(serializer): return list(getattr(meta, "included_resources", [])) +def get_included_serializers(serializer): + warnings.warn( + DeprecationWarning( + "Using `get_included_serializers(serializer)` function is deprecated." + "Use `serializer.included_serializers` instead." + ) + ) + + return serializer.included_serializers + + def get_relation_instance(resource_instance, source, serializer): try: relation_instance = operator.attrgetter(source)(resource_instance) diff --git a/rest_framework_json_api/views.py b/rest_framework_json_api/views.py index bb6f09bb..ea3970d4 100644 --- a/rest_framework_json_api/views.py +++ b/rest_framework_json_api/views.py @@ -164,14 +164,14 @@ def get_related_serializer_class(self): field_name = self.get_related_field_name() # Try get the class from related_serializers - if hasattr(parent_serializer_class, "related_serializers"): + if parent_serializer_class.related_serializers is not None: _class = parent_serializer_class.related_serializers.get( field_name, None ) if _class is None: raise NotFound - elif hasattr(parent_serializer_class, "included_serializers"): + elif parent_serializer_class.included_serializers is not None: _class = parent_serializer_class.included_serializers.get( field_name, None ) diff --git a/tests/test_serializers.py b/tests/test_serializers.py new file mode 100644 index 00000000..933b270f --- /dev/null +++ b/tests/test_serializers.py @@ -0,0 +1,35 @@ +from django.db import models + +from rest_framework_json_api import serializers +from tests.models import DJAModel, ManyToManyTarget +from tests.serializers import ManyToManyTargetSerializer + + +def test_get_included_serializers(): + class IncludedSerializersModel(DJAModel): + self = models.ForeignKey("self", on_delete=models.CASCADE) + target = models.ForeignKey(ManyToManyTarget, on_delete=models.CASCADE) + other_target = models.ForeignKey(ManyToManyTarget, on_delete=models.CASCADE) + + class Meta: + app_label = "tests" + + class IncludedSerializersSerializer(serializers.ModelSerializer): + included_serializers = { + # "self": "self", + "target": ManyToManyTargetSerializer, + "other_target": "tests.serializers.ManyToManyTargetSerializer", + } + + class Meta: + model = IncludedSerializersModel + fields = ("self", "other_target", "target") + + included_serializers = IncludedSerializersSerializer.included_serializers + expected_included_serializers = { + # "self": IncludedSerializersSerializer, + "target": ManyToManyTargetSerializer, + "other_target": ManyToManyTargetSerializer, + } + + assert included_serializers == expected_included_serializers diff --git a/tests/test_utils.py b/tests/test_utils.py index 260b6827..e471cb49 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,5 +1,4 @@ import pytest -from django.db import models from rest_framework import status from rest_framework.generics import GenericAPIView from rest_framework.response import Response @@ -20,13 +19,12 @@ ) from tests.models import ( BasicModel, - DJAModel, ForeignKeySource, ForeignKeyTarget, ManyToManySource, ManyToManyTarget, ) -from tests.serializers import BasicModelSerializer, ManyToManyTargetSerializer +from tests.serializers import BasicModelSerializer def test_get_resource_name_no_view(): @@ -342,37 +340,3 @@ class PlainRelatedResourceTypeSerializer(serializers.Serializer): serializer = PlainRelatedResourceTypeSerializer() field = serializer.fields["basic_models"] assert get_related_resource_type(field) == output - - -class IncludedSerializersModel(DJAModel): - self = models.ForeignKey("self", on_delete=models.CASCADE) - target = models.ForeignKey(ManyToManyTarget, on_delete=models.CASCADE) - other_target = models.ForeignKey(ManyToManyTarget, on_delete=models.CASCADE) - - class Meta: - app_label = "tests" - - -class IncludedSerializersSerializer(serializers.ModelSerializer): - included_serializers = { - "self": "self", - "target": ManyToManyTargetSerializer, - "other_target": "tests.serializers.ManyToManyTargetSerializer", - } - - class Meta: - model = IncludedSerializersModel - fields = ("self", "other_target", "target") - - -def test_get_included_serializers(): - included_serializers = getattr( - IncludedSerializersSerializer, "included_serializers", {} - ) - expected_included_serializers = { - "self": IncludedSerializersSerializer, - "target": ManyToManyTargetSerializer, - "other_target": ManyToManyTargetSerializer, - } - - assert included_serializers == expected_included_serializers From 09e6a616e2c881a39c5f2cc465c1ddc746e4fb1d Mon Sep 17 00:00:00 2001 From: Safa Alfulaij Date: Tue, 4 May 2021 23:33:54 +0300 Subject: [PATCH 07/15] Restore test --- example/tests/test_views.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/example/tests/test_views.py b/example/tests/test_views.py index eab6eff4..d976ed56 100644 --- a/example/tests/test_views.py +++ b/example/tests/test_views.py @@ -5,6 +5,7 @@ from django.utils import timezone from rest_framework import status from rest_framework.decorators import action +from rest_framework.exceptions import NotFound from rest_framework.request import Request from rest_framework.response import Response from rest_framework.reverse import reverse @@ -434,10 +435,10 @@ def test_get_serializer_comes_from_included_serializers(self): self.assertEqual(got, AuthorTypeSerializer) view.get_serializer_class().related_serializers = related_serializers - # def test_get_related_serializer_class_raises_error(self): - # kwargs = {"pk": self.author.id, "related_field": "unknown"} - # view = self._get_view(kwargs) - # self.assertRaises(NotFound, view.get_related_serializer_class) + def test_get_related_serializer_class_raises_error(self): + kwargs = {"pk": self.author.id, "related_field": "unknown"} + view = self._get_view(kwargs) + self.assertRaises(NotFound, view.get_related_serializer_class) def test_retrieve_related_single_reverse_lookup(self): url = reverse( From 72828cc47e37ffab2f799faa202df0a617535cd1 Mon Sep 17 00:00:00 2001 From: Safa Alfulaij Date: Tue, 4 May 2021 23:46:06 +0300 Subject: [PATCH 08/15] Add deprecated get_included_serializers test --- tests/test_utils.py | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index e471cb49..08e59235 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,4 +1,5 @@ import pytest +from django.db import models from rest_framework import status from rest_framework.generics import GenericAPIView from rest_framework.response import Response @@ -11,6 +12,7 @@ format_link_segment, format_resource_type, format_value, + get_included_serializers, get_related_resource_type, get_resource_name, undo_format_field_name, @@ -19,12 +21,13 @@ ) from tests.models import ( BasicModel, + DJAModel, ForeignKeySource, ForeignKeyTarget, ManyToManySource, ManyToManyTarget, ) -from tests.serializers import BasicModelSerializer +from tests.serializers import BasicModelSerializer, ManyToManyTargetSerializer def test_get_resource_name_no_view(): @@ -340,3 +343,35 @@ class PlainRelatedResourceTypeSerializer(serializers.Serializer): serializer = PlainRelatedResourceTypeSerializer() field = serializer.fields["basic_models"] assert get_related_resource_type(field) == output + + +def test_get_included_serializers(): + class DeprecatedIncludedSerializersModel(DJAModel): + self = models.ForeignKey("self", on_delete=models.CASCADE) + target = models.ForeignKey(ManyToManyTarget, on_delete=models.CASCADE) + other_target = models.ForeignKey(ManyToManyTarget, on_delete=models.CASCADE) + + class Meta: + app_label = "tests" + + class DeprecatedIncludedSerializersSerializer(serializers.ModelSerializer): + included_serializers = { + "target": ManyToManyTargetSerializer, + "other_target": "tests.serializers.ManyToManyTargetSerializer", + } + + class Meta: + model = DeprecatedIncludedSerializersModel + fields = ("self", "other_target", "target") + + with pytest.deprecated_call(): + included_serializers = get_included_serializers( + DeprecatedIncludedSerializersSerializer + ) + + expected_included_serializers = { + "target": ManyToManyTargetSerializer, + "other_target": ManyToManyTargetSerializer, + } + + assert included_serializers == expected_included_serializers From 0c5c06d8a6f45dc9ba2100ec6307f8d41def9cdb Mon Sep 17 00:00:00 2001 From: Safa Alfulaij Date: Wed, 5 May 2021 16:54:50 +0300 Subject: [PATCH 09/15] Simplify caching included serializers --- example/tests/integration/test_includes.py | 3 +-- rest_framework_json_api/relations.py | 5 +---- rest_framework_json_api/renderers.py | 17 +++++++-------- rest_framework_json_api/serializers.py | 24 ++++------------------ rest_framework_json_api/views.py | 4 ++-- 5 files changed, 15 insertions(+), 38 deletions(-) diff --git a/example/tests/integration/test_includes.py b/example/tests/integration/test_includes.py index 2287d207..223645d2 100644 --- a/example/tests/integration/test_includes.py +++ b/example/tests/integration/test_includes.py @@ -101,8 +101,7 @@ def test_missing_field_not_included(author_bio_factory, author_factory, client): # First author does not have a bio author = author_factory(bio=None) response = client.get(reverse("author-detail", args=[author.pk]) + "?include=bio") - data = response.json() - assert "included" not in data + assert "included" not in response.json() # Second author does author = author_factory() response = client.get(reverse("author-detail", args=[author.pk]) + "?include=bio") diff --git a/rest_framework_json_api/relations.py b/rest_framework_json_api/relations.py index bfc95b02..605f7c1c 100644 --- a/rest_framework_json_api/relations.py +++ b/rest_framework_json_api/relations.py @@ -273,10 +273,7 @@ def get_resource_type_from_included_serializer(self): inflection.singularize(field_name), inflection.pluralize(field_name), ] - includes = getattr(parent, "included_serializers", None) - if includes is None: - return None - + includes = getattr(parent, "included_serializers", dict()) for field in field_names: if field in includes.keys(): return get_resource_type_from_serializer(includes[field]) diff --git a/rest_framework_json_api/renderers.py b/rest_framework_json_api/renderers.py index dc59b4ec..3c649331 100644 --- a/rest_framework_json_api/renderers.py +++ b/rest_framework_json_api/renderers.py @@ -284,10 +284,9 @@ def extract_included( current_serializer = fields.serializer context = current_serializer.context - included_serializers = getattr(current_serializer, "included_serializers", None) - if included_serializers is None: - return - + included_serializers = getattr( + current_serializer, "included_serializers", dict() + ) included_resources = copy.copy(included_resources) included_resources = [ inflection.underscore(value) for value in included_resources @@ -695,11 +694,9 @@ def _get_included_serializers(cls, serializer, prefix="", already_seen=None): included_serializers = [] already_seen.add(serializer) - included = serializer.included_serializers - if included is None: - return [] - - for include, included_serializer in included.items(): + for include, included_serializer in getattr( + serializer, "included_serializers", dict() + ).items(): included_serializers.append(f"{prefix}{include}") included_serializers.extend( cls._get_included_serializers( @@ -720,7 +717,7 @@ def get_includes_form(self, view): except AttributeError: return - if not serializer_class.included_serializers: + if not hasattr(serializer_class, "included_serializers"): return template = loader.get_template(self.includes_template) diff --git a/rest_framework_json_api/serializers.py b/rest_framework_json_api/serializers.py index 5e60cdd3..d42bdb25 100644 --- a/rest_framework_json_api/serializers.py +++ b/rest_framework_json_api/serializers.py @@ -189,32 +189,16 @@ class SerializerMetaclass(SerializerMetaclass): def __new__(cls, name, bases, attrs): # serializer_class_path = attrs["__module__"] + "." + name - included_serializers = attrs.pop("included_serializers", None) - if included_serializers is None: - for base in bases: - included = getattr(base, "included_serializers", None) - if included is not None: - included_serializers = included - - attrs["included_serializers"] = None - if included_serializers is not None: + if attrs.get("included_serializers", None): attrs["included_serializers"] = LazySerializersDict( # serializer_class_path, - included_serializers + attrs["included_serializers"] ) - related_serializers = attrs.pop("related_serializers", None) - if related_serializers is None: - for base in bases: - related = getattr(base, "related_serializers", None) - if related is not None: - related_serializers = related - - attrs["related_serializers"] = None - if related_serializers is not None: + if attrs.get("related_serializers", None): attrs["related_serializers"] = LazySerializersDict( # serializer_class_path, - related_serializers + attrs["related_serializers"] ) return super().__new__(cls, name, bases, attrs) diff --git a/rest_framework_json_api/views.py b/rest_framework_json_api/views.py index ea3970d4..bb6f09bb 100644 --- a/rest_framework_json_api/views.py +++ b/rest_framework_json_api/views.py @@ -164,14 +164,14 @@ def get_related_serializer_class(self): field_name = self.get_related_field_name() # Try get the class from related_serializers - if parent_serializer_class.related_serializers is not None: + if hasattr(parent_serializer_class, "related_serializers"): _class = parent_serializer_class.related_serializers.get( field_name, None ) if _class is None: raise NotFound - elif parent_serializer_class.included_serializers is not None: + elif hasattr(parent_serializer_class, "included_serializers"): _class = parent_serializer_class.included_serializers.get( field_name, None ) From cdf4292a26dadfd79ec06153b79222f7949812a6 Mon Sep 17 00:00:00 2001 From: Safa ALfulaij Date: Thu, 6 May 2021 21:23:53 +0300 Subject: [PATCH 10/15] Minor --- rest_framework_json_api/utils.py | 2 +- tests/test_serializers.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index 58afd85e..03f77ed0 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -347,7 +347,7 @@ def get_included_serializers(serializer): ) ) - return serializer.included_serializers + return getattr(serializer, "included_serializers", dict()) def get_relation_instance(resource_instance, source, serializer): diff --git a/tests/test_serializers.py b/tests/test_serializers.py index 933b270f..c8f1194a 100644 --- a/tests/test_serializers.py +++ b/tests/test_serializers.py @@ -7,7 +7,7 @@ def test_get_included_serializers(): class IncludedSerializersModel(DJAModel): - self = models.ForeignKey("self", on_delete=models.CASCADE) + # self = models.ForeignKey("self", on_delete=models.CASCADE) target = models.ForeignKey(ManyToManyTarget, on_delete=models.CASCADE) other_target = models.ForeignKey(ManyToManyTarget, on_delete=models.CASCADE) From 3fc643c9c9c56fa69bec289282eb1d83ffeaa0eb Mon Sep 17 00:00:00 2001 From: Safa AlFulaij Date: Tue, 25 May 2021 22:19:36 +0300 Subject: [PATCH 11/15] Restore self in included serializers --- rest_framework_json_api/serializers.py | 40 ++++++++++++-------------- tests/test_serializers.py | 6 ++-- tests/test_utils.py | 2 ++ 3 files changed, 23 insertions(+), 25 deletions(-) diff --git a/rest_framework_json_api/serializers.py b/rest_framework_json_api/serializers.py index d42bdb25..2e72174d 100644 --- a/rest_framework_json_api/serializers.py +++ b/rest_framework_json_api/serializers.py @@ -1,5 +1,5 @@ from collections import OrderedDict -from collections.abc import MutableMapping +from collections.abc import Mapping import inflection from django.core.exceptions import ObjectDoesNotExist @@ -153,28 +153,22 @@ def validate_path(serializer_class, field_path, path): super(IncludedResourcesValidationMixin, self).__init__(*args, **kwargs) -class LazySerializersDict(MutableMapping): - def __init__(self, serializers): # klass, serializers): - # self.klass = klass +class LazySerializersDict(Mapping): + def __init__(self, klass, serializers): + self.klass = klass self.serializers = serializers def __getitem__(self, key): value = self.serializers[key] if not isinstance(value, type): - # if value == "self": - # value = self.klass - - value = import_class_from_dotted_path(value) + if value == "self": + value = self.klass + else: + value = import_class_from_dotted_path(value) self.serializers[key] = value return value - def __setitem__(self, key, field): - self.serializers[key] = field - - def __delitem__(self, key): - del self.serializers[key] - def __iter__(self): return iter(self.serializers) @@ -187,21 +181,23 @@ def __repr__(self): class SerializerMetaclass(SerializerMetaclass): def __new__(cls, name, bases, attrs): - # serializer_class_path = attrs["__module__"] + "." + name + klass = super().__new__(cls, name, bases, attrs) if attrs.get("included_serializers", None): - attrs["included_serializers"] = LazySerializersDict( - # serializer_class_path, - attrs["included_serializers"] + setattr( + klass, + "included_serializers", + LazySerializersDict(klass, attrs["included_serializers"]), ) if attrs.get("related_serializers", None): - attrs["related_serializers"] = LazySerializersDict( - # serializer_class_path, - attrs["related_serializers"] + setattr( + klass, + "related_serializers", + LazySerializersDict(klass, attrs["related_serializers"]), ) - return super().__new__(cls, name, bases, attrs) + return klass # If user imports serializer from here we can catch class definition and check diff --git a/tests/test_serializers.py b/tests/test_serializers.py index c8f1194a..6726fa8c 100644 --- a/tests/test_serializers.py +++ b/tests/test_serializers.py @@ -7,7 +7,7 @@ def test_get_included_serializers(): class IncludedSerializersModel(DJAModel): - # self = models.ForeignKey("self", on_delete=models.CASCADE) + self = models.ForeignKey("self", on_delete=models.CASCADE) target = models.ForeignKey(ManyToManyTarget, on_delete=models.CASCADE) other_target = models.ForeignKey(ManyToManyTarget, on_delete=models.CASCADE) @@ -16,7 +16,7 @@ class Meta: class IncludedSerializersSerializer(serializers.ModelSerializer): included_serializers = { - # "self": "self", + "self": "self", "target": ManyToManyTargetSerializer, "other_target": "tests.serializers.ManyToManyTargetSerializer", } @@ -27,7 +27,7 @@ class Meta: included_serializers = IncludedSerializersSerializer.included_serializers expected_included_serializers = { - # "self": IncludedSerializersSerializer, + "self": IncludedSerializersSerializer, "target": ManyToManyTargetSerializer, "other_target": ManyToManyTargetSerializer, } diff --git a/tests/test_utils.py b/tests/test_utils.py index 08e59235..43c12dcf 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -356,6 +356,7 @@ class Meta: class DeprecatedIncludedSerializersSerializer(serializers.ModelSerializer): included_serializers = { + "self": "self", "target": ManyToManyTargetSerializer, "other_target": "tests.serializers.ManyToManyTargetSerializer", } @@ -370,6 +371,7 @@ class Meta: ) expected_included_serializers = { + "self": DeprecatedIncludedSerializersSerializer, "target": ManyToManyTargetSerializer, "other_target": ManyToManyTargetSerializer, } From 4f6da21a2f8df072d6f01dc891e97413ce7e1c63 Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Wed, 14 Jul 2021 16:49:38 +0400 Subject: [PATCH 12/15] Rename klass attribute to more specific --- rest_framework_json_api/serializers.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/rest_framework_json_api/serializers.py b/rest_framework_json_api/serializers.py index 2e72174d..d5885587 100644 --- a/rest_framework_json_api/serializers.py +++ b/rest_framework_json_api/serializers.py @@ -154,15 +154,15 @@ def validate_path(serializer_class, field_path, path): class LazySerializersDict(Mapping): - def __init__(self, klass, serializers): - self.klass = klass + def __init__(self, parent, serializers): + self.parent = parent self.serializers = serializers def __getitem__(self, key): value = self.serializers[key] if not isinstance(value, type): if value == "self": - value = self.klass + value = self.parent else: value = import_class_from_dotted_path(value) self.serializers[key] = value @@ -181,23 +181,23 @@ def __repr__(self): class SerializerMetaclass(SerializerMetaclass): def __new__(cls, name, bases, attrs): - klass = super().__new__(cls, name, bases, attrs) + serializer = super().__new__(cls, name, bases, attrs) if attrs.get("included_serializers", None): setattr( - klass, + serializer, "included_serializers", - LazySerializersDict(klass, attrs["included_serializers"]), + LazySerializersDict(serializer, attrs["included_serializers"]), ) if attrs.get("related_serializers", None): setattr( - klass, + serializer, "related_serializers", - LazySerializersDict(klass, attrs["related_serializers"]), + LazySerializersDict(serializer, attrs["related_serializers"]), ) - return klass + return serializer # If user imports serializer from here we can catch class definition and check From 3cf923bcfa2165b41297fbb9141c405b1f5384e4 Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Wed, 14 Jul 2021 17:04:17 +0400 Subject: [PATCH 13/15] Clarification --- rest_framework_json_api/serializers.py | 4 ++++ rest_framework_json_api/utils.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/rest_framework_json_api/serializers.py b/rest_framework_json_api/serializers.py index d5885587..bc60f193 100644 --- a/rest_framework_json_api/serializers.py +++ b/rest_framework_json_api/serializers.py @@ -154,6 +154,10 @@ def validate_path(serializer_class, field_path, path): class LazySerializersDict(Mapping): + """ + A dictionary of serializers which lazily import dotted class path and self. + """ + def __init__(self, parent, serializers): self.parent = parent self.serializers = serializers diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index 03f77ed0..19c72809 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -342,7 +342,7 @@ def get_default_included_resources_from_serializer(serializer): def get_included_serializers(serializer): warnings.warn( DeprecationWarning( - "Using `get_included_serializers(serializer)` function is deprecated." + "Using of `get_included_serializers(serializer)` function is deprecated." "Use `serializer.included_serializers` instead." ) ) From 6850419fb1b74db8663192b46130920c2c646aaa Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Sun, 18 Jul 2021 22:42:13 +0400 Subject: [PATCH 14/15] Added CHANGELOG --- CHANGELOG.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d23a44bb..d9eaf504 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,13 @@ any parts of the framework not mentioned in the documentation should generally b ## [Unreleased] -* Deprecated `get_included_serializers(serializer)` function under `rest_framework_json_api.utils`. Use `serializer.included_serializers` and `serializer.related_serializers` instead. +### Changed + +* Moved resolving of `included_serialzers` and `related_serializers` classes to serializer's meta class. + +### Deprecated + +* Deprecated `get_included_serializers(serializer)` function under `rest_framework_json_api.utils`. Use `serializer.included_serializers` instead. ## [4.2.1] - 2021-07-06 From c863a3d7ab7c2a8397f87a56ce79c3d6f1546522 Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Sun, 18 Jul 2021 22:56:17 +0400 Subject: [PATCH 15/15] Removed obsolete `import_string` call for related serializers --- rest_framework_json_api/schemas/openapi.py | 9 +-------- rest_framework_json_api/views.py | 3 --- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/rest_framework_json_api/schemas/openapi.py b/rest_framework_json_api/schemas/openapi.py index 77a0d4ed..efe14914 100644 --- a/rest_framework_json_api/schemas/openapi.py +++ b/rest_framework_json_api/schemas/openapi.py @@ -1,7 +1,6 @@ import warnings from urllib.parse import urljoin -from django.utils.module_loading import import_string as import_class_from_dotted_path from rest_framework.fields import empty from rest_framework.relations import ManyRelatedField from rest_framework.schemas import openapi as drf_openapi @@ -379,13 +378,7 @@ def _find_related_view(self, view_endpoints, related_serializer, parent_view): """ for path, method, view in view_endpoints: view_serializer = view.get_serializer() - if not isinstance(related_serializer, type): - related_serializer_class = import_class_from_dotted_path( - related_serializer - ) - else: - related_serializer_class = related_serializer - if isinstance(view_serializer, related_serializer_class): + if isinstance(view_serializer, related_serializer): return view return None diff --git a/rest_framework_json_api/views.py b/rest_framework_json_api/views.py index bb6f09bb..6b739582 100644 --- a/rest_framework_json_api/views.py +++ b/rest_framework_json_api/views.py @@ -11,7 +11,6 @@ from django.db.models.manager import Manager from django.db.models.query import QuerySet from django.urls import NoReverseMatch -from django.utils.module_loading import import_string as import_class_from_dotted_path from rest_framework import generics, viewsets from rest_framework.exceptions import MethodNotAllowed, NotFound from rest_framework.fields import get_attribute @@ -183,8 +182,6 @@ def get_related_serializer_class(self): False ), 'Either "included_serializers" or "related_serializers" should be configured' - if not isinstance(_class, type): - return import_class_from_dotted_path(_class) return _class return parent_serializer_class