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

RESTEasy Reactive: UniInvoker signatures are wrong #15852

Closed
FroMage opened this issue Mar 18, 2021 · 3 comments · Fixed by #15942
Closed

RESTEasy Reactive: UniInvoker signatures are wrong #15852

FroMage opened this issue Mar 18, 2021 · 3 comments · Fixed by #15942
Labels
area/resteasy-reactive kind/bug Something isn't working
Milestone

Comments

@FroMage
Copy link
Member

FroMage commented Mar 18, 2021

Due to how we implement it, all its return types are Uni<?> instead of the more useful Uni<R> for methods like <R> get(Class<R>), forcing users to cast the return types to the right type:

UniInvoker invocation = target.request().rx(UniInvoker.class);
Uni<String> ret = (Uni<String>) invocation.get(String.class);

This is because we implemented it with:

public class UniInvoker extends AbstractRxInvoker<Uni<?>> {}
...
public abstract class AbstractRxInvoker<T> implements RxInvoker<T> {

    @Override
    public <R> T get(Class<R> responseType) {
        return method("GET", responseType);
    }
}

Now, for some reason the JAXRS interface is:

public interface RxInvoker<T> {

    public <R> T get(Class<R> responseType);
}

Which is pretty useless. We could mitigate this by making UniInvoker generic.

Or we could do like for MultiInvoker (for some reason this does pass the Java type system):

public class MultiInvoker extends AbstractRxInvoker<Multi<?>> {

    @SuppressWarnings("unchecked")
    @Override
    public <R> Multi<R> get(Class<R> responseType) {
        return (Multi<R>) super.get(responseType);
    }
}

Perhaps we should do both (generify and fix the signature via that weirdly allowed covariant override)?

@FroMage FroMage added kind/bug Something isn't working area/resteasy-reactive labels Mar 18, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Mar 18, 2021

/cc @geoand, @stuartwdouglas

@FroMage
Copy link
Member Author

FroMage commented Mar 18, 2021

Huh, CompletionStageRxInvoker does the same trick to fix the signature of <R> get(Class<R>):

public interface CompletionStageRxInvoker extends RxInvoker<CompletionStage> {

    @Override
    public CompletionStage<Response> get();

    @Override
    public <T> CompletionStage<T> get(Class<T> responseType);

But interestingly it also overrides get() to return a CompletionStage<Response> which is probably what we should do too?

@geoand
Copy link
Contributor

geoand commented Mar 18, 2021

But interestingly it also overrides get() to return a CompletionStage<Response> which is probably what we should do too?

That does sounds reasonable

stuartwdouglas added a commit to stuartwdouglas/quarkus that referenced this issue Mar 23, 2021
gsmet pushed a commit to stuartwdouglas/quarkus that referenced this issue Mar 23, 2021
@quarkus-bot quarkus-bot bot added this to the 1.14 - main milestone Mar 23, 2021
@gsmet gsmet modified the milestones: 1.14 - main, 1.13.0.Final Mar 23, 2021
gsmet pushed a commit to gsmet/quarkus that referenced this issue Mar 23, 2021
Fixes quarkusio#15852

(cherry picked from commit 76409e1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/resteasy-reactive kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants