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

Ensure recursive field annotation resolution #19

Merged
merged 2 commits into from
Nov 23, 2023

Conversation

lmignon
Copy link
Owner

@lmignon lmignon commented Nov 20, 2023

At the end of the registry building process, the registry contains the aggregated
model. Each aggregated model is the result of the build of a new class based on
a hierarchy of all the classes defined as 'extends' of the same base class. The
order of the classes hierarchy is defined by the order in which the classes are
loaded by the class loader or by a specific order defined by the developer when
the registry is built.

The last step of the build process is to resolve all the annotated types in the
aggregated model and rebuild the pydantic schema validator. This step is necessary
because when the developer defines a model, some fields can be annotated with a
type that refers to a class that is an extendable class. It's therefore necessary
to update the annotated type with the aggregated result of the specified
extendable class and rebuild the pydantic schema validator to take into account
the new annotated types.

Prior to this commit, the resolution of the annotated types was not done in a
recursive way and the rebuild of the pydantic schema validator was only done
just after the resolution of an aggregated class. This means that if a class A
is an extendable defining a fields annotated with a type that refers to a class
B, and if the class B is an extendable class defining a field of type C,
the annotated type of the field of the class A was resolved with the aggregated
model of the class B but we didn't resolve th annotated type of the field ot type
B with the aggregated model of the type C. Therefore when the pydantic schema
validator was rebuilt after the resolution of the class A, if the class B was
not yet resolved and therefore the pydantic schema validator was not rebuilt,
the new schema validator for the class A was not correct because it didn't take
into account the aggregated model of the class C nor the definition of extra
fields of the aggregated model of the class B.

This commit changes the resolution of the annotated types to be recursive. Therefore
when the pydantic schema validator is rebuilt, we are sure that all referenced
subtypes are resolved and defines a correct schema validator. In the
same time, when an aggregated class is resolved, it's marked as resolved to avoid
to resolve it again and rebuild the pydantic schema validator again for nothing.
In addition to resolve the initial problem, this commit also improves
the performance of the build process because schema validators rebuilds are
done only once per aggregated class.

At the end of the registry initialisation, we need to walk across all the fields defined on extendable models to replace the annoted orignal type by the one build into the registry if the field reference a extendable model. Once we replace the declared type buy the resolved one, we also need to rebuild the model schema to take into account this change. Prior to this change, the result was consistent. Indeed, the resolution mechanism was not applied recursively. As result the model rebuild for a class with a field declared as an extendable model type, could not contain  the resolved definition of fields declared into this referenced extendable model type if the resolution of the last one was not already done. The resolution mechanism is now recursive and when a annotation is resolved, we ensure that the new type is also resolved.
@lmignon lmignon force-pushed the master-recursive-mode-rebuild branch 2 times, most recently from 46184ef to 1fd4b35 Compare November 23, 2023 08:46
At the end of the registry building process, the registry contains the aggregated
model. Each aggregated model is the result of the build of a new class based on
a hierarchy of all the classes defined as 'extends' of the same base class. The
order of the classes hierarchy is defined by the order in which the classes are
loaded by the class loader or by a specific order defined by the developer when
the registry is built.

The last step of the build process is to resolve all the annotated types in the
aggregated model and rebuild the pydantic schema validator. This step is necessary
because when the developer defines a model, some fields can be annotated with a
type that refers to a class that is an extendable class. It's therefore necessary
to update the annotated type with the aggregated result of the specified
extendable class and rebuild the pydantic schema validator to take into account
the new annotated types.

Prior to this commit, the resolution of the annotated types was not done in a
recursive way and the rebuild of the pydantic schema validator was only done
just after the resolution of an aggregated class. This means that if a class A
is an extendable defining a fields annotated with a type that refers to a class
B, and if the class B is an extendable class defining a field of type C,
the annotated type of the field of the class A was resolved with the aggregated
model of the class B but we didn't resolve th annotated type of the field ot type
B with the aggregated model of the type C. Therefore when the pydantic schema
validator was rebuilt after the resolution of the class A, if the class B was
not yet resolved and therefore the pydantic schema validator was not rebuilt,
the new schema validator for the class A was not correct because it didn't take
into account the aggregated model of the class C nor the definition of extra
fields of the aggregated model of the class B.

This commit changes the resolution of the annotated types to be recursive. Therefore
when the pydantic schema validator is rebuilt, we are sure that all referenced
subtypes are resolved and defines a correct schema validator. In the
same time, when an aggregated class is resolved, it's marked as resolved to avoid
to resolve it again and rebuild the pydantic schema validator again for nothing.
In addition to resolve the initial problem, this commit also improves
the performance of the build process because schema validators rebuilds are
done only once per aggregated class.
@lmignon lmignon force-pushed the master-recursive-mode-rebuild branch from 1fd4b35 to c954629 Compare November 23, 2023 08:48
@lmignon lmignon merged commit c2aeed0 into master Nov 23, 2023
7 checks passed
@lmignon lmignon deleted the master-recursive-mode-rebuild branch November 23, 2023 08:51
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.

1 participant