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

Allow using a sub-interface of a generic interface as API with Typesafe client #2151

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lbuffler
Copy link

Goal

I want to be able to write a GraphQL api and provide a Java interface for my clients, whilst keeping GraphQL strength of allowing clients to choose exactly what data they will receive. For that, we could use generics.

So, as the writer of the GraphQL endpoint, I would for example give this to my clients:

public interface MyApi<T> {
    Uni<T> getItem();
}

T could be for example: public record MyItemSimple(String name) {} or public record MyItemFull(String name, String extraLongDescription) {}

Then, on the client side, I would be able to choose between:

@GraphQLClientApi
public interface MyApiSimple extends MyApi<MyItemSimple> {
}

or

@GraphQLClientApi
public interface MyApiFull extends MyApi<MyItemFull> {
}

depending on the data needed, and getItem() would return exactly that.

Problem

That fails with:

java.lang.UnsupportedOperationException: can't resolve type variable of a T

	at io.smallrye.graphql.client.impl.typesafe.reflection.TypeInfo.resolveTypeVariable(TypeInfo.java:111)
	at io.smallrye.graphql.client.impl.typesafe.reflection.TypeInfo.getTypeName(TypeInfo.java:97)
	at io.smallrye.graphql.client.impl.typesafe.QueryBuilder.fields(QueryBuilder.java:56)
	at io.smallrye.graphql.client.impl.typesafe.QueryBuilder.recursionCheckedFields(QueryBuilder.java:73)

The only way to get it working is to explicitly override the generic method.

Solution

This PR provides a way to fix this and makes the scenario described here as expected.

@mskacelik
Copy link
Contributor

Hi @lbuffler, thank you for the PR!
Can you also provide some tests?
In client/tck or server/integration-tests/.../client.

Also, if I am not mistaken, you would probably need to add your sub-interface implementation to the client model implementation (which has been used by Quarkus since 3.9.0). which, instead of Java reflection for query building, uses a Jandex API.

Somewhere here:

private List<MethodInfo> getAllMethodsIncludingFromSuperClasses(ClassInfo classInfo) {
)

I can take care of this part, if you want...

@lbuffler
Copy link
Author

Hi @mskacelik, thanks for your feedback.

Sure, I'll add some tests, I wanted to do it but wasn't sure where they should go.

About Quarkus, I noticed that it was failing in a different way but haven't dug more about this. Now that you say it, it's obvious it would use the Jandex index instead of reflection.

I never had the opportunity to look at Jandex, so this would be a good occasion to start, I'll look at it and let you know if I need help.

@jmartisk jmartisk requested a review from t1 July 29, 2024 07:13
@radcortez radcortez force-pushed the main branch 4 times, most recently from 6aa0d6e to e6ffcf0 Compare December 19, 2024 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants