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

Optional Embedded model and key_name not working #484

Open
anentropic opened this issue May 29, 2024 · 6 comments · May be fixed by #485
Open

Optional Embedded model and key_name not working #484

anentropic opened this issue May 29, 2024 · 6 comments · May be fixed by #485
Labels
bug Something isn't working

Comments

@anentropic
Copy link

anentropic commented May 29, 2024

Bug

I have the following model:

class CompanyRelation(EmbeddedModel):
    company_id: str = Field(key_name="companyId")


class Person(Model):
    model_config: ClassVar = {
        "collection": "Main",
        "parse_doc_with_default_factories": True,
    }

    company_relations: Optional[list[CompanyRelation]] = Field(
        key_name="companyRelation", default_factory=list
    )

It was working before I made the company_relations field Optional (which I had to because sometimes the key is missing in MongoDB records)

When I try to fetch valid records I now get errors like:

Traceback (most recent call last):
  File "/opt/python/odmantic/model.py", line 802, in model_validate_doc
    instance = cls.model_validate(obj)
               ^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/python/pydantic/main.py", line 509, in model_validate
    return cls.__pydantic_validator__.validate_python(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
pydantic_core._pydantic_core.ValidationError: 41 validation errors for Person
company_relations.0.company_id
  Field required [type=missing, input_value={'companyId': '5d19ea5ed4...'roles': ['OrderAdmin']}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.6/v/missing
company_relations.1.company_id
  Field required [type=missing, input_value={'companyId': '64df2da547...'roles': ['OrderAdmin']}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.6/v/missing
company_relations.2.company_id
  Field required [type=missing, input_value={'companyId': '64df2da647...'roles': ['OrderAdmin']}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.6/v/missing
company_relations.3.company_id
  Field required [type=missing, input_value={'companyId': '64df2da747...'roles': ['OrderAdmin']}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.6/v/missing
company_relations.4.company_id
  Field required [type=missing, input_value={'companyId': '64df2da847...'roles': ['OrderAdmin']}, input_type=dict]
    For further information visit
...

Notably the companyId field exists in the input.

If I remove the Optional it works (but can't cope with missing field).

If I replace company_id: str = Field(key_name="companyId") with companyId: str # noqa: N815 and keep the Optional it works.

So it seems to be the combination of Optional + key_name that has the problem.

Current Behavior

See above.

Also I can instantiate the CompanyRelation directly from the sub data (again bypassing whatever the Optional on parent model has done to break things)

Expected behavior

No error.

Environment

  • ODMantic version: 1.0.2
  • MongoDB version: I don't think it matters
  • Pydantic infos (output of python -c "import pydantic.utils; print(pydantic.utils.version_info())):
             pydantic version: 2.6.4
        pydantic-core version: 2.16.3
          pydantic-core build: profile=release pgo=true
               python version: 3.11.5 (main, Sep 18 2023, 15:04:25) [Clang 14.0.3 (clang-1403.0.22.14.1)]
                     platform: macOS-14.4.1-arm64-arm-64bit
             related packages: typing_extensions-4.11.0 pydantic-settings-2.2.1
  • Version of additional modules (if relevant):
    • ...

Additional context

Add any other context about the problem here.

@anentropic anentropic added the bug Something isn't working label May 29, 2024
@anentropic
Copy link
Author

anentropic commented May 29, 2024

the 'working' workarounds were tested by calling Person.model_validate_doc({...}) as per the stack trace

@anentropic
Copy link
Author

I have another field on the parent Person model like:

    subscriptions: Optional[list[Subscription]] = Field(default_factory=list)

and

class Subscription(EmbeddedModel):
    company_id: Optional[str] = Field(None, key_name="companyId")

...it doesn't seem to have the same problem

So maybe the key_name="companyRelation" on the parent field is also part of the issue

@anentropic
Copy link
Author

anentropic commented May 29, 2024

No... if I remove the key_name from parent field and amend the input data to already have company_relations key name then I still get the error

the difference above is that Subscription.company_id is Optional with None default so it can't complain about (falsely) missing value

@anentropic
Copy link
Author

shouldn't there by some stuff with alias in the generated Pydantic model fields?

https://docs.pydantic.dev/latest/concepts/alias/

@anentropic
Copy link
Author

I can see that Odmantic also doesn't respect the key_name of fields in EmbeddedModel when they are saved to MongoDB... but only when the field is Optional[list[CompanyRelation]], list[CompanyRelation] works fine

this explains why my application unit tests didn't catch the issue above before we deployed the change - the tests generate test data via the Odmantic model so it was creating records that it could read

ifm-pgarner added a commit to ifm-pgarner/odmantic that referenced this issue May 30, 2024
ifm-pgarner added a commit to ifm-pgarner/odmantic that referenced this issue May 30, 2024
anentropic added a commit to anentropic/odmantic that referenced this issue May 30, 2024
anentropic added a commit to anentropic/odmantic that referenced this issue May 30, 2024
@anentropic anentropic linked a pull request May 30, 2024 that will close this issue
@bofcarbon1
Copy link

Why did the people supporting odmantic make changes to the model configuration. It was perfect when it was flexible. Years of working with models and MongoDB collection/document. Then Type Error replaces working model. No existing documentation just failure. To be honest I thought the schema key/value pair followed by another class statement with Config: collection = 'mycollection' was simple and flexible. Now it seems I have to add a 'model_config and add another JSON structure to reference my collection and that throws another error on a field with key not found but I can see it in the object returned. Yes BSON based on JSON is a key/value pair structure. But this is obscuring terms and not needed. We have ObjectID but many of use don't use it. I do most of my queries on fields. Simply listed as key/value pairs for a model which is data that I choose to use. The whole idea of a repository tool is the flexibility to map to data and validate in the code. Not have the repository tool set outside requirements in mapping to a database table/collection or document. No we have to rewrite the code base and that is a problem. New ideas about conforming to some standard that breaks 100,000 lines of code is not the way. Set the new standards but don't break the code.

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 a pull request may close this issue.

2 participants