#25563 closed Bug (fixed)
Deferred models should be cached in their proxied model app
Reported by: | Andriy Sokolovskiy | Owned by: | Simon Charette |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | me@… | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Consider simple model and custom manager:
from django.db import models class CustomManager(models.Manager): use_in_migrations = True def get_queryset(self, *args, **kwargs): return super(CustomManager, self).get_queryset( *args, **kwargs ).defer('test') class ModelA(models.Model): test = models.CharField(blank=True, max_length=123) objects = CustomManager()
Also there is a simple test:
from django.test import TestCase from .models import ModelA class Test(TestCase): def test_defer_bug(self): ModelA.objects.create(test='test') self.assertIsInstance(ModelA.objects.last(), ModelA)
With initial migration test is passing well. But, when adding a datamigration like this (where test is evaluating a queryset, which is using custom manager with .defer()
inside):
# -*- coding: utf-8 -*- from __future__ import unicode_literals from django.db import models, migrations def test_forwards(apps, schema_editor): ModelA = apps.get_model('blankapp', 'ModelA') list(ModelA.objects.all()) def noop(apps, schema_editor): return class Migration(migrations.Migration): dependencies = [ ('blankapp', '0001_initial'), ] operations = [ migrations.RunPython(test_forwards, noop) ]
test is failing like:
====================================================================== FAIL: test_defer_bug (blankapp.tests.Test) ---------------------------------------------------------------------- Traceback (most recent call last): File "/projpath/blank/blankapp/tests.py", line 8, in test_defer_bug self.assertIsInstance(ModelA.objects.last(), ModelA) AssertionError: <ModelA_Deferred_test: ModelA_Deferred_test object> is not an instance of <class 'blankapp.models.ModelA'>
This is causing tests failures, when, for example, test contains FK assignment (there is isinstance
check).
Test if failing from
https://github.com/django/django/commit/aa5ef0d4fc67a95ac2a5103810d0c87d8c547bac
Change History (11)
comment:1 by , 9 years ago
Summary: | Datamigration causing incorrect behavior when custom manager is using .defer() → Datamigration causing incorrect tests behavior when custom manager is using .defer() |
---|
comment:2 by , 9 years ago
Summary: | Datamigration causing incorrect tests behavior when custom manager is using .defer() → Data migration causes incorrect tests behavior when custom manager is using .defer() |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 9 years ago
Component: | Migrations → Database layer (models, ORM) |
---|---|
Owner: | changed from | to
Status: | new → assigned |
Version: | 1.8 → master |
The issue lies in django.db.models.query_utils.deferred_class_factory
, it should be using the apps
of the provided model instead of the global one.
I will submit a patch in a few minutes.
comment:4 by , 9 years ago
Summary: | Data migration causes incorrect tests behavior when custom manager is using .defer() → Deferred models should be cached in their proxied model app |
---|
comment:6 by , 9 years ago
Has patch: | set |
---|
Submited a patch here. It would be great if you could test it Andriy.
Will it will be backported to 1.8? (You have changed version to master)
I guess it should be backported in 1.8, what do you think Tim?
comment:7 by , 9 years ago
Thanks for a patch, Simon.
I applied it to django1.8 and tests from my project passed fine.
comment:8 by , 9 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Looks okay to backport, just add release notes please.
That's a strange one. Could it be caused by models caching in django.apps.Apps?