On 26/08/11 16:39, Santiago Pericas-Geertsen wrote:
>
> On Aug 26, 2011, at 11:22 AM, Sergey Beryozkin wrote:
>
>>>
>>> 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();
>>
>> Why would we voluntarily sacrifice the former for the latter is beyond me.
>
> Without an explicit transition between a Target and an invocation/request, the Target interface contains a number of extraneous methods only to make the transition implicit. The proposed change helps clean this up IMO.
>
In the previous revision Invocation was a 'dead' redundant entity only
waiting to be dropped. It's back...
Cheers, Sergey
> -- Santiago
>
>>> 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
>>
>>
>> --
>> Sergey Beryozkin
>>
>> http://sberyozkin.blogspot.com
>> Talend - http://www.talend.com
>
--
Sergey Beryozkin
http://sberyozkin.blogspot.com
Talend - http://www.talend.com