[HttpClient] HttpURLConnection wrapper problems caused by deficiencies in HttpClient

[Jeff Dever <js...@sympatico.ca>]

Vincent,

We have a problem with the getHeaderField and getHeaderFieldKey methods in the
HttpURLConnection wrapper.

Problem1) You make a note about being unable to get the status line as a header,
and attempt to generate one in the wrapper.  This is non-ideal, but I see no
other choice.  readStatusLine() in HttpMethodBase is where the status line is
read, but it is never stored and is not available outside the method.

Problem 2) The headers are not returned by getResponseHeaders() in the order
that they are read from the stream.  The headers are stored in a HashMap and are
effectively just the values() which are in no particular order.

Both of these problems show deficiencies in HttpClient in my opinion, and should
be fixed.

For Problem1 we could store the status line as a string and add a getStatusLine
method.  Alternatively, we could create a header for it with the name StatusLine
(or perhaps just null) which would preserve the interface.

For Problem2 we could use a Vector in place of a HashMap for the headers so that
the order is preserved, but would have abstract away the lookups by the name
field (which there are many).  Alternatively, we could mirror the contents in
the hashmap in a vector (or array) so that we still have fast and convienent
lookups, but still have order preservation of the headers.

What do you all say?




Jeff Dever wrote:

> Patch applied.
>
> There has been no significant changes to the wrapper since it was added to
> the repository (at least untill now, that is :-)
>
> Vincent Massol wrote:
>
> > Hi,
> >
> > I haven't been following what has happened with the HttpURLConnection
> > that I have submitted some time ago but here are some corrections I made
> > to the Cactus wrapper (I will move Cactus to use the HttpClient wrapper
> > very soon now).
> >
> > Thanks
> > -Vincent
> >
> > PS: Sorry it isn't in diff format, I am very much in hurry right now.
> >
> >   ------------------------------------------------------------------------
> >                                    Name: httpurlconnectionpatches.txt
> >    httpurlconnectionpatches.txt    Type: Plain Text (text/plain)
> >                                Encoding: quoted-printable
> >
> >    Part 1.3Type: Plain Text (text/plain)
>
> --
> To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>

Re: [HttpClient] HttpURLConnection wrapper problems caused by deficienciesin HttpClient

[Ortwin Glück <or...@nose.ch>]

Jeff Dever wrote:
> Its just a string.  Would you prefer a new member and function in HttpMethod:
> String getStatusLine();

Yes please. Storing it with the headers would be abuse. Abuse is evil :-)


> A Map makes more sense than a list because the members are most commonly looked up by
> name so its just more logical.  But the order preservation is a problem.  We could
> just use a list and iterate over it for lookups, but there would be insertion issues
> with repeated entries and such.

Good point.

> As Ryan suggested, a SequencedHashMap is perfect because it is both a Map and a
> List.  It would be easy to add as well, because it implements the Map interface.

I haven't known this class before and it seems the perfect solution for 
our problem.

> The only real drawback (and this applies to HashMap as well as SequencedHashMap) is
> that they take up more space than a simple List.

Of course. But I think we should invest in those few extra bytes.


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>

Re: [HttpClient] HttpURLConnection wrapper problems caused by deficienciesin HttpClient

[Jeff Dever <js...@sympatico.ca>]

Ortwin Glück wrote:

> Jeff Dever wrote:
> > For Problem1 we could store the status line as a string and add a getStatusLine
> > method.  Alternatively, we could create a header for it with the name StatusLine
> > (or perhaps just null) which would preserve the interface.
>
> IMHO we should *not* save non-header information in the header data
> structure. The status line should have its own data structure. If this
> is a complex one, make a separate class.

Its just a string.  Would you prefer a new member and function in HttpMethod:
String getStatusLine();

>
>
> > For Problem2 we could use a Vector in place of a HashMap for the headers so that
> > the order is preserved, but would have abstract away the lookups by the name
> > field (which there are many).  Alternatively, we could mirror the contents in
> > the hashmap in a vector (or array) so that we still have fast and convienent
> > lookups, but still have order preservation of the headers.
>
> Storing headers / footers in a List is just fine. We don't need a
> HashMap since performance is not an issue here. Remember we usually
> don't have 100 Headers but maybe 10. Providing both HashMap and List is
> a code mess and does not add significant benefit.
>

A Map makes more sense than a list because the members are most commonly looked up by
name so its just more logical.  But the order preservation is a problem.  We could
just use a list and iterate over it for lookups, but there would be insertion issues
with repeated entries and such.

As Ryan suggested, a SequencedHashMap is perfect because it is both a Map and a
List.  It would be easy to add as well, because it implements the Map interface.

The only real drawback (and this applies to HashMap as well as SequencedHashMap) is
that they take up more space than a simple List.


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>

Re: [HttpClient] HttpURLConnection wrapper problems caused by deficiencies in HttpClient

[Ortwin Glück <or...@nose.ch>]

Jeff Dever wrote:
> For Problem1 we could store the status line as a string and add a getStatusLine
> method.  Alternatively, we could create a header for it with the name StatusLine
> (or perhaps just null) which would preserve the interface.

IMHO we should *not* save non-header information in the header data 
structure. The status line should have its own data structure. If this 
is a complex one, make a separate class.

> For Problem2 we could use a Vector in place of a HashMap for the headers so that
> the order is preserved, but would have abstract away the lookups by the name
> field (which there are many).  Alternatively, we could mirror the contents in
> the hashmap in a vector (or array) so that we still have fast and convienent
> lookups, but still have order preservation of the headers.

Storing headers / footers in a List is just fine. We don't need a 
HashMap since performance is not an issue here. Remember we usually 
don't have 100 Headers but maybe 10. Providing both HashMap and List is 
a code mess and does not add significant benefit.


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>