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

Allow dicts to have both patternProperties and additionalProperties #4641

Merged
merged 1 commit into from Oct 27, 2022

Conversation

jparise
Copy link

@jparise jparise commented Oct 22, 2022

Support for patternProperties was introduced in #332, but that logic unfortunately made patternProperties and additionalProperties mutually exclusive. JSON Schema supports their combination:

https://json-schema.org/understanding-json-schema/reference/object.html#additional-properties

This prevented well-typed code generation for dictionaries that use regex-constrained string keys.

@jparise
Copy link
Author

jparise commented Oct 22, 2022

This patch is for the v1.10.x branch. The main (v2) patch is #4642.

Support for `patternProperties` was introduced in pydantic#332, but that logic
unfortunately made `patternProperties` and `additionalProperties`
mutually exclusive. JSON Schema supports their combination:

https://json-schema.org/understanding-json-schema/reference/object.html#additional-properties

This prevented well-typed code generation for dictionaries that use
regex-constrained string keys.
@jparise
Copy link
Author

jparise commented Oct 22, 2022

The failing FastAPI test in this branch appears to be unrelated to this change. It's dying in an SQLAlchemy code path (sqlalchemy/engine/cursor.py:164: ValueError).

@samuelcolvin samuelcolvin merged commit 36be4c9 into pydantic:1.10.X-fixes Oct 27, 2022
@samuelcolvin
Copy link
Member

thanks so much.

@jparise jparise deleted the dict-properties branch October 27, 2022 23:12
@matejetz
Copy link

matejetz commented Mar 19, 2023

i think this introduced inconsistency regarding the json schema vs pydantic validation behavior.

in the following, pydantic rightfully raises a ValidationError

class Model(BaseModel):
    a: Dict[constr(regex="^text$"), str]

Model(a={'foo': 'bar'})  # correctly raises ValidationError

however, the json schema produced by Model.schema() will now validate the following, as 'additionalProperties': { 'type': 'string'} just allows any string properties.

{
  "a": {
    "foo": "bar"
  }
}

i also doubt this is the expected behavior when using a regex-constrained dict key. instead, imho, additionalProperties should be false by default and might possibly be changed by passing some param to Field.

@samuelcolvin
Copy link
Member

What version are you using? Might be worth trying on main.

We can't really make a change to this in v1, but we can get it right in V2.

@matejetz
Copy link

sorry, i forgot to include that my comment concerns v1, which this PR was also targeting.
why do you think you can't change this, i'd almost go as far and classify this as a bug or at least unwanted/ unexpected behavior.

i did not have a chance to take a look at v2 yet but will definitely do, looks very promising. love the lib! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants