From 21e559495b8255bba1e8a4429cd083246ab90457 Mon Sep 17 00:00:00 2001 From: Nick Pope Date: Mon, 11 Dec 2017 15:36:33 +0000 Subject: [PATCH] Fixed #29979, Refs #17337 -- Extracted AutoField field logic into a mixin and refactored AutoFields. This reduces duplication by allowing AutoField, BigAutoField and SmallAutoField to inherit from IntegerField, BigIntegerField and SmallIntegerField respectively. Doing so also allows for enabling the max_length warning check and minimum/maximum value validation for auto fields, as well as providing a mixin that can be used for other possible future auto field types such as a theoretical UUIDAutoField. --- django/db/backends/base/operations.py | 3 + django/db/backends/oracle/operations.py | 7 +- django/db/models/fields/__init__.py | 214 +++++++++--------- docs/releases/3.0.txt | 11 + .../test_ordinary_fields.py | 15 ++ tests/model_fields/test_autofield.py | 42 ++-- tests/model_fields/test_integerfield.py | 2 +- tests/validation/models.py | 5 +- 8 files changed, 171 insertions(+), 128 deletions(-) diff --git a/django/db/backends/base/operations.py b/django/db/backends/base/operations.py index d17dc8dcce..76abc6dcb2 100644 --- a/django/db/backends/base/operations.py +++ b/django/db/backends/base/operations.py @@ -26,6 +26,9 @@ class BaseDatabaseOperations: 'BigIntegerField': (-9223372036854775808, 9223372036854775807), 'PositiveSmallIntegerField': (0, 32767), 'PositiveIntegerField': (0, 2147483647), + 'SmallAutoField': (-32768, 32767), + 'AutoField': (-2147483648, 2147483647), + 'BigAutoField': (-9223372036854775808, 9223372036854775807), } set_operators = { 'union': 'UNION', diff --git a/django/db/backends/oracle/operations.py b/django/db/backends/oracle/operations.py index 016d4c9a59..3cad02fa41 100644 --- a/django/db/backends/oracle/operations.py +++ b/django/db/backends/oracle/operations.py @@ -16,13 +16,18 @@ from .utils import BulkInsertMapper, InsertVar, Oracle_datetime class DatabaseOperations(BaseDatabaseOperations): - # Oracle uses NUMBER(11) and NUMBER(19) for integer fields. + # Oracle uses NUMBER(5), NUMBER(11), and NUMBER(19) for integer fields. + # SmallIntegerField uses NUMBER(11) instead of NUMBER(5), which is used by + # SmallAutoField, to preserve backward compatibility. integer_field_ranges = { 'SmallIntegerField': (-99999999999, 99999999999), 'IntegerField': (-99999999999, 99999999999), 'BigIntegerField': (-9999999999999999999, 9999999999999999999), 'PositiveSmallIntegerField': (0, 99999999999), 'PositiveIntegerField': (0, 99999999999), + 'SmallAutoField': (-99999, 99999), + 'AutoField': (-99999999999, 99999999999), + 'BigAutoField': (-9999999999999999999, 9999999999999999999), } set_operators = {**BaseDatabaseOperations.set_operators, 'difference': 'MINUS'} diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index 1aad845470..6e924a4adf 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -898,110 +898,6 @@ class Field(RegisterLookupMixin): return getattr(obj, self.attname) -class AutoField(Field): - description = _("Integer") - - empty_strings_allowed = False - default_error_messages = { - 'invalid': _('“%(value)s” value must be an integer.'), - } - - def __init__(self, *args, **kwargs): - kwargs['blank'] = True - super().__init__(*args, **kwargs) - - def check(self, **kwargs): - return [ - *super().check(**kwargs), - *self._check_primary_key(), - ] - - def _check_primary_key(self): - if not self.primary_key: - return [ - checks.Error( - 'AutoFields must set primary_key=True.', - obj=self, - id='fields.E100', - ), - ] - else: - return [] - - def deconstruct(self): - name, path, args, kwargs = super().deconstruct() - del kwargs['blank'] - kwargs['primary_key'] = True - return name, path, args, kwargs - - def get_internal_type(self): - return "AutoField" - - def to_python(self, value): - if value is None: - return value - try: - return int(value) - except (TypeError, ValueError): - raise exceptions.ValidationError( - self.error_messages['invalid'], - code='invalid', - params={'value': value}, - ) - - def rel_db_type(self, connection): - return IntegerField().db_type(connection=connection) - - def validate(self, value, model_instance): - pass - - def get_db_prep_value(self, value, connection, prepared=False): - if not prepared: - value = self.get_prep_value(value) - value = connection.ops.validate_autopk_value(value) - return value - - def get_prep_value(self, value): - from django.db.models.expressions import OuterRef - value = super().get_prep_value(value) - if value is None or isinstance(value, OuterRef): - return value - try: - return int(value) - except (TypeError, ValueError) as e: - raise e.__class__( - "Field '%s' expected a number but got %r." % (self.name, value), - ) from e - - def contribute_to_class(self, cls, name, **kwargs): - assert not cls._meta.auto_field, "Model %s can't have more than one AutoField." % cls._meta.label - super().contribute_to_class(cls, name, **kwargs) - cls._meta.auto_field = self - - def formfield(self, **kwargs): - return None - - -class BigAutoField(AutoField): - description = _("Big (8 byte) integer") - - def get_internal_type(self): - return "BigAutoField" - - def rel_db_type(self, connection): - return BigIntegerField().db_type(connection=connection) - - -class SmallAutoField(AutoField): - description = _('Small integer') - - def get_internal_type(self): - return 'SmallAutoField' - - def rel_db_type(self, connection): - return SmallIntegerField().db_type(connection=connection) - - class BooleanField(Field): empty_strings_allowed = False default_error_messages = { @@ -2395,3 +2291,113 @@ class UUIDField(Field): 'form_class': forms.UUIDField, **kwargs, }) + + +class AutoFieldMixin: + + def __init__(self, *args, **kwargs): + kwargs['blank'] = True + super().__init__(*args, **kwargs) + + def check(self, **kwargs): + return [ + *super().check(**kwargs), + *self._check_primary_key(), + ] + + def _check_primary_key(self): + if not self.primary_key: + return [ + checks.Error( + 'AutoFields must set primary_key=True.', + obj=self, + id='fields.E100', + ), + ] + else: + return [] + + def deconstruct(self): + name, path, args, kwargs = super().deconstruct() + del kwargs['blank'] + kwargs['primary_key'] = True + return name, path, args, kwargs + + def validate(self, value, model_instance): + pass + + def get_db_prep_value(self, value, connection, prepared=False): + if not prepared: + value = self.get_prep_value(value) + value = connection.ops.validate_autopk_value(value) + return value + + def get_prep_value(self, value): + from django.db.models.expressions import OuterRef + return value if isinstance(value, OuterRef) else super().get_prep_value(value) + + def contribute_to_class(self, cls, name, **kwargs): + assert not cls._meta.auto_field, ( + "Model %s can't have more than one auto-generated field." + % cls._meta.label + ) + super().contribute_to_class(cls, name, **kwargs) + cls._meta.auto_field = self + + def formfield(self, **kwargs): + return None + + +class AutoFieldMeta(type): + """ + Metaclass to maintain backward inheritance compatibility for AutoField. + + It is intended that AutoFieldMixin become public API when it is possible to + create a non-integer automatically-generated field using column defaults + stored in the database. + + In many areas Django also relies on using isinstance() to check for an + automatically-generated field as a subclass of AutoField. A new flag needs + to be implemented on Field to be used instead. + + When these issues have been addressed, this metaclass could be used to + deprecate inheritance from AutoField and use of isinstance() with AutoField + for detecting automatically-generated fields. + """ + + @property + def _subclasses(self): + return (BigAutoField, SmallAutoField) + + def __instancecheck__(self, instance): + return isinstance(instance, self._subclasses) or super().__instancecheck__(instance) + + def __subclasscheck__(self, subclass): + return subclass in self._subclasses or super().__subclasscheck__(subclass) + + +class AutoField(AutoFieldMixin, IntegerField, metaclass=AutoFieldMeta): + + def get_internal_type(self): + return 'AutoField' + + def rel_db_type(self, connection): + return IntegerField().db_type(connection=connection) + + +class BigAutoField(AutoFieldMixin, BigIntegerField): + + def get_internal_type(self): + return 'BigAutoField' + + def rel_db_type(self, connection): + return BigIntegerField().db_type(connection=connection) + + +class SmallAutoField(AutoFieldMixin, SmallIntegerField): + + def get_internal_type(self): + return 'SmallAutoField' + + def rel_db_type(self, connection): + return SmallIntegerField().db_type(connection=connection) diff --git a/docs/releases/3.0.txt b/docs/releases/3.0.txt index 2e345f6fe5..5bb0691363 100644 --- a/docs/releases/3.0.txt +++ b/docs/releases/3.0.txt @@ -304,6 +304,12 @@ Models a certain (database-dependent) limit. Values from ``1`` to ``32767`` are safe in all databases supported by Django. +* :class:`~django.db.models.AutoField`, + :class:`~django.db.models.BigAutoField`, and + :class:`~django.db.models.SmallAutoField` now inherit from + ``IntegerField``, ``BigIntegerField`` and ``SmallIntegerField`` respectively. + System checks and validators are now also properly inherited. + * :attr:`.FileField.upload_to` now supports :class:`pathlib.Path`. Requests and Responses @@ -402,6 +408,11 @@ backends. * ``DatabaseOperations.return_insert_id()`` now requires an additional ``field`` argument with the model field. +* Entries for ``AutoField``, ``BigAutoField``, and ``SmallAutoField`` are added + to ``DatabaseOperations.integer_field_ranges`` to support the integer range + validators on these field types. Third-party backends may need to customize + the default entries. + :mod:`django.contrib.admin` --------------------------- diff --git a/tests/invalid_models_tests/test_ordinary_fields.py b/tests/invalid_models_tests/test_ordinary_fields.py index a3231ee36a..5bb1847a70 100644 --- a/tests/invalid_models_tests/test_ordinary_fields.py +++ b/tests/invalid_models_tests/test_ordinary_fields.py @@ -38,6 +38,21 @@ class AutoFieldTests(SimpleTestCase): ), ]) + def test_max_length_warning(self): + class Model(models.Model): + auto = models.AutoField(primary_key=True, max_length=2) + + field = Model._meta.get_field('auto') + self.assertEqual(field.check(), [ + DjangoWarning( + "'max_length' is ignored when used with %s." + % field.__class__.__name__, + hint="Remove 'max_length' from field", + obj=field, + id='fields.W122', + ), + ]) + @isolate_apps('invalid_models_tests') class BinaryFieldTests(SimpleTestCase): diff --git a/tests/model_fields/test_autofield.py b/tests/model_fields/test_autofield.py index 73220ae62b..db8e79e34b 100644 --- a/tests/model_fields/test_autofield.py +++ b/tests/model_fields/test_autofield.py @@ -1,32 +1,32 @@ -from django.test import TestCase +from django.db import models +from django.test import SimpleTestCase from .models import AutoModel, BigAutoModel, SmallAutoModel +from .test_integerfield import ( + BigIntegerFieldTests, IntegerFieldTests, SmallIntegerFieldTests, +) -class AutoFieldTests(TestCase): +class AutoFieldTests(IntegerFieldTests): model = AutoModel - def test_invalid_value(self): - tests = [ - (TypeError, ()), - (TypeError, []), - (TypeError, {}), - (TypeError, set()), - (TypeError, object()), - (TypeError, complex()), - (ValueError, 'non-numeric string'), - (ValueError, b'non-numeric byte-string'), - ] - for exception, value in tests: - with self.subTest(value=value): - msg = "Field 'value' expected a number but got %r." % (value,) - with self.assertRaisesMessage(exception, msg): - self.model.objects.create(value=value) - -class BigAutoFieldTests(AutoFieldTests): +class BigAutoFieldTests(BigIntegerFieldTests): model = BigAutoModel -class SmallAutoFieldTests(AutoFieldTests): +class SmallAutoFieldTests(SmallIntegerFieldTests): model = SmallAutoModel + + +class AutoFieldInheritanceTests(SimpleTestCase): + + def test_isinstance_of_autofield(self): + for field in (models.BigAutoField, models.SmallAutoField): + with self.subTest(field.__name__): + self.assertIsInstance(field(), models.AutoField) + + def test_issubclass_of_autofield(self): + for field in (models.BigAutoField, models.SmallAutoField): + with self.subTest(field.__name__): + self.assertTrue(issubclass(field, models.AutoField)) diff --git a/tests/model_fields/test_integerfield.py b/tests/model_fields/test_integerfield.py index c72cb48015..c0bb7627cf 100644 --- a/tests/model_fields/test_integerfield.py +++ b/tests/model_fields/test_integerfield.py @@ -125,7 +125,7 @@ class IntegerFieldTests(TestCase): ranged_value_field.run_validators(max_backend_value + 1) def test_types(self): - instance = self.model(value=0) + instance = self.model(value=1) self.assertIsInstance(instance.value, int) instance.save() self.assertIsInstance(instance.value, int) diff --git a/tests/validation/models.py b/tests/validation/models.py index 953370bc37..e8e18cfbce 100644 --- a/tests/validation/models.py +++ b/tests/validation/models.py @@ -130,4 +130,7 @@ try: 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 AutoField." +assert str(assertion_error) == ( + "Model validation.MultipleAutoFields can't have more than one " + "auto-generated field." +)