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 Client: Parameter "annotations" is always null in ParamConverterProvider #22870

Closed
mschorsch opened this issue Jan 13, 2022 · 5 comments · Fixed by #23090
Closed
Labels
Milestone

Comments

@mschorsch
Copy link
Contributor

mschorsch commented Jan 13, 2022

Describe the bug

The parameter annotations in ParamConverterProvider is always null.

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

@Path("/")
@RegisterProvider(MyParamConverterProvider::class)
interface MyRestService {

    @POST
    @Path("/{x}")
    fun xyz(@PathParam("x") @MyParam value: String): Uni<String>
}

@MustBeDocumented
@Retention(AnnotationRetention.RUNTIME)
@Target(AnnotationTarget.FIELD, AnnotationTarget.VALUE_PARAMETER)
annotation class MyParam

class MyParamConverter : ParamConverter<String> {

    override fun toString(value: String): String = value

    override fun fromString(value: String): String = value
}

class MyParamConverterProvider : ParamConverterProvider {

    override fun <T : Any?> getConverter(
        rawType: Class<T>?,
        genericType: Type?,
        annotations: Array<out Annotation>?
    ): ParamConverter<T>? {
	// "annotations" is alway null
        return annotations?.filterIsInstance(MyParam::class.java)?.firstOrNull()?.let {
            MyParamConverter() as ParamConverter<T>
        }
    }
}

Output of uname -a or ver

No response

Output of java -version

Java 11

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.6.2

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

Maven

Additional information

No response

@mschorsch mschorsch added the kind/bug Something isn't working label Jan 13, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Jan 13, 2022

@geoand
Copy link
Contributor

geoand commented Jan 14, 2022

@Sgitario you might be interested in this one.

@mschorsch
Copy link
Contributor Author

@geoand Will this be fixed in Quarkus 2.7.0 ?

@geoand
Copy link
Contributor

geoand commented Jan 19, 2022

Not sure, we have a lot to get done for 2.7.0

@Sgitario
Copy link
Contributor

@Sgitario you might be interested in this one.

Added to my TODO list.

Sgitario added a commit to Sgitario/quarkus that referenced this issue Jan 21, 2022
Note that this PR will introduce a performance penalty at load time (first time, when the rest client instance is loaded AND if and only if the rest client methods use param annotations - which is most of the times tho).

What I did was to always generate the `javaMethodX` static fields (before, it was also generated always, but it was done in the implementation MicroProfileRestClientEnricher). 
Plus, apart of the method information, we will also load the annotations and the generic types. This is done at load class time (static init).

Moreover, as I had to move some code from MicroProfileRestClientEnricher to 
JaxrsClientReactiveProcessor, in order to not increase the length of this class JaxrsClientReactiveProcessor, I started moving some code to ClassRestClientContext (which is protected - not available for users)

Fix quarkusio#22870
Sgitario added a commit to Sgitario/quarkus that referenced this issue Jan 22, 2022
Note that this PR will introduce a performance penalty at load time (first time, when the rest client instance is loaded AND if and only if the rest client methods use param annotations - which is most of the times tho).

What I did was to always generate the `javaMethodX` static fields (before, it was also generated always, but it was done in the implementation MicroProfileRestClientEnricher). 
Plus, apart of the method information, we will also load the annotations and the generic types. This is done at load class time (static init).

Moreover, as I had to move some code from MicroProfileRestClientEnricher to 
JaxrsClientReactiveProcessor, in order to not increase the length of this class JaxrsClientReactiveProcessor, I started moving some code to ClassRestClientContext (which is protected - not available for users)

Fix quarkusio#22870
Sgitario added a commit to Sgitario/quarkus that referenced this issue Jan 27, 2022
Note that this PR will introduce a performance penalty at load time (first time, when the rest client instance is loaded AND if and only if the rest client methods use param annotations - which is most of the times tho).

What I did was to always generate the `javaMethodX` static fields (before, it was also generated always, but it was done in the implementation MicroProfileRestClientEnricher).
Plus, apart of the method information, we will also load the annotations and the generic types. This is done at load class time (static init).

Moreover, as I had to move some code from MicroProfileRestClientEnricher to
JaxrsClientReactiveProcessor, in order to not increase the length of this class JaxrsClientReactiveProcessor, I started moving some code to ClassRestClientContext (which is protected - not available for users)

Fix quarkusio#22870
Sgitario added a commit to Sgitario/quarkus that referenced this issue Jan 31, 2022
Note that this PR will introduce a performance penalty at load time (first time, when the rest client instance is loaded AND if and only if the rest client methods use param annotations - which is most of the times tho).

What I did was to always generate the `javaMethodX` static fields (before, it was also generated always, but it was done in the implementation MicroProfileRestClientEnricher).
Plus, apart of the method information, we will also load the annotations and the generic types. This is done at load class time (static init).

Moreover, as I had to move some code from MicroProfileRestClientEnricher to
JaxrsClientReactiveProcessor, in order to not increase the length of this class JaxrsClientReactiveProcessor, I started moving some code to ClassRestClientContext (which is protected - not available for users)

Fix quarkusio#22870
Sgitario added a commit to Sgitario/quarkus that referenced this issue Feb 1, 2022
Note that this PR will introduce a performance penalty at load time (first time, when the rest client instance is loaded AND if and only if the rest client methods use param annotations - which is most of the times tho).

What I did was to always generate the `javaMethodX` static fields (before, it was also generated always, but it was done in the implementation MicroProfileRestClientEnricher).
Plus, apart of the method information, we will also load the annotations and the generic types. This is done at load class time (static init).

Moreover, as I had to move some code from MicroProfileRestClientEnricher to
JaxrsClientReactiveProcessor, in order to not increase the length of this class JaxrsClientReactiveProcessor, I started moving some code to ClassRestClientContext (which is protected - not available for users)

Fix quarkusio#22870
Sgitario added a commit to Sgitario/quarkus that referenced this issue Feb 4, 2022
Note that this PR will introduce a performance penalty at load time (first time, when the rest client instance is loaded AND if and only if the rest client methods use param annotations - which is most of the times tho).

What I did was to always generate the `javaMethodX` static fields (before, it was also generated always, but it was done in the implementation MicroProfileRestClientEnricher).
Plus, apart of the method information, we will also load the annotations and the generic types. This is done at load class time (static init).

Moreover, as I had to move some code from MicroProfileRestClientEnricher to
JaxrsClientReactiveProcessor, in order to not increase the length of this class JaxrsClientReactiveProcessor, I started moving some code to ClassRestClientContext (which is protected - not available for users)

Fix quarkusio#22870
Sgitario added a commit to Sgitario/quarkus that referenced this issue Feb 4, 2022
Note that this PR will introduce a performance penalty at load time (first time, when the rest client instance is loaded AND if and only if the rest client methods use param annotations - which is most of the times tho).

What I did was to always generate the `javaMethodX` static fields (before, it was also generated always, but it was done in the implementation MicroProfileRestClientEnricher).
Plus, apart of the method information, we will also load the annotations and the generic types. This is done at load class time (static init).

Moreover, as I had to move some code from MicroProfileRestClientEnricher to
JaxrsClientReactiveProcessor, in order to not increase the length of this class JaxrsClientReactiveProcessor, I started moving some code to ClassRestClientContext (which is protected - not available for users)

Fix quarkusio#22870
@quarkus-bot quarkus-bot bot added this to the 2.8 - main milestone Feb 4, 2022
@gsmet gsmet modified the milestones: 2.8 - main, 2.7.1.Final Feb 7, 2022
gsmet pushed a commit to gsmet/quarkus that referenced this issue Feb 7, 2022
Note that this PR will introduce a performance penalty at load time (first time, when the rest client instance is loaded AND if and only if the rest client methods use param annotations - which is most of the times tho).

What I did was to always generate the `javaMethodX` static fields (before, it was also generated always, but it was done in the implementation MicroProfileRestClientEnricher).
Plus, apart of the method information, we will also load the annotations and the generic types. This is done at load class time (static init).

Moreover, as I had to move some code from MicroProfileRestClientEnricher to
JaxrsClientReactiveProcessor, in order to not increase the length of this class JaxrsClientReactiveProcessor, I started moving some code to ClassRestClientContext (which is protected - not available for users)

Fix quarkusio#22870

(cherry picked from commit 7f453ca)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants