From 53469f355dcf5c1dd1bf064ed64abc21325f11b0 Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Mon, 24 Jun 2024 21:13:34 +0200 Subject: [PATCH 1/3] Re-enabled overwriting of URL field (#1237) Enabled overwriting of URL field URL_FIELD_NAME is usually used as self-link in links. However it should be allowed to be overwritten as long as not HyperlinkedIdentifyField has been used. --- CHANGELOG.md | 6 ++++ rest_framework_json_api/renderers.py | 11 ++++---- rest_framework_json_api/serializers.py | 10 +++++-- tests/conftest.py | 6 ++++ tests/models.py | 8 ++++++ tests/serializers.py | 10 +++++++ tests/test_views.py | 38 ++++++++++++++++++++++++++ tests/views.py | 8 ++++++ 8 files changed, 89 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12ca5017..07ea2cd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Note that in line with [Django REST framework policy](https://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] + +### Fixed + +* Re-enabled overwriting of url field (regression since 7.0.0) + ## [7.0.1] - 2024-06-06 ### Added diff --git a/rest_framework_json_api/renderers.py b/rest_framework_json_api/renderers.py index 5980b95d..03a95b30 100644 --- a/rest_framework_json_api/renderers.py +++ b/rest_framework_json_api/renderers.py @@ -75,13 +75,11 @@ def extract_attributes(cls, fields, resource): and relationships are not returned. """ - invalid_fields = {"id", api_settings.URL_FIELD_NAME} - return { format_field_name(field_name): value for field_name, value in resource.items() if field_name in fields - and field_name not in invalid_fields + and field_name != "id" and not is_relationship_field(fields[field_name]) } @@ -449,7 +447,10 @@ def _filter_sparse_fields(cls, serializer, fields, resource_name): if field.field_name in sparse_fields # URL field is not considered a field in JSON:API spec # but a link so need to keep it - or field.field_name == api_settings.URL_FIELD_NAME + or ( + field.field_name == api_settings.URL_FIELD_NAME + and isinstance(field, relations.HyperlinkedIdentityField) + ) } return fields @@ -486,7 +487,7 @@ def build_json_resource_obj( resource_data["relationships"] = relationships # Add 'self' link if field is present and valid if api_settings.URL_FIELD_NAME in resource and isinstance( - fields[api_settings.URL_FIELD_NAME], relations.RelatedField + fields[api_settings.URL_FIELD_NAME], relations.HyperlinkedIdentityField ): resource_data["links"] = {"self": resource[api_settings.URL_FIELD_NAME]} diff --git a/rest_framework_json_api/serializers.py b/rest_framework_json_api/serializers.py index 3ba9de86..370ae37b 100644 --- a/rest_framework_json_api/serializers.py +++ b/rest_framework_json_api/serializers.py @@ -6,6 +6,7 @@ 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 +from rest_framework.relations import HyperlinkedIdentityField # star import defined so `rest_framework_json_api.serializers` can be # a simple drop in for `rest_framework.serializers` @@ -94,9 +95,12 @@ def _readable_fields(self): field for field in readable_fields if field.field_name in sparse_fields - # URL field is not considered a field in JSON:API spec - # but a link so need to keep it - or field.field_name == api_settings.URL_FIELD_NAME + # URL_FIELD_NAME is the field used as self-link to resource + # however only when it is a HyperlinkedIdentityField + or ( + field.field_name == api_settings.URL_FIELD_NAME + and isinstance(field, HyperlinkedIdentityField) + ) # ID is a required field which might have been overwritten # so need to keep it or field.field_name == "id" diff --git a/tests/conftest.py b/tests/conftest.py index 682d8342..865244e0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,6 +8,7 @@ ManyToManySource, ManyToManyTarget, NestedRelatedSource, + URLModel, ) @@ -36,6 +37,11 @@ def model(db): return BasicModel.objects.create(text="Model") +@pytest.fixture +def url_instance(db): + return URLModel.objects.create(text="Url", url="https://example.com") + + @pytest.fixture def foreign_key_target(db): return ForeignKeyTarget.objects.create(name="Target") diff --git a/tests/models.py b/tests/models.py index 812ee5bf..63cb6434 100644 --- a/tests/models.py +++ b/tests/models.py @@ -18,6 +18,14 @@ class Meta: ordering = ("id",) +class URLModel(DJAModel): + url = models.URLField() + text = models.CharField(max_length=100) + + class Meta: + ordering = ("id",) + + # Models for relations tests # ManyToMany class ManyToManyTarget(DJAModel): diff --git a/tests/serializers.py b/tests/serializers.py index c312b83a..ef8a51cf 100644 --- a/tests/serializers.py +++ b/tests/serializers.py @@ -8,6 +8,7 @@ ManyToManySource, ManyToManyTarget, NestedRelatedSource, + URLModel, ) @@ -17,6 +18,15 @@ class Meta: model = BasicModel +class URLModelSerializer(serializers.ModelSerializer): + class Meta: + fields = ( + "text", + "url", + ) + model = URLModel + + class ForeignKeyTargetSerializer(serializers.ModelSerializer): class Meta: fields = ("name",) diff --git a/tests/test_views.py b/tests/test_views.py index 45f8aaca..6dfde90b 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -21,6 +21,7 @@ ForeignKeyTargetViewSet, ManyToManySourceViewSet, NestedRelatedSourceViewSet, + URLModelViewSet, ) @@ -182,6 +183,42 @@ def test_list_with_default_included_resources(self, client, foreign_key_source): } ] == result["included"] + @pytest.mark.urls(__name__) + def test_list_allow_overwriting_url_field(self, client, url_instance): + """ + Test overwriting of url is possible. + + URL_FIELD_NAME which is set to 'url' per default is used as self in links. + However if field is overwritten and not a HyperlinkedIdentityField it should be allowed + to use as a attribute as well. + """ + + url = reverse("urlmodel-list") + response = client.get(url) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + assert data == [ + { + "type": "URLModel", + "id": str(url_instance.pk), + "attributes": {"text": "Url", "url": "https://example.com"}, + } + ] + + @pytest.mark.urls(__name__) + def test_list_allow_overwiritng_url_with_sparse_fields(self, client, url_instance): + url = reverse("urlmodel-list") + response = client.get(url, data={"fields[URLModel]": "text"}) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + assert data == [ + { + "type": "URLModel", + "id": str(url_instance.pk), + "attributes": {"text": "Url"}, + } + ] + @pytest.mark.urls(__name__) def test_retrieve(self, client, model): url = reverse("basic-model-detail", kwargs={"pk": model.pk}) @@ -495,6 +532,7 @@ def patch(self, request, *args, **kwargs): # configuration in general router = SimpleRouter() router.register(r"basic_models", BasicModelViewSet, basename="basic-model") +router.register(r"url_models", URLModelViewSet) router.register(r"foreign_key_sources", ForeignKeySourceViewSet) router.register(r"foreign_key_targets", ForeignKeyTargetViewSet) router.register( diff --git a/tests/views.py b/tests/views.py index dba769a6..7958c6b9 100644 --- a/tests/views.py +++ b/tests/views.py @@ -5,6 +5,7 @@ ForeignKeyTarget, ManyToManySource, NestedRelatedSource, + URLModel, ) from tests.serializers import ( BasicModelSerializer, @@ -13,6 +14,7 @@ ForeignKeyTargetSerializer, ManyToManySourceSerializer, NestedRelatedSourceSerializer, + URLModelSerializer, ) @@ -22,6 +24,12 @@ class BasicModelViewSet(ModelViewSet): ordering = ["text"] +class URLModelViewSet(ModelViewSet): + serializer_class = URLModelSerializer + queryset = URLModel.objects.all() + ordering = ["url"] + + class ForeignKeySourceViewSet(ModelViewSet): serializer_class = ForeignKeySourceSerializer queryset = ForeignKeySource.objects.all() From d1163cedd81a62cb55f6acab3e85bb7f9e7d1c0f Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Fri, 28 Jun 2024 13:33:41 +0200 Subject: [PATCH 2/3] Ensured that no fields are returned when sparse fields is set to an empty value (#1240) --- CHANGELOG.md | 1 + rest_framework_json_api/renderers.py | 2 +- rest_framework_json_api/serializers.py | 2 +- tests/test_views.py | 13 +++++++++++++ 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07ea2cd8..d60e4861 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ any parts of the framework not mentioned in the documentation should generally b ### Fixed * Re-enabled overwriting of url field (regression since 7.0.0) +* Ensured that no fields are rendered when sparse fields is set to an empty value. (regression since 7.0.0) ## [7.0.1] - 2024-06-06 diff --git a/rest_framework_json_api/renderers.py b/rest_framework_json_api/renderers.py index 03a95b30..8c19934f 100644 --- a/rest_framework_json_api/renderers.py +++ b/rest_framework_json_api/renderers.py @@ -439,7 +439,7 @@ def _filter_sparse_fields(cls, serializer, fields, resource_name): sparse_fieldset_value = request.query_params.get( sparse_fieldset_query_param ) - if sparse_fieldset_value: + if sparse_fieldset_value is not None: sparse_fields = sparse_fieldset_value.split(",") return { field_name: field diff --git a/rest_framework_json_api/serializers.py b/rest_framework_json_api/serializers.py index 370ae37b..d59dbd88 100644 --- a/rest_framework_json_api/serializers.py +++ b/rest_framework_json_api/serializers.py @@ -89,7 +89,7 @@ def _readable_fields(self): sparse_fieldset_value = request.query_params.get( sparse_fieldset_query_param ) - if sparse_fieldset_value: + if sparse_fieldset_value is not None: sparse_fields = sparse_fieldset_value.split(",") return ( field diff --git a/tests/test_views.py b/tests/test_views.py index 6dfde90b..349d37da 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -219,6 +219,19 @@ def test_list_allow_overwiritng_url_with_sparse_fields(self, client, url_instanc } ] + @pytest.mark.urls(__name__) + def test_list_with_sparse_fields_empty_value(self, client, model): + url = reverse("basic-model-list") + response = client.get(url, data={"fields[BasicModel]": ""}) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + assert data == [ + { + "type": "BasicModel", + "id": str(model.pk), + } + ] + @pytest.mark.urls(__name__) def test_retrieve(self, client, model): url = reverse("basic-model-detail", kwargs={"pk": model.pk}) From 971a8796b79fc4fb9daf26c83d406c19219b428d Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Fri, 28 Jun 2024 17:08:57 +0200 Subject: [PATCH 3/3] Release 7.0.2 (#1241) --- CHANGELOG.md | 4 ++-- rest_framework_json_api/__init__.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d60e4861..8c3c7720 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,11 +8,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Note that in line with [Django REST framework policy](https://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] +## [7.0.2] - 2024-06-28 ### Fixed -* Re-enabled overwriting of url field (regression since 7.0.0) +* Allow overwriting of url field again (regression since 7.0.0) * Ensured that no fields are rendered when sparse fields is set to an empty value. (regression since 7.0.0) ## [7.0.1] - 2024-06-06 diff --git a/rest_framework_json_api/__init__.py b/rest_framework_json_api/__init__.py index 9a8c48a3..3008837c 100644 --- a/rest_framework_json_api/__init__.py +++ b/rest_framework_json_api/__init__.py @@ -1,5 +1,5 @@ __title__ = "djangorestframework-jsonapi" -__version__ = "7.0.1" +__version__ = "7.0.2" __author__ = "" __license__ = "BSD" __copyright__ = ""