ContentType should purge orphaned models

[Rob Hudson <treborhudson@…>]

When working with models there are often times when one refactors model class names which leaves orphaned content types in the database. Content types should clean themselves when they notice an app no longer contains models in the database.

[Rob Hudson <treborhudson@…>]

First attempt at a patch for this. This is a diff from git so this may need tweaking...

[Rob Hudson <treborhudson@…>]

See: ​http://groups.google.com/group/django-developers/browse_thread/thread/c0a99afe82d4eaa8

The first patch works but I haven't tested fully. I had assumed I would also have to figure out how to manage the various permissions tables but it seems to be taking care of itself. I looked to see if the delete() on the ContentType object had some trickery to update the permissions but didn't find anything.

This doesn't remove the orphaned model tables themselves either, which is probably good... the idea for this patch was to avoid having to delete rows in the contenttype and various permissions tables, dropping a table is easy enough.

[Rob Hudson <treborhudson@…>]

Just an update on this patch: I figured out the "trickery" in that Permissions has a FK to ContentType and therefore takes care of itself when a content type is deleted. Nice.

This patch accomplishes what I think it should...

• If a model in an installed app gets orphaned, it removes that content type.
• If a content type is renamed, it removes the old one and installs the new one.
  • It does not track or understand a renaming happened and migrate permissions or anything else. That's a bit outside the scope of this, I think.

I'll work on adding docs and test cases if I can.

-Rob

[Malcolm Tredinnick]

I like this patch and assuming all the concerns below are not real problems, it's close to reday. A few small things to fix and one large thing to ponder:

1. create_contenttypes() now really needs a docstring, because it's not just creating new content types. It's also cleaning up old ones. Looks like it's a bit fiddly to move the "remove" processing out from the "create" loop, so it'll have to stay as one method. But at least document what it's doing in a broad way.
2. We're strong believers in the English language and full sentences. The comment you've got to needs an initial capital and final full stop.
3. The AUTHORS file is in alphabetical order by surname. Try to find where your name belongs (hint: it's not at the end).
4. Do you need to get app_label from the models? What happens if a model has Meta.app_label set?

Now here's the big problem: is this a safe thing to do? I haven't followed through all the code, but I would guess it's not impossible for models in different locations to have the same value for their Meta.app_label attribute. I'm not sure if Django works well in that case, or if django.core.management sorts that out and groups things properly (I suspect not), or if it's something we want to support. But right now it might be possible. If so, that would mean you could end up calling create_contenttypes() multiple times for the same app_label.

At the moment, I suspect that just means not as much cleaning up gets done, so it fails safely (no still-valid data is deleted), right? I think this bears some thinking about; I don't know the answer yet, because it just occurred to me when I was reviewing the patch.

Not sure it's going to really be possible to test this, since the ContentTypes table is flushed for each new test anyway. If you can come up with something, great. However, I can't think of how to do it offhand.

What docs do you think are necessary? A quick observational note in the contrib apps docs? It's almost doing the natural thing and could pass without comment. By all means, write whatever you think is good, but I wouldn't hold back the patch if you think there's nothing worth saying.

[Rob Hudson <treborhudson@…>]

Fixing points 1 through 3 from Malcolm. Point #4 will need more time to ponder.

[Rob Hudson <treborhudson@…>]

Making the doc string more correct.

[Rob Hudson <treborhudson@…>]

Discussion and a design decision needed request regarding Malcolm's point 4 above posted to the developers list here:
​http://groups.google.com/group/django-developers/browse_thread/thread/60de62f7a1b05f17/

Marking as design decision needed.

[Rob Hudson <treborhudson@…>]

Per Adrian and Jacob's suggestion, changing method name to update since it's now doing more than creating.

[Malcolm Tredinnick]

(In [6287]) Fixed #5177 -- Changed content type creation to also remove the types for any orphaned entries (so it's now an "update" feature). Thanks, Rob Hudson.