Same model returned by pydantic_model_creator calls with different arguments

henadzit commented 1 month ago

Description

This PR fixes the issue where calling pydantic_model_creator multiple times with different arguments without specifying a name might return the same model due to caching (_MODEL_INDEX in creator.py). The example below demonstrates the issue.

from tortoise import fields
from tortoise.contrib.pydantic import pydantic_model_creator
from tortoise.models import Model

class People(Model):
    id = fields.IntField(pk=True)
    created_at = fields.DatetimeField(auto_now_add=True)
    modified_at = fields.DatetimeField(auto_now=True)

    first_name = fields.CharField(max_length=50)
    last_name = fields.CharField(max_length=50)

Person = pydantic_model_creator(People)
PersonIn = pydantic_model_creator(People, exclude_readonly=True)

print(list(Person.model_fields))  
# [id, created_at, modified_at, first_name_last_name]
print(list(PersonIn.model_fields))  
# [id, created_at, modified_at, first_name_last_name] 
# However, it should be ['first_name', 'last_name'] because of exclude_readonly=True

print(Person is PersonIn)  
# True (!)

This issues affects the models that have no backward or forward reference to any other model.

This PR:

Makes pydantic_model_creator to create verbatim names for "root" models even if they do not have relations
Adds the exclude_readonly to the hash used for naming unnamed models so pydantic_model_creator(People) and pydantic_model_creator(People, exclude_readonly=True)

Motivation and Context

The issues that will be fixed by this PR:

There was an attempt to solve this issue previously but it was never merged. The approach used does not seem to work anymore.

How Has This Been Tested?

Tested the snippets from the issues above
Added unit tests

Checklist:

[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[ ] I have added the changelog accordingly.
[x] I have read the CONTRIBUTING document.
[x] I have added tests to cover my changes.
[x] All new and existing tests passed.

henadzit commented 1 month ago

Hey @waketzheng @waketzheng, any chance you can review this PR? Please let me know if more information or more tests are required! Thanks!

coveralls commented 1 month ago

Pull Request Test Coverage Report for Build 11407291191

Details

2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.008%) to 89.021%

Totals
Change from base Build 11323175489:	0.008%
Covered Lines:	5980
Relevant Lines:	6603

💛 - Coveralls

henadzit commented 1 month ago

Thank you for venturing into pydantic model created, it's one of more convoluted pieces of code there :)

Thank you for maintaining tortoise-orm! I'm glad to be of help!

There was a PR (https://github.com/tortoise/tortoise-orm/pull/735) a while ago that was partially solving the issue but never got merged, I think it can be closed now.

tortoise / tortoise-orm