Combination of `many=True` and a dotted source doesn't allow a default #7550

stephenfin · 2020-09-26T13:28:39Z

Checklist

I have verified that that issue exists against the master branch of Django REST framework.
I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
This is not a usage question. (Those should be directed to the discussion group instead.)
This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
I have reduced the issue to the simplest possible case.
I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)

Steps to reproduce

Consider a model with a many-many relationship to itself via a through model.

class FooModel(models.Model):
    text = models.CharField(max_length=100)
    bar = models.ForeignKey(
        'BarModel', null=True, blank=True, on_delete=models.SET_NULL,
        related_name='foos', related_query_name='foo')


class BarModel(models.Model):
    pass

We can attempt to serialize this using something like the following.

        class _FooSerializer(serializers.ModelSerializer):
            class Meta:
                model = FooModel
                fields = ('id', 'text')

        class FooSerializer(serializers.ModelSerializer):
            other_foos = _FooSerializer(source='bar.foos', many=True)

            class Meta:
                model = FooModel
                fields = ('id', 'other_foos')

(NOTE: I haven't tested this yet. My case is a lot more complicated so I'll try reduce that is this reproducer isn't valid)

Expected behavior

This is intended to flatten the default output from:

{
    "id": 1,
    "bar": {
        "foos": [
            {
                "id": 2,
                "text": "abc"
            }
        ]
    }
}

to

{
    "id": 1,
    "bar": [
        {
            "id": 2,
            "text": "abc"
        }
    ]
}

Actual behavior

We get an exception:

Got AttributeError when attempting to get a value for field `related` on serializer `PatchDetailSerializer`.
The serializer field might be named incorrectly and not match any attribute or key on the `Patch` instance.
Original exception text was: 'NoneType' object has no attribute 'patches'.

As discussed in #5489, you need to have a default value if you wish to use dotted notation with a nullable value.

class FooSerializer(serializers.ModelSerializer):
    bar = _FooSerializer(source='bar.foos', default=None)

    class Meta:
        fields = ('id', 'bar')

This fixes things for empty case. However, we also need to specify many=True to correct the not-empty case otherwise we get the following error:

'RelatedManager' object has no attribute 'pk'

If we do that, we now get:

'NoneType' object has no attribute 'patches'

for the empty case 😞

The text was updated successfully, but these errors were encountered:

The docs note: When serializing fields with dotted notation, it may be necessary to provide a `default` value if any object is not present or is empty during attribute traversal. However, this doesn't work for fields with 'many=True'. When using these, the default is simply ignored. The solution is simple: do in 'ManyRelatedField' what we were already doing for 'Field', namely, catch possible 'AttributeError' and 'KeyError' exceptions and return the default if there is one set. Signed-off-by: Stephen Finucane <stephen@that.guru> Closes: encode#7550

Signed-off-by: Stephen Finucane <stephen@that.guru>

This wasn't writeable for reasons I haven't been able to figure out. However, it's not actually needed: the 'PatchSerializer' can do the job just fine, given enough information. This exposes a bug in DRF, which has been reported upstream [1]. While we wait for that fix, or some variant of it, to be merged, we must monkey patch the library. [1] encode/django-rest-framework#7550 [2] encode/django-rest-framework#7574 Signed-off-by: Stephen Finucane <stephen@that.guru> Reported-by: Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de> Closes: #379 Cc: Daniel Axtens <dja@axtens.net> Cc: Rohit Sarkar <rohitsarkar5398@gmail.com>

This wasn't writeable for reasons I haven't been able to figure out. However, it's not actually needed: the 'PatchSerializer' can do the job just fine, given enough information. This exposes a bug in DRF, which has been reported upstream [1]. While we wait for that fix, or some variant of it, to be merged, we must monkey patch the library. Conflicts: patchwork/api/patch.py NOTE(stephenfin): Conflicts are due to the absence of commit d3d4f9f ("Add Django 3.0 support") which we do not want to backport here. [1] encode/django-rest-framework#7550 [2] encode/django-rest-framework#7574 Signed-off-by: Stephen Finucane <stephen@that.guru> Reported-by: Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de> Closes: #379 Cc: Daniel Axtens <dja@axtens.net> Cc: Rohit Sarkar <rohitsarkar5398@gmail.com> (cherry picked from commit fe07f30)

* Handle unset fields with 'many=True' The docs note: When serializing fields with dotted notation, it may be necessary to provide a `default` value if any object is not present or is empty during attribute traversal. However, this doesn't work for fields with 'many=True'. When using these, the default is simply ignored. The solution is simple: do in 'ManyRelatedField' what we were already doing for 'Field', namely, catch possible 'AttributeError' and 'KeyError' exceptions and return the default if there is one set. Signed-off-by: Stephen Finucane <stephen@that.guru> Closes: #7550 * Add test cases for #7550 Signed-off-by: Stephen Finucane <stephen@that.guru>

* Handle unset fields with 'many=True' The docs note: When serializing fields with dotted notation, it may be necessary to provide a `default` value if any object is not present or is empty during attribute traversal. However, this doesn't work for fields with 'many=True'. When using these, the default is simply ignored. The solution is simple: do in 'ManyRelatedField' what we were already doing for 'Field', namely, catch possible 'AttributeError' and 'KeyError' exceptions and return the default if there is one set. Signed-off-by: Stephen Finucane <stephen@that.guru> Closes: encode#7550 * Add test cases for encode#7550 Signed-off-by: Stephen Finucane <stephen@that.guru>

stephenfin mentioned this issue Oct 3, 2020

Handle unset fields with 'many=True' #7574

Merged

stephenfin added a commit to stephenfin/django-rest-framework that referenced this issue Oct 3, 2020

Add test cases for encode#7550

393951c

Signed-off-by: Stephen Finucane <stephen@that.guru>

stephenfin added a commit to stephenfin/django-rest-framework that referenced this issue Oct 3, 2020

Add test cases for encode#7550

658b4bd

Signed-off-by: Stephen Finucane <stephen@that.guru>

tomchristie closed this as completed Mar 3, 2021

encode locked and limited conversation to collaborators Mar 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

This issue was moved to a discussion.

Combination of `many=True` and a dotted source doesn't allow a default #7550

Combination of `many=True` and a dotted source doesn't allow a default #7550

stephenfin commented Sep 26, 2020 •

edited

Loading

This issue was moved to a discussion.

Uh oh!

This issue was moved to a discussion.

Combination of many=True and a dotted source doesn't allow a default #7550

Combination of many=True and a dotted source doesn't allow a default #7550

Comments

stephenfin commented Sep 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Steps to reproduce

Expected behavior

Actual behavior

This issue was moved to a discussion.

Combination of `many=True` and a dotted source doesn't allow a default #7550

Combination of `many=True` and a dotted source doesn't allow a default #7550

stephenfin commented Sep 26, 2020 •

edited

Loading