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

Fix json validation of complex types in strict models #818

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JacobHayes
Copy link

@JacobHayes JacobHayes commented Feb 25, 2024

I'd like to use SQLModel with strict models exposed via FastAPI. However, there seem to be a few edge cases specifically with date, UUID, and similar "complex" types that aren't native json types where the "strict validate json" behavior is not used and instead SQLModel/FastAPI/Pydantic is expecting the exact type (eg: a date or UUID instance) which obviously fails when we're parsing from a json string. For example, this script:

from datetime import date

from pydantic import BaseModel, ConfigDict, TypeAdapter
from sqlmodel import SQLModel

class Pydantic(BaseModel):
    value: date

class PydanticStrict(Pydantic):
    model_config = ConfigDict(strict=True)

class SQL(SQLModel):
    value: date

class SQLStrict(SQL):
    model_config = ConfigDict(strict=True)

json = '{"value": "1970-01-02"}'
for model in [Pydantic, PydanticStrict, SQL, SQLStrict]:
    print(f"{model.__name__}.model_validate_json: {model.model_validate_json(json)}")
    print(f"TypeAdapter({model.__name__}).validate_json: {TypeAdapter(model).validate_json(json)}")
    print()

errors for the SQLStrict model on main calling model_validate_json and TypeAdapter.validate_json while all of the other models work as expected.


This PR implements SQLModel.model_validate_json by splitting sqlmodel_validate into a main helper with *_python and *_json variants that defer to the appropriate Pydantic method.

This fixes the direct .model_validate_json calls, but there still seem to be some separate issues with the TypeAdapter. I'm not sure yet what's wrong with TypeAdaptor around SQLModels (given it works fine with PydanticStrict above), but this seems to be the main blocker for fixing the usage in FastAPI so I've kept this PR as a draft for now while I figure that out - I'd appreciate any guidance!

@@ -335,6 327,46 @@ def sqlmodel_validate(
setattr(new_obj, key, value)
return new_obj

def sqlmodel_validate_python(

Choose a reason for hiding this comment

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

Is it me or is this redefined later in this file?

Copy link

Choose a reason for hiding this comment

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

It is wrapped inside an if else block for pydantic v1 vs v2 at line 72.

@alejsdev alejsdev added the bug Something isn't working label Jul 12, 2024
@novag
Copy link

novag commented Nov 5, 2024

Hi @JacobHayes,

This PR has been inactive for some time. Are you still able to work on it? If not, please let us know so we can continue the development from this point.

@tiangolo What is your perspective on this? Maybe you have an idea regarding the TypeAdapter.

@JacobHayes
Copy link
Author

I'm not quite sure what to look into next and external PRs don't seem to be too prioritized (no judgement there, I know it takes a lot of work 😃), so I personally won't spend much more time on this.

The PR should be open to edits by maintainers or I'm happy to close if you all prefer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants