Skip to content

Commit 6c609f8

Browse files
authored
Ensured that sparse fields only applies when rendering not when parsing (#1221)
* Added missing name field to ForeignKeySourceSerializer * Only extract attributes provided by serialized data * Added changelog
1 parent 0eabc39 commit 6c609f8

12 files changed

+156
-86
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ any parts of the framework not mentioned in the documentation should generally b
2323
* Avoided that an empty attributes dict is rendered in case serializer does not
2424
provide any attribute fields.
2525
* Avoided shadowing of exception when rendering errors (regression since 4.3.0).
26+
* Ensured that sparse fields only applies when rendering, not when parsing.
27+
* Adjusted that sparse fields properly removes meta fields when not defined.
2628

2729
### Removed
2830

example/tests/integration/test_non_paginated_responses.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ def test_multiple_entries_no_pagination(multiple_entries, client):
1414
expected = {
1515
"data": [
1616
{
17-
"type": "posts",
17+
"type": "entries",
1818
"id": "1",
1919
"attributes": {
2020
"headline": multiple_entries[0].headline,
@@ -70,7 +70,7 @@ def test_multiple_entries_no_pagination(multiple_entries, client):
7070
},
7171
},
7272
{
73-
"type": "posts",
73+
"type": "entries",
7474
"id": "2",
7575
"attributes": {
7676
"headline": multiple_entries[1].headline,

example/tests/integration/test_pagination.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ def test_pagination_with_single_entry(single_entry, client):
1414
expected = {
1515
"data": [
1616
{
17-
"type": "posts",
17+
"type": "entries",
1818
"id": "1",
1919
"attributes": {
2020
"headline": single_entry.headline,

example/tests/integration/test_sparse_fieldsets.py

+1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ def test_sparse_fieldset_valid_fields(client, entry):
1515
entry = data[0]
1616
assert entry["attributes"].keys() == {"headline"}
1717
assert entry["relationships"].keys() == {"blog"}
18+
assert "meta" not in entry
1819

1920

2021
@pytest.mark.parametrize(

example/tests/test_filters.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ def test_search_keywords(self):
470470
expected_result = {
471471
"data": [
472472
{
473-
"type": "posts",
473+
"type": "entries",
474474
"id": "7",
475475
"attributes": {
476476
"headline": "ANTH3868X",

example/tests/unit/test_renderer_class_methods.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,8 @@ def test_extract_attributes():
102102
assert sorted(JSONRenderer.extract_attributes(fields, resource)) == sorted(
103103
expected
104104
), "Regular fields should be extracted"
105-
assert sorted(JSONRenderer.extract_attributes(fields, {})) == sorted(
106-
{"username": ""}
105+
assert (
106+
JSONRenderer.extract_attributes(fields, {}) == {}
107107
), "Should not extract read_only fields on empty serializer"
108108

109109

example/views.py

+4-1
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,10 @@ class BlogCustomViewSet(JsonApiViewSet):
112112

113113
class EntryViewSet(ModelViewSet):
114114
queryset = Entry.objects.all()
115-
resource_name = "posts"
115+
# TODO it should not be supported to overwrite resource name
116+
# of endpoints with serializers as includes and sparse fields
117+
# cannot be aware of it
118+
# resource_name = "posts"
116119

117120
def get_serializer_class(self):
118121
return EntrySerializer

rest_framework_json_api/renderers.py

+71-51
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,26 @@
1717
from rest_framework.settings import api_settings
1818

1919
import rest_framework_json_api
20-
from rest_framework_json_api import utils
2120
from rest_framework_json_api.relations import (
2221
HyperlinkedMixin,
2322
ManySerializerMethodResourceRelatedField,
2423
ResourceRelatedField,
2524
SkipDataMixin,
2625
)
26+
from rest_framework_json_api.utils import (
27+
format_errors,
28+
format_field_name,
29+
format_field_names,
30+
get_included_resources,
31+
get_related_resource_type,
32+
get_relation_instance,
33+
get_resource_id,
34+
get_resource_name,
35+
get_resource_type_from_instance,
36+
get_resource_type_from_serializer,
37+
get_serializer_fields,
38+
is_relationship_field,
39+
)
2740

2841

2942
class JSONRenderer(renderers.JSONRenderer):
@@ -57,31 +70,20 @@ class JSONRenderer(renderers.JSONRenderer):
5770
def extract_attributes(cls, fields, resource):
5871
"""
5972
Builds the `attributes` object of the JSON:API resource object.
60-
"""
61-
data = {}
62-
for field_name, field in iter(fields.items()):
63-
# ID is always provided in the root of JSON:API so remove it from attributes
64-
if field_name == "id":
65-
continue
66-
# don't output a key for write only fields
67-
if fields[field_name].write_only:
68-
continue
69-
# Skip fields with relations
70-
if utils.is_relationship_field(field):
71-
continue
7273
73-
# Skip read_only attribute fields when `resource` is an empty
74-
# serializer. Prevents the "Raw Data" form of the browsable API
75-
# from rendering `"foo": null` for read only fields
76-
try:
77-
resource[field_name]
78-
except KeyError:
79-
if fields[field_name].read_only:
80-
continue
74+
Ensures that ID which is always provided in a JSON:API resource object
75+
and relationships are not returned.
76+
"""
8177

82-
data.update({field_name: resource.get(field_name)})
78+
invalid_fields = {"id", api_settings.URL_FIELD_NAME}
8379

84-
return utils.format_field_names(data)
80+
return {
81+
format_field_name(field_name): value
82+
for field_name, value in resource.items()
83+
if field_name in fields
84+
and field_name not in invalid_fields
85+
and not is_relationship_field(fields[field_name])
86+
}
8587

8688
@classmethod
8789
def extract_relationships(cls, fields, resource, resource_instance):
@@ -107,14 +109,14 @@ def extract_relationships(cls, fields, resource, resource_instance):
107109
continue
108110

109111
# Skip fields without relations
110-
if not utils.is_relationship_field(field):
112+
if not is_relationship_field(field):
111113
continue
112114

113115
source = field.source
114-
relation_type = utils.get_related_resource_type(field)
116+
relation_type = get_related_resource_type(field)
115117

116118
if isinstance(field, relations.HyperlinkedIdentityField):
117-
resolved, relation_instance = utils.get_relation_instance(
119+
resolved, relation_instance = get_relation_instance(
118120
resource_instance, source, field.parent
119121
)
120122
if not resolved:
@@ -166,7 +168,7 @@ def extract_relationships(cls, fields, resource, resource_instance):
166168
field,
167169
(relations.PrimaryKeyRelatedField, relations.HyperlinkedRelatedField),
168170
):
169-
resolved, relation = utils.get_relation_instance(
171+
resolved, relation = get_relation_instance(
170172
resource_instance, f"{source}_id", field.parent
171173
)
172174
if not resolved:
@@ -189,7 +191,7 @@ def extract_relationships(cls, fields, resource, resource_instance):
189191
continue
190192

191193
if isinstance(field, relations.ManyRelatedField):
192-
resolved, relation_instance = utils.get_relation_instance(
194+
resolved, relation_instance = get_relation_instance(
193195
resource_instance, source, field.parent
194196
)
195197
if not resolved:
@@ -222,9 +224,7 @@ def extract_relationships(cls, fields, resource, resource_instance):
222224
for nested_resource_instance in relation_instance:
223225
nested_resource_instance_type = (
224226
relation_type
225-
or utils.get_resource_type_from_instance(
226-
nested_resource_instance
227-
)
227+
or get_resource_type_from_instance(nested_resource_instance)
228228
)
229229

230230
relation_data.append(
@@ -243,7 +243,7 @@ def extract_relationships(cls, fields, resource, resource_instance):
243243
)
244244
continue
245245

246-
return utils.format_field_names(data)
246+
return format_field_names(data)
247247

248248
@classmethod
249249
def extract_relation_instance(cls, field, resource_instance):
@@ -289,7 +289,7 @@ def extract_included(
289289
continue
290290

291291
# Skip fields without relations
292-
if not utils.is_relationship_field(field):
292+
if not is_relationship_field(field):
293293
continue
294294

295295
try:
@@ -341,7 +341,7 @@ def extract_included(
341341

342342
if isinstance(field, ListSerializer):
343343
serializer = field.child
344-
relation_type = utils.get_resource_type_from_serializer(serializer)
344+
relation_type = get_resource_type_from_serializer(serializer)
345345
relation_queryset = list(relation_instance)
346346

347347
if serializer_data:
@@ -350,11 +350,9 @@ def extract_included(
350350
nested_resource_instance = relation_queryset[position]
351351
resource_type = (
352352
relation_type
353-
or utils.get_resource_type_from_instance(
354-
nested_resource_instance
355-
)
353+
or get_resource_type_from_instance(nested_resource_instance)
356354
)
357-
serializer_fields = utils.get_serializer_fields(
355+
serializer_fields = get_serializer_fields(
358356
serializer.__class__(
359357
nested_resource_instance, context=serializer.context
360358
)
@@ -378,10 +376,10 @@ def extract_included(
378376
)
379377

380378
if isinstance(field, Serializer):
381-
relation_type = utils.get_resource_type_from_serializer(field)
379+
relation_type = get_resource_type_from_serializer(field)
382380

383381
# Get the serializer fields
384-
serializer_fields = utils.get_serializer_fields(field)
382+
serializer_fields = get_serializer_fields(field)
385383
if serializer_data:
386384
new_item = cls.build_json_resource_obj(
387385
serializer_fields,
@@ -414,7 +412,8 @@ def extract_meta(cls, serializer, resource):
414412
meta_fields = getattr(meta, "meta_fields", [])
415413
data = {}
416414
for field_name in meta_fields:
417-
data.update({field_name: resource.get(field_name)})
415+
if field_name in resource:
416+
data.update({field_name: resource[field_name]})
418417
return data
419418

420419
@classmethod
@@ -434,6 +433,24 @@ def extract_root_meta(cls, serializer, resource):
434433
data.update(json_api_meta)
435434
return data
436435

436+
@classmethod
437+
def _filter_sparse_fields(cls, serializer, fields, resource_name):
438+
request = serializer.context.get("request")
439+
if request:
440+
sparse_fieldset_query_param = f"fields[{resource_name}]"
441+
sparse_fieldset_value = request.query_params.get(
442+
sparse_fieldset_query_param
443+
)
444+
if sparse_fieldset_value:
445+
sparse_fields = sparse_fieldset_value.split(",")
446+
return {
447+
field_name: field
448+
for field_name, field, in fields.items()
449+
if field_name in sparse_fields
450+
}
451+
452+
return fields
453+
437454
@classmethod
438455
def build_json_resource_obj(
439456
cls,
@@ -449,11 +466,15 @@ def build_json_resource_obj(
449466
"""
450467
# Determine type from the instance if the underlying model is polymorphic
451468
if force_type_resolution:
452-
resource_name = utils.get_resource_type_from_instance(resource_instance)
469+
resource_name = get_resource_type_from_instance(resource_instance)
453470
resource_data = {
454471
"type": resource_name,
455-
"id": utils.get_resource_id(resource_instance, resource),
472+
"id": get_resource_id(resource_instance, resource),
456473
}
474+
475+
# TODO remove this filter by rewriting extract_relationships
476+
# so it uses the serialized data as a basis
477+
fields = cls._filter_sparse_fields(serializer, fields, resource_name)
457478
attributes = cls.extract_attributes(fields, resource)
458479
if attributes:
459480
resource_data["attributes"] = attributes
@@ -468,7 +489,7 @@ def build_json_resource_obj(
468489

469490
meta = cls.extract_meta(serializer, resource)
470491
if meta:
471-
resource_data["meta"] = utils.format_field_names(meta)
492+
resource_data["meta"] = format_field_names(meta)
472493

473494
return resource_data
474495

@@ -485,7 +506,7 @@ def render_relationship_view(
485506

486507
def render_errors(self, data, accepted_media_type=None, renderer_context=None):
487508
return super().render(
488-
utils.format_errors(data), accepted_media_type, renderer_context
509+
format_errors(data), accepted_media_type, renderer_context
489510
)
490511

491512
def render(self, data, accepted_media_type=None, renderer_context=None):
@@ -495,7 +516,7 @@ def render(self, data, accepted_media_type=None, renderer_context=None):
495516
request = renderer_context.get("request", None)
496517

497518
# Get the resource name.
498-
resource_name = utils.get_resource_name(renderer_context)
519+
resource_name = get_resource_name(renderer_context)
499520

500521
# If this is an error response, skip the rest.
501522
if resource_name == "errors":
@@ -531,7 +552,7 @@ def render(self, data, accepted_media_type=None, renderer_context=None):
531552

532553
serializer = getattr(serializer_data, "serializer", None)
533554

534-
included_resources = utils.get_included_resources(request, serializer)
555+
included_resources = get_included_resources(request, serializer)
535556

536557
if serializer is not None:
537558
# Extract root meta for any type of serializer
@@ -558,7 +579,7 @@ def render(self, data, accepted_media_type=None, renderer_context=None):
558579
else:
559580
resource_serializer_class = serializer.child
560581

561-
fields = utils.get_serializer_fields(resource_serializer_class)
582+
fields = get_serializer_fields(resource_serializer_class)
562583
force_type_resolution = getattr(
563584
resource_serializer_class, "_poly_force_type_resolution", False
564585
)
@@ -581,7 +602,7 @@ def render(self, data, accepted_media_type=None, renderer_context=None):
581602
included_cache,
582603
)
583604
else:
584-
fields = utils.get_serializer_fields(serializer)
605+
fields = get_serializer_fields(serializer)
585606
force_type_resolution = getattr(
586607
serializer, "_poly_force_type_resolution", False
587608
)
@@ -640,7 +661,7 @@ def render(self, data, accepted_media_type=None, renderer_context=None):
640661
)
641662

642663
if json_api_meta:
643-
render_data["meta"] = utils.format_field_names(json_api_meta)
664+
render_data["meta"] = format_field_names(json_api_meta)
644665

645666
return super().render(render_data, accepted_media_type, renderer_context)
646667

@@ -690,7 +711,6 @@ def get_includes_form(self, view):
690711
serializer_class = view.get_serializer_class()
691712
except AttributeError:
692713
return
693-
694714
if not hasattr(serializer_class, "included_serializers"):
695715
return
696716

0 commit comments

Comments
 (0)