vitalik / django-ninja

💨 Fast, Async-ready, Openapi, type hints based framework for building APIs
https://django-ninja.dev
MIT License
7.23k stars 429 forks source link

[BUG] [Regression] Resolver is called with dict not object #1002

Open msopacua opened 11 months ago

msopacua commented 11 months ago

Describe the bug When you create a schema using schema = Schema(field_name='value') you have a resolver for field_name, then the resolver is called with a dictionary. If the field is required, this results in a "missing" validation error.

Versions (please complete the following information):

Root cause Traced it to _run_root_validator in schema.Schema.

When called as above, Pydantic calls __pydantic_self__.__pydantic_validator__.validate_python(data, self_instance=__pydantic_self__) where data is the dictionary of kwargs passed to the constructor.

This gets to run_root_validator which instantiates a DjangoGetter with the the data values = DjangoGetter(values, cls, info.context). When the handler is called, the getter is called to get the value for field_name. Because a resolver exists, the resolver is called:

        if resolver:
            value = resolver(getter=self)

Because the resolver only expects a model class as per documentation, dict.field_name fails and the value for the field_name cannot be found, eventually resulting in PyDantic throwing a missing value for a required field.

Confirmed that writing the resolver as:

    def resolve_field_name(obj: _M | dict):
        if isinstance(obj, dict):
            return obj["field_name"]
        return obj.field_name

fixes the issue.

Constructing a schema like this worked in 0.22.

adi1 commented 11 months ago

I'm not sure if this is related, but rather than opening a new issue I will mention it here.

It appears that in 1.x of django-ninja, possibly because of what was mentioned above, the resolvers are called also on request input validation. Is this intended behaviour? I'm having trouble following this area in the documentation.

In my case, I can also determine if I'm dealing with input data or model serialization by checking if 'response_status' in context but this seems like a hack.

nuarhu commented 10 months ago

Same here. I had a test that was running fine with version django-ninja 0.22.1 and pydantic 1.10.13 but is now failing with ninja 1.1.0 and pydantic 2.5.3.

I try to condense it into a short example with the following highlights:

    class Contact(Model): # Django Model
        categories = ManyToManyField(...)
        tags = DalTaggableManager()  # taggit
        # other fields omitted

    class ContactSchema(ModelSchema):
        # other fields omitted
        categories: List[CategorySchema] = []
        tags: List[str] = []

        model_config = ConfigDict(
            alias_generator=to_camel_case,
            populate_by_name=True
        )

        class Meta:
            model = Contact
            fields = [
                'categories',
                'tags',
            ]

        @staticmethod
        def resolve_categories(obj):
            LOGGER.debug('>>>>>>>>>>>>>>>>>< RESOLVE CATS %s', obj)
            return obj.categories_display()

        @staticmethod
        def resolve_tags(obj):
            LOGGER.debug('>>>>>>>>>>>>>>>>>< RESOLVE TAGS %s', obj)
            return list(obj.tags.values_list('name', flat=True))

    @contacts_router.put('/me/', response={200: ContactSchema, 401: str, 400: ContactErrorsSchema}, by_alias=True,
                         exclude_unset=True)
    def put_current_user(request, payload: ContactSchema):
        LOGGER.debug('>>>>>>>>>>>>>>< request %s %s', request.body, payload)
        contact, unauth = get_contact_for_request(request)

The request contains the categories and tags. The resolvers get called and the obj at this point is the request body as python dictionary. The calls to obj.categories_display and obj.tags at this point result in None and as a consequence, the payload contains only empty lists of categories and tags.

If this is now the new behaviour, a hint in the documentation would probably be helpful.