jsr339-experts@jax-rs-spec.java.net

[jsr339-experts] Re: Updating requests/responses in Filters

From: Bill Burke <bburke_at_redhat.com>
Date: Fri, 17 Feb 2012 11:22:29 -0500

So, getRequest/ResponseBuilder would not return a copy anymore, but a
fresh empty one?

Should copy() be clone() instead?

On 2/17/12 8:35 AM, Marek Potociar wrote:
> Fellow experts,
>
> currently the latest version of FilterContext API provides following methods for working with Req/Resp:
>
> Request getRequest();
> Response getResponse();
>
> void setRequest(Request req);
> void setResponse(Response res);
>
> Request.RequestBuilder getRequestBuilder();
> Response.ResponseBuilder getResponseBuilder();
>
> Response.ResponseBuilder createResponse();
>
> Personally I find the combination of getter+setter and also builder getter confusing. It may lead to assumptions, that
> following code updates the request headers in the context if value of request Content-length is greater than 1024:
>
> if (context.getRequest().getHeaders().getLength()> 1024) {
> context.getRequestBuilder().header("foo", "bar").build();
> }
>
> ...but it doesn't. The correct version of the code above is:
>
> if (context.getRequest().getHeaders().getLength()> 1024) {
> final RequestBuilder rb = context.getRequestBuilder();
> rb.header("foo", "bar");
> context.setRequest(rb.build());
> }
>
> Thus I would propose following changes:
>
> C1. introduce a new method in the Request/Response:
>
> public Reqest/ResponseBuilder copy();
>
> C2. remove the getRequest/ResponseBuilder() methods from FilterContext.
>
> The updated version of the correct code from above would then look like:
>
> if (context.getRequest().getHeaders().getLength()> 1024) {
> final RequestBuilder rb = context.getRequest().copy();
> rb.header("foo", "bar");
> context.setRequest(rb.build());
> }
>
> Note that the amount of code remains exactly the same, but the code itself much more clearly conveys the fact that we
> are working with a new copy of the request, not some internal mutable version of the request associated with the request
> context (although such optimization is, of course, still possible but remains hidden as an internal implementation detail).
>
> Additionally, seems to me that createResponse() method is redundant, as static Response factory methods could (and
> perhaps should) be used instead. Thus I'd like to propose another change:
>
> C3. remove the createResponse() method from the FilterContext.
>
> Also, I feel we miss a more generic header replacement method in both, Request& Response builders:
>
> C4. add replaceAll(MultivaluedMap<String, String> headers) method to Request& Response builder API
>
> I'd like to proceed with the proposed changes next week. Let me know if you have any objections or comments by Tuesday
> Feb 21, CoB.
>
> Thanks,
> Marek

-- 
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com