Hi Bill, Markus, Sergey, and others,
On 08/25/2011 04:23 AM, Bill Burke wrote:
>
> I took GIT HEAD and made the changes I was arguing about in previous emails:
>
> https://github.com/patriot1burke/redhat-jaxrs-2.0-proposals-rev2/tree/master/jax-rs-api-r3/src
>
> View the README.txt there for changes.
I made more changes in order to find a good compromise between API clarity and simplicity on one hand and fluent context
consistency on the other:
http://java.net/projects/jax-rs-spec/sources/git/show/src/jax-rs-api/src/main/java/javax/ws/rs/client/
The most notable change is the introduction of Entity concept in the fluent API as well as ability to process invocation
as a HttpRequest via Invocation.asRequest().
The idea is that the API can be used as "higher-level" - fluent only:
HttpResponse r = client.target(...).request("text/plain").get();
Or mixed-mode, "low-level", flexible, via invocation:
Invocation i = client.target(...).request("text/plain").buildGet();
i.asRequest().redirect(...)...; // do any magic with the nested request you want
HttpResponse r = i.invoke();
Markus, Sergey: I have reintroduced the target.request(), you didn't like. However I tried to do it in a way that does
not bring any extra method into the invocation chain. Instead of:
client.target().accept("text/plain").get();
You can now write:
client.target().request("text/plain").get();
IMO, the resulting chain reads quite well.
Bill: Let me respond to some of the proposed changes in your README.txt:
* Took out path(), queryParam)(), etc. from HttpRequest to avoid "inconsistent" fluency in client api
I am ok with this simplification. Those methods were not initially in my early proposals anyway. For filters, having
path-related getters + redirect() is enough. I have made this change, including a revised path/uri getters -
all methods dealing with base uri were removed as they does not seem to make sense on the client side and can be coveed
by injected UriInfo on the server side, unless I am missing something.
* Took out redirect(UriBuilder) method from HttpRequest as you need path() queryParameter, etc. to fill in path parameters
See my comment above - there are valid cases to support redirection or any other path modification in filters. Removing
such support completely would cause a significant lost in filter functionality.
* Moved Headers.getAllowed() to ResponseHeaders as this is a response thang
It's actually not a response header. According to the spec:
"The Allow header field MAY be provided with a PUT request to recommend the methods to be supported by the new or
modified resource. The server is not required to support these methods and SHOULD include an Allow header in the
response giving the actual supported methods."
* Merged Headers and Headers.Builder as all implementing interfaces implement both classes anyways
* Merged RequestHeaders and RequestHeaders.Builder as all implementing interfaces implement both classes anyways
* Merged ResponseHeaders and ResponseHeaders.Builder as all implementing interfaces implement both classes anyways
How about injection? I was hoping one could inject e.g. read-only request headers.
FWIW, in fact, if you were not pushing so much against builders and for interface reduction, I would have separated
current HttpRequest/Response into mutable and immutable interfaces using Builders. That's btw. more aligned with JAX-RS
1.x style as well as better respect SoC principle. (Also from my past experience it seems to make a lot of sense to keep
mutable and immutable interfaces separated, but this is admittedly hard to generalize upon.)
* Moved Headers.allowed() Headers.cacheControl to ResponseHeaders as they are response headers
According to the HTTP spec, both of the headers have valid usages on both request and response.
* Combined AsyncInvoker and SyncInvoker by renaming AsyncInvoker methods to putAsync(), etc. All this is within an
Invocation interface
In the fluent interface I prefer async().put() over putAsync() or asyncPut(). For that matter I decided to keep the
Sync/AsyncInvoker interfaces. Those interfaces nicely encapsulate the concept of sync/async uniform interface.
* Invocation interface extends HttpRequest
Instead of this, I added Invocation.asRequest() method, that exposes the internal, mutable request structure.
* Target now only implements Configurable
* Target now has an invocation() method.
I removed the extra interfaces from Target and re-introduced request([...]) including overloaded versions that take
accepted types as input. It's a good compromise between having and not having an intermediate request method.
Additionally, based on your feedback I have updated header value getters to return String values, not Objects.
In summary, I tried my best to include as many simplification suggestions as possible while at the same time
strengthening the API flow context consistency without a compromise in the flexibility. While all these are quite
contradictory requirements, I think I managed to accomodate them and I am happy with the result.
Marek