Marek - I apologize, I think I was a bit childish - thanks for being
patient. More comments to follow
On 31/08/11 18:37, Marek Potociar wrote:
> FYI - forgot to reply to EG list.
>
> On 08/31/2011 07:37 PM, Marek Potociar wrote:
>>
>>
>> On 08/31/2011 07:13 PM, Sergey Beryozkin wrote:
>>> 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
>>
>> Not at all. Overloads with different parameter types are perfectly ok. The problem is in the same type of the params
>> with completely different meaning. This becomes more prominent with 3+ args in the method. Anyway, this is not the only
>> issue with the proposed method. Breaking Entity API and limiting the accept to a single parameter are the ones that I'm
>> much more concerned about.
>>
>>>
>>>>>
>>>>>> 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
>>
>> I'm sorry, perhaps I did not read that carefully. Anyway, I was just confirming your assertion is valid.
>>
>>>
>>>> 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()...
>>
>> I'm getting lost here. Did you have a chance to look at the latest API sources and examples yet?
>>
>>>>>
>>>>>>> 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.
>>
>> Sure you can, this is the equivalent:
>>
>> request("foo").post(entity(new Book(), "bar"));
>>
>> ...and if you really need the full flexibility, there's the Invocation API:
>>
>> i.asRequest().type("bar").accept("foo").entity(new Book()).method("POST").redirect("http://out.of.here/");
>> i.invoke();
>>
>>> Fine with parking this thread though
>>>
>>> Just my 2c
>>
>> Ok, this thread is really long enough and deserves retiring...
>>
>> Still, thanks for the feedback,
>> Marek
>>
>>
--
Sergey Beryozkin
http://sberyozkin.blogspot.com
Talend - http://www.talend.com