ContentType.objects.get_for_models() in migrations does not works for multiple models

[HMaker]

I am trying to use migrations to create default groups, I tried to run the following procedure with migrations.RunPython()

def create_normal_users_group(apps, *args):
    auth = SimpleNamespace(**apps.all_models['auth'])
    myapp = SimpleNamespace(**apps.all_models['myapp'])
    ContentType = apps.get_model('contenttypes', 'ContentType')
    group = auth.group.objects.create(name='Normal Users')
    contenttypes = ContentType.objects.get_for_models(myapp.user, myapp.proxy) # there are more models...
   # ...

but it raises AttributeError

  File "/home/user/projects/myapp/.venv/lib/python3.8/site-packages/django/contrib/contenttypes/models.py", line 89, in get_for_models
    opts_models = needed_opts.pop(ct.model_class()._meta, [])
AttributeError: 'ContentType' object has no attribute 'model_class'

it works when I pass a single model to ContentType.objects.get_for_models(), but get_for_models() is supposed to work with multiple models.

EDIT:
Adding model_class() to ContentType makes it work

def create_normal_users_group(apps, *args):
    auth = SimpleNamespace(**apps.all_models['auth'])
    myapp = SimpleNamespace(**apps.all_models['myapp'])
    ContentType = apps.get_model('contenttypes', 'ContentType')
    # this makes it work =========================
    def model_class(self):
        return apps.get_model(self.app_label, self.model)
    ContentType.model_class = model_class
    # ========================================
    group = auth.group.objects.create(name='Normal Users')
    contenttypes = ContentType.objects.get_for_models(myapp.user, myapp.proxy) # there are more models...
   # ...

[Simon Charette]

Since ContentTypeManager is marked as use_in_migrations = True it should be able to handle this case.

There's two possible solutions here:

1. Create an AbstractContentType(models.Model) abstract model class that include most of the logic ContentType and have the latter subclass the former. From there CreateModel(bases) ​should be adjusted to pass this base instead.
2. Adjust ContentTypeManager.get_for_models to avoid calling .model_class() and inline its logic in the manager method instead.

Feels like 2. is the less disruptive approach and easier to test. Would you be interested in writing a patch PR for it?

[HMaker]

OK I will implement 2nd solution.

[HMaker]

I tried to reproduce this problem but I could not, check my ​partial patch, it's based on 3.1.4 branch.

[Keryn Knight]

Replying to Heraldo Lucena:

I tried to reproduce this problem but I could not [...]

My guess would be that the ContentTypeManager instance already had a partially filled (or empty) cache when you encountered the error, such that self._get_from_cache(modelinstance._meta) worked (didn't throw a KeyError looking in the self._cache).

• If you pass in 2 model classes Group and User and both are already in its internal cache, then needed_opts is empty and needed_opts.pop(ct.model_class()._meta, []) doesn't occur.
• If you pass the same 2 classes and only 1 or 0 of them are in the self._cache already, you'd fill the missing items into needed_opts and hit the line which has caused you problems.

[HMaker]

I thought migrations would be automatically rolled back after each test case, but that did not happen and last test case could not run properly since all migrations were already applied. Now the regression test can reproduce reported problem.

​PR made.

[GitHub <noreply@…>]

In 84206607:

Fixed #32833 -- Fixed ContentTypeManager.get_for_models() crash when using in migrations.

Co-authored-by: Heraldo Lucena <23155511+HMaker@…>

[Sarah Boyce <42296566+sarahboyce@…>]

In f1705c8:

Fixed #35545, Refs #32833 -- Fixed ContentTypeManager.get_for_models() crash in CreateModel migrations.

Thank you to Csirmaz Bendegúz for the report and Simon Charettes for the review.

[Sarah Boyce <42296566+sarahboyce@…>]

In 6317803:

[5.1.x] Fixed #35545, Refs #32833 -- Fixed ContentTypeManager.get_for_models() crash in CreateModel migrations.

Thank you to Csirmaz Bendegúz for the report and Simon Charettes for the review.

Backport of f1705c8780c0a7587654fc736542d55fe4a7f29b from main.