You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by Dmitry Potapov <po...@gmail.com> on 2013/09/03 21:47:34 UTC

Javadocs inconsistency concerning [Basic]HttpResponse in HttpCore-4.3

Hello everyone,

I'm a bit confused with some inconsistencies in javadocs concerning
HttpResponse. Probably you can confirm that some of these inconsistencies are
typos or bugs:
  1. [Basic]HttpResponse: getLocale/setLocale is marked as deprecated:
    "use org.apache.http.impl.DefaultHttpRequestFactory"
    Is org.apache.http.impl.DefaultHttpResponseFactory implied here?

  2. HttpResponse: setStatusCode has the following note in the description:
    "The reason phrase will be updated according to the new status code,
    based on the current locale".
    But in the BasicHttpResponse.setStatusCode implementation I see that
    'reasonPhrase' is left untouched. My first desire was to add the following
    line to this function (as it seems to be most elegant solution):
    "this.reasonPhrase = getReason(code)"
    but BasicHttpResponse.getReason is marked deprecated now, so this fix
    doesn't seem to be acceptable.

What is the correct way to set HttpResponse status code with reason phrase
update in HttpCore 4.3?

-- 
Dmitry

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Re: Javadocs inconsistency concerning [Basic]HttpResponse in HttpCore-4.3

Posted by Oleg Kalnichevski <ol...@apache.org>.
On Wed, 2013-09-04 at 15:20 +0400, Dmitry Potapov wrote:
> My case is trivial:
> 1. Download ElementalHttpServer.java from
> http://hc.apache.org/httpcomponents-core-4.3.x/examples.html
> 2. Compile
> 3. Run with '/tmp' as command-line argument
> 4. curl -v localhost:8080/non-existing-file
> 
> Expected result: Status line is 'HTTP/1.1 404 Not Found'
> Actual Result on HttpCore 4.3.x: Status line is 'HTTP/1.1 404 OK'
> Actual Result on HttpCore 4.2.x: Status line is 'HTTP/1.1 404 Not Found'
> 
> The root cause for this difference is that
> HttpResponse.setStatusCode() in HttpCore 4.3.x doesn't change reason
> phrase, despite of javadocs, which promises reason phrase update.
> This is all about trivial cases.
> 
> For non-trivial cases HttpResponseFactory interface is not very
> useful, because it won't copy custom headers, I've already set, to the
> new HttpResponse object.
> 

Please raise a JIRA for this defect.

Oleg


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Re: Javadocs inconsistency concerning [Basic]HttpResponse in HttpCore-4.3

Posted by Dmitry Potapov <po...@gmail.com>.
My case is trivial:
1. Download ElementalHttpServer.java from
http://hc.apache.org/httpcomponents-core-4.3.x/examples.html
2. Compile
3. Run with '/tmp' as command-line argument
4. curl -v localhost:8080/non-existing-file

Expected result: Status line is 'HTTP/1.1 404 Not Found'
Actual Result on HttpCore 4.3.x: Status line is 'HTTP/1.1 404 OK'
Actual Result on HttpCore 4.2.x: Status line is 'HTTP/1.1 404 Not Found'

The root cause for this difference is that
HttpResponse.setStatusCode() in HttpCore 4.3.x doesn't change reason
phrase, despite of javadocs, which promises reason phrase update.
This is all about trivial cases.

For non-trivial cases HttpResponseFactory interface is not very
useful, because it won't copy custom headers, I've already set, to the
new HttpResponse object.

-- 
Best regards,
Dmitry

On Wed, Sep 4, 2013 at 1:41 PM, Oleg Kalnichevski <ol...@apache.org> wrote:
> On Tue, 2013-09-03 at 23:47 +0400, Dmitry Potapov wrote:
>> Hello everyone,
>>
>> I'm a bit confused with some inconsistencies in javadocs concerning
>> HttpResponse. Probably you can confirm that some of these inconsistencies are
>> typos or bugs:
>>   1. [Basic]HttpResponse: getLocale/setLocale is marked as deprecated:
>>     "use org.apache.http.impl.DefaultHttpRequestFactory"
>>     Is org.apache.http.impl.DefaultHttpResponseFactory implied here?
>
> It is a typo. Naturally it should be DefaultHttpResponseFactory.
> Corrected in SVN trunk:
>
> http://svn.apache.org/r1519957
>
>>   2. HttpResponse: setStatusCode has the following note in the description:
>>     "The reason phrase will be updated according to the new status code,
>>     based on the current locale".
>>     But in the BasicHttpResponse.setStatusCode implementation I see that
>>     'reasonPhrase' is left untouched. My first desire was to add the following
>>     line to this function (as it seems to be most elegant solution):
>>     "this.reasonPhrase = getReason(code)"
>>     but BasicHttpResponse.getReason is marked deprecated now, so this fix
>>     doesn't seem to be acceptable.
>>
>
> #getReason and Locate related methods got deprecated as violating the
> separation of concerns principle. HTTP message objects are meant to be
> plain java beans containing no special processing or protocol logic.
> Assembly of complex HTTP message objects should be done a responsibility
> of factory classes.
>
>> What is the correct way to set HttpResponse status code with reason phrase
>> update in HttpCore 4.3?
>>
>
> For non-trivial cases it should be DefaultHttpResponseFactory or a
> custom implementation of the HttpResponseFactory interface.
>
> Hope this helps
>
> Oleg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Re: Javadocs inconsistency concerning [Basic]HttpResponse in HttpCore-4.3

Posted by Oleg Kalnichevski <ol...@apache.org>.
On Tue, 2013-09-03 at 23:47 +0400, Dmitry Potapov wrote:
> Hello everyone,
> 
> I'm a bit confused with some inconsistencies in javadocs concerning
> HttpResponse. Probably you can confirm that some of these inconsistencies are
> typos or bugs:
>   1. [Basic]HttpResponse: getLocale/setLocale is marked as deprecated:
>     "use org.apache.http.impl.DefaultHttpRequestFactory"
>     Is org.apache.http.impl.DefaultHttpResponseFactory implied here?

It is a typo. Naturally it should be DefaultHttpResponseFactory.
Corrected in SVN trunk:

http://svn.apache.org/r1519957

>   2. HttpResponse: setStatusCode has the following note in the description:
>     "The reason phrase will be updated according to the new status code,
>     based on the current locale".
>     But in the BasicHttpResponse.setStatusCode implementation I see that
>     'reasonPhrase' is left untouched. My first desire was to add the following
>     line to this function (as it seems to be most elegant solution):
>     "this.reasonPhrase = getReason(code)"
>     but BasicHttpResponse.getReason is marked deprecated now, so this fix
>     doesn't seem to be acceptable.
> 

#getReason and Locate related methods got deprecated as violating the
separation of concerns principle. HTTP message objects are meant to be
plain java beans containing no special processing or protocol logic.
Assembly of complex HTTP message objects should be done a responsibility
of factory classes.

> What is the correct way to set HttpResponse status code with reason phrase
> update in HttpCore 4.3?
> 

For non-trivial cases it should be DefaultHttpResponseFactory or a
custom implementation of the HttpResponseFactory interface.

Hope this helps

Oleg


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org