Raise error when ModelSerializer used with abstract model.

[cancan101]

With:

class FullName(models.Model):
    name_first = models.CharField(max_length=200, blank=True)
...
    class Meta:
        abstract = True

class Person(FullName, models.Model):
...

I am trying:

class FullNameSerializer(serializers.ModelSerializer):
    class Meta:
        model = FullName

class PersonSerializer(serializers.ModelSerializer):
    full_name = FullNameSerializer(source='*')
    class Meta:
        model = Person

but I get:

AttributeError at /api/XXXXX/
'NoneType' object has no attribute 'rel'
Request Method: GET
Request URL:    http://127.0.0.1:8033/api/XXXXX/
Django Version: 1.7.5
Exception Type: AttributeError
Exception Value:    
'NoneType' object has no attribute 'rel'

[maryokhin]

I don't think that you should be able to do that. An abstract model is just a Python class for sharing code with other models, it's not actually a model, so here you're referencing to a serializer which is backed by a model that doesn't actually exist.

[maryokhin]

I'm not sure why you need it it to be an abstract model, but even in this set-up you can reference it as a single field. Or if you really need a separate model for the full name, make a it one-to-one relation with Person containing FullName. (which would probably also make more sense conceptually)
https://docs.djangoproject.com/en/1.7/topics/db/examples/one_to_one/

[cancan101]

The one to one model will end up create another table which I might not want.

[tomchristie]

I don't think this has been sufficiently narrowed down to be treated as an issue.
Eg. "'NoneType' object has no attribute 'rel'"
Unclear where you're passing None (or what attribute on the object is None) and which field rel is referring too.

As a side note: almost any issue that comes to us with ModelSerializer hasn't been sufficiently narrowed down. Given that you can always reduce a ModelSerializer class into the plain Serializer class that it generates, I'd always recommend either:

• Raising an issue that demonstrates that a plain Serializer class has some incorrect/unwanted behavior.
• Raising an issue that demonstrates that the set of fields/validators that a ModelSerializer class generates is incorrect/unwanted.

[cancan101]

Here is a more compact example:

class Name(models.Model):
    first = models.CharField(max_length=255)
    class Meta:
        abstract = True

class NameSerializer(serializers.ModelSerializer):
    class Meta:
        model = Name

trying to print repr(NameSerializer()) yields:

Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File "/Users/alex/.virtualenvs/django-rest/lib/python2.7/site-packages/rest_framework/serializers.py", line 440, in __repr__
    return unicode_to_repr(representation.serializer_repr(self, indent=1))
  File "/Users/alex/.virtualenvs/django-rest/lib/python2.7/site-packages/rest_framework/utils/representation.py", line 75, in serializer_repr
    fields = serializer.fields
  File "/Users/alex/.virtualenvs/django-rest/lib/python2.7/site-packages/rest_framework/serializers.py", line 312, in fields
    for key, value in self.get_fields().items():
  File "/Users/alex/.virtualenvs/django-rest/lib/python2.7/site-packages/rest_framework/serializers.py", line 883, in get_fields
    info = model_meta.get_field_info(model)
  File "/Users/alex/.virtualenvs/django-rest/lib/python2.7/site-packages/rest_framework/utils/model_meta.py", line 65, in get_field_info
    pk = _get_pk(opts)
  File "/Users/alex/.virtualenvs/django-rest/lib/python2.7/site-packages/rest_framework/utils/model_meta.py", line 78, in _get_pk
    while pk.rel and pk.rel.parent_link:
AttributeError: 'NoneType' object has no attribute 'rel'

[cancan101]

The issue appears to be an assumption that a model has a primary key. Specifically get_field_info calls into _get_pk which has issue if the model does not have a pk (in this case an abstract model).

[tomchristie]

Okay, that's much better - retitled the issue.

[tomchristie]

Given that the create() and update() methods wouldn't work for abstract models is that actually something that we want to support? What's the motivation?

[maryokhin]

Well technically it's not a model (in the common sense of the word), but just a Python class, which is why it's probably not ModelSerializer's domain.

[tomchristie]

Acceptable resolutions:

• Raise an exception loudly when used with an abstract model.
• Gracefully ignore any pk/url/relationships fields (sounds like a recipe for just introducing complexity and confusing folks)

First def sounds preferable to me.

[maryokhin]

As a side note for conscience, does anyone know if Django's ModelForm supports abstract models?

[tomchristie]

@maryokhin It doesn't. Anyways decision done - raise a big loud error if ModelSerializer is used with an abstract model. Use a regular Serializer if you have a valid reason to want something that maps to an abstract model class.

[tomchristie]

Retitled the issue given the latest assessment.

[cancan101]

In thinking about this more, it really would be cool to have some way of generating a Serializer from an abstract Model. Perhaps the name of this Serializer should not be ModelSerializer. Where this comes up is that the example at the top where I use a FullName abstract base class and would like to not have to Repeat Myself when writing the Serializer:

class FullNameSerializer(serializers.Serializer):
    name_first = serializers.CharField(allow_blank=True, max_length=MAX_LENGTH_NAME_FIELDS, required=False)
    name_middle = serializers.CharField(allow_blank=True, max_length=MAX_LENGTH_NAME_FIELDS, required=False)
    name_last = serializers.CharField(allow_blank=True, max_length=MAX_LENGTH_NAME_FIELDS, required=False)

I lose the nice fact that the ModelSerializer stays in sync with changes to the Model.

[ekiourk]

I am going to have a look

[siolag161]

Hi, first of all thanks for the great work.

This is my first time I work with DRF and run right into this issue. I agree with cancan101 and think it still makes sense for DRY reasons. Or maybe there's a better way for doing this?

[tomchristie]

Given that it currently doesn't work, we should in either case make sure that it currently loudly raises an error.

We could reconsider this if someone supplies a patch that does make it work with abstract models.
However I'd still expect my judgement in that case would be to reject that patch as having too much implicit hidden behavior wrt. how relationships get treated in that state.

DRY is a poor tradeoff if it means introducing too much implicit, hidden behavior.

[kang-kimchhay]

okay