Models from app state aren't compatible (Cannot assign "<A: A object (1)>": "B.a_fk" must be a "A" instance)

[Feuermurmel]

I'm running into a problem when I apply a migration with Migrator.apply_initial_migration() and then try to construct model instances using the returned app state. One model has a foreign key to another model, but the foreign key field does not accept an instance of that model (see below for the exact output and exception).

I tried to reduce the setup as much a possible. In the end, two additional models and 2 indexes were necessary to trigger the issue. I tested it with django-test-migrations 1.2.0 and with Django 3.2.13 and 4.0.4. The attached ZIP archive contains a complete project which reproduces the issue:

app-state-bug.zip

To run the test, use the following couple of commands:

python3.10 -m venv venv
venv/bin/pip install django~=3.2 pytest-django django-test-migrations
venv/bin/pytest test_bad.py

When running this, you should see the following output:

___________________________________ test_bad ___________________________________

migrator = <django_test_migrations.migrator.Migrator object at 0x105c44430>

    def test_bad(migrator):
        state = migrator.apply_initial_migration(('my_app', '0002_foo'))
    
        A = state.apps.get_model('my_app', 'A')
        B = state.apps.get_model('my_app', 'B')
    
        print(id(A), id(B.a_fk.field.related_model))
    
>       B.objects.create(a_fk=A.objects.create(foo=1))

test_bad.py:9: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
venv/lib/python3.10/site-packages/django/db/models/manager.py:85: in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
venv/lib/python3.10/site-packages/django/db/models/query.py:512: in create
    obj = self.model(**kwargs)
venv/lib/python3.10/site-packages/django/db/models/base.py:541: in __init__
    _setattr(self, field.name, rel_obj)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <django.db.models.fields.related_descriptors.ForwardManyToOneDescriptor object at 0x105c47520>
instance = <B: B object (None)>, value = <A: A object (1)>

    def __set__(self, instance, value):
        """
        Set the related instance through the forward relation.
    
        With the example above, when setting ``child.parent = parent``:
    
        - ``self`` is the descriptor managing the ``parent`` attribute
        - ``instance`` is the ``child`` instance
        - ``value`` is the ``parent`` instance on the right of the equal sign
        """
        # An object must be an instance of the related class.
        if value is not None and not isinstance(
            value, self.field.remote_field.model._meta.concrete_model
        ):
>           raise ValueError(
                'Cannot assign "%r": "%s.%s" must be a "%s" instance.'
                % (
                    value,
                    instance._meta.object_name,
                    self.field.name,
                    self.field.remote_field.model._meta.object_name,
                )
E               ValueError: Cannot assign "<A: A object (1)>": "B.a_fk" must be a "A" instance.

venv/lib/python3.10/site-packages/django/db/models/fields/related_descriptors.py:235: ValueError

[Feuermurmel]

I found a workaround:

diff --git a/test_bad.py b/test_bad.py
index b6b5779..a385cde 100644
--- a/test_bad.py
+++ b/test_bad.py
@@ -1,5 +1,6 @@
 def test_bad(migrator):
     state = migrator.apply_initial_migration(('my_app', '0002_foo'))
+    state.clear_delayed_apps_cache()
 
     A = state.apps.get_model('my_app', 'A')
     B = state.apps.get_model('my_app', 'B')

The RunPython operation calls this from django.db.migrations.operations.special.RunPython.database_forwards() before passing apps to the function.

[sobolevn]

Should we always call this function on our side? What do you think? 🤔

[Feuermurmel]

I guess so? 🤔 I think the RunPython operation is the only case where the migration state is exposed to user code and there, clear_delayed_apps_cache() is called on it right before the state is passed to the callable. So it seems to me that this is the right approach.

[sobolevn]

Then, PR is welcome. Let's test it!

[skarzi]

hi 👋

Thanks for really well-prepared issue!

It seems like we should call it, because it's also used in migrate command.