django-polymorphic
django-polymorphic copied to clipboard
Creating polymorphic instances in migrations results in invalid model instances (polymorphic_ctype_id is null)
When you create an instance of a polymorphic model in a (data) migration, the polymorphic_ctype_id
will be left null
. The reason is that this field is set in PolymorphicModel.pre_save_polymorphic
which is called from save
. However, save is never called when running migrations, because in migrations we don't use the actual model classes but the 'historical models' maintained by the migrations engine. As the documentation says: "Because it’s impossible to serialize arbitrary Python code, these historical models will not have any custom methods that you have defined. " This includes the custom save
(and pre_save_polymorphic
) methods.
The result is that naively creating a polymorphic model instance from migrations will create instances without a valid polymorphic_ctype
FK which will cause problems down the road when you try to access these instances. And this is pretty evil as it won't necessarily be caught by tests.
This has been reported before (see https://github.com/django-polymorphic/django-polymorphic/issues/197 ), but closed due to a misunderstanding probably with a reference to the documentation explaining how to migrate existing objects. But this is a distinct problem.
Now besides this being a documentation problem (i.e. undocumented), also raises the question if polymorphic_ctype
should be non-nullable. (I don't know what's the decision behind allowing null values, maybe there is a good reason, but getting any instance with a null polymorphic_ctype
will fail with an exception which hints that it shouldn't be allowed.)
Setting polymorphic_ctype
non-nullable would at least prevent creating invalid data. As far as I can see, the only solution is to manually set the polymorphic_ctype
as mentioned in the documentation for migrating existing models. (But note, that the documentation is wrong about how to do that. I'll open another issue for that.)
Thanks for sharing this - i just couldn't figure out why it wasn't working...
Also see #148.
Regarding documentation being wrong, that was resolved in #389.
Note that there is also the related problem of deleting polymorphic instances in a data migration: I think in this case, the parent(s) won't be deleted, essentially leaving them around with an also invalid, though non-null, polymorphic_ctype
. This leads to the same kinds of problems down the road.
Not sure if there should be a separate issue for this.
I also stumbled across the problem of polymorphic behaviour not being applied during (data) migrations the unpleasant way and have been looking for a solution to make sure the expected behaviour is applied since. So far, no luck for something general, but if it were easy, then I guess it would already be part of the library.
I feel like it would be nice to have a hard to miss warning about data migrations in the documentation somewhere, though, given the time consuming surprises I've had.
In case it's useful to someone, these are the best workarounds I've found for data migrations:
Creation
Just make sure to set the poylmorphic_ctype
explicitly on all model instances created. As is documented, the correct value can be found by adding:
ContentType = apps.get_model('contenttypes', 'ContentType')
new_ct = ContentType.objects.get_for_model(MyModel)
Deletion
I delete the base model instead of the child and rely on cascading down the parent pointers to delete all children. Finding the base class seems to be accomplished most flexibly by checking the model which defined the polymorphic_ctype
field:
ModelBaseClass = MyChildModel._meta.get_field("polymorphic_ctype").model
Caveats:
- I'm not sure if a model is allowed to have multiple inheritance of Polymorphic bases, or if the name
polymorphic_ctype
can be customised, but in those cases, this approach obviously won't work. - If the Polymorphic base model is allowed to be abstract, then I'm not sure whether
get_field("polymorphic_ctype").model
will return the concrete polymorphic base, or the abstract base. The latter would break this approach. - If the
pk
of a child model may not match thepk
of the base model any case, it may be a bit ugly to figure out which base instances to delete in a generic manner, which supports an arbitrary chain of inheritance. - If parent pointers may be set not to cascade, this will also not work.
Well, since you do know when writing the migration what the base class is, you can refer to it directly :) . Actually that is the preferred way since the code can change and you don't know what MyChildModel._meta.get_field("polymorphic_ctype").model
will be at an unknown time in the furure. (Though it's unlikely that the base class will change for a model, it's not out of question.) This is a generic principle to keep in mind when writing Django migrations. Even if it's just your app and not a reusable, published one. It can bite e.g. when you have to setup a new environment (e.g. because you add a new dev to the team) and run old migrations you have long forgotten about.
Also, the pk of the child is set to be a FK on the parent's PK so they are guaranteed to be the same and it also guarantees that deleting the parent instance will cascade the delete (or that the deletion will fail if for some reason the tables weren't created properly).