Django GenericRelation doesn't save related object's id - is this a bug or am I doing it wrong?

Question

I have a model with a generic relation (call it A), when creating an instance of this object I pass an instance of another model (call it B) as the initializer of the content_object field (via kwargs of the constructor).

If I don't save B before creating A then when saving A the content_object_id is saved to the db as NULL. If I save B before passing it to the constructor of A then everything's allright.

It's not logical. I assumed that the ID of the related object (B) is fetched when doing A.save() and it should throw some kind of an exception if B isn't saved yet but it just fails silently. I don't like the current solution (saving B beforhand) because we don't know yet if I will be always willing to keep the object, not just scrap it, and there are performance considerations - what if I will add some another data and save it once more shortly after.

class BaseNodeData(models.Model):
    ...
    extnodedata_content_type = models.ForeignKey(ContentType, null=True)
    extnodedata_object_id = models.PositiveIntegerField(null=True)
    extnodedata = generic.GenericForeignKey(ct_field='extnodedata_content_type', fk_field='extnodedata_object_id')

class MarkupNodeData(models.Model):
    raw_content = models.TextField()

Suppose we do:

markup = MarkupNodeData(raw_content='...')
base = BaseNodeData(..., extnodedata=markup)
markup.save()
base.save()
# both records are inserted to the DB but base is stored with extnodedata_object_id=NULL

markup = MarkupNodeData(raw_content='...')
base = BaseNodeData(..., extnodedata=markup)
base.save()
markup.save()
# no exception is thrown and everything is the same as above

markup = MarkupNodeData(raw_content='...')
markup.save()
base = BaseNodeData(..., extnodedata=markup)
base.save()
# this works as expected

Of course I can do it this way, but it doesn't change anything:

base = BaseNodeData(...)
base.extnodedata = markup

My question is - is this a bug in django which I should report or maybe I'm doing something wrong. Docs on GenericRelations aren't exactly verbose.

Answer 1

Thank you for your answers. I decided to take some more time to investigate the django sources and came up with a solution myself. I subclassed the GenericForeignKey. The code should be self-explanatory.

from django.contrib.contenttypes import generic
from django.db.models import signals

class ImprovedGenericForeignKey(generic.GenericForeignKey):
    """
    Corrects the behaviour of GenericForeignKey so even if you firstly
    assign an object to this field and save it after its PK gets saved.

    If you assign a not yet saved object to this field an exception is 
    thrown upon saving the model.
    """

    class IncompleteData(Exception):
        message = 'Object assigned to field "%s" doesn\'t have a PK (save it first)!'

        def __init__(self, field_name):
            self.field_name = field_name

        def __str__(self):
            return self.message % self.field_name

    def contribute_to_class(self, cls, name):
        signals.pre_save.connect(self.instance_pre_save, sender=cls, weak=False)
        super(ImprovedGenericForeignKey, self).contribute_to_class(cls, name)

    def instance_pre_save(self, sender, instance, **kwargs):
        """
        Ensures that if GenericForeignKey has an object assigned
        that the fk_field stores the object's PK.
        """

        """ If we already have pk set don't do anything... """
        if getattr(instance, self.fk_field) is not None: return

        value = getattr(instance, self.name)

        """
        If no objects is assigned then we leave it as it is. If null constraints
        are present they should take care of this, if not, well, it's not my fault;)
        """
        if value is not None:
            fk = value._get_pk_val()

            if fk is None:
                raise self.IncompleteData(self.name)

            setattr(instance, self.fk_field, fk)

I think that this should be considered a bug in django, so I will report it and see how it turns out.

Answer 2

An instance of B has a pk of None prior to save, and your extnodedata_object_id field allows null values, so the save of the A instance is valid.

It sounds like what might work for you is to override A's save to properly handle new instances of B; e.g. (untested):

def save(self, *args, **kwargs):
    b = self.extnodedata
    if b and b.pk is None:
        b.save()
        self.extnodedata = b
    return super(BaseNodeData, self).save(*args, **kwargs)

Answer 3

I agree that it is strange that you can save A without saving B beforehand.

But I can not think of a case where you will set a relation to an object that you don't want to keep. Makes no sense having a relation to an object that does not exist ;-) Thus saving B beforehand is fine for me.

Not sure whether this helps, but the create method, as posted in this question, might give you an idea what you can also do with generic relations.