Hi Marek
On 05/04/12 16:11, Marek Potociar wrote:
> Hello experts,
>
> Please review the reworked filtering API proposal. To review, please
> checkout the remote branch:
>
> git clone ssh:///<user_name>/_at_git.java.net/jax-rs-spec~git jaxrs-api
> cd jaxrs-api
> git checkout -t new-filter-api origin/new-filter-api
>
> The API code is under src/jax-rs-api. Examples are under src/examples.
> The git commit history will help you to better see the actual changes.
>
> What has changed (quite a lot):
>
> * filtering API has been split into server (container) and client part.
> o client side: ClientRequestFilter, ClientRequestContext,
> ClientResponseFilter, ClientResponseContext
> o server side: ContainerRequestFilter,
> ContainerRequestContext, ContainerResponseFilter,
> ContainerResponseContext, ResourceFilter
> o Response remains a common component (went through some
> modifications though - see bellow)
> * Container filters are by default pre-matching
> o There is a new @PostMatching name-binding annotation to
> support global resource (post-matching) filters and interceptors
> o DynamicBinding interface has been refactored into
> DynamicBinder provider interface and javadoc has been clarified.
> * Request changes have been rolled back - removed RequestHeaders,
> RequestBuilder & Request API methods.
> * Some of the removed methods were added to the filter context
> interfaces
> * ResponseHeaders has been removed, Response API has been updated to
> support common use cases and some of the ResponseHeaders methods
> * Client API has been updated to accommodate the Req/Resp API changes.
> * Examples have been updated to use the new filtering API. (Positive
> fact is that in all cases the use of the new API meant less code
> in the example filter implementation code.)
>
> Other changes:
>
> * MessageProcessingException has been moved from core into root package
> * Client-side Target has been renamed to WebTarget to avoid a name
> clash with java.lang.annotation.Target from Java SE.
>
>
> Looking forward to your feedback. If possible, we would like to put the
> proposal into the next EDR that is planned for 4/20, so please, make
> sure to send your feedback by Friday next week CoB (4/13).
>
I like a lot many of the changes you did, thanks for the effort (as
usual, honestly :-)).
Looking at WebTarget reminded me how I wished we could have an entity
called WebClient whose goal is to consume or post to a given resource
(target) as opposed to the Client explicitly acting as a factory :-),
but I recall you had some pretty convincing arguments at a time
supporting the current approach. I also start liking the explicit
requirement to specify Accept via WebTarget.request...
I'm not too keen about 'Container' prefixes added to Request/Response
filters handled in scope of the server processing, I'd be happy with the
'container' in the package highlighting the 'location'.
PostMatching, ResourceInfo, and as Bill noted, DynamicBinder are nice
additions.
Response class:
I think it's become simpler, specifically it seems a number of methods
to do with the buffering support is 1 down, and say getMetadata() docs
have been fixed, but I believe some more minor work needs to be done
(perhaps at the JavaDoc level). Example, Response.getEntity() would
return what I set with ResponseBuilder.entity(). What would
Response.readEntity() do when called on the server side ? Perhaps
IllegalStateException ? I can start another thread for it.
ContainerRequestContext & ContainerResponseContext: I agree with you
that HttpHeaders represents the request headers, so it's not reusable
within ContainerResponseContext but may be within
ContainerRequestContext ? I guess you wanted it to be consistent between
the two, meaning ContainerRequestContext & ContainerResponeContext offer
the same interface toward retrieving the headers.
I wonder if ResponseHeaders still does make sense. We'd then have
ContainerRequestContext.getHeaders():HttpHeaders,
ContainerResponseContext.getHeaders():ResponseHeaders
Response.getHeaders():ResponseHeaders ?
Overall, nice effort
Cheers, Sergey
P.S I'll open a JIRA to do with ReaderInterctorContext returning a
modifiable Map<String, String>, in all other cases where modifiable maps
are provided it is Map<String, Object>
> Thank you,
> Marek