Refs #32508 -- Raised Type/ValueError instead of using "assert" in django.db.models.

Co-authored-by: Mariusz Felisiak <felisiak.mariusz@gmail.com>
This commit is contained in:
Daniyal 2021-03-24 11:15:08 +05:30 committed by Mariusz Felisiak
parent 08f0778885
commit f479df7f8d
20 changed files with 168 additions and 79 deletions

View File

@ -947,12 +947,12 @@ class Model(metaclass=ModelBase):
field.delete_cached_value(self) field.delete_cached_value(self)
def delete(self, using=None, keep_parents=False): def delete(self, using=None, keep_parents=False):
if self.pk is None:
raise ValueError(
"%s object can't be deleted because its %s attribute is set "
"to None." % (self._meta.object_name, self._meta.pk.attname)
)
using = using or router.db_for_write(self.__class__, instance=self) using = using or router.db_for_write(self.__class__, instance=self)
assert self.pk is not None, (
"%s object can't be deleted because its %s attribute is set to None." %
(self._meta.object_name, self._meta.pk.attname)
)
collector = Collector(using=using) collector = Collector(using=using)
collector.collect([self], keep_parents=keep_parents) collector.collect([self], keep_parents=keep_parents)
return collector.delete() return collector.delete()

View File

@ -2484,10 +2484,11 @@ class AutoFieldMixin:
return value return value
def contribute_to_class(self, cls, name, **kwargs): def contribute_to_class(self, cls, name, **kwargs):
assert not cls._meta.auto_field, ( if cls._meta.auto_field:
"Model %s can't have more than one auto-generated field." raise ValueError(
% cls._meta.label "Model %s can't have more than one auto-generated field."
) % cls._meta.label
)
super().contribute_to_class(cls, name, **kwargs) super().contribute_to_class(cls, name, **kwargs)
cls._meta.auto_field = self cls._meta.auto_field = self

View File

@ -815,13 +815,13 @@ class ForeignKey(ForeignObject):
try: try:
to._meta.model_name to._meta.model_name
except AttributeError: except AttributeError:
assert isinstance(to, str), ( if not isinstance(to, str):
"%s(%r) is invalid. First parameter to ForeignKey must be " raise TypeError(
"either a model, a model name, or the string %r" % ( '%s(%r) is invalid. First parameter to ForeignKey must be '
self.__class__.__name__, to, 'either a model, a model name, or the string %r' % (
RECURSIVE_RELATIONSHIP_CONSTANT, self.__class__.__name__, to, RECURSIVE_RELATIONSHIP_CONSTANT,
)
) )
)
else: else:
# For backwards compatibility purposes, we need to *try* and set # For backwards compatibility purposes, we need to *try* and set
# the to_field during FK construction. It won't be guaranteed to # the to_field during FK construction. It won't be guaranteed to
@ -1169,18 +1169,20 @@ class ManyToManyField(RelatedField):
try: try:
to._meta to._meta
except AttributeError: except AttributeError:
assert isinstance(to, str), ( if not isinstance(to, str):
"%s(%r) is invalid. First parameter to ManyToManyField must be " raise TypeError(
"either a model, a model name, or the string %r" % '%s(%r) is invalid. First parameter to ManyToManyField '
(self.__class__.__name__, to, RECURSIVE_RELATIONSHIP_CONSTANT) 'must be either a model, a model name, or the string %r' % (
) self.__class__.__name__, to, RECURSIVE_RELATIONSHIP_CONSTANT,
)
)
if symmetrical is None: if symmetrical is None:
symmetrical = (to == RECURSIVE_RELATIONSHIP_CONSTANT) symmetrical = (to == RECURSIVE_RELATIONSHIP_CONSTANT)
if through is not None: if through is not None and db_table is not None:
assert db_table is None, ( raise ValueError(
"Cannot specify a db_table if an intermediary model is used." 'Cannot specify a db_table if an intermediary model is used.'
) )
kwargs['rel'] = self.rel_class( kwargs['rel'] = self.rel_class(

View File

@ -214,9 +214,10 @@ class TruncBase(TimezoneMixin, Transform):
copy = super().resolve_expression(query, allow_joins, reuse, summarize, for_save) copy = super().resolve_expression(query, allow_joins, reuse, summarize, for_save)
field = copy.lhs.output_field field = copy.lhs.output_field
# DateTimeField is a subclass of DateField so this works for both. # DateTimeField is a subclass of DateField so this works for both.
assert isinstance(field, (DateField, TimeField)), ( if not isinstance(field, (DateField, TimeField)):
"%r isn't a DateField, TimeField, or DateTimeField." % field.name raise TypeError(
) "%r isn't a DateField, TimeField, or DateTimeField." % field.name
)
# If self.output_field was None, then accessing the field will trigger # If self.output_field was None, then accessing the field will trigger
# the resolver to assign it to self.lhs.output_field. # the resolver to assign it to self.lhs.output_field.
if not isinstance(copy.output_field, (DateField, DateTimeField, TimeField)): if not isinstance(copy.output_field, (DateField, DateTimeField, TimeField)):

View File

@ -161,10 +161,11 @@ class Index:
column_names[0][:7], column_names[0][:7],
'%s_%s' % (names_digest(*hash_data, length=6), self.suffix), '%s_%s' % (names_digest(*hash_data, length=6), self.suffix),
) )
assert len(self.name) <= self.max_name_length, ( if len(self.name) > self.max_name_length:
'Index too long for multiple database support. Is self.suffix ' raise ValueError(
'longer than 3 characters?' 'Index too long for multiple database support. Is self.suffix '
) 'longer than 3 characters?'
)
if self.name[0] == '_' or self.name[0].isdigit(): if self.name[0] == '_' or self.name[0].isdigit():
self.name = 'D%s' % self.name[1:] self.name = 'D%s' % self.name[1:]

View File

@ -291,10 +291,14 @@ class QuerySet:
'QuerySet indices must be integers or slices, not %s.' 'QuerySet indices must be integers or slices, not %s.'
% type(k).__name__ % type(k).__name__
) )
assert ((not isinstance(k, slice) and (k >= 0)) or if (
(isinstance(k, slice) and (k.start is None or k.start >= 0) and (isinstance(k, int) and k < 0) or
(k.stop is None or k.stop >= 0))), \ (isinstance(k, slice) and (
"Negative indexing is not supported." (k.start is not None and k.start < 0) or
(k.stop is not None and k.stop < 0)
))
):
raise ValueError('Negative indexing is not supported.')
if self._result_cache is not None: if self._result_cache is not None:
return self._result_cache[k] return self._result_cache[k]
@ -480,7 +484,8 @@ class QuerySet:
# PostgreSQL via the RETURNING ID clause. It should be possible for # PostgreSQL via the RETURNING ID clause. It should be possible for
# Oracle as well, but the semantics for extracting the primary keys is # Oracle as well, but the semantics for extracting the primary keys is
# trickier so it's not done yet. # trickier so it's not done yet.
assert batch_size is None or batch_size > 0 if batch_size is not None and batch_size <= 0:
raise ValueError('Batch size must be a positive integer.')
# Check that the parents share the same concrete model with the our # Check that the parents share the same concrete model with the our
# model to detect the inheritance pattern ConcreteGrandParent -> # model to detect the inheritance pattern ConcreteGrandParent ->
# MultiTableParent -> ProxyChild. Simply checking self.model._meta.proxy # MultiTableParent -> ProxyChild. Simply checking self.model._meta.proxy
@ -900,10 +905,10 @@ class QuerySet:
Return a list of date objects representing all available dates for Return a list of date objects representing all available dates for
the given field_name, scoped to 'kind'. the given field_name, scoped to 'kind'.
""" """
assert kind in ('year', 'month', 'week', 'day'), \ if kind not in ('year', 'month', 'week', 'day'):
"'kind' must be one of 'year', 'month', 'week', or 'day'." raise ValueError("'kind' must be one of 'year', 'month', 'week', or 'day'.")
assert order in ('ASC', 'DESC'), \ if order not in ('ASC', 'DESC'):
"'order' must be either 'ASC' or 'DESC'." raise ValueError("'order' must be either 'ASC' or 'DESC'.")
return self.annotate( return self.annotate(
datefield=Trunc(field_name, kind, output_field=DateField()), datefield=Trunc(field_name, kind, output_field=DateField()),
plain_field=F(field_name) plain_field=F(field_name)
@ -916,10 +921,13 @@ class QuerySet:
Return a list of datetime objects representing all available Return a list of datetime objects representing all available
datetimes for the given field_name, scoped to 'kind'. datetimes for the given field_name, scoped to 'kind'.
""" """
assert kind in ('year', 'month', 'week', 'day', 'hour', 'minute', 'second'), \ if kind not in ('year', 'month', 'week', 'day', 'hour', 'minute', 'second'):
"'kind' must be one of 'year', 'month', 'week', 'day', 'hour', 'minute', or 'second'." raise ValueError(
assert order in ('ASC', 'DESC'), \ "'kind' must be one of 'year', 'month', 'week', 'day', "
"'order' must be either 'ASC' or 'DESC'." "'hour', 'minute', or 'second'."
)
if order not in ('ASC', 'DESC'):
raise ValueError("'order' must be either 'ASC' or 'DESC'.")
if settings.USE_TZ: if settings.USE_TZ:
if tzinfo is None: if tzinfo is None:
tzinfo = timezone.get_current_timezone() tzinfo = timezone.get_current_timezone()

View File

@ -567,14 +567,14 @@ class Query(BaseExpression):
The 'connector' parameter describes how to connect filters from the The 'connector' parameter describes how to connect filters from the
'rhs' query. 'rhs' query.
""" """
assert self.model == rhs.model, \ if self.model != rhs.model:
"Cannot combine queries on two different base models." raise TypeError('Cannot combine queries on two different base models.')
if self.is_sliced: if self.is_sliced:
raise TypeError('Cannot combine queries once a slice has been taken.') raise TypeError('Cannot combine queries once a slice has been taken.')
assert self.distinct == rhs.distinct, \ if self.distinct != rhs.distinct:
"Cannot combine a unique query with a non-unique query." raise TypeError('Cannot combine a unique query with a non-unique query.')
assert self.distinct_fields == rhs.distinct_fields, \ if self.distinct_fields != rhs.distinct_fields:
"Cannot combine queries with different distinct fields." raise TypeError('Cannot combine queries with different distinct fields.')
# Work out how to relabel the rhs aliases, if necessary. # Work out how to relabel the rhs aliases, if necessary.
change_map = {} change_map = {}

View File

@ -347,3 +347,8 @@ class BulkCreateTests(TestCase):
) )
with self.assertRaisesMessage(ValueError, msg): with self.assertRaisesMessage(ValueError, msg):
NullableFields.objects.bulk_create([NullableFields(auto_field=parent)]) NullableFields.objects.bulk_create([NullableFields(auto_field=parent)])
def test_invalid_batch_size_exception(self):
msg = 'Batch size must be a positive integer.'
with self.assertRaisesMessage(ValueError, msg):
Country.objects.bulk_create([], batch_size=-1)

View File

@ -98,11 +98,12 @@ class DatesTests(TestCase):
def test_dates_fails_when_given_invalid_kind_argument(self): def test_dates_fails_when_given_invalid_kind_argument(self):
msg = "'kind' must be one of 'year', 'month', 'week', or 'day'." msg = "'kind' must be one of 'year', 'month', 'week', or 'day'."
with self.assertRaisesMessage(AssertionError, msg): with self.assertRaisesMessage(ValueError, msg):
Article.objects.dates("pub_date", "bad_kind") Article.objects.dates("pub_date", "bad_kind")
def test_dates_fails_when_given_invalid_order_argument(self): def test_dates_fails_when_given_invalid_order_argument(self):
with self.assertRaisesMessage(AssertionError, "'order' must be either 'ASC' or 'DESC'."): msg = "'order' must be either 'ASC' or 'DESC'."
with self.assertRaisesMessage(ValueError, msg):
Article.objects.dates("pub_date", "year", order="bad order") Article.objects.dates("pub_date", "year", order="bad order")
@override_settings(USE_TZ=False) @override_settings(USE_TZ=False)

View File

@ -199,3 +199,16 @@ class DateTimesTests(TestCase):
Article.objects.create(pub_date=dt, published_on=dt.date(), title="Don't put dates into datetime functions!") Article.objects.create(pub_date=dt, published_on=dt.date(), title="Don't put dates into datetime functions!")
with self.assertRaisesMessage(ValueError, "Cannot truncate DateField 'published_on' to DateTimeField"): with self.assertRaisesMessage(ValueError, "Cannot truncate DateField 'published_on' to DateTimeField"):
list(Article.objects.datetimes('published_on', 'second')) list(Article.objects.datetimes('published_on', 'second'))
def test_datetimes_fails_when_given_invalid_kind_argument(self):
msg = (
"'kind' must be one of 'year', 'month', 'week', 'day', 'hour', "
"'minute', or 'second'."
)
with self.assertRaisesMessage(ValueError, msg):
Article.objects.datetimes('pub_date', 'bad_kind')
def test_datetimes_fails_when_given_invalid_order_argument(self):
msg = "'order' must be either 'ASC' or 'DESC'."
with self.assertRaisesMessage(ValueError, msg):
Article.objects.datetimes('pub_date', 'year', order='bad order')

View File

@ -655,7 +655,8 @@ class DateFunctionTests(TestCase):
with self.assertRaisesMessage(ValueError, msg): with self.assertRaisesMessage(ValueError, msg):
list(DTModel.objects.annotate(truncated=Trunc('start_datetime', 'year', output_field=IntegerField()))) list(DTModel.objects.annotate(truncated=Trunc('start_datetime', 'year', output_field=IntegerField())))
with self.assertRaisesMessage(AssertionError, "'name' isn't a DateField, TimeField, or DateTimeField."): msg = "'name' isn't a DateField, TimeField, or DateTimeField."
with self.assertRaisesMessage(TypeError, msg):
list(DTModel.objects.annotate(truncated=Trunc('name', 'year', output_field=DateTimeField()))) list(DTModel.objects.annotate(truncated=Trunc('name', 'year', output_field=DateTimeField())))
with self.assertRaisesMessage(ValueError, "Cannot truncate DateField 'start_date' to DateTimeField"): with self.assertRaisesMessage(ValueError, "Cannot truncate DateField 'start_date' to DateTimeField"):

View File

@ -280,6 +280,12 @@ class DeletionTests(TestCase):
with self.assertRaisesMessage(TypeError, msg): with self.assertRaisesMessage(TypeError, msg):
M.objects.all()[0:5].delete() M.objects.all()[0:5].delete()
def test_pk_none(self):
m = M()
msg = "M object can't be deleted because its id attribute is set to None."
with self.assertRaisesMessage(ValueError, msg):
m.delete()
def test_m2m(self): def test_m2m(self):
m = M.objects.create() m = M.objects.create()
r = R.objects.create() r = R.objects.create()

View File

@ -87,9 +87,14 @@ class DistinctOnTests(TestCase):
self.assertSequenceEqual(qset, expected) self.assertSequenceEqual(qset, expected)
self.assertEqual(qset.count(), len(expected)) self.assertEqual(qset.count(), len(expected))
# Combining queries with different distinct_fields is not allowed. # Combining queries with non-unique query is not allowed.
base_qs = Celebrity.objects.all() base_qs = Celebrity.objects.all()
with self.assertRaisesMessage(AssertionError, "Cannot combine queries with different distinct fields."): msg = 'Cannot combine a unique query with a non-unique query.'
with self.assertRaisesMessage(TypeError, msg):
base_qs.distinct('id') & base_qs
# Combining queries with different distinct_fields is not allowed.
msg = 'Cannot combine queries with different distinct fields.'
with self.assertRaisesMessage(TypeError, msg):
base_qs.distinct('id') & base_qs.distinct('name') base_qs.distinct('id') & base_qs.distinct('name')
# Test join unreffing # Test join unreffing

View File

@ -1591,6 +1591,18 @@ class OtherModelTests(SimpleTestCase):
]) ])
class MultipleAutoFieldsTests(TestCase):
def test_multiple_autofields(self):
msg = (
"Model invalid_models_tests.MultipleAutoFields can't have more "
"than one auto-generated field."
)
with self.assertRaisesMessage(ValueError, msg):
class MultipleAutoFields(models.Model):
auto1 = models.AutoField(primary_key=True)
auto2 = models.AutoField(primary_key=True)
@isolate_apps('invalid_models_tests') @isolate_apps('invalid_models_tests')
class JSONFieldTests(TestCase): class JSONFieldTests(TestCase):
@skipUnlessDBFeature('supports_json_field') @skipUnlessDBFeature('supports_json_field')

View File

@ -147,3 +147,12 @@ class ForeignKeyTests(TestCase):
) )
with self.assertRaisesMessage(FieldError, msg): with self.assertRaisesMessage(FieldError, msg):
Related._meta.get_field('child').related_fields Related._meta.get_field('child').related_fields
def test_invalid_to_parameter(self):
msg = (
"ForeignKey(1) is invalid. First parameter to ForeignKey must be "
"either a model, a model name, or the string 'self'"
)
with self.assertRaisesMessage(TypeError, msg):
class MyModel(models.Model):
child = models.ForeignKey(1, models.CASCADE)

View File

@ -59,6 +59,34 @@ class ManyToManyFieldTests(SimpleTestCase):
assert_app_model_resolved('model_fields') assert_app_model_resolved('model_fields')
assert_app_model_resolved('tests') assert_app_model_resolved('tests')
def test_invalid_to_parameter(self):
msg = (
"ManyToManyField(1) is invalid. First parameter to "
"ManyToManyField must be either a model, a model name, or the "
"string 'self'"
)
with self.assertRaisesMessage(TypeError, msg):
class MyModel(models.Model):
m2m = models.ManyToManyField(1)
@isolate_apps('model_fields')
def test_through_db_table_mutually_exclusive(self):
class Child(models.Model):
pass
class Through(models.Model):
referred = models.ForeignKey(Child, on_delete=models.CASCADE)
referent = models.ForeignKey(Child, on_delete=models.CASCADE)
msg = 'Cannot specify a db_table if an intermediary model is used.'
with self.assertRaisesMessage(ValueError, msg):
class MyModel(models.Model):
m2m = models.ManyToManyField(
Child,
through='Through',
db_table='custom_name',
)
class ManyToManyFieldDBTests(TestCase): class ManyToManyFieldDBTests(TestCase):

View File

@ -173,8 +173,11 @@ class SimpleIndexesTests(SimpleTestCase):
# suffix can't be longer than 3 characters. # suffix can't be longer than 3 characters.
long_field_index.suffix = 'suff' long_field_index.suffix = 'suff'
msg = 'Index too long for multiple database support. Is self.suffix longer than 3 characters?' msg = (
with self.assertRaisesMessage(AssertionError, msg): 'Index too long for multiple database support. Is self.suffix '
'longer than 3 characters?'
)
with self.assertRaisesMessage(ValueError, msg):
long_field_index.set_name_with_model(Book) long_field_index.set_name_with_model(Book)
@isolate_apps('model_indexes') @isolate_apps('model_indexes')

View File

@ -417,9 +417,10 @@ class Queries1Tests(TestCase):
def test_heterogeneous_qs_combination(self): def test_heterogeneous_qs_combination(self):
# Combining querysets built on different models should behave in a well-defined # Combining querysets built on different models should behave in a well-defined
# fashion. We raise an error. # fashion. We raise an error.
with self.assertRaisesMessage(AssertionError, 'Cannot combine queries on two different base models.'): msg = 'Cannot combine queries on two different base models.'
with self.assertRaisesMessage(TypeError, msg):
Author.objects.all() & Tag.objects.all() Author.objects.all() & Tag.objects.all()
with self.assertRaisesMessage(AssertionError, 'Cannot combine queries on two different base models.'): with self.assertRaisesMessage(TypeError, msg):
Author.objects.all() | Tag.objects.all() Author.objects.all() | Tag.objects.all()
def test_ticket3141(self): def test_ticket3141(self):
@ -1226,10 +1227,11 @@ class Queries3Tests(TestCase):
# This shouldn't create an infinite loop. # This shouldn't create an infinite loop.
self.assertQuerysetEqual(Valid.objects.all(), []) self.assertQuerysetEqual(Valid.objects.all(), [])
def test_ticket8683(self): def test_datetimes_invalid_field(self):
# An error should be raised when QuerySet.datetimes() is passed the # An error should be raised when QuerySet.datetimes() is passed the
# wrong type of field. # wrong type of field.
with self.assertRaisesMessage(AssertionError, "'name' isn't a DateField, TimeField, or DateTimeField."): msg = "'name' isn't a DateField, TimeField, or DateTimeField."
with self.assertRaisesMessage(TypeError, msg):
Item.objects.datetimes('name', 'month') Item.objects.datetimes('name', 'month')
def test_ticket22023(self): def test_ticket22023(self):
@ -2405,13 +2407,17 @@ class QuerySetSupportsPythonIdioms(TestCase):
def test_slicing_negative_indexing_not_supported_for_single_element(self): def test_slicing_negative_indexing_not_supported_for_single_element(self):
"""hint: inverting your ordering might do what you need""" """hint: inverting your ordering might do what you need"""
with self.assertRaisesMessage(AssertionError, "Negative indexing is not supported."): msg = 'Negative indexing is not supported.'
with self.assertRaisesMessage(ValueError, msg):
Article.objects.all()[-1] Article.objects.all()[-1]
def test_slicing_negative_indexing_not_supported_for_range(self): def test_slicing_negative_indexing_not_supported_for_range(self):
"""hint: inverting your ordering might do what you need""" """hint: inverting your ordering might do what you need"""
with self.assertRaisesMessage(AssertionError, "Negative indexing is not supported."): msg = 'Negative indexing is not supported.'
with self.assertRaisesMessage(ValueError, msg):
Article.objects.all()[0:-5] Article.objects.all()[0:-5]
with self.assertRaisesMessage(ValueError, msg):
Article.objects.all()[-1:]
def test_invalid_index(self): def test_invalid_index(self):
msg = 'QuerySet indices must be integers or slices, not str.' msg = 'QuerySet indices must be integers or slices, not str.'

View File

@ -32,8 +32,10 @@ class CustomManyToManyField(RelatedField):
) )
self.swappable = swappable self.swappable = swappable
self.db_table = db_table self.db_table = db_table
if kwargs['rel'].through is not None: if kwargs['rel'].through is not None and self.db_table is not None:
assert self.db_table is None, "Cannot specify a db_table if an intermediary model is used." raise ValueError(
'Cannot specify a db_table if an intermediary model is used.'
)
super().__init__( super().__init__(
related_name=related_name, related_name=related_name,
related_query_name=related_query_name, related_query_name=related_query_name,

View File

@ -125,18 +125,3 @@ class GenericIPAddressTestModel(models.Model):
class GenericIPAddrUnpackUniqueTest(models.Model): class GenericIPAddrUnpackUniqueTest(models.Model):
generic_v4unpack_ip = models.GenericIPAddressField(null=True, blank=True, unique=True, unpack_ipv4=True) generic_v4unpack_ip = models.GenericIPAddressField(null=True, blank=True, unique=True, unpack_ipv4=True)
# A model can't have multiple AutoFields
# Refs #12467.
assertion_error = None
try:
class MultipleAutoFields(models.Model):
auto1 = models.AutoField(primary_key=True)
auto2 = models.AutoField(primary_key=True)
except AssertionError as exc:
assertion_error = exc
assert str(assertion_error) == (
"Model validation.MultipleAutoFields can't have more than one "
"auto-generated field."
)