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.

Sign up for GitHub

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

Jump to bottom

Fix extra fields behavior for TypedDict and make additionalProperties explicit in JSON schema #6115

Merged
merged 3 commits into from Jun 13, 2023
Merged

Fix extra fields behavior for TypedDict and make additionalProperties explicit in JSON schema #6115

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d63e8c9
Fix extra fields behavior for TypedDict and make additionalProperties…
adriangb Jun 13, 2023
faa1d4a
Remove mode logic:
adriangb Jun 13, 2023
efd91df
coalesce logic
adriangb Jun 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 1 addition & 3 deletions docs/usage/models.md
View file
Open in desktop
Expand Up @@ -791,11 +791,9 @@ try:
except ValidationError as e:
print(e)
"""
2 validation errors for list[typed-dict]
1 validation error for list[typed-dict]
0.id
Input should be a valid integer, unable to parse string as an integer [type=int_parsing, input_value='wrong', input_type=str]
0.other
Extra inputs are not permitted [type=extra_forbidden, input_value='no', input_type=str]
"""
```

Expand Down
1 change: 0 additions & 1 deletion pydantic/_internal/_generate_schema.py
View file
Open in desktop
Expand Up @@ -746,7 +746,6 @@ def _typed_dict_schema(self, typed_dict_cls: Any, origin: Any) -> core_schema.Co
td_schema = core_schema.typed_dict_schema(
fields,
computed_fields=[self._computed_field_schema(d) for d in decorators.computed_fields.values()],
extra_behavior='forbid',
ref=typed_dict_ref,
metadata=metadata,
config=core_config,
Expand Down
48 changes: 35 additions & 13 deletions pydantic/json_schema.py
View file
Open in desktop
Expand Up @@ -24,7 +24,7 @@
)

import pydantic_core
from pydantic_core import CoreSchema, PydanticOmit, core_schema
from pydantic_core import CoreConfig, CoreSchema, PydanticOmit, core_schema
from pydantic_core.core_schema import ComputedField
from typing_extensions import Literal, assert_never

Expand Down Expand Up @@ -888,7 +888,16 @@ def typed_dict_schema(self, schema: core_schema.TypedDictSchema) -> JsonSchemaVa
]
if self.mode == 'serialization':
named_required_fields.extend(self._name_required_computed_fields(schema.get('computed_fields', [])))
return self._named_required_fields_schema(named_required_fields)
json_schema = self._named_required_fields_schema(named_required_fields)
config: CoreConfig | None = schema.get('config', None)

extra = (config or {}).get('extra_fields_behavior', 'ignore')
if extra == 'forbid':
json_schema['additionalProperties'] = False
elif extra == 'allow':
json_schema['additionalProperties'] = True

return json_schema

@staticmethod
def _name_required_computed_fields(
Expand Down Expand Up @@ -962,19 +971,26 @@ def model_schema(self, schema: core_schema.ModelSchema) -> JsonSchemaValue:
cls = cast('type[BaseModel]', schema['cls'])
config = cls.model_config
title = config.get('title')
forbid_additional_properties = config.get('extra') == 'forbid'
extra = config.get('extra', 'ignore')

extra = config.get('extra', 'ignore')
if extra == 'forbid':
additional_properties = False
elif extra == 'allow':
additional_properties = True
Copy link
Contributor

@dmontagu dmontagu Jun 13, 2023 •
edited

Choose a reason for hiding this comment

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

Did you see somewhere saying that this should be set to True when additionalProperties are allowed? Looking at https://json-schema.org/understanding-json-schema/reference/object.html#additional-properties I only see it saying it should be False or a valid JSON schema. (That's why in my dmontagu/6082 branch I set it to {}, which is the JSON schema for Any.)

If True means the same thing as {} here then this is fine, I just haven't seen that before.

Copy link
Contributor

Choose a reason for hiding this comment

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

(This could also be handled in _update_class_schema, I just see it was passed through without modification down there)

Copy link
Member Author

Choose a reason for hiding this comment

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

True seems ot be valid, at least according to SwaggerUI

else:
additional_properties = None

json_schema_extra = config.get('json_schema_extra')
json_schema = self._update_class_schema(
json_schema, title, forbid_additional_properties, cls, json_schema_extra
)
json_schema = self._update_class_schema(json_schema, title, additional_properties, cls, json_schema_extra)

return json_schema

def _update_class_schema(
self,
json_schema: JsonSchemaValue,
title: str | None,
forbid_additional_properties: bool,
additional_properties: bool | None,
cls: type[Any],
json_schema_extra: dict[str, Any] | JsonSchemaExtraCallable | None,
) -> JsonSchemaValue:
Expand All @@ -987,8 +1003,8 @@ def _update_class_schema(
# referenced_schema['title'] = title
schema_to_update.setdefault('title', title)

if forbid_additional_properties:
schema_to_update['additionalProperties'] = False
if additional_properties is not None:
schema_to_update['additionalProperties'] = additional_properties

if isinstance(json_schema_extra, (staticmethod, classmethod)):
# In older versions of python, this is necessary to ensure staticmethod/classmethods are callable
Expand Down Expand Up @@ -1062,11 +1078,17 @@ def dataclass_schema(self, schema: core_schema.DataclassSchema) -> JsonSchemaVal
config: ConfigDict = getattr(cls, '__pydantic_config__', cast('ConfigDict', {}))

title = config.get('title') or cls.__name__
forbid_additional_properties = config.get('extra') == 'forbid'

extra = config.get('extra', 'ignore')
if extra == 'forbid':
additional_properties = False
elif extra == 'allow':
additional_properties = True
else:
additional_properties = None

json_schema_extra = config.get('json_schema_extra')
json_schema = self._update_class_schema(
json_schema, title, forbid_additional_properties, cls, json_schema_extra
)
json_schema = self._update_class_schema(json_schema, title, additional_properties, cls, json_schema_extra)

# Dataclass-specific handling of description
if is_dataclass(cls) and not hasattr(cls, '__pydantic_validator__'):
Expand Down
114 changes: 113 additions & 1 deletion tests/test_json_schema.py
View file
Open in desktop
Expand Up @@ -31,8 +31,9 @@

import pytest
from pydantic_core import CoreSchema, core_schema, to_json
from typing_extensions import Annotated, Literal
from typing_extensions import Annotated, Literal, TypedDict

import pydantic
from pydantic import (
BaseModel,
Field,
Expand Down Expand Up @@ -2083,6 +2084,117 @@ class Model(BaseModel):
}


def test_model_with_extra_allow():
class Model(BaseModel):
model_config = ConfigDict(extra='allow')
a: str

assert Model.model_json_schema() == {
'title': 'Model',
'type': 'object',
'properties': {'a': {'title': 'A', 'type': 'string'}},
'required': ['a'],
'additionalProperties': True,
}


def test_model_with_extra_ignore():
class Model(BaseModel):
model_config = ConfigDict(extra='ignore')
a: str

assert Model.model_json_schema() == {
'title': 'Model',
'type': 'object',
'properties': {'a': {'title': 'A', 'type': 'string'}},
'required': ['a'],
}


def test_dataclass_with_extra_allow():
@pydantic.dataclasses.dataclass
class Model:
__pydantic_config__ = ConfigDict(extra='allow')
a: str

assert TypeAdapter(Model).json_schema() == {
'title': 'Model',
'type': 'object',
'properties': {'a': {'title': 'A', 'type': 'string'}},
'required': ['a'],
'additionalProperties': True,
}


def test_dataclass_with_extra_ignore():
@pydantic.dataclasses.dataclass
class Model:
__pydantic_config__ = ConfigDict(extra='ignore')
a: str

assert TypeAdapter(Model).json_schema() == {
'title': 'Model',
'type': 'object',
'properties': {'a': {'title': 'A', 'type': 'string'}},
'required': ['a'],
}


def test_dataclass_with_extra_forbid():
Copy link
Contributor

@dmontagu dmontagu Jun 13, 2023 •
edited

Choose a reason for hiding this comment

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

There are at least a couple cases with mismatched test names and contents (including at least this one and test_typeddict_with_extra_forbid); I would suggest parametrizing to make it easier to review:

@pytest.mark.parametrize(
    'extra,additional_properties', [('allow', {}), ('ignore', None), ('forbid', False), (None, None)]
)
def test_basemodel_extra(extra, additional_properties):
    class A(BaseModel):
        if extra is not None:
            model_config = dict(extra=extra)
        data: str

    json_schema = A.model_json_schema()
    if additional_properties is None:
        assert 'additionalProperties' not in json_schema
    else:
        assert json_schema['additionalProperties'] == additional_properties

and similar for typeddict, dataclass

Copy link
Member Author

Choose a reason for hiding this comment

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

I just fixed the one name

@pydantic.dataclasses.dataclass
class Model:
__pydantic_config__ = ConfigDict(extra='ignore')
a: str

assert TypeAdapter(Model).json_schema() == {
'title': 'Model',
'type': 'object',
'properties': {'a': {'title': 'A', 'type': 'string'}},
'required': ['a'],
}


def test_typeddict_with_extra_allow():
class Model(TypedDict):
__pydantic_config__ = ConfigDict(extra='allow') # type: ignore
a: str

assert TypeAdapter(Model).json_schema() == {
'title': 'Model',
'type': 'object',
'properties': {'a': {'title': 'A', 'type': 'string'}},
'required': ['a'],
'additionalProperties': True,
}


def test_typeddict_with_extra_ignore():
class Model(TypedDict):
__pydantic_config__ = ConfigDict(extra='ignore') # type: ignore
a: str

assert TypeAdapter(Model).json_schema() == {
'title': 'Model',
'type': 'object',
'properties': {'a': {'title': 'A', 'type': 'string'}},
'required': ['a'],
}


def test_typeddict_with_extra_forbid():
@pydantic.dataclasses.dataclass
class Model:
__pydantic_config__ = ConfigDict(extra='ignore')
a: str

assert TypeAdapter(Model).json_schema() == {
'title': 'Model',
'type': 'object',
'properties': {'a': {'title': 'A', 'type': 'string'}},
'required': ['a'],
}


@pytest.mark.parametrize(
'annotation,kwargs,field_schema',
[
Expand Down
18 changes: 16 additions & 2 deletions tests/test_types_typeddict.py
View file
Open in desktop
Expand Up @@ -176,10 +176,24 @@ class User(TypedDict):
name: str
age: int

val = TypeAdapter(User)
ta = TypeAdapter(User)

assert ta.validate_python({'name': 'pika', 'age': 7, 'rank': 1}) == {'name': 'pika', 'age': 7}

class UserExtraAllow(User):
__pydantic_config__ = ConfigDict(extra='allow')

ta = TypeAdapter(UserExtraAllow)

assert ta.validate_python({'name': 'pika', 'age': 7, 'rank': 1}) == {'name': 'pika', 'age': 7, 'rank': 1}

class UserExtraForbid(User):
__pydantic_config__ = ConfigDict(extra='forbid')

ta = TypeAdapter(UserExtraForbid)

with pytest.raises(ValidationError) as exc_info:
val.validate_python({'name': 'pika', 'age': 7, 'rank': 1})
ta.validate_python({'name': 'pika', 'age': 7, 'rank': 1})
# insert_assert(exc_info.value.errors(include_url=False))
assert exc_info.value.errors(include_url=False) == [
{'type': 'extra_forbidden', 'loc': ('rank',), 'msg': 'Extra inputs are not permitted', 'input': 1}
Expand Down