Hi Marek
On 31/08/11 17:58, Marek Potociar wrote:
> Hi Sergey,
>
> On 08/31/2011 12:03 PM, Sergey Beryozkin wrote:
>> On 31/08/11 10:48, Marek Potociar wrote:
>>> Varargs just add extra confusion but the problem "which is which" is there even for a method(String, String).
>>
>> what do you "which is which" ? I'm proposing to bin the option to specify multiple accepts in a request(...) and have
>> two well-document option, one which tales a single arg - that is accept, the one which takes 2 options takes
>> Content-Type as the 1st param, Accept - as a 2nd one
>
> If you look at the code it is not immediately clear which string is which without knowing the exact details of the
> method javadoc. The javadoc or source code is the only way how to infer the important information about the position of
> each header in the method. You can't tell it by looking at the code using the API or by inspecting the compiled class.
> IOW, unless you know the exact ordering, you are still lost even if you know that the method takes CT and Accept as
> arguments.
>
Hmm... That sounds to me like an arg against having overloaded methods
in general ? I don't understand the above argument, sorry
>>
>>> Additionally, as I said, content type really belongs to the part where entity is provided).
>>>
>>
>> This is just too rigid - request is a transitional method and it does make an exception for Accept, presumably to let
>> people avoid typing request(). I'm proposing to do another shortcut
>
> Actually, in the current API it DOES make an exception for Accept headers. It is done this way as GET is assumed to be
> by far most prevailing request. According to some sources it may be somewhere at 80% of all client-side invocations.
> Next most popular method (although far behind GET) is POST. As it happens, POST, too, often expects some entity in
> response. In summary, Accept is the client side header that is expected to be present in more than 90% of the use cases.
> Thus such a prominent place in the client API.
>
I think I was actually asserting that there was indeed an exception made
for Accept but not for CT
> Another way to look at it is that we decided to keep the transitional request() method. This made the most prevailing
> GET request look unnecessary verbose:
>
> client.target(...).request().accept(...).get();
>
> The proposed shortcut to merge request().accept(...) into request(...) is justified at this point by the fact that GET
> is the prevailing use case and use of Accept is even more prevailing. Additionally, the shortcut does not interfere with
> the proposed Entity definition API. I am afraid none of the above can be said about the request(A, CT).
>
>>>
>>> Not to me. It's perhaps a matter of taste, but I interpret the above as "Give me text or json in return for the posted
>>> item (represented as unknown media type)".
>>>
>>
>> Why you keep saying that :-) ?
>
> Because that's the way I see it... :-)
>
>> I've said twice that I proposed to drop a request(String... accept) because IMHO it's not
>> the most common case. And have 2 methods one taking Accept only and one - Ct and Accept only and thus in the latter case
>> CT would still be close enough to post...
>
> Seems to me we differ in the opinion on what is the most common use case. Also, putting aside the issue with breaking
> the Entity definition API, if we do what you suggest, we would need to introduce .accept() method to accommodate users
> who need the ability of defining multiple accepted media types. So instead of:
>
> target.request(a,b,c).get();
>
> One would need to do:
>
> target.request(a).accept(b).accept(c).get();
>
> ...and that just looks plain awful.
>
Hmm, I thought we still had a plain request()...and now I see there's no
even specific accept()...
>>
>>>> By the way, does one really has to use entity wrapper all the time or one of its static
>>>> helpers, no way to set content-type freely ?
>>>> I'm presuming the following is allowed
>>>> inv.header("Content-Type", "text/xml").accept("text/xml").post(new Book()) ?
>>>
>>> Yes, if you want the extra freedom, there's the Invocation you can use. The difference is that invocation does not have
>>> named HTTP methods. It only has generic invoke/submit. Also, it does not have any header/entity mutators, but it
>>> provides access to HttpRequest. So the above code would need to be written as:
>>>
>>> invocation.asRequest().type("text/xml").accept("text/xml").entity(new Book()).method("POST"); // modify request freely
>>> invocation.invoke(); // invoke the modified request
>>>
>>> Yes, it's more verbose. But this is not the target use case of th API, although it's certainly good we provide the
>>> flexibility for users that truly need it.
>>>
>> Well, well...As said I was warming up but seeing these example makes me dizzy...Having no explicit setter for CT without
>> having to resort to the above unreadable sequence ? I'm sorry - I do appreciate you are working very hard - I honestly do.
>
> Thanks. But not sure we are on the same page. The above is the Invocation API. Flexible, but not so straightforward. Of
> course there is an explicit way for setting the CT in the fluent API:
>
> client.target(...).request().post(Entity.entity(data, "application/xml"));
>
> Or, in case of XML (or JSON, plain text, HTML or form-urlencoded) using one of the provided Entity convenience methods:
>
> client.target(...).request().post(Entity.xml(data));
>
Nope, no explicit way is there...
Anyway - I can live with all of it - I just don't like it at all, all
these static imports, Entity.xml, etc.
Having no ability to type
contentType("bar").accept("foo").post(new Book())
is wrong. Fine with parking this thread though
Just my 2c
Sergey
> Cheers,
> Marek
--
Sergey Beryozkin
http://sberyozkin.blogspot.com
Talend - http://www.talend.com