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

Use java.util.List for lists #185

Closed
lorenzleutgeb opened this issue Jun 18, 2020 · 1 comment · Fixed by #186
Closed

Use java.util.List for lists #185

lorenzleutgeb opened this issue Jun 18, 2020 · 1 comment · Fixed by #186
Assignees
Labels
bug Something isn't working
Milestone

Comments

@lorenzleutgeb
Copy link
Contributor

lorenzleutgeb commented Jun 18, 2020

TL;DR: I think that the implementation of this generator misinterprets the semantics of lists in GraphQL. It generates Collection<T> where it should generate List<T>.

There's mismatch in what classes generated by the library are expected to provide (lists) and what they actually provide (collections).

Steps to Reproduce

Generate code for

type Foo { bar: [String] }

Expected Result

The Java equivalent of bar should have type List<String>, and this should of course generalize.

Actual Result

The Java equivalent of bar has type Collection<String>.

Additional context

Semantics of Lists in GraphQL

The GraphQL specification (draft) considers a list to be

List values are serialized as ordered lists, where each item in the list is serialized as per the item type.

(emphasis mine) which clearly references ordering of elements as a characteristic property of values of the list type.

Also there is other tooling out there, like Hasura which clearly rely on the property of lists being ordered.

Semantics of Lists in this generator

The Collection type in Java, however, does not guarantee ordering of elements (consider HashSet as a counterexample).

@lorenzleutgeb lorenzleutgeb added the bug Something isn't working label Jun 18, 2020
@kobylynskyi
Copy link
Owner

@lorenzleutgeb thanks for finding and fixing this!
It's good that I haven't released 2.0.0 yet (planned to do it yesterday, but did not have time). Hopefully will do today if someone will review 179 (maybe you could?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants