jsr339-experts@jax-rs-spec.java.net

[jsr339-experts] Re: Removing command pattern simplifies things

From: Marek Potociar <marek.potociar_at_oracle.com>
Date: Wed, 31 Aug 2011 19:37:58 +0200

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
>
>