-
-
Notifications
You must be signed in to change notification settings - Fork 525
fix PatchDict errors with inherited schemas #1324 #1480
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
Conversation
ninja/patch_dict.py
Outdated
excluded_bases = ( | ||
"<class 'ninja.schema.Schema'>", | ||
"<class 'ninja.orm.ModelSchema'>", | ||
"<class 'pydantic.main.BaseModel'>", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can use actual classes here instead of their repr (which may break in some potential new version)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, I have updated to use the classes instead of their repr.
ninja/patch_dict.py
Outdated
|
||
|
||
def get_schema_annotations(schema_cls: Type[Any]) -> Dict[str, Any]: | ||
schema = schema_cls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what you do here with while True
you can do with class.__mro__
some thing like this:
def get_schema_annotations(schema_cls: Type[Any]) -> Dict[str, Any]:
excluded_bases = {Schema, ModelSchema, BaseModel}
annotations = {}
for base in reversed(schema_cls.__mro__):
if base in excluded_bases:
break
annotations.update(getattr(base, '__annotations__', {}))
return annotations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach doesn’t work because when calling reversed(schema_cls.__mro__)
, it will always return an empty annotations
dictionary. That’s because it will hit one of the excluded classes first:
class SomeSchema(Schema):
name: str
age: int
category: Optional[str] = None
# reversed(SomeSchema.__mro__)
# [<class 'object'>, <class 'pydantic.main.BaseModel'>, <class 'ninja.schema.Schema'>, <class 'test_patch_dict.SomeSchema'>]
I updated to this solution:
def get_schema_annotations(schema_cls: Type[Any]) -> Dict[str, Any]:
annotations: Dict[str, Any] = {}
excluded_bases = {Schema, ModelSchema, BaseModel}
bases = schema_cls.mro()[:-1]
final_bases = reversed([b for b in bases if b not in excluded_bases])
for base in final_bases:
annotations.update(getattr(base, '__annotations__', {}))
return annotations
Thank you @LeandroDeJesus-S |
Fixed the error that occurs when using
PatchDict
with a schema that inherits from other schemas #1324Problematic Scenario
Consider the following schemas:
When using these in a route, the method
ninja.patch_dict.create_patch_schema
relies solely on the__annotations__
attribute of the schema class. This means it only considers the annotations declared in the current schema, ignoring inherited fields. This leads to the following error:Proposed Solution
To address this issue, I created a helper function that recursively collects annotations from all parent classes. It traverses the class hierarchy using the
__base__
attribute until it reaches a base schema class (Schema
,ModelSchema
, orBaseModel
), then merges all the collected__annotations__
in reverse order to preserve field overrides from the child class.