From 1506c71a95cd7f58fbc6363edf2ef742c58d2487 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Mon, 3 Mar 2014 20:12:42 -0500 Subject: [PATCH] Fixed #12030 -- Validate integer field range at the model level. Thanks to @timgraham for the review. --- django/db/backends/__init__.py | 18 +++++ django/db/backends/mysql/base.py | 6 ++ django/db/backends/oracle/base.py | 9 +++ django/db/backends/sqlite3/base.py | 4 + django/db/models/fields/__init__.py | 10 +++ docs/releases/1.7.txt | 5 ++ tests/model_fields/models.py | 18 ++++- tests/model_fields/tests.py | 109 +++++++++++++++++++++------- 8 files changed, 153 insertions(+), 26 deletions(-) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index 2a93d9a9ce..a8e0cc526c 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -617,6 +617,16 @@ class BaseDatabaseOperations(object): """ compiler_module = "django.db.models.sql.compiler" + # Integer field safe ranges by `internal_type` as documented + # in docs/ref/models/fields.txt. + integer_field_ranges = { + 'SmallIntegerField': (-32768, 32767), + 'IntegerField': (-2147483648, 2147483647), + 'BigIntegerField': (-9223372036854775808, 9223372036854775807), + 'PositiveSmallIntegerField': (0, 32767), + 'PositiveIntegerField': (0, 2147483647), + } + def __init__(self, connection): self.connection = connection self._cache = None @@ -1101,6 +1111,14 @@ class BaseDatabaseOperations(object): """ return params + def integer_field_range(self, internal_type): + """ + Given an integer field internal type (e.g. 'PositiveIntegerField'), + returns a tuple of the (min_value, max_value) form representing the + range of the column type bound to the field. + """ + return self.integer_field_ranges[internal_type] + # Structure returned by the DB-API cursor.description interface (PEP 249) FieldInfo = namedtuple('FieldInfo', diff --git a/django/db/backends/mysql/base.py b/django/db/backends/mysql/base.py index e79f980e21..61a8ab72fc 100644 --- a/django/db/backends/mysql/base.py +++ b/django/db/backends/mysql/base.py @@ -223,6 +223,12 @@ class DatabaseFeatures(BaseDatabaseFeatures): class DatabaseOperations(BaseDatabaseOperations): compiler_module = "django.db.backends.mysql.compiler" + # MySQL stores positive fields as UNSIGNED ints. + integer_field_ranges = dict(BaseDatabaseOperations.integer_field_ranges, + PositiveSmallIntegerField=(0, 4294967295), + PositiveIntegerField=(0, 18446744073709551615), + ) + def date_extract_sql(self, lookup_type, field_name): # http://dev.mysql.com/doc/mysql/en/date-and-time-functions.html if lookup_type == 'week_day': diff --git a/django/db/backends/oracle/base.py b/django/db/backends/oracle/base.py index 0c799055fb..830a8a9862 100644 --- a/django/db/backends/oracle/base.py +++ b/django/db/backends/oracle/base.py @@ -121,6 +121,15 @@ class DatabaseFeatures(BaseDatabaseFeatures): class DatabaseOperations(BaseDatabaseOperations): compiler_module = "django.db.backends.oracle.compiler" + # Oracle uses NUMBER(11) and NUMBER(19) for integer fields. + integer_field_ranges = { + 'SmallIntegerField': (-99999999999, 99999999999), + 'IntegerField': (-99999999999, 99999999999), + 'BigIntegerField': (-9999999999999999999, 9999999999999999999), + 'PositiveSmallIntegerField': (0, 99999999999), + 'PositiveIntegerField': (0, 99999999999), + } + def autoinc_sql(self, table, column): # To simulate auto-incrementing primary keys in Oracle, we have to # create a sequence and a trigger. diff --git a/django/db/backends/sqlite3/base.py b/django/db/backends/sqlite3/base.py index 3bb85e042d..9709e8449f 100644 --- a/django/db/backends/sqlite3/base.py +++ b/django/db/backends/sqlite3/base.py @@ -292,6 +292,10 @@ class DatabaseOperations(BaseDatabaseOperations): return 'django_power(%s)' % ','.join(sub_expressions) return super(DatabaseOperations, self).combine_expression(connector, sub_expressions) + def integer_field_range(self, internal_type): + # SQLite doesn't enforce any integer constraints + return (None, None) + class DatabaseWrapper(BaseDatabaseWrapper): vendor = 'sqlite' diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index 8dfe7e79e5..4ebb6049b3 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -1561,6 +1561,16 @@ class IntegerField(Field): } description = _("Integer") + def __init__(self, *args, **kwargs): + field_validators = kwargs.setdefault('validators', []) + internal_type = self.get_internal_type() + min_value, max_value = connection.ops.integer_field_range(internal_type) + if min_value is not None: + field_validators.append(validators.MinValueValidator(min_value)) + if max_value is not None: + field_validators.append(validators.MaxValueValidator(max_value)) + super(IntegerField, self).__init__(*args, **kwargs) + def get_prep_value(self, value): value = super(IntegerField, self).get_prep_value(value) if value is None: diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt index a058d40054..8cb0961a4e 100644 --- a/docs/releases/1.7.txt +++ b/docs/releases/1.7.txt @@ -678,6 +678,11 @@ Models Previously this used to work if the field accepted integers as input as it took the primary key. +* Integer fields are now validated against database backend specific min and + max values based on their :meth:`internal_type `. + Previously model field validation didn't prevent values out of their associated + column data type range from being saved resulting in an integrity error. + Signals ^^^^^^^ diff --git a/tests/model_fields/models.py b/tests/model_fields/models.py index 5fe4104851..16bfd61308 100644 --- a/tests/model_fields/models.py +++ b/tests/model_fields/models.py @@ -55,11 +55,27 @@ class BigS(models.Model): s = models.SlugField(max_length=255) -class BigInt(models.Model): +class SmallIntegerModel(models.Model): + value = models.SmallIntegerField() + + +class IntegerModel(models.Model): + value = models.IntegerField() + + +class BigIntegerModel(models.Model): value = models.BigIntegerField() null_value = models.BigIntegerField(null=True, blank=True) +class PositiveSmallIntegerModel(models.Model): + value = models.PositiveSmallIntegerField() + + +class PositiveIntegerModel(models.Model): + value = models.PositiveIntegerField() + + class Post(models.Model): title = models.CharField(max_length=100) body = models.TextField() diff --git a/tests/model_fields/tests.py b/tests/model_fields/tests.py index 421672e4d2..754ad20e5d 100644 --- a/tests/model_fields/tests.py +++ b/tests/model_fields/tests.py @@ -7,6 +7,7 @@ import warnings from django import test from django import forms +from django.core import validators from django.core.exceptions import ValidationError from django.db import connection, transaction, models, IntegrityError from django.db.models.fields import ( @@ -21,9 +22,10 @@ from django.utils import six from django.utils.functional import lazy from .models import ( - Foo, Bar, Whiz, BigD, BigS, BigInt, Post, NullBooleanModel, + Foo, Bar, Whiz, BigD, BigS, BigIntegerModel, Post, NullBooleanModel, BooleanModel, PrimaryKeyCharModel, DataModel, Document, RenamedField, - DateTimeModel, VerboseNameField, FksToBooleans, FkToChar, FloatModel) + DateTimeModel, VerboseNameField, FksToBooleans, FkToChar, FloatModel, + SmallIntegerModel, IntegerModel, PositiveSmallIntegerModel, PositiveIntegerModel) class BasicFieldTests(test.TestCase): @@ -131,7 +133,6 @@ class DecimalFieldTests(test.TestCase): self.assertEqual(f._format(None), None) def test_get_db_prep_lookup(self): - from django.db import connection f = models.DecimalField(max_digits=5, decimal_places=1) self.assertEqual(f.get_db_prep_lookup('exact', None, connection=connection), [None]) @@ -212,7 +213,6 @@ class DateTimeFieldTests(unittest.TestCase): class BooleanFieldTests(unittest.TestCase): def _test_get_db_prep_lookup(self, f): - from django.db import connection self.assertEqual(f.get_db_prep_lookup('exact', True, connection=connection), [True]) self.assertEqual(f.get_db_prep_lookup('exact', '1', connection=connection), [True]) self.assertEqual(f.get_db_prep_lookup('exact', 1, connection=connection), [True]) @@ -451,33 +451,92 @@ class ValidationTest(test.TestCase): self.assertRaises(ValidationError, f.clean, None, None) -class BigIntegerFieldTests(test.TestCase): - def test_limits(self): - # Ensure that values that are right at the limits can be saved - # and then retrieved without corruption. - maxval = 9223372036854775807 - minval = -maxval - 1 - BigInt.objects.create(value=maxval) - qs = BigInt.objects.filter(value__gte=maxval) +class IntegerFieldTests(test.TestCase): + model = IntegerModel + documented_range = (-2147483648, 2147483647) + + def test_documented_range(self): + """ + Ensure that values within the documented safe range pass validation, + can be saved and retrieved without corruption. + """ + min_value, max_value = self.documented_range + + instance = self.model(value=min_value) + instance.full_clean() + instance.save() + qs = self.model.objects.filter(value__lte=min_value) self.assertEqual(qs.count(), 1) - self.assertEqual(qs[0].value, maxval) - BigInt.objects.create(value=minval) - qs = BigInt.objects.filter(value__lte=minval) + self.assertEqual(qs[0].value, min_value) + + instance = self.model(value=max_value) + instance.full_clean() + instance.save() + qs = self.model.objects.filter(value__gte=max_value) self.assertEqual(qs.count(), 1) - self.assertEqual(qs[0].value, minval) + self.assertEqual(qs[0].value, max_value) + + def test_backend_range_validation(self): + """ + Ensure that backend specific range are enforced at the model + validation level. ref #12030. + """ + field = self.model._meta.get_field('value') + internal_type = field.get_internal_type() + min_value, max_value = connection.ops.integer_field_range(internal_type) + + if min_value is not None: + instance = self.model(value=min_value - 1) + expected_message = validators.MinValueValidator.message % { + 'limit_value': min_value + } + with self.assertRaisesMessage(ValidationError, expected_message): + instance.full_clean() + instance.value = min_value + instance.full_clean() + + if max_value is not None: + instance = self.model(value=max_value + 1) + expected_message = validators.MaxValueValidator.message % { + 'limit_value': max_value + } + with self.assertRaisesMessage(ValidationError, expected_message): + instance.full_clean() + instance.value = max_value + instance.full_clean() def test_types(self): - b = BigInt(value=0) - self.assertIsInstance(b.value, six.integer_types) - b.save() - self.assertIsInstance(b.value, six.integer_types) - b = BigInt.objects.all()[0] - self.assertIsInstance(b.value, six.integer_types) + instance = self.model(value=0) + self.assertIsInstance(instance.value, six.integer_types) + instance.save() + self.assertIsInstance(instance.value, six.integer_types) + instance = self.model.objects.get() + self.assertIsInstance(instance.value, six.integer_types) def test_coercing(self): - BigInt.objects.create(value='10') - b = BigInt.objects.get(value='10') - self.assertEqual(b.value, 10) + self.model.objects.create(value='10') + instance = self.model.objects.get(value='10') + self.assertEqual(instance.value, 10) + + +class SmallIntegerFieldTests(IntegerFieldTests): + model = SmallIntegerModel + documented_range = (-32768, 32767) + + +class BigIntegerFieldTests(IntegerFieldTests): + model = BigIntegerModel + documented_range = (-9223372036854775808, 9223372036854775807) + + +class PositiveSmallIntegerFieldTests(IntegerFieldTests): + model = PositiveSmallIntegerModel + documented_range = (0, 32767) + + +class PositiveIntegerFieldTests(IntegerFieldTests): + model = PositiveIntegerModel + documented_range = (0, 2147483647) class TypeCoercionTests(test.TestCase):