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