Skip to content

Lenient handling of empty representations list in EntitiesDataFetcher #1057

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

Closed
roookeee opened this issue Sep 16, 2024 · 1 comment
Closed
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@roookeee
Copy link

The EntitiesDataFetcher erronously returns null when an empty list is passed in representations.
This is caused by a wrong usage of Mono.zip which coerces empty lists to an empty mono, see here. The empty monoList should be preserved and passed to EntitiesDataFetcher::toDataFetcherResult.

While this will probably never occur in production (why would federation send an empty list?) it's just not spec conforming, see here. The _entities endpoints returns a non-nullable result and the current implementation returns null in the outlined scenario, which yields the following error in our tests:

org.springframework.graphql.client.FieldAccessException: Invalid field '_entities', errors: [{message=The field at path '/_entities' was declared as a non null type, but the code involved in retrieving data has wrongly returned a null value.  The graphql specification requires that the parent field be set to null, or if that is non nullable that it bubble up null to its parent and so on. The non-nullable type is '[_Entity]' within parent type 'Query', path=[_entities], extensions={classification=NullValueInNonNullableField}}]

We noticed this issue while trying to migrate from another GraphQL framework to Spring GraphQL. Some previously green tests are red because of this.

An easy fix would be to add an .defaultIfEmpty(emptyList()) after the Mono.zip call, but it seems a bit hacky to me - I am not that familiar with the Mono APIs, better solutions might exist.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 16, 2024
@rstoyanchev rstoyanchev added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 16, 2024
@rstoyanchev rstoyanchev self-assigned this Sep 16, 2024
@rstoyanchev rstoyanchev added this to the 1.3.3 milestone Sep 16, 2024
@rstoyanchev rstoyanchev changed the title EntitiesDataFetcher returns non-spec-confirming null value for empty representations list Lenient handling of empty representations list in EntitiesDataFetcher Sep 16, 2024
@roookeee
Copy link
Author

roookeee commented Sep 17, 2024

Thank you for the quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants