From 5e3463f6bcec818431f0e1f4649d6a5bd944c459 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Mon, 22 Oct 2018 09:49:34 -0400 Subject: [PATCH] Fixed #27595 -- Made ForeignKey.get_col() follow target chains. Previously, foreign relationships were followed only one level deep which prevents foreign keys to foreign keys from being resolved appropriately. This was causing issues such as improper database value conversion for UUIDField on SQLite because the resolved expression's output field's internal type wasn't correct. Added tests to make sure unlikely foreign reference cycles don't cause recursion errors. Refs #24343. Thanks oyooyo for the report and Wayne Merry for the investigation. --- django/db/models/fields/related.py | 8 +++++++- tests/model_fields/test_foreignkey.py | 25 +++++++++++++++++++++++++ tests/model_fields/test_uuid.py | 6 +++++- 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index d19d70d30c..2c30a15700 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -977,7 +977,13 @@ class ForeignKey(ForeignObject): return converters def get_col(self, alias, output_field=None): - return super().get_col(alias, output_field or self.target_field) + if output_field is None: + output_field = self.target_field + while isinstance(output_field, ForeignKey): + output_field = output_field.target_field + if output_field is self: + raise ValueError('Cannot resolve output_field.') + return super().get_col(alias, output_field) class OneToOneField(ForeignKey): diff --git a/tests/model_fields/test_foreignkey.py b/tests/model_fields/test_foreignkey.py index 7c99d8e34a..51e76c4052 100644 --- a/tests/model_fields/test_foreignkey.py +++ b/tests/model_fields/test_foreignkey.py @@ -103,3 +103,28 @@ class ForeignKeyTests(TestCase): fk = models.ForeignKey(Foo, models.CASCADE) self.assertEqual(Bar._meta.get_field('fk').to_python('1'), 1) + + @isolate_apps('model_fields') + def test_fk_to_fk_get_col_output_field(self): + class Foo(models.Model): + pass + + class Bar(models.Model): + foo = models.ForeignKey(Foo, models.CASCADE, primary_key=True) + + class Baz(models.Model): + bar = models.ForeignKey(Bar, models.CASCADE, primary_key=True) + + col = Baz._meta.get_field('bar').get_col('alias') + self.assertIs(col.output_field, Foo._meta.pk) + + @isolate_apps('model_fields') + def test_recursive_fks_get_col(self): + class Foo(models.Model): + bar = models.ForeignKey('Bar', models.CASCADE, primary_key=True) + + class Bar(models.Model): + foo = models.ForeignKey(Foo, models.CASCADE, primary_key=True) + + with self.assertRaisesMessage(ValueError, 'Cannot resolve output_field'): + Foo._meta.get_field('bar').get_col('alias') diff --git a/tests/model_fields/test_uuid.py b/tests/model_fields/test_uuid.py index bc1c8d5bc0..6b6af3ea7e 100644 --- a/tests/model_fields/test_uuid.py +++ b/tests/model_fields/test_uuid.py @@ -170,8 +170,12 @@ class TestAsPrimaryKey(TestCase): self.assertEqual(r.uuid_fk, u2) def test_two_level_foreign_keys(self): + gc = UUIDGrandchild() # exercises ForeignKey.get_db_prep_value() - UUIDGrandchild().save() + gc.save() + self.assertIsInstance(gc.uuidchild_ptr_id, uuid.UUID) + gc.refresh_from_db() + self.assertIsInstance(gc.uuidchild_ptr_id, uuid.UUID) class TestAsPrimaryKeyTransactionTests(TransactionTestCase):