Fix extra fields behavior for TypedDict and make additionalProperties explicit in JSON schema #6115
Conversation
… explicit in JSON schema
adriangb
changed the title
Deploying with
|
| Latest commit: |
efd91df
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e93c944c.pydantic-docs2.pages.dev |
| Branch Preview URL: | https://extra-json-schema.pydantic-docs2.pages.dev |
|
please review |
| } | ||
|
|
||
|
|
||
| def test_dataclass_with_extra_forbid(): |
There was a problem hiding this comment.
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_propertiesand similar for typeddict, dataclass
There was a problem hiding this comment.
I just fixed the one name
pydantic/json_schema.py
Outdated
| if extra == 'forbid': | ||
| additional_properties = False | ||
| elif extra == 'allow': | ||
| additional_properties = True |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
(This could also be handled in _update_class_schema, I just see it was passed through without modification down there)
There was a problem hiding this comment.
True seems ot be valid, at least according to SwaggerUI
Closes #6082
Selected Reviewer: @Kludex