On 08/22/2011 05:49 PM, Sergey Beryozkin wrote:
> I was actually looking forward to Bill's revision being approved too - thought there was some agreement about
> finalizing the chain with get()/etc in most cases and generally I thought it was step forward toward the simplification of the client api.
While we agree with Bill in the end result (i.e. what the end-user sees in code), we seem to differ in the solution.
Bill prefers API simplicity over consistency. I prefer API consistency over simplicity, esp. in case of a fluent API.
When it comes to the ease of use, both versions are comparably easy to use.
>
> More comments soon, just back form holidays - particularly keen to see how the idea of Active links has been implemented -
There's nothing fancy to it - I just renamed client.Link to client.Target to distinguish it as an active link. Then I
introduced a Link POJO header as well as client.target(Link) method. This is still a fresh area however, so there should
be a room for suggestions and improvements.
Marek
>
> thanks, Sergey
> ________________________________________
> From: Bill Burke [bburke_at_redhat.com]
> Sent: 22 August 2011 15:21
> To: jsr339-experts_at_jax-rs-spec.java.net
> Subject: [jsr339-experts] Re: client revisions
>
> First the good things: I like the addition of the Feature interface.
> It should be allowed to be annotated with @Provider and scanned and
> automatically registered too. You did a good job of refactoring my
> interceptor proposal. I like the class names and hierarchy there.
>
> Now the bad...
>
> At first glance the class hierarchy is ridiculous. The Javadoc will not
> be very useful to first-time users as the class hierarchy is hard to
> understand even for somebody that has been working on JAX-RS 2.0 (i.e.
> me). I dread the day when I have to update my O'Reilly book to explain it.
>
> I want you to do an exercise:
>
> 1. Count how many Builder interfaces you have
> 2. Calculate the average of how many interfaces an interface implements
> 3. Load this revision into a graphical class hierarchy browser and view
> all the criss-crossing lines.
>
> Then maybe you'll see how complicated you've made this API in order to
> make it "pure". IMO, ditch this, revert to the previous version, and
> let's move on from there.
>
> I also think there are a lot of problems with it:
>
> * How do you set the Content-Type of a Target? What I'm getting at is,
> what's the point of having Target implement the SyncInvoker interface?
>
> target.put(entity) makes no sense as there's no way of setting the
> content-type of the entity.
>
> * The only way to go from a Target to an Invocation/Invocation.Builder
> is to set a header on Target. What if you don't want/need to set a
> header? i.e. a get or delete request?
>
> * Client.Builder and ClientFactoryBuilder and ClientFactory can be
> combined. There's no need to have a factory of a factory of a factory.
> Please see my earlier proposal (4-5 weeks ago) on a better way to do this.
>
>
> All and all, I'm really disappointed with this revision. I think the
> previous revision plus the additional changes I proposed (in the GIT i
> linked) was far superior had a greatly simplified class hierarchy and
> was just as simple to use.
>
>
>
>
>
>
>
>
>
>
> On 8/17/11 11:33 AM, Marek Potociar wrote:
>> 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.
>>>
>>>
>>>
>
> --
> Bill Burke
> JBoss, a division of Red Hat
> http://bill.burkecentral.com