On 09/25/2011 07:00 PM, Sergey Beryozkin wrote:
> Hi Marek
>
> here are some comments:
>
> - a form(...) entity qualifier in the example showing post(form(new Form(...)))
> seems redundant given that Form is a dedicated class capturing a form submission
Form is a concept used both on the server as well as the client side. As such it cannot extend client-side Entity. One
solution would be to create a special overload for post() to accept Form directly. But I'm not sure it's worth the
effort. Perhaps renaming Entity.form(Form) to Entity.entity(Form) (or Entity.encoded(Form) etc.) would do the trick?
>
> - example invocationFlexibility concerns me a bit.
>
> I've checked the source and I see no accept() method at all - Invocation.Builder has a header(...) method, so I can use it.
> Target allows for
>
> target.request()
> and
> target.request(acceptMediaTypes)
>
> The former option requires me to do header("Accept", "text/xml") but still lets me do acceptLanguage(...). And if
> Request.RequestBuilder did have accept() then
>
> target.request().buildGet().asRequestBuilder().accept("text/html").invoke()
>
> vs
>
> target.request(acceptMediaTypes).get();
>
>
> seems 'unfair' toward target.request() :-).
Target.request() is not supposed to be used if you expect to receive a response back. In such case
target.request(type...) should be used. It's ok, if you see the API as being unfair to the Target.request() in this
scenario. We want it to be unfair.
Also, if you happen to have a more esoteric use case in which you don't know what you are going to invoke or what you
can accept at the time of constructing the invocation, then you have 3 options:
1. Rethink the logic & design of your code as chances are that there is a simpler or more elegant solution for your
problem (e.g. maybe you want to use filters or maybe you want to deffer the invocation construction to some later point
in time).
2. Use generic Invocation.Builder.header(...) method (but only after you failed with #1)
3. Go one level lower and use the generic Invocation API + Invocation.asRequestBuilder() to perform the "last-minute"
modifications of the request before invoking.
> Can we have accept() on Invocation.Builder ?
There's no reason for that. Target.request(acceptedMediaTypes) serves the same purpose in the main (anticipated) API
flow. If you remember, we discussed in the past that combining request().accept(types) into single request(types) will
help us retain SoC between Target and I.B in the API while reducing the number of method invocations to a minimum at the
same.
Cheers,
Marek
>
> Cheers, Sergey
>
>
>
> On 20/09/11 15:30, Marek Potociar wrote:
>> Hello experts,
>>
>> Please review the proposed change I have just commited:
>> http://java.net/projects/jax-rs-spec/sources/git/revision/f48ae3d9a98a6194df3abe708347083bf78afc91
>>
>> The change branches off the configuration flow of client-side artifacts from the main request-to-response execution
>> flow. From my recent discussion with Sergey on this list as well as from further feedback I have received privately it
>> seems the mixed mutable and immutable methods on Targets may indeed cause some user confusion.
>>
>> To reduce the confusion we decided to make Target "dirty" immutable - Target will not expose any methods that would
>> return same Target instance, however Target's configuration state can be indirectly modified (in a separate method flow)
>> by accessing its Configuration instance and invoking mutators on that instance. Configuration interface is a
>> rename/replacement for former Configurable<T> interface. Please, look at the samples, they should be rather
>> self-explanatory.
>>
>> We decided to use this solution because it allows for keeping the Target immutable wrt. URI which helps us preserve
>> URI-based identity contract (equals/hashCode) and makes it possible to use Target instances safely with e.g. Java
>> collection classes etc.
>>
>> Kind regards,
>> Marek
>>
>>
>
>