vitalik / django-ninja

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

[testing] Add `resolver_match` to the mocked request object in the Ninja's TestClient #1061

Open zigcccc opened 8 months ago

zigcccc commented 8 months ago

The problem

Previously, the Ninja's TestClient did not include the resolver_match in the mocked request object. That's causing issue when having the following setup:

# in schemas.py

class EntityUpdateSchema(Schema):
    id: int

    class Meta:
        exclude: ("id",)
        fields_optional = "__all__"
        model = Entity

    @staticmethod
    def resolve_id(obj, context):
        return context["request"].resolver_match.kwargs.get("id")

The above schema will infer the id property from the path. So, assuming you have the following path:

@router.patch("/entities/{id}/")
def patch_entity(request: HttpRequest, id: int, partial_entity_data: EntityUpdateSchema):
    ...

the EntityUpdateSchema will be populated with the id received from the path. I find this useful for when we need to do some validation against the object and we need to query the object that we're currently working with.

And the good news is - this works perfectly! When I call the endpoint, everything is behaving as it should :chef-kiss:

So, what's the problem?

When writing tests, though, and using the TestClient from Ninja, this is not working at all. Doing client.patch("/entities/123") will never populate the schema field, and we will get a validation error back, saying the id field is missing.

That is because resolver_match is never set as the property of the mocked request. So, doing request.resolver_match.kwargs.get("id") will always result in None.

The solution

The solution for this is quite simple - when we build the mock request object, we need to set the resolver_match to the ninja resolver that we get back from the URL. By doing that, the schema is again able to infer the id and everything works as it should, even in tests.

I went ahead and created a PR for this: https://github.com/vitalik/django-ninja/pull/1060 Using the solution that I've introduced in this PR, everything works as expected when testing views. While I could just create my own TestClient that would inherit from Ninja's test client, and override the _resolve method (which I'm happy to do if for some reason we don't want this in the framework overall), I assume this would be beneficial to others as well, hence the PR 🙂

shijl0925 commented 7 months ago

I found a temporary solution

`class EntityUpdateSchema(Schema): id: int

class Meta:
    exclude: ("id",)
    fields_optional = "__all__"
    model = Entity

@staticmethod
def resolve_id(obj, context):
    from unittest.mock import Mock
    request = context["request"]

    if isinstance(request, Mock):
        from crum import get_current_request
        request = get_current_request()`
zigcccc commented 7 months ago

@shijl0925 a simpler workaround approach is to extend the TestClient class with your own implementation:

class ApiTestClient(TestClient):
    def _resolve(
        self, method: str, path: str, data: Dict, request_params: Any
    ) -> Tuple[Callable[..., Any], Mock, Dict]:
        url_path = path.split("?")[0].lstrip("/")

        for url in self.urls:
            match = url.resolve(url_path)
            if match:
                request = self._build_request(method, path, data, request_params)
                request.resolver_match = match
                return match.func, request, match.kwargs

        raise Exception(f'Cannot resolve "{path}"')

and then in your test, instead of using TestClient from ninja, use your own test client:

def setUp(self):
        self.client = ApiTestClient(router)

By doing that, tests will work without any mocks

image