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

Handling Accept with both Consumes/Produces and HeaderParam annotations in JAX-RS contract #890

Closed
bwangfdu opened this issue Feb 1, 2019 · 4 comments
Labels
bug Unexpected or incorrect behavior regression Bugs and issues related to unintended breaking changes

Comments

@bwangfdu
Copy link

bwangfdu commented Feb 1, 2019

I am facing the API of JAX-RS 2.0 contract with this definition:

@GET
    @Path("/orders/{externalOrderId}")
    @Produces({ "application/vnd.mycompany.application.order.v1+json", "application/vnd.mycompany.application.order+json", "application/vnd.mycompany.application.order.v2+json" })
    @ApiOperation(value = "Orders_GetOrderDetail", tags={  })
    @ApiResponses(value = { 
        @ApiResponse(code = 200, message = "OK", response = Order.class),
        @ApiResponse(code = 400, message = "Bad request", response = BadRequestResponseModel.class),
        @ApiResponse(code = 404, message = "Not Found", response = NotFoundResponseModel.class),
        @ApiResponse(code = 412, message = "Precondition Failed", response = PreconditionFailedModel.class),
        @ApiResponse(code = 500, message = "Internal Server Error", response = InternalErrorResponseModel.class) })
    public Order ordersGetOrderDetail(@PathParam("externalOrderId") String externalOrderId, @HeaderParam("Accept") String accept, @HeaderParam("Authorization") String authorization);

This is a design for supporting multiple data model and choose reply data model by the Accept header. But when I using OpenFeign 10.0.1, it put all the Produces and Accept into accept parameter into header.
We suppose to send only the header value in the parameter in this scenario.

And another issue is the @Conumes, when there are multiple values, it send all in Content-Type header. I think that may not make sense to have a body with multiple types.

I am not sure whether this is a mismatching of JAX-RS 2.0 definition or whatever.

@bwangfdu bwangfdu changed the title Handling Accept with both Consumes and HeaderParam annotations in JAX-RS contract Handling Accept with both Consumes/Produces and HeaderParam annotations in JAX-RS contract Feb 1, 2019
@bwangfdu
Copy link
Author

bwangfdu commented Feb 1, 2019

I find version 10.1.0 fixed by using the only first @consumes value as the Content-Type. But there is another bug which I reported in #891

@kdavisk6
Copy link
Member

kdavisk6 commented Feb 1, 2019

That is intentional. The JAXRS annotations we support are meant to help with setting up clients but do not implement the JAXRS specification. What you've run into one of those situations. The @Produces annotation is an alias for the "Accept" header. This is because @Produces has no real meaning from a client perspective. We map it so users can create interfaces that "feign" a server. A similar situation is with the @Consumes annotation, it is an alias for the "Content-Type" header, which you have discovered.

@kdavisk6
Copy link
Member

kdavisk6 commented Feb 1, 2019

There is a bug in the JAXRSContract. It usage of the template to modify the headers and queries is incorrect. It is no longer possible to modify the query and header collections directly, they must be modified using the appropriate header and query method. I'll take a look.

@bwangfdu
Copy link
Author

bwangfdu commented Feb 1, 2019

My forked project fixed it, but add some additional dealing with Content-Type and Accept to make them overwritable from a parameter with the same name.
Please see https://github.com/bwangfdu/feign/blob/master/jaxrs/src/main/java/feign/jaxrs/JAXRSContract.java
The previous merge requests built failed because I modify the code in web and missed here and there.

There is a bug in the JAXRSContract. It usage of the template to modify the headers and queries is incorrect. It is no longer possible to modify the query and header collections directly, they must be modified using the appropriate header and query method. I'll take a look.

@kdavisk6 kdavisk6 added bug Unexpected or incorrect behavior regression Bugs and issues related to unintended breaking changes labels Feb 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected or incorrect behavior regression Bugs and issues related to unintended breaking changes
Projects
None yet
Development

No branches or pull requests

2 participants