Well, these comments refer to the previous revision, but I'll continue
as I guess it could be relevant anyway...
On 26/08/11 11:07, Marek Potociar wrote:
>
>
> On 08/25/2011 01:38 PM, Sergey Beryozkin wrote:
>> Hi
>>
>> Here are some comments about Target and Invocation [1]
>>
>> 1. Target.getUriBuilder()
>>
>> what is the idea behind this method ? In other words, what is the expected outcome of
>>
>> client.target("http://bar").getUriBuilder().build() ?
>>
>> I'm presuming getUriBuilder() needs to return a cloned builder. But I'm still not sure what is next after
>> getUriBuilder() is invoked ?
>
> UriBuilder is just another representation of the target URI. Some users may want to use it as a shortcut for:
>
> UriBuilder.fromUri(target.getUri());
>
> The returned uri builder is disconnected from the internal state of the target.
>
I'm still not getting though what I'd use this method for ?
To avoid typing UriBuilder.fromUri(target.getUri()); ?
I can understand why UriInfo needs such methods - but not sure what I
can do with getUriBuilder().build() or similar in context of working
with URI
>>
>> 2. I'm wondering if Target path& query methods accepting Annotations should also be dropped ? (assuming they were
>> originally added to support the validation at the low-level api level)
>>
>
> Good catch - removed. Thanks.
>
>> 3. Would it make sense to have Target.fragment(String) ?
>
> I'm not clear on the purpose of such method. Can you elaborate? Please also look at the existing methods and specify how
> it would differ from the related ones.
>
to add #bar or similar to the end of URI - I did not spot it at Target
in the prev revision
>> 4. Can we consider adding a back() method ? That would allow Target glide back to the previous segment after multiple
>> path() invocations...
>
> Seems to me that current set of methods can cover the use cases where back() would be usable. The method would add more
> complexity without a real benefit (you would probably also need methods for finding out where would back() take you in
> any given moment, etc.).
>
How the current rev can help with iterating over a set of relative URIs ?
I propose to have back() returning strictly to the prev path segment,
optionally to the base URI
>> 5. Target.prepare()
>>
>> I can see that the idea here is to create an invocable entity which can be used in batched invocations. I think the fact
>> that Target can be used to perform simple and sophisticated HTTP invocations without relying on prepare()/request() is
>> very good.
>>
>> So Target.prepare() kind of does make sense and I think it's clean it's done - I've set Target as needed, including all
>> the headers, etc and then I do prepare() and then I finalize it with selecting a verb.
>>
>> The thing I don't understand why would anyone use Invocation.
>> If it's a standalone Invocation, a single one, then Target just works
>> perfectly well on its own, for sync or async support of its own.
>>
>> There's no need to have Invocation to duplicate what Target can do for
>> simple single and specific invocations.
>>
>> So then we'd use Target.prepare() to create many Invocations and submit them to the queue or batch processor which can
>> happily do invoke() for every Invocation in the collection.
>>
>> And I don't know what it means in practice. I think it can only work for empty post pings or similar...
>>
>> IMHO, if we can come up with a good practical example of using a collection of Invocations then Target.prepare() should
>> stay.
>> If not then Target.prepare() and Invocation should be dropped which
>> IMHO can only improve API in the positive way
>
> In my local sandbox the prepare() is already dropped.
It's a bit painful for me to comment on this one :-),
Cheers, Sergey
>
> Marek
>
>>
>> Thanks, Sergey
>>
>>
>>
>> [1] http://java.net/projects/jax-rs-spec/sources/git/show/src/jax-rs-api/src/main/java/javax/ws/rs/client/
>>
>>
--
Sergey Beryozkin
http://sberyozkin.blogspot.com
Talend - http://www.talend.com