#32787 closed New feature (wontfix)
ContentTypes are created instead of renamed when using SeparateDatabaseAndState
| Reported by: | David Seddon | Owned by: | nobody |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | 3.2 |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Unreviewed | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
When renaming a model using SeparateDatabaseAndState, rather than renaming the existing ContentType, a new one is created. This breaks any existing generic foreign keys pointing to the model.
Cause
When a model is renamed in the standard way, the content types framework uses the pre_migrate signal to rename the ContentType model, ensuring that existing generic foreign keys continue to work. Code here: https://github.com/django/django/blob/main/django/contrib/contenttypes/management/__init__.py#L79
If SeparateDatabaseAndState is used to control the migration, the isinstance check will miss the fact that there is a RenameModel operation, as it just checks the containing SeparateDatabaseAndState operation. It therefore fails to inject a RenameContentType operation.
Later in the migration process, in create_contenttypes will be triggered by the post_migrate signal. Since there is now a missing ContentType for the new model name, it will create it.
Discussion
While SeparateDatabaseAndState is of course meant to be a low level operation, it's difficult to anticipate this behaviour. Since the ContentTypes table is being changed by in any event, it would seem to me preferable to do it in a backward-compatible way.
Alternatively, emitting a warning might be worthwhile: the developer could then manually add a RenameContentType to the migration file once they're aware of the issue.
I think this could be solved by adjusting the check so that it drills down to the database_operations if the operation is a SeparateDatabaseAndState instance.
Change History (2)
comment:1 by , 4 years ago
| Component: | contrib.contenttypes → Database layer (models, ORM) |
|---|---|
| Resolution: | → wontfix |
| Status: | new → closed |
| Type: | Bug → New feature |
comment:2 by , 4 years ago
Thanks for the reply!
If you want to use it to rename models you need to also update content types in the database_operations.
That's what we recommend to do now at my workplace, but it was hard-won knowledge involving some forensic work to get to the bottom of what was going on.
I would argue is that as it stands, the content types hooks are a bit of a footgun even for experienced developers who need to use SeparateDatabaseAndState. My expectation would have been that using SeparateDatabaseAndState suppressed all database actions, but in fact it does perform a database action, just the wrong one.
I wonder if an alternative mitigation would be to output to the console what contenttypes is doing in the pre_migrate/post_migrate steps, just so developers are aware this is happening.
Anyway I do appreciate this is probably a pretty niche issue. Let me know if you change your mind, I'd be happy to put together a pull request.
SeparateDatabaseAndStateis "a highly specialized operation that lets you mix and match the database (schema-changing) and state (autodetector-powering) aspects of operations." (see docs). If you want to use it to rename models you need to also update content types in thedatabase_operations. Django will not analyze raw SQL queries, there is also now way to provide a sensible warning.