From 15c9b36c473555ce3d09b075fb91584cc8be4abc Mon Sep 17 00:00:00 2001 From: Daniel Townsend Date: Tue, 14 Jan 2025 17:51:37 +0000 Subject: [PATCH 1/2] if a column is nullable, make it optional by default in `create_pydantic_model` --- docs/src/piccolo/serialization/index.rst | 37 ++++++-- piccolo/columns/base.py | 4 +- piccolo/utils/pydantic.py | 14 ++- tests/apps/fixtures/commands/test_shared.py | 1 + tests/utils/test_pydantic.py | 94 ++++++++++++++++++--- 5 files changed, 127 insertions(+), 23 deletions(-) diff --git a/docs/src/piccolo/serialization/index.rst b/docs/src/piccolo/serialization/index.rst index 446e6f20f..f7b2effda 100644 --- a/docs/src/piccolo/serialization/index.rst +++ b/docs/src/piccolo/serialization/index.rst @@ -191,23 +191,45 @@ So if we want to disallow extra fields, we can do: Required fields ~~~~~~~~~~~~~~~ -You can specify which fields are required using the ``required`` -argument of :class:`Column `. For example: +If a column has ``null=True``, then it creates an ``Optional`` field in the +Pydantic model: .. code-block:: python class Band(Table): - name = Varchar(required=True) + name = Varchar(null=True) + + BandModel = create_pydantic_model(Band) + + # This is equivalent to: + from pydantic import BaseModel + + class BandModel(BaseModel): + name: Optional[str] = None + +If the column has ``null=True``, but we still want the user to provide a value, +then we can pass ``required=True`` to :class:`Column `: + +.. code-block:: python + + class Band(Table): + name = Varchar(null=True, required=True) BandModel = create_pydantic_model(Band) + # This is equivalent to: + from pydantic import BaseModel + + class BandModel(BaseModel): + name: str + # Omitting the field raises an error: >>> BandModel() ValidationError - name field required -You can override this behaviour using the ``all_optional`` argument. An example -use case is when you have a model which is used for filtering, then you'll want -all fields to be optional. +If you don't want any of your fields to be required, you can use the +``all_optional`` argument. An example use case is when you have a model which +is used for filtering: .. code-block:: python @@ -217,11 +239,10 @@ all fields to be optional. BandFilterModel = create_pydantic_model( Band, all_optional=True, - model_name='BandFilterModel', ) # This no longer raises an exception: - >>> BandModel() + >>> BandFilterModel() Subclassing the model ~~~~~~~~~~~~~~~~~~~~~ diff --git a/piccolo/columns/base.py b/piccolo/columns/base.py index 4725b78ad..1232e4382 100644 --- a/piccolo/columns/base.py +++ b/piccolo/columns/base.py @@ -153,7 +153,7 @@ class ColumnMeta: unique: bool = False index: bool = False index_method: IndexMethod = IndexMethod.btree - required: bool = False + required: bool = ... help_text: t.Optional[str] = None choices: t.Optional[t.Type[Enum]] = None secret: bool = False @@ -459,7 +459,7 @@ def __init__( unique: bool = False, index: bool = False, index_method: IndexMethod = IndexMethod.btree, - required: bool = False, + required: bool = ..., help_text: t.Optional[str] = None, choices: t.Optional[t.Type[Enum]] = None, db_column_name: t.Optional[str] = None, diff --git a/piccolo/utils/pydantic.py b/piccolo/utils/pydantic.py index 3c88c8764..8c28f9d28 100644 --- a/piccolo/utils/pydantic.py +++ b/piccolo/utils/pydantic.py @@ -243,10 +243,20 @@ def create_pydantic_model( for column in piccolo_columns: column_name = column._meta.name - is_optional = True if all_optional else not column._meta.required + ####################################################################### + # Work out if the field should be optional + + if all_optional: + is_optional = True + elif column._meta.required is not ...: + # The user can force the field to be optional or not, irrespective + # of whether it's nullable in the database. + is_optional = not column._meta.required + else: + is_optional = column._meta.null ####################################################################### - # Work out the column type + # Work out the field type if isinstance(column, (JSON, JSONB)): if deserialize_json: diff --git a/tests/apps/fixtures/commands/test_shared.py b/tests/apps/fixtures/commands/test_shared.py index 34e2af4ee..ccbf99e3d 100644 --- a/tests/apps/fixtures/commands/test_shared.py +++ b/tests/apps/fixtures/commands/test_shared.py @@ -50,6 +50,7 @@ def test_shared(self): "unique_col": "hello", "null_col": None, "not_null_col": "hello", + "double_precision_col": 1.0, } ], } diff --git a/tests/utils/test_pydantic.py b/tests/utils/test_pydantic.py index 82096603d..20f6d6c88 100644 --- a/tests/utils/test_pydantic.py +++ b/tests/utils/test_pydantic.py @@ -48,12 +48,17 @@ class Director(Table): pydantic_model = create_pydantic_model(table=Director) self.assertEqual( - pydantic_model.model_json_schema()["properties"]["email"]["anyOf"][ - 0 - ]["format"], + pydantic_model.model_json_schema()["properties"]["email"][ + "format" + ], "email", ) + self.assertEqual( + pydantic_model.model_json_schema()["properties"]["email"]["type"], + "string", + ) + with self.assertRaises(ValidationError): pydantic_model(email="not a valid email") @@ -121,8 +126,8 @@ class Band(Table): self.assertEqual( pydantic_model.model_json_schema()["properties"]["members"][ - "anyOf" - ][0]["items"]["type"], + "items" + ]["type"], "string", ) @@ -132,7 +137,7 @@ def test_multidimensional_array(self): """ class Band(Table): - members = Array(Array(Varchar(length=255)), required=True) + members = Array(Array(Varchar(length=255))) pydantic_model = create_pydantic_model(table=Band) @@ -223,8 +228,8 @@ class Concert(Table): self.assertEqual( pydantic_model.model_json_schema()["properties"]["start_time"][ - "anyOf" - ][0]["format"], + "format" + ], "time", ) @@ -281,13 +286,80 @@ class Ticket(Table): self.assertEqual(json, '{"code":"' + str(ticket_.code) + '"}') self.assertEqual( - pydantic_model.model_json_schema()["properties"]["code"]["anyOf"][ - 0 - ]["format"], + pydantic_model.model_json_schema()["properties"]["code"]["format"], "uuid", ) +class TestRequired(TestCase): + """ + Using the `required` attribute, we can force the field to be required or + not (overriding `column._meta.null`) + """ + + def test_required(self): + """ + Make a null column required. + """ + + class Director(Table): + name = Varchar(null=True, required=True) + + pydantic_model = create_pydantic_model(table=Director) + + self.assertEqual( + pydantic_model.model_json_schema()["properties"]["name"]["type"], + "string", + ) + + with self.assertRaises(pydantic.ValidationError): + pydantic_model(name=None) + + def test_not_required(self): + """ + Make a column not required. + """ + + class Director(Table): + name = Varchar(null=False, required=False) + + pydantic_model = create_pydantic_model(table=Director) + + self.assertEqual( + pydantic_model.model_json_schema()["properties"]["name"]["anyOf"], + [ + {"maxLength": 255, "type": "string"}, + {"type": "null"}, + ], + ) + + # Shouldn't raise an error: + pydantic_model(name=None) + + def test_all_optional(self): + """ + Makes all columns not required - useful for filters. + """ + + class Director(Table): + name = Varchar(null=False) + + pydantic_model = create_pydantic_model( + table=Director, all_optional=True + ) + + self.assertEqual( + pydantic_model.model_json_schema()["properties"]["name"]["anyOf"], + [ + {"maxLength": 255, "type": "string"}, + {"type": "null"}, + ], + ) + + # Shouldn't raise an error: + pydantic_model(name=None) + + class TestColumnHelpText(TestCase): """ Make sure that columns with `help_text` attribute defined have the From 3cbae15c47d2c74efa9bb7cd2f493e718274ab82 Mon Sep 17 00:00:00 2001 From: Daniel Townsend Date: Tue, 14 Jan 2025 18:32:28 +0000 Subject: [PATCH 2/2] fix linter errors --- piccolo/columns/base.py | 4 ++-- piccolo/utils/pydantic.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/piccolo/columns/base.py b/piccolo/columns/base.py index 1232e4382..bfb7e7495 100644 --- a/piccolo/columns/base.py +++ b/piccolo/columns/base.py @@ -153,7 +153,7 @@ class ColumnMeta: unique: bool = False index: bool = False index_method: IndexMethod = IndexMethod.btree - required: bool = ... + required: t.Optional[bool] = None help_text: t.Optional[str] = None choices: t.Optional[t.Type[Enum]] = None secret: bool = False @@ -459,7 +459,7 @@ def __init__( unique: bool = False, index: bool = False, index_method: IndexMethod = IndexMethod.btree, - required: bool = ..., + required: t.Optional[bool] = None, help_text: t.Optional[str] = None, choices: t.Optional[t.Type[Enum]] = None, db_column_name: t.Optional[str] = None, diff --git a/piccolo/utils/pydantic.py b/piccolo/utils/pydantic.py index 8c28f9d28..36630235c 100644 --- a/piccolo/utils/pydantic.py +++ b/piccolo/utils/pydantic.py @@ -248,7 +248,7 @@ def create_pydantic_model( if all_optional: is_optional = True - elif column._meta.required is not ...: + elif column._meta.required is not None: # The user can force the field to be optional or not, irrespective # of whether it's nullable in the database. is_optional = not column._meta.required