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 doesn't fully recognize the bean param when it's in generic interface #42807

Open
tran4774 opened this issue Aug 27, 2024 · 11 comments
Labels
area/rest kind/bug Something isn't working

Comments

@tran4774
Copy link
Contributor

tran4774 commented Aug 27, 2024

Describe the bug

When I use a generic interface defined for the controller, the @BeanParam doesn't fully recognize the query param defined in the class.

I have 2 models (called filters): PageFilter is the base filter and UserFilter extends it

public abstract class PageFilter {
    @RestQuery
    @DefaultValue(value = "0")
    protected Integer number;
    @RestQuery
    @DefaultValue(value = "20")
    protected Integer size;
    @RestQuery
    private List<String> sort;
}
public class UserFilter extends PageFilter {
    @RestQuery
    private String username;
    @RestQuery
    private String fullName;
}

And I have a generic interface like this

public interface IGetInfoPageController<ID, T extends BaseData<ID>, F extends PageFilter> {

    @GET
    @Path("/list")
    @Produces(MediaType.APPLICATION_JSON)
    default RestResponse<BaseResponse<BasePagingResponse<T>>> getInfoPageWithFilter(@BeanParam @Valid F filter) {
        // some logic code
    }
}

I implement that in the controller:

@Path("/api/user")
@Tag(name = "User Controller")
public class UserController implements IGetInfoPageController<String, UserInfo, UserFilter> {
   //Some method and logic
}

I want a bean param UserFilter containing 5 fields: number, size, sort, username, and fullName. But username and fullName are always null. When I checked OpenAPI UI, it only showed 3 fields in PageFilter

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

Linux 6.8.0-41-generic #41-Ubuntu SMP PREEMPT_DYNAMIC Fri Aug 2 20:41:06 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

java 17.0.8 2023-07-18 LTS Java(TM) SE Runtime Environment Oracle GraalVM 17.0.8+9.1 (build 17.0.8+9-LTS-jvmci-23.0-b14) Java HotSpot(TM) 64-Bit Server VM Oracle GraalVM 17.0.8+9.1 (build 17.0.8+9-LTS-jvmci-23.0-b14, mixed mode, sharing)

Quarkus version or git rev

3.13.3

Build tool (ie. output of mvnw --version or gradlew --version)

Gradle 8.8

Additional information

No response

@tran4774 tran4774 added the kind/bug Something isn't working label Aug 27, 2024
Copy link

quarkus-bot bot commented Aug 27, 2024

/cc @FroMage (resteasy-reactive), @geoand (resteasy-reactive), @stuartwdouglas (resteasy-reactive)

@geoand
Copy link
Contributor

geoand commented Aug 27, 2024

I'm sure @FroMage is going to love this one 😄

@tran4774
Copy link
Contributor Author

Any fix for this issue? I have tried quarkus-spring-web and it still occur

@nasonawa
Copy link

Hi @tran4774,

i reproduced the issue at my end, seems like fullName and username are not shown in the OpenAPI UI, but when i tried with the from "curl" command it was working for me
curl -X 'GET' 'http://localhost:8080/api/user/list?fullName=123213&number=123&size=20&sort=string&sort=string&username=123123' -H 'accept: text/plain

@FroMage
Copy link
Member

FroMage commented Aug 28, 2024

TBH I'm even surprised Quarkus REST works with generic methods like these. I don't recall adding support for that, so it must have been someone else.

Not surprised bean params or openapi doesn't work with this. It requires quite some generics knowledge to get it to work.

And it could be worse: the bean param classes don't use generics, that wouldn't work either.

Anyway, I definitely won't have time to fix this soon, sorry, but I can help you if you can provide a PR.

Also, as @nasonawa said, it's fairly possible this actually works in Quarkus REST, and this is just openapi not supporting this.

But it's going to be tricky to verify this without actually reifying the method, since you can't access the extra fields in the interface:

public interface IGetInfoPageController<ID, T extends BaseData<ID>, F extends PageFilter> {

    @GET
    @Path("/list")
    @Produces(MediaType.APPLICATION_JSON)
    default RestResponse<BaseResponse<BasePagingResponse<T>>> getInfoPageWithFilter(@BeanParam @Valid F filter) {
        // here you can only access filter.username by downcasting
    }
}

@Path("/api/user")
@Tag(name = "User Controller")
public class UserController implements IGetInfoPageController<String, UserInfo, UserFilter> {
   //Some method and logic
}

If you start overriding getInfoPageWithFilter in UserController I suspect it will start working, since you'd be reifying the type argument.

Apparently, you said:

But username and fullName are always null

Which means you did test it and it did not set the extra fields, which leads me to suspect that it's the parameter injector which is wrong in not applying type arguments. I'm sure we're looking up an argument of type PageFilter from CDI to fill the bean param.

I'm saying this because I'm pretty sure the bean params injection will be generated properly (there's no generics there), so if we passed a UserFilter instance, it would automatically get filled up. So we're probably passing a PageFilter.

You can check this by printing the class of the @BeanParam @Valid F filter argument. Let us know?

@tran4774
Copy link
Contributor Author

I have tried curl way before (like @nasonawa) but it's still null. Here is my code after adding the log

public interface IGetInfoPageController<ID, T extends BaseData<ID>, F extends PageFilter> {

    @GET
    @Path("/list")
    @Produces(MediaType.APPLICATION_JSON)
    default RestResponse<BaseResponse<BasePagingResponse<T>>> getInfoPageWithFilter(@BeanParam @Valid F filter) {
        log.info("Bean param type: {}", filter.getClass().getName());
        log.info("Bean param: {}", Json.encode(filter));
        // some logic
    }
}

The curl command is

curl -X 'GET' \
  'http://localhost:8080/api/user/list?number=0&size=20&username=123123&fullName=123213' \
  -H 'accept: application/json'

And here is my log
image
@FroMage If I can create PR, where is the place you resolve bean params?

@FroMage
Copy link
Member

FroMage commented Aug 30, 2024

Ah, so I'm wrong and we're making the proper UserFilter, I'll have to take a look in the debugger to see what goes wrong, then.

@nasonawa
Copy link

nasonawa commented Sep 2, 2024

Hi @FroMage and @tran4774 sharing my working code and output just for your reference,

Screenshot from 2024-09-02 10-53-25

public interface IGetInfoPageController<ID, F extends PageFilter> {

    @GET
    @Path("/list")
    @Produces(MediaType.TEXT_PLAIN)
    default String getInfoPageWithFilter(@BeanParam @Valid F filter){
        System.out.println(filter.getClass().getName());
        System.out.println(filter);
        return "Hello";
    }
}

The only difference is that i am returning plain text response and i think that should not affect how bean param works.

@tran4774
Copy link
Contributor Author

Hi @FroMage, any action for this issue? I need it resolved to complete my code base

@FroMage
Copy link
Member

FroMage commented Oct 1, 2024

Sorry, I didn't get the time to look at this yet

@tran4774
Copy link
Contributor Author

tran4774 commented Nov 13, 2024

@FroMage
After several debug in the deployment phase, I figured out the problem that the transformed bytecode of UserFilter wasn't generated to implement ResteasyReactiveInjectionTarget like its abstract class PageFilter.
I think the root cause is org.jboss.resteasy.reactive.common.processor.EndpointIndexer.collectEndpoints() can't get the actual method of UserController.getInfoPageWithFilter(@BeanParam @Valid UserFilter filter) in this loop:

for (DotName httpMethod : httpAnnotationToMethod.keySet()) {
    // Here is the point
    List<MethodInfo> methods = currentClassInfo.methods();
    for (MethodInfo info : methods) {
        AnnotationInstance annotation = annotationStore.getAnnotation(info, httpMethod);
        if (annotation != null) {
            if (!hasProperModifiers(info)) {
                continue;
            }
            String descriptor = methodDescriptor(info);
            if (seenMethods.contains(descriptor)) {
                continue;
            }
            seenMethods.add(descriptor);
            ret.add(new FoundEndpoint(currentClassInfo, classNameBindings, info, httpMethod));
        }
    }
}

I think we can bind type variables from actual class info to this method. But I cannot cover that. I hope this information will help.

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

No branches or pull requests

4 participants