On 08/26/2011 05:22 PM, 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. I thought we were nearly there...instead we
> are back to where we were at the get go
Please have a look at samples or try to play with the API a little bit. You may find that there is no verbosity added
while the SoC principle is preserved. Typically you would write .accept(...) anyway. Now you don't have to. Instead of
saying "Client - targeting someuri - accept text/plain - get" you now say "Client - targeting someuri - request
text/plain - get".
Marek
>
> Sergey
>
>
>>
>> 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
>
>