OpenFeign / feign

Feign makes writing java http clients easier
Apache License 2.0
9.51k stars 1.93k forks source link

Jaxrs3 / Jakarta contract does not support Jakarta annotations #2643

Closed Kennelly57 closed 1 week ago

Kennelly57 commented 2 weeks ago

Hello there,

First of all, thank you to the maintainers for putting good work into this open source library. I appreciate all of the work that has been done to support the client.

However, I believe I have found an issue with the annotation processing for the JAXRS contracts that use the newer Jakarta packages. This should apply to the Jakarta contract, and the JAXRS3 contract that has deprecated the Jakarta contract.

When I attempt to create a client using our annotated interface as a target, none of the annotations are properly picked up. I get the following error, despite these annotations theoretically being valid. I believe the issue is due to the Jakarta based annotations not being processed as the matching javax annotations with the same names.

Screenshot 2024-11-05 at 1 57 54 PM

Our code works as expected when using 13.1 (see below for snippets), but not on any of the later versions. More specifically, I believe this commit seems to be responsible for breaking it: https://github.com/OpenFeign/feign/commit/00b04bc5bc2ece38270e41cc4d39072cc0fb8382. I understand that it is attempting to use bytecode transformations, but I am not confident these are being run correctly when importing the packages via Gradle. When I check the imported dependencies, I can see that the underlying JAXRS2Contract and JAXRSContract still use the javax imports, which again seems to point to a compatibility issue between Javax and Jakarta annotations.

Edit: After continuing to look at the problem, I believe I configured Gradle to select the proper variant of Jaxrs, using Jakarta imports. However, on runtime, and when inspecting the underlying source, I still see the older Javax files being used. I do not work much with Maven, but this leads me to believe that the underlying bytecode transformer (I think from org.eclipse.transformer) is not being run before the build is sent to maven central. This perhaps might point to this solution not being viable for Gradle users.

Screenshot 2024-11-05 at 2 49 17 PM

Is there another way we could restructure the packages to remove some code deduplication while retaining functionality for Jakarta packages?

HealthCheckApi serviceClient = Feign.builder().contract(new JAXRS3Contract())
                .encoder(new JacksonEncoder(objectMapper))
                .decoder(new JacksonDecoder(objectMapper)).options(new Options(CONNECT_TIMEOUT_MILLIS,
                        TimeUnit.MILLISECONDS,
                        READ_TIMEOUT_MILLIS,
                        TimeUnit.MILLISECONDS,
                        true))
                .target(HealthCheckApi.class, baseUrl);
import com.codahale.metrics.health.HealthCheck;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import jakarta.ws.rs.GET; // using Jakarta packages
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;
import java.util.Map;
import org.immutables.value.Value;

@Produces(MediaType.APPLICATION_JSON)
@Path("/healthcheck")
public interface HealthCheckApi {

    @Value.Immutable
    @JsonSerialize(as = ImmutableNamedHealthCheck.class)
    @JsonDeserialize(as = ImmutableNamedHealthCheck.class)
    interface NamedHealthCheck {
        String name();

        HealthCheck healthCheck();
    }

    @GET
    Map<String, HealthCheckResult> getStatus();
}
okrische commented 1 week ago

You most likely have feign-jaxrs AND feign-jaxrs3 in the classpath. This will not work. I fell into the same trap. You can see it in the dependencies, that each one pulls in, the feign-jaxrs3 pulls in a dependency that has classifier "jakarta". So there is for example this class, i think it is feign.JAXRSContract, that is implemented more than once in these dependencies, once for javax, once for jakarta. And depending on what is seen first in the classpath, that one will be used. So, maybe feign-jaxrs3 sees the feign.JAXRScontract from the dependencies pulled by feign-jaxrs and then you have this mismatch.

You have to decide, if you either use jaxrs or jaxrs3 in your app.

(I wish it had to be done differently. This basically means, the API must be annotated twice, once with javax-annotations and once with jakarta-annotations. And in your application you must decide: do i use javax or do i use jakarta. But you can not used both at the same time. Lots of migration effort...)

Kennelly57 commented 1 week ago

@okrische Thank you for the pointer, Olaf, you seem to have been correct! I had a dependency that was sneaking in some javax dependencies. The classpath was a little bit messy because we are upgrading some libraries which have moved from Javax -> Jakarta.

Once I took that dependency out, and then ensured that there were no javax dependencies on the classpath, transitive or otherwise, the Jaxrs3 contract seems to work. Closing this issue.