From 51e0b329ee2697fafa11580eece6c5f22dc32d73 Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Sun, 23 Jun 2024 20:50:05 +0200 Subject: [PATCH] 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()