New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed #35238 -- Fixed database serialization crash when base managers use prefetch_related(). #17889
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello! Thank you for your contribution 💪
As it's your first contribution be sure to check out the patch review checklist.
If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!
If you have any design or process questions then you can ask in the Django forum.
Welcome aboard ⛵️!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AlexCLeduc Thanks 👍 Please add a regression test and release note (5.0.3.txt
). You can find a similar regression fix in 38eaf2f.
You can add a regression test to the diff --git a/tests/backends/base/test_creation.py b/tests/backends/base/test_creation.py
index 9593e13462..7e760e8884 100644
--- a/tests/backends/base/test_creation.py
+++ b/tests/backends/base/test_creation.py
@@ -14,6 +14,7 @@ from ..models import (
Object,
ObjectReference,
ObjectSelfReference,
+ SchoolBus,
SchoolClass,
)
@@ -250,6 +251,22 @@ class TestDeserializeDbFromString(TransactionTestCase):
self.assertIn('"model": "backends.schoolclass"', data)
self.assertIn('"year": 1000', data)
+ def test_serialize_db_to_string_base_manager_with_prefetch_related(self):
+ sclass = SchoolClass.objects.create(
+ year=2000, last_updated=datetime.datetime.now()
+ )
+ bus = SchoolBus.objects.create(number=1)
+ bus.schoolclasses.add(sclass)
+ with mock.patch("django.db.migrations.loader.MigrationLoader") as loader:
+ # serialize_db_to_string() serializes only migrated apps, so mark
+ # the backends app as migrated.
+ loader_instance = loader.return_value
+ loader_instance.migrated_apps = {"backends"}
+ data = connection.creation.serialize_db_to_string()
+ self.assertIn('"model": "backends.schoolbus"', data)
+ self.assertIn('"model": "backends.schoolclass"', data)
+ self.assertIn(f'"schoolclasses": [{sclass.pk}]', data)
+
class SkipTestClass:
def skip_function(self):
diff --git a/tests/backends/models.py b/tests/backends/models.py
index 99e9e86f44..1ed108c2b8 100644
--- a/tests/backends/models.py
+++ b/tests/backends/models.py
@@ -32,6 +32,20 @@ class SchoolClass(models.Model):
objects = SchoolClassManager()
+class SchoolBusManager(models.Manager):
+ def get_queryset(self):
+ return super().get_queryset().prefetch_related("schoolclasses")
+
+
+class SchoolBus(models.Model):
+ number = models.IntegerField()
+ schoolclasses = models.ManyToManyField("SchoolClass")
+ objects = SchoolBusManager()
+
+ class Meta:
+ base_manager_name = "objects"
+
+
class VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ(models.Model):
primary_key_is_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.AutoField(
primary_key=True
|
… use prefetch_related(). Regression in 1391356 following deprecation in eedbf930287cb72e9afab1f7208c24b1146b0c4ec.
e120c6d
to
a084c5d
Compare
@AlexCLeduc Thanks 👍 Welcome aboard ⛵ I pushed tiny final edits. |
Fixes #35238