Fixed #25535 -- Made ForeignObject checks less strict.
Check that the foreign object `from_fields` are a subset of any unique constraints on the foreign model.
This commit is contained in:
parent
533c10998a
commit
80dac8c33e
|
@ -464,22 +464,35 @@ class ForeignObject(RelatedField):
|
||||||
if not self.foreign_related_fields:
|
if not self.foreign_related_fields:
|
||||||
return []
|
return []
|
||||||
|
|
||||||
has_unique_field = any(rel_field.unique
|
unique_foreign_fields = {
|
||||||
for rel_field in self.foreign_related_fields)
|
frozenset([f.name])
|
||||||
if not has_unique_field and len(self.foreign_related_fields) > 1:
|
for f in self.remote_field.model._meta.get_fields()
|
||||||
|
if getattr(f, 'unique', False)
|
||||||
|
}
|
||||||
|
unique_foreign_fields.update({
|
||||||
|
frozenset(ut)
|
||||||
|
for ut in self.remote_field.model._meta.unique_together
|
||||||
|
})
|
||||||
|
foreign_fields = {f.name for f in self.foreign_related_fields}
|
||||||
|
|
||||||
|
has_unique_constraint = any(u <= foreign_fields for u in unique_foreign_fields)
|
||||||
|
|
||||||
|
if not has_unique_constraint and len(self.foreign_related_fields) > 1:
|
||||||
field_combination = ', '.join("'%s'" % rel_field.name
|
field_combination = ', '.join("'%s'" % rel_field.name
|
||||||
for rel_field in self.foreign_related_fields)
|
for rel_field in self.foreign_related_fields)
|
||||||
model_name = self.remote_field.model.__name__
|
model_name = self.remote_field.model.__name__
|
||||||
return [
|
return [
|
||||||
checks.Error(
|
checks.Error(
|
||||||
"None of the fields %s on model '%s' have a unique=True constraint."
|
"No subset of the fields %s on model '%s' is unique"
|
||||||
|
% (field_combination, model_name),
|
||||||
|
hint="Add a unique=True on any of the fields %s of '%s', or add them all or a "
|
||||||
|
"subset to a unique_together constraint."
|
||||||
% (field_combination, model_name),
|
% (field_combination, model_name),
|
||||||
hint=None,
|
|
||||||
obj=self,
|
obj=self,
|
||||||
id='fields.E310',
|
id='fields.E310',
|
||||||
)
|
)
|
||||||
]
|
]
|
||||||
elif not has_unique_field:
|
elif not has_unique_constraint:
|
||||||
field_name = self.foreign_related_fields[0].name
|
field_name = self.foreign_related_fields[0].name
|
||||||
model_name = self.remote_field.model.__name__
|
model_name = self.remote_field.model.__name__
|
||||||
return [
|
return [
|
||||||
|
|
|
@ -2,6 +2,8 @@ import datetime
|
||||||
from operator import attrgetter
|
from operator import attrgetter
|
||||||
|
|
||||||
from django.core.exceptions import FieldError
|
from django.core.exceptions import FieldError
|
||||||
|
from django.db import models
|
||||||
|
from django.db.models.fields.related import ForeignObject
|
||||||
from django.test import TestCase, skipUnlessDBFeature
|
from django.test import TestCase, skipUnlessDBFeature
|
||||||
from django.utils import translation
|
from django.utils import translation
|
||||||
|
|
||||||
|
@ -391,3 +393,61 @@ class MultiColumnFKTests(TestCase):
|
||||||
""" See: https://code.djangoproject.com/ticket/21566 """
|
""" See: https://code.djangoproject.com/ticket/21566 """
|
||||||
objs = [Person(name="abcd_%s" % i, person_country=self.usa) for i in range(0, 5)]
|
objs = [Person(name="abcd_%s" % i, person_country=self.usa) for i in range(0, 5)]
|
||||||
Person.objects.bulk_create(objs, 10)
|
Person.objects.bulk_create(objs, 10)
|
||||||
|
|
||||||
|
def test_check_composite_foreign_object(self):
|
||||||
|
class Parent(models.Model):
|
||||||
|
a = models.PositiveIntegerField()
|
||||||
|
b = models.PositiveIntegerField()
|
||||||
|
|
||||||
|
class Meta:
|
||||||
|
unique_together = (
|
||||||
|
('a', 'b'),
|
||||||
|
)
|
||||||
|
|
||||||
|
class Child(models.Model):
|
||||||
|
a = models.PositiveIntegerField()
|
||||||
|
b = models.PositiveIntegerField()
|
||||||
|
value = models.CharField(max_length=255)
|
||||||
|
|
||||||
|
parent = ForeignObject(
|
||||||
|
Parent,
|
||||||
|
on_delete=models.SET_NULL,
|
||||||
|
from_fields=('a', 'b'),
|
||||||
|
to_fields=('a', 'b'),
|
||||||
|
related_name='children',
|
||||||
|
)
|
||||||
|
|
||||||
|
field = Child._meta.get_field('parent')
|
||||||
|
errors = field.check(from_model=Child)
|
||||||
|
|
||||||
|
self.assertEqual(errors, [])
|
||||||
|
|
||||||
|
def test_check_subset_composite_foreign_object(self):
|
||||||
|
class Parent(models.Model):
|
||||||
|
a = models.PositiveIntegerField()
|
||||||
|
b = models.PositiveIntegerField()
|
||||||
|
c = models.PositiveIntegerField()
|
||||||
|
|
||||||
|
class Meta:
|
||||||
|
unique_together = (
|
||||||
|
('a', 'b'),
|
||||||
|
)
|
||||||
|
|
||||||
|
class Child(models.Model):
|
||||||
|
a = models.PositiveIntegerField()
|
||||||
|
b = models.PositiveIntegerField()
|
||||||
|
c = models.PositiveIntegerField()
|
||||||
|
d = models.CharField(max_length=255)
|
||||||
|
|
||||||
|
parent = ForeignObject(
|
||||||
|
Parent,
|
||||||
|
on_delete=models.SET_NULL,
|
||||||
|
from_fields=('a', 'b', 'c'),
|
||||||
|
to_fields=('a', 'b', 'c'),
|
||||||
|
related_name='children',
|
||||||
|
)
|
||||||
|
|
||||||
|
field = Child._meta.get_field('parent')
|
||||||
|
errors = field.check(from_model=Child)
|
||||||
|
|
||||||
|
self.assertEqual(errors, [])
|
||||||
|
|
|
@ -5,6 +5,7 @@ import warnings
|
||||||
|
|
||||||
from django.core.checks import Error, Warning as DjangoWarning
|
from django.core.checks import Error, Warning as DjangoWarning
|
||||||
from django.db import models
|
from django.db import models
|
||||||
|
from django.db.models.fields.related import ForeignObject
|
||||||
from django.test import ignore_warnings
|
from django.test import ignore_warnings
|
||||||
from django.test.testcases import skipIfDBFeature
|
from django.test.testcases import skipIfDBFeature
|
||||||
from django.test.utils import override_settings
|
from django.test.utils import override_settings
|
||||||
|
@ -507,9 +508,9 @@ class RelativeFieldTests(IsolatedModelsTestCase):
|
||||||
errors = field.check()
|
errors = field.check()
|
||||||
expected = [
|
expected = [
|
||||||
Error(
|
Error(
|
||||||
"None of the fields 'country_id', 'city_id' on model 'Person' "
|
"No subset of the fields 'country_id', 'city_id' on model 'Person' is unique",
|
||||||
"have a unique=True constraint.",
|
hint="Add a unique=True on any of the fields 'country_id', 'city_id' of 'Person', "
|
||||||
hint=None,
|
"or add them all or a subset to a unique_together constraint.",
|
||||||
obj=field,
|
obj=field,
|
||||||
id='fields.E310',
|
id='fields.E310',
|
||||||
)
|
)
|
||||||
|
@ -1395,3 +1396,79 @@ class M2mThroughFieldsTests(IsolatedModelsTestCase):
|
||||||
obj=field,
|
obj=field,
|
||||||
id='fields.E337')]
|
id='fields.E337')]
|
||||||
self.assertEqual(expected, errors)
|
self.assertEqual(expected, errors)
|
||||||
|
|
||||||
|
def test_superset_foreign_object(self):
|
||||||
|
class Parent(models.Model):
|
||||||
|
a = models.PositiveIntegerField()
|
||||||
|
b = models.PositiveIntegerField()
|
||||||
|
c = models.PositiveIntegerField()
|
||||||
|
|
||||||
|
class Meta:
|
||||||
|
unique_together = (
|
||||||
|
('a', 'b', 'c'),
|
||||||
|
)
|
||||||
|
|
||||||
|
class Child(models.Model):
|
||||||
|
a = models.PositiveIntegerField()
|
||||||
|
b = models.PositiveIntegerField()
|
||||||
|
value = models.CharField(max_length=255)
|
||||||
|
|
||||||
|
parent = ForeignObject(
|
||||||
|
Parent,
|
||||||
|
on_delete=models.SET_NULL,
|
||||||
|
from_fields=('a', 'b'),
|
||||||
|
to_fields=('a', 'b'),
|
||||||
|
related_name='children',
|
||||||
|
)
|
||||||
|
|
||||||
|
field = Child._meta.get_field('parent')
|
||||||
|
errors = field.check(from_model=Child)
|
||||||
|
expected = [
|
||||||
|
Error(
|
||||||
|
"No subset of the fields 'a', 'b' on model 'Parent' is unique",
|
||||||
|
hint="Add a unique=True on any of the fields 'a', 'b' of 'Parent', or add them "
|
||||||
|
"all or a subset to a unique_together constraint.",
|
||||||
|
obj=field,
|
||||||
|
id='fields.E310',
|
||||||
|
),
|
||||||
|
]
|
||||||
|
self.assertEqual(expected, errors)
|
||||||
|
|
||||||
|
def test_insersection_foreign_object(self):
|
||||||
|
class Parent(models.Model):
|
||||||
|
a = models.PositiveIntegerField()
|
||||||
|
b = models.PositiveIntegerField()
|
||||||
|
c = models.PositiveIntegerField()
|
||||||
|
d = models.PositiveIntegerField()
|
||||||
|
|
||||||
|
class Meta:
|
||||||
|
unique_together = (
|
||||||
|
('a', 'b', 'c'),
|
||||||
|
)
|
||||||
|
|
||||||
|
class Child(models.Model):
|
||||||
|
a = models.PositiveIntegerField()
|
||||||
|
b = models.PositiveIntegerField()
|
||||||
|
d = models.PositiveIntegerField()
|
||||||
|
value = models.CharField(max_length=255)
|
||||||
|
|
||||||
|
parent = ForeignObject(
|
||||||
|
Parent,
|
||||||
|
on_delete=models.SET_NULL,
|
||||||
|
from_fields=('a', 'b', 'd'),
|
||||||
|
to_fields=('a', 'b', 'd'),
|
||||||
|
related_name='children',
|
||||||
|
)
|
||||||
|
|
||||||
|
field = Child._meta.get_field('parent')
|
||||||
|
errors = field.check(from_model=Child)
|
||||||
|
expected = [
|
||||||
|
Error(
|
||||||
|
"No subset of the fields 'a', 'b', 'd' on model 'Parent' is unique",
|
||||||
|
hint="Add a unique=True on any of the fields 'a', 'b', 'd' of 'Parent', or add "
|
||||||
|
"them all or a subset to a unique_together constraint.",
|
||||||
|
obj=field,
|
||||||
|
id='fields.E310',
|
||||||
|
),
|
||||||
|
]
|
||||||
|
self.assertEqual(expected, errors)
|
||||||
|
|
Loading…
Reference in New Issue