graphql-python / graphene-django

Build powerful, efficient, and flexible GraphQL APIs with seamless Django integration.
http://docs.graphene-python.org/projects/django/en/latest/
MIT License
4.28k stars 766 forks source link

graphene_django.utils.testing.graphql_query and Django 4.2+ #1501

Open hokkaido opened 6 months ago

hokkaido commented 6 months ago

What is the current behavior? This is with graphene-django==3.2.0 and Django==4.2.10 and it concerns the graphql_query function from graphene_django.utils.testing. Django 4.2.+ prefixes the header keys automatically with HTTP_, making the documentation of the graphql_query in that situation wrong.

Pseudo Code:

    headers = {
        'HTTP_AUTHORIZATION': f'Bearer {JWT_TOKEN}'
    }
    return graphql_query(.., .., client=client, headers=headers, graphql_url="/graphql/")

If we then later dump request.META we will see HTTP_HTTP_AUTHORIZATION.

The part of graphene where it handles the Django version can be found here: https://github.com/graphql-python/graphene-django/blob/4d0484f312f8259cb06eb3abe2b14944933c31eb/graphene_django/utils/testing.py#L60

Further context You see Django's behavior described here in an example: https://docs.djangoproject.com/en/4.2/topics/testing/tools/#django.test.Client.get

c = Client() c.get( ... "/customers/details/", ... {"name": "fred", "age": 7}, ... headers={"accept": "application/json"}, ... ) …will send the HTTP header HTTP_ACCEPT to the details view, which is a good way to test code paths that use the django.http.HttpRequest.accepts() method.

Fix So I suggest at a minimum to update the documentation / description of client_query that with Django 4.2+ headers don't have to be prefixed with HTTP_. Ideally the function it self would take care of it however.

https://github.com/graphql-python/graphene-django/blob/4d0484f312f8259cb06eb3abe2b14944933c31eb/graphene_django/utils/testing.py#L33-L36

kiendang commented 6 months ago

Ah ok this is valid. I didn't read the Django doc carefully when doing #1465. I think the correct solution to this is to revert #1465 and add a check making sure that headers keys all start with HTTP_. What do you think?