Markus,
Many thanks for the review. See my comments inlined.
On 08/20/2011 07:25 PM, Markus KARG wrote:
> Marek,
>
> thank you for this update. Please find my comments below. I know all that was discussed before, I just want to give you feedback about my view of the current situation.
>
> * Fluent API: I already mentioned that before but even after weeks of thinking over it, I have to repeat my opinion as it is unchanged still (I hoped that there might come a day when I like the API in its current form, but it didn't come): I understand that building a fluent API is really complex and there are lots of technical implications to consider. But from the user's view, the API has a strong smell of being more designed for wrapping technical implementation constrains and less for being a "user's natural language". I mean, I understand that for the flexibility we need additional "hidden" classes and all that, but for the user, it just looks some kind of "odd" to have things like "x...request()...post()" instead of just "x.post()". Again, I understand your arguments, but as the API is for the user, I want to tell how it looks from a user's view (as an ISV employee I know from my own projects that it is hard to understand the user's view sometimes just having made som
e internally technically brilliant solution). I do not know a better solution, I just wanted to comment that this complexity could lead to a rather lot of people not wanting to use that API, which obviously is not a desireable situation.
I tried to address this in the following fix:
http://java.net/projects/jax-rs-spec/sources/git/revision/64934b368c20c86a6d3e2a9c85948ade93760296
Let me know if you see any issues. Following diff provides a short list of supported fluent API flows:
http://java.net/projects/jax-rs-spec/sources/git/diff/src/examples/src/main/java/jaxrs/examples/client/BasicExamples.java?rev1=6766a4ad13b19792187886e0b66330bd59c5aca7&rev2=64934b368c20c86a6d3e2a9c85948ade93760296
>
> * Extensions: As you know I am the maintainer of the WebDAV extensions and so I really liked the server side extension points in JAX-RS 1.0. Maybe I missed it, but I did not see an explicit extension point for protocol extensions on the client side other than providing a complete chain starting with a client factory. In fact I do not see why I should provide a complete class chain just to add some more http verbs. Can you give a code example how it would look like if I want to provide a ".propfind()" or ".search()" method that can be used with the same simplicity than ".get()" or ".post()"?
While the API does not have a convenient support for extensions such as WebDAV currently, it is still possible to define
simple a wrapping extensions as outlined in this new sample:
http://java.net/projects/jax-rs-spec/sources/git/revision/e3760c66cf448a066809c0c83b54ccb036bbc141
While there is certainly some overhead associated with this approach, it is justifiable since we don't expect this type
of extension to be the major use case.
Alternatively, we could provide some adapter-based solution, that would look like e.g.:
client.target("someuri").adapt(WebDavInvoker).search(e);
client.target("someuri").accept("text/plain").adapt(WebDavInvoker).search(e);
The disadvantage being here the need to adapt every request separately.
>
> * Cache: I understood that by democratic vote the EG opted for not providing cache support, but I need to tell you that your cache sample shows how complex it is to actually add caching using the current API proposal. Do you really want each application to provide that same rather complex code again and again (note that your code actually is not a valid http cache implementation as it completely ignores any cache headers so you are forcing people to invent that on their own)? I doubt that application coders will actually do that, but rather either will copy your example (getting a false implementation) or will use a proprietary API -- so the benefit of defining a vendor-independent API fades.
>
> * Events: If I have understood the client API correctly, it will return one single response from a Future as soon as the first http response is received -- so more events can be received only by starting another request. Does that mean that JAX-RS 2.0 will only support long term polling but not directly support event streaming? This is no critic, I just want to clearly understand the proposal's intention in that area.
If I understand the question correctly, you are asking about whether a future response returned as a result of a single
request can be use to retrieve multiple responses. The answer here would be *no*. The API is intended to support only
simple req/resp interactions.
>
> * Hypermedia: On the server side JAX-RS 2.0 will come with "automatic hyperlinking" as part of the HATEOS support. Maybe I missed it, but I did not find any kind of special support for that on the client side. Is this intentionally not there, or did I just miss it?
Well, it's me, who missed it :) I had it in a stash and did not check it in. The latest master branch contains the added
Link support. This part is still fresh and open for further enhancements and discussion. Also, a builder for Link is
still missing.
Marek
>
> Regards
> Markus
>
>> -----Original Message-----
>> From: Marek Potociar [mailto:marek.potociar_at_oracle.com]
>> Sent: Mittwoch, 17. August 2011 17:33
>> To: jsr339-experts_at_jax-rs-spec.java.net
>> Subject: [jsr339-experts] Re: client revisions
>>
>> Sorry for a delayed response. I have reviewed the proposed changes and
>> based on the proposal bellow as well as based on
>> the feedback from Sergey, Guilherme and Markus I have the updated the
>> proposed client API:
>>
>> http://java.net/projects/jax-rs-spec/sources/git/show/src
>>
>> The main changes include:
>>
>> CORE HTTP REQUEST & RESPONSE API
>>
>> - Introduced Headers, RequestHeaders and ResponseHeaders + inner
>> Builder classes to hold the header-related APIs reused
>> in client-side fluent API as well as core HttpRequest and HttpResponse
>> classes
>>
>> - Introduced new core exception to decouple core req/resp classes from
>> the client API
>>
>> - Cleaned up and extended the HttpRequest/Response API ...somewhat :)
>>
>> CLIENT BOOTSTRAPPING AND CONFIGURATION
>>
>> - Added typed Feature support - see Feature and Configurable interfaces
>>
>> - Removed redundant ClientConfiguration interface as well as
>> DefaultClientConfiguration impl class
>>
>> FLUENT CLIENT API
>>
>> - Link renamed to Target
>> This is to distinguish the "active link" from the plain link header
>> POJO (Link header POJO still to be introduced).
>>
>> - Streamlined the fluent client API: client -> target -> invocation
>> builder [ -> invocation] -> response
>> -- Invocation API was reduced to just support the command pattern for
>> request processing
>> -- new Sync/AsyncInvoker interfaces were introuced to support HTTP
>> method at the end of the fluent API method chain
>> -- Invocation.Builder was introduced to enforce the consistency in the
>> fluent method invocation chain (e.g. the API does
>> not allow you to set a POST method and then invoke a GET). Such
>> "advanced" features were removed from the fluent API,
>> but the flexibility for filters and handlers is retained since those
>> work with mutable HttpRequest/Response interfaces.
>>
>> Now, I know that with this fluent client API change we somewhat closed
>> the loop with the client API proposals. The API
>> resembles the original proposal once again and the class hierarchy is
>> more complex compared to the previous version.
>> Yet, this is not necessarily a bad thing IMHO. The problem with the
>> older version of the API is that it was trying to do
>> too many things: it was supposed to be fluent, consistent and easy-to
>> use, but also remain simple and lean (in terms of
>> class hierarchy) as well as super-flexible at the same time. The main
>> point in providing a fluent API is however to be
>> primarily consistent and easy to use by the end users. Reduced
>> flexibility and slightly increased complexity of the
>> class hierarchy is a reasonable price to pay. It is the usability of
>> the fluent API that is important, not how the
>> javadoc looks like or how many (hidden) classes are there. And, again,
>> please notice that the original flexibility
>> requirement is still preserved in the places where it makes most sense
>> - filters and handlers are still using the
>> HttpRequest/Response interfaces.
>>
>> With that, I am looking forward to your feedback,
>> Marek
>>
>> On 07/05/2011 05:30 PM, Bill Burke wrote:
>>> I made some changes to client API to simplifiy it a little.
>>>
>>> https://github.com/patriot1burke/redhat-jaxrs-2.0-proposals-rev2
>>>
>>> * removed queue from Invocation interface
>>> * Added an AsyncInvocation interface
>>> * Added get/post/delete/etc. methods to AsyncInvocation and
>> Invocation
>>> * removed invoke/queue methods from Client as they are redundant and
>> I don't get why they are there.
>>> * removed HttpRequest.Builder as its not needed anymore
>>> * Link no longer extends HttpRequest.Builder
>>> * added request/async methods to Link
>>> * Gutted and simplified ClientFactory. No need for all those extra
>> abstractions.
>>>
>>> You now are forced to put a get(), Post() or whatever at end of
>> method call unless you are doing a custom HTTP method
>>> like PATCH. It now looks like:
>>>
>>> Client c = ...;
>>>
>>> Invocation invocation = c.request();
>>> HttpResponse response = invocation.header("foo", "bar").get();
>>> String str = invocation.header("foo", "bar").get(String.class);
>>>
>>> AsyncInvocation async = c.async();
>>> Future<HttpResponse> f = async.header("foo", "bar").get();
>>> Future<String> f = async.header("foo", "bar").get(String.class);
>>>
>>> Generic invocations are:
>>>
>>> Invocation invocation = c.request();
>>> HttpResponse response = invocation.header("foo",
>> "bar").method("PATCH").invoke();
>>>
>>>
>>> Alternatively, you could combine Invocation/AsyncInvocation, but
>> you'd have to rename get() to getAsync(), etc. I think
>>> originally it wasn't done this way because of the explosion of
>> methods within Invocation.
>>>
>>>
>>>
>