Inheriting multiple Serializers gives unexpected order of overloading of fields

[svilendobrev]

Checklist

• [ x] I have verified that that issue exists against the master branch of Django REST framework.
• [ x] I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
• [ x] This is not a usage question. (Those should be directed to the discussion group instead.)
• [ x] 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.)
• [ x] 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

inheriting a serializer from several other serializers. Some of them have fields with same name, and that name is not present localy in inheriting one

Expected behavior

from those same-name fields, the first as in the order of inheritance should win

Actual behavior

last one wins.

This is because in SerializerMetaclass._get_declared_fields():
a) bases is walked in reverse
b) fields are prepended (i.e. reversing order once more)
c) repeating field-names are added just as any other; check is only against local-attrs
d) result list goes into ordereddict... hence last wins.

Easiest remedy is to reverse bases once more on entrance to this func...
or to avoid breaking unknown amounts of code that relies on current behavior, keep it as is, and put big warning somewhere as documentation.

have fun

[carltongibson]

Hi.

From the description, and from experience of working with inheritance and serialisers, the behaviour you describe sounds expected.

I'm going to close this as such. If you can expand your "Steps to Reproduce" to include an actual (and minimal) code reproduce then I'm happy to re-open. (As it stands the issue is not actionable.)

Thanks.

[mwhansen]

Hi @carltongibson,

I've run into this before. Here's a minimal example:

>>> from rest_framework import serializers
>>> class First(serializers.Serializer):
...     a = serializers.CharField()
>>> class Second(serializers.Serializer):
...     a = serializers.IntegerField()
>>> class MySerializer(First, Second):
...     pass
>>> MySerializer()
MySerializer():
    a = IntegerField()

Under normal Python MRO, you'd expect the a field on MySerializer to be a CharField() since that's what's on First. However, we get an IntegerField due to the combination of reversing the bases and prepending the fields as described above.

[mwhansen]

It would have the intuitive behavior if

django-rest-framework/rest_framework/serializers.py

Line 304 in 0e4811e

for base in reversed(bases):

iterated over the bases in the normal, rather than reverse, order.

[rpkilby]

Processing the mro in reverse is correct, since we want classes higher up in the mro to be overridden by those lower in the mro. The issue is the subsequent line, that prepends fields, effectively reversing order again (as pointed out by @svilendobrev).

Reopening as this looks valid, and the above is enough of a test case to work with.