Are Generics not supported? #281
Comments
|
How to handle generics is not yet discussed in the spec. We can support it in SmallRye but need to discuss how... You are welcome to get this going :) |
|
OK, so it is expected behavior. |
|
This is a bit weird. Now Gradle is telling |
|
Not sure if this will help, but, in Quarkus SchemaBuilder is only used on build (and not on runtime). |
|
Yes that will explain it and makes sense. |
|
The more I think about it the Generic used for a page are important. And maybe it's not that difficult (just guessing though) as SmallRye already supports |
|
Yea I don't think it's difficult. Just need to do it, and make sure that we boil it up to the spec. |
|
I just hit the same problem, trying to implement the same thing, paged result :-D It is a bit funny, because if graphql @query method returns generic class this way (in my case PageType is not interface but class with some implementations, not sure if it is important): then quarkus starts, but graphql schema says that this method returns array of When I created subclass of the generic class with concrete type in it, then I finished with the same exception, which is even stranger. I'm not sure if graphql schema supports generics somehow, I think that no. So I'd say that if generics part is set to exact class in Java (as in our cases, sorry , not sure correct words for this, hope it is clear) then graphql implementation should be able to take and use it without any problem in the graphql schema. I think support for generics is really important to provide convenient functionality for java developers as generics are used a lot. |
|
Yea like we said, generics is not supported. Mostly because we have not discussed what the expected behaviour is. I think there is more than one use case, so we need to flesh this out. There is an issues on the Spec (MP) side, but we can try and implement this in SmallRye first and then move it to the spec. |
|
@marceloverdijk are you working on this? I'd like to look at it if you are not working on it. |
|
@velias No I'm currently not working on it. |
|
I started some investigation around this. This is ticket to discuss Generics directly to GraphQL spec graphql/graphql-spec#190 . They are not there (and some of the spec authors says that intentionally), and it is question if they will ever be. We can still workaround it by allowing only generics with exactly defined types (no any wildchars) in the in/out object trees, and generating GraphQL schema in a way it contains extra class for each exact variant of the generic with given type. I'll try to implement this, I expect correct place is |
|
And BTW we have to patch |
(inclusing interfaces)
(inclusing interfaces)
|
Thanks @phillip-kruger for merging my PR. My only concern is that it is not documented anywhere beside this issue. Do you plan any documentation for smallrye-graphql to cover areas not covered by the spec itself? There are extensions in functionality, some config params specific for this impl etc which should be documented. |
|
Thanks @velias , yes we can maybe for now document in the Readme until we know how documentation will work , it's in discussion at the moment on a SmallRye level |
|
I just tried to use generics in GraphQL in Quarkus 1.8 and app fails during startup :-( quarkusio/quarkus#12230 created. |
|
Hi @velias - that is weird. Quarkus 1.8.1.Final contains SmallRye GraphQL1.0.9 that contains your PR and should work. Are you investigating ? |
|
Yep, I'm going to create small reproducer Quarkus app and investigate where exactly the problem is. It is somehow related to introspection to build GraphQL scheme in quarkus deployment step. |
|
Thanks :) |
|
Has this issue been resolved? I' am still getting this error |
|
What error are you getting, can you share your example ? |
|
Maybe some of my subsequent patches is not released yet in quarkus? What do you use @apellizzn ? |
|
@velias I am using quarkus package test.graphql;
import test.graphql.dtos.PaginationInfo;
import java.util.List;
public interface Connection<T> {
List<T> getData();
PaginationInfo getPagination();
}package test.graphql;
import test.dtos.PaginationInfo;
import lombok.Builder;
import java.util.List;
@Builder
public class FruitConnection implements Connection<Fruit> {
private List<Fruit> data;
private PaginationInfo paginationInfo;
@Override
public List<Fruit> getData() {
return data;
}
@Override
public PaginationInfo getPagination() {
return paginationInfo;
}
}package test.graphql;
import lombok.Builder;
import lombok.Data;
@Data
@Builder
public class Fruit {
private String name;
}package test.graphql;
import test.dtos.PaginationInfo;
import org.eclipse.microprofile.graphql.GraphQLApi;
import org.eclipse.microprofile.graphql.Query;
import java.util.ArrayList;
@GraphQLApi
public class FruitsResource {
@Query
public FruitConnection all() {
return FruitConnection.builder()
.data(new ArrayList<>() {{ add(new Fruit("apple")); }})
.paginationInfo(new PaginationInfo())
.build();
}
}I get the following error compiling Build step io.quarkus.smallrye.graphql.deployment.SmallRyeGraphQLProcessor#buildExecutionService threw an exception: io.smallrye.graphql.schema.SchemaBuilderException: Don't know what to do with [T] of kind [TYPE_VARIABLE] as parent object reference is missing or incompleteDo you think this error is related to this issue? |
|
Can you double check this with the latest (1.9.2) version ? |
|
Tried with 1.9.2.Final and got this error when I start the app |
|
@phillip-kruger any plan when smallrye-graphql 1.0.15 will be in Quarkus? It contains some patches around generic interfaces, but I'm not fully sure if it covers this case. |
|
@apellizzn and create new issue for it please, as this one is closed |
|
@apellizzn OK, I was able to create testing project and reproduce this error. So please create new issue and I'll try to patch it. Would be good to have patch in smallrye-graphql 1.0.15 and next Quarkus |
|
@velias - I am busy pulling 1.0.15 in to Quarkus 1.10.0 (CR release tomorrow). We can still patch after that with 1.0.16 |
|
I have patch for this problem, any chance to get it into 1.0.15 if I create PR now? |
|
To be honest, I forgot to rebase, and just realized during the rebase that the problem had been patched 9 days ago. So it should be patched in 1.0.15 and my PR only adds test to cover this problem (I like to cover all errors with tests not to be reintroduced later ;-) |
|
1.0.15 already released and PR open to get it into Q. But We can do a 1.0.16 and still try and get it into Q before the 1.10.0 release. |
Ok great, lets get the tests into master. Please do a PR |
…d patched in previous commit
Added test for interface problem reported in #281 comments and patched
I'm having:
But this gives:
Note that in the
@GraphQLApiI don't directly refer to thePageType<T>interface.It would be nice to have some sort of Page contract in my Java classes.
The text was updated successfully, but these errors were encountered: