Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IntField: add MinMaxValidator #1868

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
2 changes: 2 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ Fixed
Changed
^^^^^^^
- add benchmarks for `get_for_dialect` (#1862)
- optimize `get_for_dialect` (#1863)
- add validator to `IntField` (#1866)

0.24
====
Expand Down
2 changes: 1 addition & 1 deletion tests/fields/test_fk.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
from tests import testmodels
from tortoise.contrib import test
from tortoise.exceptions import (
IntegrityError,
NoValuesFetched,
OperationalError,
IntegrityError,
ValidationError,
)
from tortoise.queryset import QuerySet
Expand Down
9 changes: 9 additions & 0 deletions tests/fields/test_fk_with_unique.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,15 @@ async def test_delete_by_name(self):
with self.assertRaises(IntegrityError):
await student.save()

async def test_update_by_nonexistent_id(self):
if not self._db.capabilities.supports_transactions:
raise test.SkipTest("transactions not supported")
school = await testmodels.School.create(id=1024, name="School1")
student = await testmodels.Student.create(name="Sang-Heon Jeon", school=school)
student.school_id = 1025
with self.assertRaises(IntegrityError):
await student.save()

async def test_student__uninstantiated_create(self):
school = await testmodels.School(id=1024, name="School1")
with self.assertRaisesRegex(OperationalError, "You should first call .save()"):
Expand Down
49 changes: 48 additions & 1 deletion tests/fields/test_int.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,31 @@
from tests import testmodels
from tortoise.contrib import test
from tortoise.exceptions import IntegrityError
from tortoise.exceptions import ValidationError, IntegrityError
from tortoise.expressions import F
from tortoise.fields import IntField, SmallIntField, BigIntField


class TestIntFields(test.TestCase):
async def test_empty(self):
with self.assertRaises(IntegrityError):
await testmodels.IntFields.create()

async def test_create_too_great(self):
too_great = IntField.LE + 1
with self.assertRaisesRegex(
ValidationError,
"Value should be less or equal to 2147483647 and greater or equal to -2147483648",
):
await testmodels.IntFields.create(intnum=too_great)

async def test_create_too_small(self):
too_small = IntField.GE - 1
with self.assertRaisesRegex(
ValidationError,
"Value should be less or equal to 2147483647 and greater or equal to -2147483648",
):
await testmodels.IntFields.create(intnum=too_small)

async def test_create(self):
obj0 = await testmodels.IntFields.create(intnum=2147483647)
obj = await testmodels.IntFields.get(id=obj0.id)
Expand Down Expand Up @@ -65,6 +82,20 @@ async def test_empty(self):
with self.assertRaises(IntegrityError):
await testmodels.SmallIntFields.create()

async def test_create_too_great(self):
too_great = SmallIntField.LE + 1
with self.assertRaisesRegex(
ValidationError, "Value should be less or equal to 32767 and greater or equal to -32768"
):
await testmodels.SmallIntFields.create(smallintnum=too_great)

async def test_create_too_small(self):
too_small = SmallIntField.GE - 1
with self.assertRaisesRegex(
ValidationError, "Value should be less or equal to 32767 and greater or equal to -32768"
):
await testmodels.SmallIntFields.create(smallintnum=too_small)

async def test_create(self):
obj0 = await testmodels.SmallIntFields.create(smallintnum=32767)
obj = await testmodels.SmallIntFields.get(id=obj0.id)
Expand Down Expand Up @@ -107,6 +138,22 @@ async def test_empty(self):
with self.assertRaises(IntegrityError):
await testmodels.BigIntFields.create()

async def test_create_too_great(self):
too_great = BigIntField.LE + 1
with self.assertRaisesRegex(
ValidationError,
"Value should be less or equal to 9223372036854775807 and greater or equal to -9223372036854775808",
):
await testmodels.BigIntFields.create(intnum=too_great)

async def test_create_too_small(self):
too_small = BigIntField.GE - 1
with self.assertRaisesRegex(
ValidationError,
"Value should be less or equal to 9223372036854775807 and greater or equal to -9223372036854775808",
):
await testmodels.BigIntFields.create(intnum=too_small)

async def test_create(self):
obj0 = await testmodels.BigIntFields.create(intnum=9223372036854775807)
obj = await testmodels.BigIntFields.get(id=obj0.id)
Expand Down
9 changes: 9 additions & 0 deletions tests/fields/test_o2o_with_unique.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,15 @@ async def test_delete_by_name(self):
with self.assertRaises(IntegrityError):
await principal.save()

async def test_update_by_nonexistent_id(self):
if not self._db.capabilities.supports_transactions:
raise test.SkipTest("transactions not supported")
school = await testmodels.School.create(id=1024, name="School1")
principal = await testmodels.Principal.create(name="Sang-Heon Jeon", school=school)
principal.school_id = 1025
with self.assertRaises(IntegrityError):
await principal.save()

async def test_principal__uninstantiated_create(self):
school = await testmodels.School(id=1024, name="School1")
with self.assertRaisesRegex(OperationalError, "You should first call .save()"):
Expand Down
6 changes: 4 additions & 2 deletions tortoise/fields/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ def validate(self, value: Any) -> None:
"""
for v in self.validators:
if self.null and value is None:
continue
return
henadzit marked this conversation as resolved.
Show resolved Hide resolved
try:
if isinstance(value, Enum):
v(value.value)
Expand Down Expand Up @@ -358,7 +358,9 @@ def get_for_dialect(self, dialect: str, key: str) -> Any:
if isinstance(dialect_value, property):
return getattr(dialect_cls(self), key)
return dialect_value
return getattr(self, key, None) # there is nothing special defined, return the value of self
return getattr(
self, key, None
) # there is nothing special defined, return the value of self

def describe(self, serializable: bool) -> dict:
"""
Expand Down
70 changes: 52 additions & 18 deletions tortoise/fields/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from pypika_tortoise.terms import Term

from tortoise import timezone
from tortoise.exceptions import ConfigurationError, FieldError
from tortoise.exceptions import ConfigurationError, FieldError, ValidationError
from tortoise.fields.base import Field
from tortoise.timezone import get_default_timezone, get_timezone, get_use_tz, localtime
from tortoise.validators import MaxLengthValidator
Expand Down Expand Up @@ -71,21 +71,57 @@ class IntField(Field[int], int):

``primary_key`` (bool):
True if field is Primary Key.
``ge`` (int):
Values may be greater or even, defaults to -2147483648
``le`` (int):
Values may be less or even, defaults to 2147483647
"""

SQL_TYPE = "INT"
allows_generated = True
GE = -2147483648
LE = 2147483647

def __init__(self, primary_key: Optional[bool] = None, **kwargs: Any) -> None:
def __init__(
self,
primary_key: Optional[bool] = None,
ge: Optional[int] = None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extending the field configuration with ge and le introduces the second way of setting up validation. I think we should have a single way of doing it to keep things simple - through validators. If some one needs specific validation, they can add a validator. So I suggest to remove these options from the constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ge and le are currently baked in in the constraints property. Instead of that I wanted them to be customizable, a bit like with max_char in CharField where this is reflected during validation and also by the constraints property.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ge and le are currently baked in in the constraints property. Instead of that I wanted them to be customizable, a bit like with max_char

The int field constraints are related to the nature of the data type. I think it is different from business logic validation and we already have a way of doing it. We shouldn't have two ways of doing the same thing.

le: Optional[int] = None,
**kwargs: Any,
) -> None:
if primary_key or kwargs.get("pk"):
kwargs["generated"] = bool(kwargs.get("generated", True))
super().__init__(primary_key=primary_key, **kwargs)
if ge is None:
self.ge = self.GE
else:
if ge < self.GE:
raise ConfigurationError(f"'ge' must be >= {self.GE}")
self.ge = ge
if le is None:
self.le = self.LE
else:
if le > self.LE:
raise ConfigurationError(f"'le' must be <= {self.LE}")
self.le = le

def to_db_value(self, value: Any, instance: "Union[Type[Model], Model]") -> Any:
if value is not None:
henadzit marked this conversation as resolved.
Show resolved Hide resolved
if not isinstance(value, self.field_type):
value = self.field_type(value) # pylint: disable=E1102
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which cases is it required?

if not self.ge <= value <= self.le:
raise ValidationError(
f"Value should be less or equal to {self.le} and greater or equal to {self.ge}"
)

self.validate(value)
return value

@property
def constraints(self) -> dict:
return {
"ge": -2147483648,
"le": 2147483647,
"ge": self.ge,
"le": self.le,
}

class _db_postgres:
Expand All @@ -110,16 +146,15 @@ class BigIntField(IntField):

``primary_key`` (bool):
True if field is Primary Key.
``ge`` (int):
Values may be greater or even, defaults to -9223372036854775808
``le`` (int):
Values may be less or even, defaults to 9223372036854775807
"""

SQL_TYPE = "BIGINT"

@property
def constraints(self) -> dict:
return {
"ge": -9223372036854775808,
"le": 9223372036854775807,
}
GE = -9223372036854775808
LE = 9223372036854775807

class _db_postgres:
GENERATED_SQL = "BIGSERIAL NOT NULL PRIMARY KEY"
Expand All @@ -141,16 +176,15 @@ class SmallIntField(IntField):

``primary_key`` (bool):
True if field is Primary Key.
``ge`` (int):
Values may be greater or even, defaults to -32768
``le`` (int):
Values may be less or even, defaults to 32767
"""

SQL_TYPE = "SMALLINT"

@property
def constraints(self) -> dict:
return {
"ge": -32768,
"le": 32767,
}
GE = -32768
LE = 32767

class _db_postgres:
GENERATED_SQL = "SMALLSERIAL NOT NULL PRIMARY KEY"
Expand Down