From a325fb1f9b14b46288d0e1342407be4a6db2bdb1 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Sun, 7 Feb 2016 18:05:14 -0500 Subject: [PATCH] Fixed #26162 -- Checked query name clashes of hidden relationships. Although reverse accessor clashes should be skipped query name can't be hidden. Thanks to Ian Foote and Tim Graham for the review. --- django/db/models/fields/related.py | 19 +++++----- docs/releases/1.8.10.txt | 3 ++ docs/releases/1.9.3.txt | 3 ++ .../test_relative_fields.py | 36 +++++++++++++++---- 4 files changed, 44 insertions(+), 17 deletions(-) diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 08d2aa7a43..a2cd42b7ce 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -194,12 +194,6 @@ class RelatedField(Field): if not isinstance(self.remote_field.model, ModelBase): return [] - # If the field doesn't install backward relation on the target model (so - # `is_hidden` returns True), then there are no clashes to check and we - # can skip these fields. - if self.remote_field.is_hidden(): - return [] - # Consider that we are checking field `Model.foreign` and the models # are: # @@ -211,12 +205,15 @@ class RelatedField(Field): # foreign = models.ForeignKey(Target) # m2m = models.ManyToManyField(Target) - rel_opts = self.remote_field.model._meta # rel_opts.object_name == "Target" + rel_opts = self.remote_field.model._meta + # If the field doesn't install a backward relation on the target model + # (so `is_hidden` returns True), then there are no clashes to check + # and we can skip these fields. + rel_is_hidden = self.remote_field.is_hidden() rel_name = self.remote_field.get_accessor_name() # i. e. "model_set" rel_query_name = self.related_query_name() # i. e. "model" - field_name = "%s.%s" % (opts.object_name, - self.name) # i. e. "Model.field" + field_name = "%s.%s" % (opts.object_name, self.name) # i. e. "Model.field" # Check clashes between accessor or reverse query name of `field` # and any other field name -- i.e. accessor for Model.foreign is @@ -225,7 +222,7 @@ class RelatedField(Field): for clash_field in potential_clashes: clash_name = "%s.%s" % (rel_opts.object_name, clash_field.name) # i. e. "Target.model_set" - if clash_field.name == rel_name: + if not rel_is_hidden and clash_field.name == rel_name: errors.append( checks.Error( "Reverse accessor for '%s' clashes with field name '%s'." % (field_name, clash_name), @@ -255,7 +252,7 @@ class RelatedField(Field): clash_name = "%s.%s" % ( # i. e. "Model.m2m" clash_field.related_model._meta.object_name, clash_field.field.name) - if clash_field.get_accessor_name() == rel_name: + if not rel_is_hidden and clash_field.get_accessor_name() == rel_name: errors.append( checks.Error( "Reverse accessor for '%s' clashes with reverse accessor for '%s'." % (field_name, clash_name), diff --git a/docs/releases/1.8.10.txt b/docs/releases/1.8.10.txt index c9e77552e6..1ceada6f54 100644 --- a/docs/releases/1.8.10.txt +++ b/docs/releases/1.8.10.txt @@ -11,3 +11,6 @@ Bugfixes * Fixed a crash on PostgreSQL that prevented using ``TIME_ZONE=None`` and ``USE_TZ=False`` (:ticket:`26177`). + +* Added system checks for query name clashes of hidden relationships + (:ticket:`26162`). diff --git a/docs/releases/1.9.3.txt b/docs/releases/1.9.3.txt index ac62926583..994a018fbc 100644 --- a/docs/releases/1.9.3.txt +++ b/docs/releases/1.9.3.txt @@ -14,3 +14,6 @@ Bugfixes * Fixed a crash on PostgreSQL that prevented using ``TIME_ZONE=None`` and ``USE_TZ=False`` (:ticket:`26177`). + +* Added system checks for query name clashes of hidden relationships + (:ticket:`26162`). diff --git a/tests/invalid_models_tests/test_relative_fields.py b/tests/invalid_models_tests/test_relative_fields.py index ba0354d5e1..9734e82631 100644 --- a/tests/invalid_models_tests/test_relative_fields.py +++ b/tests/invalid_models_tests/test_relative_fields.py @@ -1008,45 +1008,69 @@ class ExplicitRelatedNameClashTests(SimpleTestCase): @isolate_apps('invalid_models_tests') class ExplicitRelatedQueryNameClashTests(SimpleTestCase): - def test_fk_to_integer(self): + def test_fk_to_integer(self, related_name=None): self._test_explicit_related_query_name_clash( target=models.IntegerField(), relative=models.ForeignKey('Target', models.CASCADE, + related_name=related_name, related_query_name='clash')) - def test_fk_to_fk(self): + def test_hidden_fk_to_integer(self, related_name=None): + self.test_fk_to_integer(related_name='+') + + def test_fk_to_fk(self, related_name=None): self._test_explicit_related_query_name_clash( target=models.ForeignKey('Another', models.CASCADE), relative=models.ForeignKey('Target', models.CASCADE, + related_name=related_name, related_query_name='clash')) - def test_fk_to_m2m(self): + def test_hidden_fk_to_fk(self): + self.test_fk_to_fk(related_name='+') + + def test_fk_to_m2m(self, related_name=None): self._test_explicit_related_query_name_clash( target=models.ManyToManyField('Another'), relative=models.ForeignKey('Target', models.CASCADE, + related_name=related_name, related_query_name='clash')) - def test_m2m_to_integer(self): + def test_hidden_fk_to_m2m(self): + self.test_fk_to_m2m(related_name='+') + + def test_m2m_to_integer(self, related_name=None): self._test_explicit_related_query_name_clash( target=models.IntegerField(), relative=models.ManyToManyField('Target', + related_name=related_name, related_query_name='clash')) - def test_m2m_to_fk(self): + def test_hidden_m2m_to_integer(self): + self.test_m2m_to_integer(related_name='+') + + def test_m2m_to_fk(self, related_name=None): self._test_explicit_related_query_name_clash( target=models.ForeignKey('Another', models.CASCADE), relative=models.ManyToManyField('Target', + related_name=related_name, related_query_name='clash')) - def test_m2m_to_m2m(self): + def test_hidden_m2m_to_fk(self): + self.test_m2m_to_fk(related_name='+') + + def test_m2m_to_m2m(self, related_name=None): self._test_explicit_related_query_name_clash( target=models.ManyToManyField('Another'), relative=models.ManyToManyField('Target', + related_name=related_name, related_query_name='clash')) + def test_hidden_m2m_to_m2m(self): + self.test_m2m_to_m2m(related_name='+') + def _test_explicit_related_query_name_clash(self, target, relative): class Another(models.Model): pass