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

null values in body are not passed through to graphql backend #114

Closed
greemo opened this issue Aug 5, 2022 · 6 comments · Fixed by #477
Closed

null values in body are not passed through to graphql backend #114

greemo opened this issue Aug 5, 2022 · 6 comments · Fixed by #477
Assignees
Labels
enhancement New feature or request in-progress Currently being implemented

Comments

@greemo
Copy link

greemo commented Aug 5, 2022

The Wundergraph operation looks like this:

mutation someUpdate($id: Int!, $name: String, $description: String, $icon: URL, $image: URL, $labels: [String!]) {
    object: someUpdate(id: $id, name: $name, description: $description, icon: $icon, image: $image, labels: $labels)
    {
        id
    }
}

I have the semantics that we can clear a value by sending an update with a null value.
This is different to sending an update when some values are not set, which means don't change the values not included in the call.

For example, in the below call, I want to clear the existing "name" attribute on the object.

--- ClientRequest start ---

POST /api/main/operations/someUpdate HTTP/1.1
Host: localhost:9991
Accept: */*
Content-Length: 23
Content-Type: application/json
User-Agent: curl/7.68.0

{"id": 0, "name": null}



--- ClientRequest end ---



--- DebugTransport ---

Request:

POST / HTTP/1.1
Host: localhost:8000
Accept: application/json
Content-Type: application/json

{"variables":{"id":0},"query":"mutation($id: Int!, $name: String, $description: String, $icon: URL, $image: URL, $labels: [String!]){object: someUpdate(id: $id, name: $name, description: $description, icon: $icon, image: $image, labels: $labels){id}}"}

Duration: 121 ms

Response:

HTTP/1.1 200 OK
Content-Length: 28
Content-Type: application/json
Date: Fri, 05 Aug 2022 04:52:27 GMT
Server: uvicorn

{"data":{"object":{"id":0}}}

--- DebugTransport

You can see by the above, the Wundergraph server removes the "name" attribute, as it is null. If I set it to something else, it works ok.

The desired behaviour is for the wundergraph server to pass through null-valued arguments.

@jensneuse
Copy link
Member

Thanks for reporting. We've got people who wanted to have the exact opposite behaviour. I guess we should make this configurable. Do you think this need to have granular configuration or could it be just at the datasource level? E.g. switch it on or off via boolean config param for the whole origin.

@greemo
Copy link
Author

greemo commented Aug 5, 2022

Hi Jens, thanks for the quick response.

In GraphQL, one can have different semantics for non-inclusion vs inclusion as null. Therefore I believe the default should be to pass nulls through. Otherwise an intentional feature of GraphQL backends is not available through Wundergraph.

I see this filtering null values as an additional filtering feature, rather than the way the system should behave by default.

If someone wishes to have this filtering nulls behaviour, and they want it in the Go codebase for performance reasons, rather than a hook, I can see the value in that. I imagine enabling the filter for the entire datasource would be sufficient. I would do it wherever it's easiest and most performant.

@jensneuse
Copy link
Member

Sounds reasonable to me, although the reason is not performance. In some cases, you don't want to send nulls because the server handles them differently than empty fields, e.g. in a patch.

@greemo
Copy link
Author

greemo commented Aug 6, 2022

Yes, this is the reason I meant.
Regarding performance, I meant doing the filtering in the Go code instead of in the Node hooks, where it could also be done. :)

@stale
Copy link

stale bot commented Oct 5, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale label Oct 5, 2022
@stale stale bot closed this as completed Oct 15, 2022
@devsergiy devsergiy reopened this Dec 22, 2022
@devsergiy devsergiy added enhancement New feature or request and removed meta: stale labels Dec 22, 2022
@jensneuse
Copy link
Member

Hey @spetrunin nice find :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request in-progress Currently being implemented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants