Skip to content

Commit

Permalink
Ensured that URL and id field are kept when using sparse fields (#1231)
Browse files Browse the repository at this point in the history
Ensured that URL and id field are not filtered out when using sparse fields

URL field is considered a field in DRF but is not in JSON:API spec
therefore we may not exclude it.

ID on the other hand is a required field and may not be filtered.
  • Loading branch information
sliverc authored Jun 4, 2024
1 parent 21493c1 commit dbe939a
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 9 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ any parts of the framework not mentioned in the documentation should generally b

* Added `429 Too Many Requests` as a possible error response in the OpenAPI schema.

### Fixed

* Ensured that URL and id field are kept when using sparse fields (regression since 7.0.0)

## [7.0.0] - 2024-05-02

### Added
Expand Down
5 changes: 4 additions & 1 deletion rest_framework_json_api/renderers.py
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,10 @@ def _filter_sparse_fields(cls, serializer, fields, resource_name):
return {
field_name: field
for field_name, field, in fields.items()
if field_name in sparse_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
}

return fields
Expand Down
7 changes: 6 additions & 1 deletion rest_framework_json_api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,15 @@ 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
# ID is a required field which might have been overwritten
# so need to keep it
or field.field_name == "id"
)
except AttributeError:
# no type on serializer, must be used only as only nested
# no type on serializer, may only be used nested
pass

return readable_fields
Expand Down
12 changes: 12 additions & 0 deletions tests/serializers.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from rest_framework.settings import api_settings

from rest_framework_json_api import serializers
from tests.models import (
BasicModel,
Expand Down Expand Up @@ -32,6 +34,16 @@ class Meta:
)


class ForeignKeySourcetHyperlinkedSerializer(serializers.HyperlinkedModelSerializer):
class Meta:
model = ForeignKeySource
fields = (
"name",
"target",
api_settings.URL_FIELD_NAME,
)


class ManyToManyTargetSerializer(serializers.ModelSerializer):
class Meta:
fields = ("name",)
Expand Down
64 changes: 57 additions & 7 deletions tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
from tests.serializers import BasicModelSerializer, ForeignKeyTargetSerializer
from tests.views import (
BasicModelViewSet,
ForeignKeySourcetHyperlinkedViewSet,
ForeignKeySourceViewSet,
ForeignKeyTargetViewSet,
ManyToManySourceViewSet,
NestedRelatedSourceViewSet,
)
Expand Down Expand Up @@ -87,7 +89,7 @@ def test_list(self, client, model):

@pytest.mark.urls(__name__)
def test_list_with_include_foreign_key(self, client, foreign_key_source):
url = reverse("foreign-key-source-list")
url = reverse("foreignkeysource-list")
response = client.get(url, data={"include": "target"})
assert response.status_code == status.HTTP_200_OK
result = response.json()
Expand Down Expand Up @@ -156,7 +158,7 @@ def test_list_with_include_nested_related_field(

@pytest.mark.urls(__name__)
def test_list_with_invalid_include(self, client, foreign_key_source):
url = reverse("foreign-key-source-list")
url = reverse("foreignkeysource-list")
response = client.get(url, data={"include": "invalid"})
assert response.status_code == status.HTTP_400_BAD_REQUEST
result = response.json()
Expand Down Expand Up @@ -195,7 +197,7 @@ def test_retrieve(self, client, model):

@pytest.mark.urls(__name__)
def test_retrieve_with_include_foreign_key(self, client, foreign_key_source):
url = reverse("foreign-key-source-detail", kwargs={"pk": foreign_key_source.pk})
url = reverse("foreignkeysource-detail", kwargs={"pk": foreign_key_source.pk})
response = client.get(url, data={"include": "target"})
assert response.status_code == status.HTTP_200_OK
result = response.json()
Expand All @@ -208,6 +210,20 @@ def test_retrieve_with_include_foreign_key(self, client, foreign_key_source):
}
] == result["included"]

@pytest.mark.urls(__name__)
def test_retrieve_hyperlinked_with_sparse_fields(self, client, foreign_key_source):
url = reverse(
"foreignkeysourcehyperlinked-detail", kwargs={"pk": foreign_key_source.pk}
)
response = client.get(url, data={"fields[ForeignKeySource]": "name"})
assert response.status_code == status.HTTP_200_OK
data = response.json()["data"]
assert data["attributes"] == {"name": foreign_key_source.name}
assert "relationships" not in data
assert data["links"] == {
"self": f"http://testserver/foreign_key_sources/{foreign_key_source.pk}/"
}

@pytest.mark.urls(__name__)
def test_patch(self, client, model):
data = {
Expand Down Expand Up @@ -239,7 +255,7 @@ def test_delete(self, client, model):

@pytest.mark.urls(__name__)
def test_create_with_sparse_fields(self, client, foreign_key_target):
url = reverse("foreign-key-source-list")
url = reverse("foreignkeysource-list")
data = {
"data": {
"id": None,
Expand Down Expand Up @@ -379,6 +395,28 @@ def test_patch_with_custom_id(self, client):
}
}

@pytest.mark.urls(__name__)
def test_patch_with_custom_id_with_sparse_fields(self, client):
data = {
"data": {
"id": 2_193_102,
"type": "custom",
"attributes": {"body": "hello"},
}
}

url = reverse("custom-id")

response = client.patch(f"{url}?fields[custom]=body", data=data)
assert response.status_code == status.HTTP_200_OK
assert response.json() == {
"data": {
"type": "custom",
"id": "2176ce", # get_id() -> hex
"attributes": {"body": "hello"},
}
}


# Routing setup

Expand Down Expand Up @@ -415,13 +453,16 @@ class CustomModelSerializer(serializers.Serializer):
id = serializers.IntegerField()


class CustomIdModelSerializer(serializers.Serializer):
class CustomIdSerializer(serializers.Serializer):
id = serializers.SerializerMethodField()
body = serializers.CharField()

def get_id(self, obj):
return hex(obj.id)[2:]

class Meta:
resource_name = "custom"


class CustomAPIView(APIView):
parser_classes = [JSONParser]
Expand All @@ -443,14 +484,23 @@ class CustomIdAPIView(APIView):
resource_name = "custom"

def patch(self, request, *args, **kwargs):
serializer = CustomIdModelSerializer(CustomModel(request.data))
serializer = CustomIdSerializer(
CustomModel(request.data), context={"request": self.request}
)
return Response(status=status.HTTP_200_OK, data=serializer.data)


# TODO remove basename and use default (lowercase of model)
# this makes using HyperlinkedIdentityField easier and reduces
# configuration in general
router = SimpleRouter()
router.register(r"basic_models", BasicModelViewSet, basename="basic-model")
router.register(r"foreign_key_sources", ForeignKeySourceViewSet)
router.register(r"foreign_key_targets", ForeignKeyTargetViewSet)
router.register(
r"foreign_key_sources", ForeignKeySourceViewSet, basename="foreign-key-source"
r"foreign_key_sources_hyperlinked",
ForeignKeySourcetHyperlinkedViewSet,
"foreignkeysourcehyperlinked",
)
router.register(
r"many_to_many_sources", ManyToManySourceViewSet, basename="many-to-many-source"
Expand Down
15 changes: 15 additions & 0 deletions tests/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@
from tests.models import (
BasicModel,
ForeignKeySource,
ForeignKeyTarget,
ManyToManySource,
NestedRelatedSource,
)
from tests.serializers import (
BasicModelSerializer,
ForeignKeySourceSerializer,
ForeignKeySourcetHyperlinkedSerializer,
ForeignKeyTargetSerializer,
ManyToManySourceSerializer,
NestedRelatedSourceSerializer,
)
Expand All @@ -25,6 +28,18 @@ class ForeignKeySourceViewSet(ModelViewSet):
ordering = ["name"]


class ForeignKeySourcetHyperlinkedViewSet(ModelViewSet):
serializer_class = ForeignKeySourcetHyperlinkedSerializer
queryset = ForeignKeySource.objects.all()
ordering = ["name"]


class ForeignKeyTargetViewSet(ModelViewSet):
serializer_class = ForeignKeyTargetSerializer
queryset = ForeignKeyTarget.objects.all()
ordering = ["name"]


class ManyToManySourceViewSet(ModelViewSet):
serializer_class = ManyToManySourceSerializer
queryset = ManyToManySource.objects.all()
Expand Down

0 comments on commit dbe939a

Please sign in to comment.