RE: HTTP client API

[Mark A. Claassen]

I realize that no one probably knows who I am, however I have been using Java 
since the 1.x days.  I have probably spoken to several of you at previous 
JavaOne’s.  I am also a member of the CAP program.

Anyway, I would like to say that I like the idea of the incubator modules.  
These things are hard to get right the first time.  The idea of having 
something “locked down” on the first release seems to raise the bar so high 
that helpful features can end up getting delayed.

Mark

Disclaimer:
The opinions provided herein do not necessarily state or reflect those of 
Donnell Systems, Inc.(DSI). DSI makes no warranty for and assumes no legal 
liability or responsibility for the posting.

From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Michael 
McMahon
Sent: Tuesday, November 29, 2016 10:47 AM
To: Tobias Thierer
Cc: OpenJDK Network Dev list
Subject: Re: HTTP client API

Hi Tobias,

Apologies for the long delay in responding. I'll hopefully catch up on your 
subsequent comments soon.
Just to give an overall update on the status of this work first. There is a 
plan afoot to make the JDK 9
module system support something we are calling "incubator modules". These are 
modules that
can be delivered with the JDK, but are not standard parts of the platform (Java 
SE). So, they do not
resolve by default and will have runtime/compile time warnings to indicate that 
the contained APIs
are potentially subject to change. The plan is to make this new http client API 
one of these
incubator modules in JDK 9. It will use a different package namespace to 
reflect this.

The upshot is that while we would like the API to be as complete as possible, 
it will not
be fixed or standardized in JDK 9. But by delivering the API and implementation 
with JDK 9, it
will get some exposure and use, so that we can standardize it in JDK 10, and 
move it from
its incubator module and namespace back into Java SE in the java.net.http 
package.

I've added some answers to your questions below, and will get to the followup 
messages soon.

Thanks,
Michael

On 31/10/2016, 18:13, Tobias Thierer wrote:
Hi Michael -

thanks a lot for your response! Further comments below.

How would you rank simplicity, versatility and performance as goals for the 
HTTP Client API? For example the blocking vs. non-blocking API choices will 
largely depend on trade-offs, so I'd like to understand the target applications 
and requirements that these trade-offs were based on.

To my mind, we should support both blocking and non-blocking in the API, and by 
specifying them both in
the API (rather than relying on blocking behavior being a wrapper around 
non-blocking) then we get the
potential benefits of both (reduced thread switching with blocking, versus less 
thread usage with non-blocking).


On Fri, Oct 28, 2016 at 12:28 PM, Michael McMahon 
<michael.x.mcma...@oracle.com<mailto:michael.x.mcma...@oracle.com>> wrote:

The intention behind those interfaces is primarily to provide a conversion 
between ByteBuffers and higher level objects that users
are interested in. So, HttpRequest.BodyProcessor generates ByteBuffers for 
output and HttpResponse.ByteBuffer consumes them
on input. For response bodies, there is this additional implicit requirement of 
needing to do something with the incoming data
and that is why the option of discarding it exists (the details of that API are 
certainly open to discussion)

On Publisher/Subscriber (Flow) versus InputStream/OutputStream, we wanted a non 
blocking API

I can understand the desire to have a non blocking API, especially if 
performance is seen as more important for the API than usability. Cronet (the 
Chromium networking stack) API 
[documentation<https://chromium.googlesource.com/chromium/src/+/lkgr/components/cronet>]
 has a

UrlRequest.read(ByteBuffer) <-> callback.onReadCompleted(UrlRequest, 
UrlResponseInfo, ByteBuffer)

call cycle that is not too different from OpenJDK 9 HTTP client's

   subscription.request(1) <-> BodyProcessor.onNext(ByteBuffer)

so the general idea is shared by others (I'm meeting with some Cronet folks 
later this week so I hope to gather their input / views soon). Note that in 
Cronet, the ByteBuffer is provided as part of each call requesting more data, 
whereas in OpenJDK 9 it's returned once BodyProcessor.getBuffer().

On this point, I'd say that there is a high degree of usability with the 
proposed API when using
the standard handlers/processors. Again, the blocking variant is very easy to 
use. The non-blocking
variant with CompletableFuture is not much harder, once the programmer invests 
some effort in understanding that
model.

Implementing your own processor does require familiarity with the Flow types 
and their underlying philosophy, but I see
those cases as outside the core use cases. It's likely that additional 
processor/handlers would be added in time also.


At the same time, the first and third goal menti

Re: HTTP client API

[Michael McMahon]

Hi Tobias,

Apologies for the long delay in responding. I'll hopefully catch up on 
your subsequent comments soon.
Just to give an overall update on the status of this work first. There 
is a plan afoot to make the JDK 9
module system support something we are calling "incubator modules". 
These are modules that
can be delivered with the JDK, but are not standard parts of the 
platform (Java SE). So, they do not
resolve by default and will have runtime/compile time warnings to 
indicate that the contained APIs
are potentially subject to change. The plan is to make this new http 
client API one of these
incubator modules in JDK 9. It will use a different package namespace to 
reflect this.


The upshot is that while we would like the API to be as complete as 
possible, it will not
be fixed or standardized in JDK 9. But by delivering the API and 
implementation with JDK 9, it
will get some exposure and use, so that we can standardize it in JDK 10, 
and move it from
its incubator module and namespace back into Java SE in the 
java.net.http package.


I've added some answers to your questions below, and will get to the 
followup messages soon.


Thanks,
Michael

On 31/10/2016, 18:13, Tobias Thierer wrote:

Hi Michael -

thanks a lot for your response! Further comments below.

How would you rank simplicity, versatility and performance as goals 
for the HTTP Client API? For example the blocking vs. non-blocking API 
choices will largely depend on trade-offs, so I'd like to understand 
the target applications and requirements that these trade-offs were 
based on.


To my mind, we should support both blocking and non-blocking in the API, 
and by specifying them both in
the API (rather than relying on blocking behavior being a wrapper around 
non-blocking) then we get the
potential benefits of both (reduced thread switching with blocking, 
versus less thread usage with non-blocking).


On Fri, Oct 28, 2016 at 12:28 PM, Michael McMahon 
> 
wrote:



The intention behind those interfaces is primarily to provide a
conversion between ByteBuffers and higher level objects that users
are interested in. So, HttpRequest.BodyProcessor generates
ByteBuffers for output and HttpResponse.ByteBuffer consumes them
on input. For response bodies, there is this additional implicit
requirement of needing to do something with the incoming data
and that is why the option of discarding it exists (the details of
that API are certainly open to discussion)

On Publisher/Subscriber (Flow) versus InputStream/OutputStream, we
wanted a non blocking API


I can understand the desire to have a non blocking API, especially if 
performance is seen as more important for the API than usability. 
Cronet (the Chromium networking stack) API [documentation 
] 
has a


UrlRequest.read(ByteBuffer) <-> 
callback.onReadCompleted(UrlRequest, UrlResponseInfo, ByteBuffer)


call cycle that is not too different from OpenJDK 9 HTTP client's

   subscription.request(1) <-> BodyProcessor.onNext(ByteBuffer)

so the general idea is shared by others (I'm meeting with some Cronet 
folks later this week so I hope to gather their input / views soon). 
Note that in Cronet, the ByteBuffer is provided as part of each call 
requesting more data, whereas in OpenJDK 9 it's returned once 
BodyProcessor.getBuffer().


On this point, I'd say that there is a high degree of usability with the 
proposed API when using
the standard handlers/processors. Again, the blocking variant is very 
easy to use. The non-blocking
variant with CompletableFuture is not much harder, once the programmer 
invests some effort in understanding that

model.

Implementing your own processor does require familiarity with the Flow 
types and their underlying philosophy, but I see
those cases as outside the core use cases. It's likely that additional 
processor/handlers would be added in time also.


At the same time, the first and third goal mentioned in JEP 110 
 are


  * /Must be easy to use for common cases, including a simple blocking
mode./
  * /A simple and concise API which caters for 80-90% of application
needs.
/

Blocking, pull-based APIs tend to be simpler to use because (a) one 
can just step through a for loop in a debugger rather than needing 
breakpoints, and (b) state can be held on the stack rather than in 
fields [see related blog post 
], 
so I'm struggling to follow the trade-offs here. Obviously, blocking 
APIs have some disadvantages as well - e.g. context switching overhead 
in the case of separate Threads per request. Is there any place other 
than JEP 110 where I could read up on principles or target 
applications that drove the trade-offs for this API?


But, I 

Re: HTTP client API

[Tobias Thierer]

Hi Michael -

Some follow-up/updates on items from my previous email inline below:

On Mon, Oct 31, 2016 at 6:13 PM, Tobias Thierer  wrote:

>
> On Fri, Oct 28, 2016 at 12:28 PM, Michael McMahon <
> michael.x.mcma...@oracle.com> wrote:
>
>>
>> The intention behind those interfaces is primarily to provide a
>> conversion between ByteBuffers and higher level objects that users
>> are interested in. So, HttpRequest.BodyProcessor generates ByteBuffers
>> for output and HttpResponse.ByteBuffer consumes them
>> on input. For response bodies, there is this additional implicit
>> requirement of needing to do something with the incoming data
>> and that is why the option of discarding it exists (the details of that
>> API are certainly open to discussion)
>>
>> On Publisher/Subscriber (Flow) versus InputStream/OutputStream, we wanted
>> a non blocking API
>>
>
> I can understand the desire to have a non blocking API, especially if
> performance is seen as more important for the API than usability. Cronet
> (the Chromium networking stack) API [documentation
> ]
> has a
>
> UrlRequest.read(ByteBuffer) <-> callback.onReadCompleted(UrlRequest,
> UrlResponseInfo, ByteBuffer)
>
> call cycle that is not too different from OpenJDK 9 HTTP client's
>
>subscription.request(1) <-> BodyProcessor.onNext(ByteBuffer)
>
> so the general idea is shared by others (I'm meeting with some Cronet
> folks later this week so I hope to gather their input / views soon).
>

I've meanwhile met with Cronet folks and now agree with your view that an
async API is useful for applications that prioritize performance over
simplicity. That said, I still think that many applications will prefer the
simplicity of a blocking API - either provided as part of the platform or
as a separate library.

FYI as an experiment to see how easy this could be, I've prototyped a
blocking API on top of the async API, specifically an adapter from OpenJDK
9's HttpResponse.BodyProcessor to an InputStream.

The way I've prototyped it was to use a PipedInputStream internally whose
buffer size is 32KBytes (2 * the size of the ByteBuffers used to deliver
the response); the number of bytes free in that internal buffer

   (a) decreases when onResponseBodyChunk() copies bytes into the
associated PipedOutputStream,
   (b) increases when the application reads bytes from the PipedInputStream.

The PipedInputStream is wrapped in another InputStream that calls
flowController.accept(1) every time (b) frees up enough bytes in the
internal buffer to fit everything that may be delivered in the next call to
onResponseBodyChunk(); this ensures that onResponseBodyChunk() never
blocks. The InputStream wrapper methods also take care of throwing
IOException after onResponseError(Throwable) occurred.

All of this seemed fairly straightforward, but I did run into some problems:

   - I haven't succeeded in building OpenJDK 9 from source, following the
   instructions (I always run into a NullPointerException in XMLDTDProcessor
   during the BUILD_TOOLS_CORBA part of "make all"), so I've been unable to
   empirically try / benchmark my prototype code.
   - Obviously, copying data into/out of the PipedInputStream's internal
   buffer is overhead; this consciously trades performance for simplicity of
   the API. (I note that ResponseContent.copyBuffer() also performs copying
   but I haven't checked if this can be avoided).
   - flowController.accept(1) is only called when the underlying
   PipedInputStream.read() returns, and then there will still be additional
   latency before more data is delivered to onResponseBodyChunk(). These
   delays will impair performance more than should ideally be required but I
   haven't been able to quantify this through benchmarks because of my
   problems of building OpenJDK 9 from source.
  - Further contributing to this latency is the fact that the wrapper
  doesn't have good control over how many bytes will be delivered
during the
  next onResponseBodyChunk(). Note that I've run into this same
limitation on
  my previous prototype for the use case of parsing image data.
Further note
  that Cronet's API *does* give the application control over how many
  bytes at most will be delivered during the next
onReadCompleted(UrlRequest,
  UrlResponseInfo, ByteBuffer) - see my previous email. I
previously thought
  that it was the BodyProcessor's job to construct the ByteBuffer
objects but
  I may be mistaken - in the latest version of the API, this seems to be an
  implementation detail of HttpClientImpl and not part of the public API.

On the question of push versus pull, there seems to be differing
>> philosophies around this, but
>> actually the publish/subscribe model provided by the Flow types has
>> characteristics of both push and pull.
>> It is primarily push() as in Subscriber.onNext() delivers the next buffer
>> to 

Re: HTTP client API

[Tobias Thierer]

On Mon, Oct 31, 2016 at 6:13 PM, Tobias Thierer  wrote:

> On Fri, Oct 28, 2016 at 12:28 PM, Michael McMahon <
> michael.x.mcma...@oracle.com> wrote:
>>
>> 2.) HttpHeaders: I love that there is a type for this abstraction! But:
>>
>>-
>>
>>Why is the type an interface rather than a concrete, final class?
>>Since this is a pure value type, there doesn’t seem to be much point in
>>allowing alternative implementations?
>>
>> You could conceive of different implementations, with different tradeoffs
>> between parsing early or lazily for instance.
>>
>
> What parsing are you referring to?
>

P.S. In case you were referring to lazy HPACK decompression [RFC 7541
]: I'll have to read up the details to
understand if and how much work could be done lazily, but I can see the
consideration. Such a lazy implementation would need some new (potentially
package private) way to be passed the un-processed header data.

Note that RFC 7540 section 4.3
 mandates that at least
some minimal header processing must be done eagerly to verify the
compression and to maintain the state needed for decompression.

For what it's worth, though, to allow other implementations, HttpHeaders
need not be an interface; it should be sufficient for just the method
Map asMap() to be abstract or overridable? Of course, if
HttpHeaders were shrunk to only that one method, then an interface would be
fine.

Tobias

Re: HTTP client API

[Tobias Thierer]

Hi Michael -

thanks a lot for your response! Further comments below.

How would you rank simplicity, versatility and performance as goals for the
HTTP Client API? For example the blocking vs. non-blocking API choices will
largely depend on trade-offs, so I'd like to understand the target
applications and requirements that these trade-offs were based on.

On Fri, Oct 28, 2016 at 12:28 PM, Michael McMahon <
michael.x.mcma...@oracle.com> wrote:

>
> The intention behind those interfaces is primarily to provide a conversion
> between ByteBuffers and higher level objects that users
> are interested in. So, HttpRequest.BodyProcessor generates ByteBuffers for
> output and HttpResponse.ByteBuffer consumes them
> on input. For response bodies, there is this additional implicit
> requirement of needing to do something with the incoming data
> and that is why the option of discarding it exists (the details of that
> API are certainly open to discussion)
>
> On Publisher/Subscriber (Flow) versus InputStream/OutputStream, we wanted
> a non blocking API
>

I can understand the desire to have a non blocking API, especially if
performance is seen as more important for the API than usability. Cronet
(the Chromium networking stack) API [documentation
]
has a

UrlRequest.read(ByteBuffer) <-> callback.onReadCompleted(UrlRequest,
UrlResponseInfo, ByteBuffer)

call cycle that is not too different from OpenJDK 9 HTTP client's

   subscription.request(1) <-> BodyProcessor.onNext(ByteBuffer)

so the general idea is shared by others (I'm meeting with some Cronet folks
later this week so I hope to gather their input / views soon). Note that in
Cronet, the ByteBuffer is provided as part of each call requesting more
data, whereas in OpenJDK 9 it's returned once BodyProcessor.getBuffer().

At the same time, the first and third goal mentioned in JEP 110
 are

   - *Must be easy to use for common cases, including a simple blocking
   mode.*
   -
*A simple and concise API which caters for 80-90% of application needs.  *

Blocking, pull-based APIs tend to be simpler to use because (a) one can
just step through a for loop in a debugger rather than needing breakpoints,
and (b) state can be held on the stack rather than in fields [see related
blog post
],
so I'm struggling to follow the trade-offs here. Obviously, blocking APIs
have some disadvantages as well - e.g. context switching overhead in the
case of separate Threads per request. Is there any place other than JEP 110
where I could read up on principles or target applications that drove the
trade-offs for this API?

A concrete example that I tried to prototype was reading an image file
header & body from the network, potentially aborting the request depending
on some metadata in the file header; using the OpenJDK 9 HTTP client API,
this involved a ton of boilerplate. Part of the boilerplate was because I
wasn't sure if onNext() might get a partially filled ByteBuffer that might
cut off the image file header or even an individual int32 value in that
header. Is it guaranteed that the ByteBuffer returned by
BodyProcessor.getBuffer will be full on each call to onNext(ByteBuffer),
except possibly the last? If not, perhaps it should be guaranteed?

Related to this, it appears that the documentation for
Subscription.request(long
n)

specifies
that *n* specifically *denotes the number of future calls* to onNext()
.
If this wasn't the case, BodyProcessor could have defined *n* to refer to a
number of bytes (delivered as a single onNext() call if they fit into the
supplied ByteBuffer), which might have been a really neat way for the
application to control the sizes of chunks that it gets passed. For
example, an application might want to get the file header in one go, and
then afterwards process some fixed size data record for each call. Oh well.
Do you happen to have an idea for how the application could be more in
control of the chunk sizes?


> and had originally defined
> something that was quite similar to the asynchronous model used by Flow,
> but the Flow types have a nice mechanism for
> managing flow control asynchronously. So, it made sense to use the
> standard API.
>

Regarding managing flow control, I assume you're referring to
subscription.request() which I assume is tied to WINDOW_UPDATE frames in
HTTP 2? Also, I assume that Subscription.cancel() will result in a
RST_STREAM frame being sent to the server, is that correct?

Is there a recommended way for an application to limit stream concurrency (RFC
7540 section 5.1.2 

Re: HTTP client API

[Michael McMahon]

Hi Tobias

Thanks for the feedback (and thanks to Anthony for answering some of the 
questions)

My thoughts are below.


On 24/10/2016, 20:33, Tobias Thierer wrote:


Hi Michael and others -


Thanks for publishing the latest HTTP client API docs 
(already 
slightly outdated again), as well as for publishing the current draft 
code in the sandbox repository!



Below is some concrete feedback, questions and brainstorming on how to

(a) increase the usefulness or

(b) decrease the semantic weight

of the API. Note that most of this is driven only by inspection of the 
API and some brief exploration of the implementation code, not (yet) 
by a substantial effort to write proof of concept client applications. 
I’d love if I could help make this API as useful to applications as 
possible, so I’d appreciate your feedback on how I can best do that 
and what the principles were that guided your design choices.



1.) The HttpRequest.BodyProcessor 
and 
HttpResponse.BodyProcessor 
abstractions 
seem particularly hard to grasp / have a high semantics weight.


 *

What purpose does the abstraction of a BodyProcessoraim to fulfill
beyond what the (simpler) abstraction of a Bodycould be?

 o

Instead of describing the abstraction as a “processor” of
ByteBuffers / Java objects, wouldn’t it be simpler to say to
say that request / response bodiesare ByteBuffer / Java object
sources/ sinks? What is the advantage of the
Publisher / Subscriber API over plain
old InputStream / OutputStream based APIs?



The intention behind those interfaces is primarily to provide a 
conversion between ByteBuffers and higher level objects that users
are interested in. So, HttpRequest.BodyProcessor generates ByteBuffers 
for output and HttpResponse.ByteBuffer consumes them
on input. For response bodies, there is this additional implicit 
requirement of needing to do something with the incoming data
and that is why the option of discarding it exists (the details of that 
API are certainly open to discussion)


On Publisher/Subscriber (Flow) versus InputStream/OutputStream, we 
wanted a non blocking API and had originally defined
something that was quite similar to the asynchronous model used by Flow, 
but the Flow types have a nice mechanism for
managing flow control asynchronously. So, it made sense to use the 
standard API.



 o

The term “processor” and the description of “converting
incoming buffers of data to some user-defined object type T”
is especially confusing (increases the semantic weight of the
abstraction) given that there is an implementation that
discards all data

(and
its generic type is called Urather than T). A BodyProcessor
that has no input but generates the digits of Pi is also
conceivable. Perhaps call these BodySource / BodySink,
ByteBufferPublisher / ByteBufferSubscriber, or just Body?

 o

The fact that you felt the need to introduce an abstraction
HttpResponse.BodyHandler whose name is similar to but whose
semantics are different from HttpResponse.BodyProcessor is
another indication that these concepts could be clarified and
named better.



We're certainly open to suggestions for improved names, but I think it 
is useful to have a way to explicitly discard a response body, 
especially since HTTP/2 has an efficient way to do that.


I am looking at the moment at simplifications of the server push API, 
and it will be helpful if the request and response body processor
types have shorter names, to help with that. So, I'll take a look at the 
possibility of using something using BodySource/BodySink.



 o

To explore how well the abstractions “fit”, I played with some
draft code implementing the API on top of another one; one
thing I found particularly challenging was the control flow
progression:
HttpClient.send(request, bodyHandler)
-> bodyProcessor = bodyHandler.apply(); // called by the library
-> bodyProcessor.onSubscribe() / onNext()
because it is push based and forces an application to
relinquish control to the library rather than pulling data out
of the library.
Perhaps the Response BodyHandler abstraction could be
eliminated altogether? For example, wouldn’t it be sufficient
to abort downloading the body once an application thread has a
chance to look at the Response object? Perhaps once a buffer
is full, the download of the further 

Re: HTTP client API

[Anthony Vanelverdinghe]

Hi Tobias

As far as I know, the latest published Javadoc is at [1] (which was 
linked to in [2]). However, this Javadoc doesn't exactly match the 
current code either.


Below are some quick replies to some of your feedback.

1.)
> What is the advantage of the Publisher / 
Subscriber API over plain old InputStream / OutputStream 
based APIs?
InputStream / OutputStream are blocking APIs, whereas Publisher / 
Subscriber are part of the Flow API, which corresponds to the Reactive 
Streams specification [3].


> there is an implementation that discards all data [...] (and its 
generic type is called U rather than T)
The generic type is called U to avoid shadowing the type parameter in 
the enclosing HttpResponse.BodyProcessor


> one thing I found particularly challenging was the control flow 
progression [...] because it is push based and forces an application to 
relinquish control to the library rather than pulling data out of the 
library.
The application is still in control by means of its Flow.Subscription 
(see Flow.Subscriber::onSubscribe), isn't it?


> asByteArrayConsumer(Consumer> consumer): Why is this 
an Optional? What logic decides whether an empty response body will be 
represented as a present byte[0] or an absent value?
As the Javadoc says, it's an Optional to be able to mark the end of the 
body. In case of an empty response body, the Consumer will therefore 
receive an Optional.empty() value.


2.) what happens when name is null, is specified by the following note 
in the package Javadoc: "Unless otherwise stated, null parameter values 
will cause methods of all classes in this package to throw 
NullPointerException."


3.) allowing custom redirection policies was brought up before [4][5], 
proposing that a redirection policy would simply be a 
BiFunction. Besides, 
analog to e.g. java.nio.file.StandardOpenOption, there would also be a 
StandardRedirectionPolicy enum [6].


4.) it doesn't, as is specified in HttpClient.Builder::version

5.) this was brought up in [7], and was addressed, I believe, in the 
latest API [1] by replacing CookieManager with java.net.CookieHandler. 
However, this change isn't in the current code (yet?).


6.) I'd assume the Executor is used to handle asynchronous requests 
(HttpClient::sendAsync in [1]). So by using a fixed thread pool, one can 
control the maximum number of concurrent requests.


Hope this helps,
Anthony

[1] http://cr.openjdk.java.net/~michaelm/httpclient/api.1/
[2] 
http://mail.openjdk.java.net/pipermail/net-dev/2016-September/010238.html

[3] http://www.reactive-streams.org/
[4] http://mail.openjdk.java.net/pipermail/net-dev/2016-February/009547.html
[5] http://mail.openjdk.java.net/pipermail/net-dev/2016-March/009608.html
[6] 
http://jep110-anthonyvdotbe.rhcloud.com/api/jep110/StandardRedirectionPolicy.html

[7] http://mail.openjdk.java.net/pipermail/net-dev/2016-August/010187.html


On 24/10/2016 21:33, Tobias Thierer wrote:


Hi Michael and others -


Thanks for publishing the latest HTTP client API docs 
(already 
slightly outdated again), as well as for publishing the current draft 
code in the sandbox repository!



Below is some concrete feedback, questions and brainstorming on how to

(a) increase the usefulness or

(b) decrease the semantic weight

of the API. Note that most of this is driven only by inspection of the 
API and some brief exploration of the implementation code, not (yet) 
by a substantial effort to write proof of concept client applications. 
I’d love if I could help make this API as useful to applications as 
possible, so I’d appreciate your feedback on how I can best do that 
and what the principles were that guided your design choices.



1.) The HttpRequest.BodyProcessor 
and 
HttpResponse.BodyProcessor 
abstractions 
seem particularly hard to grasp / have a high semantics weight.


 *

What purpose does the abstraction of a BodyProcessoraim to fulfill
beyond what the (simpler) abstraction of a Bodycould be?

 o

Instead of describing the abstraction as a “processor” of
ByteBuffers / Java objects, wouldn’t it be simpler to say to
say that request / response bodiesare ByteBuffer / Java object
sources/ sinks? What is the advantage of the
Publisher / Subscriber API over plain
old InputStream / OutputStream based APIs?

 o

The term “processor” and the description of “converting
incoming buffers of data to some user-defined object type T”
is especially confusing (increases the semantic weight of the
abstraction) given that there is an implementation that
discards all data

Re: HTTP client API

[Chris Hegarty]

On 22/09/16 08:54, Chris Hegarty wrote:

On 22 Sep 2016, at 00:14, Michael McMahon  wrote:


Martin

The source is available in the JDK 9 sandbox (http-client-branch) at
http://hg.openjdk.java.net/jdk9/sandbox/

I think it has been updated to reflect the API as described below.


Apologies, it has not been update yet. I will do it over the next day, or so, 
and
post a note when it is done.


Done.

This was a little rushed. Further implementation clean up will
be necessary.

-Chris.


-Chris.


Michael.

On 21/09/2016, 14:14, Martin Buchholz wrote:



On Wed, Sep 7, 2016 at 10:15 AM, Michael McMahon  
wrote:

[1] http://cr.openjdk.java.net/~michaelm/httpclient/api.1/

[2] 
http://cr.openjdk.java.net/~michaelm/httpclient/specdiffout.1/package-summary.html

Could we have actual source code for experimentation by interested parties?

Re: HTTP client API

[Chris Hegarty]

On 22 Sep 2016, at 00:14, Michael McMahon  wrote:
> 
> Martin
> 
> The source is available in the JDK 9 sandbox (http-client-branch) at
> http://hg.openjdk.java.net/jdk9/sandbox/
> 
> I think it has been updated to reflect the API as described below.

Apologies, it has not been update yet. I will do it over the next day, or so, 
and
post a note when it is done.

-Chris.

> Michael.
> 
> On 21/09/2016, 14:14, Martin Buchholz wrote:
>> 
>> 
>> On Wed, Sep 7, 2016 at 10:15 AM, Michael McMahon 
>>  wrote:
>> 
>> [1] http://cr.openjdk.java.net/~michaelm/httpclient/api.1/
>> 
>> [2] 
>> http://cr.openjdk.java.net/~michaelm/httpclient/specdiffout.1/package-summary.html
>> 
>> Could we have actual source code for experimentation by interested parties?

Re: HTTP client API

[Michael McMahon]

Martin

The source is available in the JDK 9 sandbox (http-client-branch) at
http://hg.openjdk.java.net/jdk9/sandbox/

I think it has been updated to reflect the API as described below.

Michael.

On 21/09/2016, 14:14, Martin Buchholz wrote:



On Wed, Sep 7, 2016 at 10:15 AM, Michael McMahon 
> 
wrote:



[1] http://cr.openjdk.java.net/~michaelm/httpclient/api.1/


[2]

http://cr.openjdk.java.net/~michaelm/httpclient/specdiffout.1/package-summary.html




Could we have actual source code for experimentation by interested 
parties?

Re: HTTP client API

[Martin Buchholz]

On Wed, Sep 7, 2016 at 10:15 AM, Michael McMahon <
michael.x.mcma...@oracle.com> wrote:

>
> [1] http://cr.openjdk.java.net/~michaelm/httpclient/api.1/
>
> [2] http://cr.openjdk.java.net/~michaelm/httpclient/
> specdiffout.1/package-summary.html
>

Could we have actual source code for experimentation by interested parties?

Re: HTTP client API

[Michael McMahon]

Hi Anthony,

On 08/09/2016, 18:49, Anthony Vanelverdinghe wrote:

Hi Michael

What's the rationale for not turning all public classes into 
interfaces, since none of them contain any actual implementation code?


We could do that with little or no impact on the implementation or on 
calling code.

I will look into it.

On another note, I fail to see the point of 
HttpClient.Builder::priority: as far as I understand, HTTP/2 priority 
only comes into play when multiple resources depend on the same 
resource. But since there's no way to express dependencies between 
resources, how would this be useful?


There was a desire expressed for the concept of a default priority, 
which would not have any impact on client
behavior, but could still affect the behavior of the server. Client 
applications might be able to differentiate

requests with different priorities by using different HttpClient instances.

Lastly, some minor remarks: I believe the following factory methods 
should be moved for consistency

- HttpRequest::noBody --> HttpRequest.BodyProcessor::noBody
- HttpResponse::multiFile --> HttpResponse.MultiProcessor::asFiles

and that "? extends T" should be used instead of "T", in the return 
type of the following methods

- HttpResponse.BodyHandler::apply, i.e. BodyProcessor
- HttpResponse.BodyProcessor::getBody, i.e. CompletionStage
- HttpResponse.MultiProcessor::onRequest, i.e. 
Optional



Agreed on both points.

One other thing. I just want to acknowledge, since I haven't before, the 
work you did, prototyping
the change to the request and response processor API, and the use of the 
Flow Publisher and
Subscriber classes in particular. I think that has been a very useful 
addition to the API.


Thanks,
Michael


On 7/09/2016 19:15, Michael McMahon wrote:

Hi Wenbo,

First, sorry for the delay in replying. We took your comments and 
prototyped how the major ones

might be accommodated. In particular, we did the following:

- moved "business logic" out of HttpRequest. The methods for sending 
requests now
   exist in HttpClient. Given that requests are no longer associated 
with a client and can
   be sent through any client, this means that some properties of a 
request that used
   to be inherited from the client can no longer be visible in the 
HttpRequest object,
   but that is not a problem as far as I can see. This changes the 
look of the sample code

   in HttpRequest, but doesn't make it any harder to read.

- changed some method names as suggested eg newBuilder()

- added protected constructors to the public classes. This allows 
alternative implementations

   for use in mocking/test frameworks etc.

- changed the PUT, POST, GET methods in the request builder to only 
set the method type.
   PUT, POST takes the request body processor as parameter. A new 
build() method returns

   the HttpRequest.

- added a method to HttpResponse which returns the "final" request 
that was sent on the wire
   for the particular response, which might be different from the 
user initiated request.


I put an updated apidoc [1] and specdiff [2] up to show the changes. 
In particular, the sample

code described in the HttpClient docs is updated to reflect the changes.

Thanks,
Michael

[1] http://cr.openjdk.java.net/~michaelm/httpclient/api.1/

[2] 
http://cr.openjdk.java.net/~michaelm/httpclient/specdiffout.1/package-summary.html


On 26/08/2016, 07:59, Wenbo Zhu wrote:

Hi Michael,

Thanks for the update! The adoption of the Flow API is indeed a big 
improvement (async flow-control is very hard to get right).


Attached is a feedback doc on this new version. One specific idea to 
discuss is whether it's possible to release the new HTTP client API 
as a standalone library (that works on JDK 9).


Thanks,
Wenbo



On Mon, Aug 22, 2016 at 12:56 AM, Michael McMahon 
> 
wrote:


There is an updated version of the HTTP client API doc [1] and a
specdiff [2] showing the changes
proposed from the current version in JDK9 dev.

The main changes are:

1) The request and response processors are now based on
Flow.Publisher and Flow.Subscriber

2) Response bodies are retrieved synchronously with the response
headers as part of the
HttpRequest.response() and responseAsync() methods

3) Because of the change above, to allow code to examine the
headers and decide what to do
with the body before retrieving it, there is a new entity
called a HttpResponse.BodyHandler
which is given the status code and headers, and which
returns a HttpResponse.BodyProcessor.
Static implementations of both body handlers and body
processors are provided, to make the
simple cases easy to code.

We are currently working in the sandbox repository again and
will have these changes
in the main JDK9 dev forest soon.

Thanks,

Michael

[1] 

Re: HTTP client API

[Anthony Vanelverdinghe]

Hi Michael

What's the rationale for not turning all public classes into interfaces, 
since none of them contain any actual implementation code?


On another note, I fail to see the point of 
HttpClient.Builder::priority: as far as I understand, HTTP/2 priority 
only comes into play when multiple resources depend on the same 
resource. But since there's no way to express dependencies between 
resources, how would this be useful?


Lastly, some minor remarks: I believe the following factory methods 
should be moved for consistency

- HttpRequest::noBody --> HttpRequest.BodyProcessor::noBody
- HttpResponse::multiFile --> HttpResponse.MultiProcessor::asFiles

and that "? extends T" should be used instead of "T", in the return type 
of the following methods

- HttpResponse.BodyHandler::apply, i.e. BodyProcessor
- HttpResponse.BodyProcessor::getBody, i.e. CompletionStage
- HttpResponse.MultiProcessor::onRequest, i.e. Optional>


Kind regards,
Anthony


On 7/09/2016 19:15, Michael McMahon wrote:

Hi Wenbo,

First, sorry for the delay in replying. We took your comments and 
prototyped how the major ones

might be accommodated. In particular, we did the following:

- moved "business logic" out of HttpRequest. The methods for sending 
requests now
   exist in HttpClient. Given that requests are no longer associated 
with a client and can
   be sent through any client, this means that some properties of a 
request that used
   to be inherited from the client can no longer be visible in the 
HttpRequest object,
   but that is not a problem as far as I can see. This changes the 
look of the sample code

   in HttpRequest, but doesn't make it any harder to read.

- changed some method names as suggested eg newBuilder()

- added protected constructors to the public classes. This allows 
alternative implementations

   for use in mocking/test frameworks etc.

- changed the PUT, POST, GET methods in the request builder to only 
set the method type.
   PUT, POST takes the request body processor as parameter. A new 
build() method returns

   the HttpRequest.

- added a method to HttpResponse which returns the "final" request 
that was sent on the wire
   for the particular response, which might be different from the user 
initiated request.


I put an updated apidoc [1] and specdiff [2] up to show the changes. 
In particular, the sample

code described in the HttpClient docs is updated to reflect the changes.

Thanks,
Michael

[1] http://cr.openjdk.java.net/~michaelm/httpclient/api.1/

[2] 
http://cr.openjdk.java.net/~michaelm/httpclient/specdiffout.1/package-summary.html


On 26/08/2016, 07:59, Wenbo Zhu wrote:

Hi Michael,

Thanks for the update! The adoption of the Flow API is indeed a big 
improvement (async flow-control is very hard to get right).


Attached is a feedback doc on this new version. One specific idea to 
discuss is whether it's possible to release the new HTTP client API 
as a standalone library (that works on JDK 9).


Thanks,
Wenbo



On Mon, Aug 22, 2016 at 12:56 AM, Michael McMahon 
> 
wrote:


There is an updated version of the HTTP client API doc [1] and a
specdiff [2] showing the changes
proposed from the current version in JDK9 dev.

The main changes are:

1) The request and response processors are now based on
Flow.Publisher and Flow.Subscriber

2) Response bodies are retrieved synchronously with the response
headers as part of the
HttpRequest.response() and responseAsync() methods

3) Because of the change above, to allow code to examine the
headers and decide what to do
with the body before retrieving it, there is a new entity
called a HttpResponse.BodyHandler
which is given the status code and headers, and which returns
a HttpResponse.BodyProcessor.
Static implementations of both body handlers and body
processors are provided, to make the
simple cases easy to code.

We are currently working in the sandbox repository again and will
have these changes
in the main JDK9 dev forest soon.

Thanks,

Michael

[1] http://cr.openjdk.java.net/~michaelm/httpclient/api/


[2]

http://cr.openjdk.java.net/~michaelm/httpclient/specdiffout/package-summary.html

Re: HTTP client API

[Michael McMahon]

Hi Wenbo,

First, sorry for the delay in replying. We took your comments and 
prototyped how the major ones

might be accommodated. In particular, we did the following:

- moved "business logic" out of HttpRequest. The methods for sending 
requests now
   exist in HttpClient. Given that requests are no longer associated 
with a client and can
   be sent through any client, this means that some properties of a 
request that used
   to be inherited from the client can no longer be visible in the 
HttpRequest object,
   but that is not a problem as far as I can see. This changes the look 
of the sample code

   in HttpRequest, but doesn't make it any harder to read.

- changed some method names as suggested eg newBuilder()

- added protected constructors to the public classes. This allows 
alternative implementations

   for use in mocking/test frameworks etc.

- changed the PUT, POST, GET methods in the request builder to only set 
the method type.
   PUT, POST takes the request body processor as parameter. A new 
build() method returns

   the HttpRequest.

- added a method to HttpResponse which returns the "final" request that 
was sent on the wire
   for the particular response, which might be different from the user 
initiated request.


I put an updated apidoc [1] and specdiff [2] up to show the changes. In 
particular, the sample

code described in the HttpClient docs is updated to reflect the changes.

Thanks,
Michael

[1] http://cr.openjdk.java.net/~michaelm/httpclient/api.1/

[2] 
http://cr.openjdk.java.net/~michaelm/httpclient/specdiffout.1/package-summary.html


On 26/08/2016, 07:59, Wenbo Zhu wrote:

Hi Michael,

Thanks for the update! The adoption of the Flow API is indeed a big 
improvement (async flow-control is very hard to get right).


Attached is a feedback doc on this new version. One specific idea to 
discuss is whether it's possible to release the new HTTP client API as 
a standalone library (that works on JDK 9).


Thanks,
Wenbo



On Mon, Aug 22, 2016 at 12:56 AM, Michael McMahon 
> 
wrote:


There is an updated version of the HTTP client API doc [1] and a
specdiff [2] showing the changes
proposed from the current version in JDK9 dev.

The main changes are:

1) The request and response processors are now based on
Flow.Publisher and Flow.Subscriber

2) Response bodies are retrieved synchronously with the response
headers as part of the
HttpRequest.response() and responseAsync() methods

3) Because of the change above, to allow code to examine the
headers and decide what to do
with the body before retrieving it, there is a new entity
called a HttpResponse.BodyHandler
which is given the status code and headers, and which returns
a HttpResponse.BodyProcessor.
Static implementations of both body handlers and body
processors are provided, to make the
simple cases easy to code.

We are currently working in the sandbox repository again and will
have these changes
in the main JDK9 dev forest soon.

Thanks,

Michael

[1] http://cr.openjdk.java.net/~michaelm/httpclient/api/


[2]

http://cr.openjdk.java.net/~michaelm/httpclient/specdiffout/package-summary.html

Re: HTTP client API

[Chris Hegarty]

Martin, Wenbo,

We are considering the feedback, will take it under advisement, and
reply in due course.

-Chris.

> On 26 Aug 2016, at 22:14, Martin Buchholz  wrote:
> 
> I don't know much about http, but I believe those who say that it's very hard 
> to get the API right.  Perhaps it would be best to deliver a http client 
> library as an independent beta package for use with jdk 8+ with a final 
> version becoming part of jdk 10?  Perhaps there can be a productive 
> collaboration here with companies for which http is bread-n-butter?
> 
> As a precedent, jsr166 historically released independent jars in this way in 
> the jsr166* packages
> http://gee.cs.oswego.edu/dl/concurrency-interest/index.html 
> 
> 

Re: HTTP client API

[Martin Buchholz]

I don't know much about http, but I believe those who say that it's very
hard to get the API right.  Perhaps it would be best to deliver a http
client library as an independent beta package for use with jdk 8+ with a
final version becoming part of jdk 10?  Perhaps there can be a productive
collaboration here with companies for which http is bread-n-butter?

As a precedent, jsr166 historically released independent jars in this way
in the jsr166* packages
http://gee.cs.oswego.edu/dl/concurrency-interest/index.html

Re: HTTP client API

[Pavel Rappo]

http://cr.openjdk.java.net/~chegar/docs/sandbox.html

> On 23 Aug 2016, at 21:20, Martin Buchholz  wrote:
> 
> I tried this:
> 
> (Install mercurial trees extension.)
>  hg tclone http://hg.openjdk.java.net/jdk9/sandbox jdk9-sandbox
>  hg tupdate -r http-client-branch
> 
> Is that right?
> I'm not sure how one would get differences against stock jdk9.
> Is there a doc about how to effectively use the sandbox repository?

Re: HTTP client API

[Martin Buchholz]

On Tue, Aug 23, 2016 at 12:57 PM, Wenbo Zhu  wrote:

>
>> We are currently working in the sandbox repository again and will have
>> these changes
>> in the main JDK9 dev forest soon.
>>
> Is it possible to access the API code somewhere before then, mainly as a
> convenience for reviewing the APIs?
>

I tried this:

(Install mercurial trees extension.)
 hg tclone http://hg.openjdk.java.net/jdk9/sandbox jdk9-sandbox
 hg tupdate -r http-client-branch

Is that right?
I'm not sure how one would get differences against stock jdk9.
Is there a doc about how to effectively use the sandbox repository?

Re: HTTP client API

[Wenbo Zhu]

On Mon, Aug 22, 2016 at 12:56 AM, Michael McMahon <
michael.x.mcma...@oracle.com> wrote:

> There is an updated version of the HTTP client API doc [1] and a specdiff
> [2] showing the changes
> proposed from the current version in JDK9 dev.
>
> The main changes are:
>
> 1) The request and response processors are now based on Flow.Publisher and
> Flow.Subscriber
>
> 2) Response bodies are retrieved synchronously with the response headers
> as part of the
> HttpRequest.response() and responseAsync() methods
>
> 3) Because of the change above, to allow code to examine the headers and
> decide what to do
> with the body before retrieving it, there is a new entity called a
> HttpResponse.BodyHandler
> which is given the status code and headers, and which returns a
> HttpResponse.BodyProcessor.
> Static implementations of both body handlers and body processors are
> provided, to make the
> simple cases easy to code.
>
> We are currently working in the sandbox repository again and will have
> these changes
> in the main JDK9 dev forest soon.
>
Is it possible to access the API code somewhere before then, mainly as a
convenience for reviewing the APIs?



>
> Thanks,
>
> Michael
>
> [1] http://cr.openjdk.java.net/~michaelm/httpclient/api/
>
> [2] http://cr.openjdk.java.net/~michaelm/httpclient/specdiffout/
> package-summary.html
>
>

Re: Http client API

[Vasiliy Baranov]

On 17.08.2012 14:30, Michael McMahon wrote:

On 09/08/12 19:15, Chris Hegarty wrote:

Michael,

Looking good, some comments.

1) Why the use of CookieManager, rather than CookieHandler? I would
expect that CookieHandler would be a cleaner API



CookieHandler is a very low level API, which doesn't offer much
functionality
beyond managing the cookies yourself, which is what you would have to
do, if there
were no cookie capabilities in the http api. So, I don't know what we
would gain from using it.

While CookieManager isn't perfect (and we'd like to suggest a few
improvements)
it is a higher level API that does take care of cookie management with
some common
policies pre-defined.


java.net.CookieHandler is an abstract class, almost an interface, that 
represents everything an HTTP client needs to manage cookies. 
java.net.CookieManager is a concrete implementation of 
java.net.CookieHandler. At least that is the case in the current 
java.net.* design.


That said, shouldn't new HttpClient depend on the more abstract 
java.net.CookieHandler, rather than the more concrete 
java.net.CookieManager, for the sake of API cleanness and extensibility? 
If one prefers java.net.CookieManager over java.net.CookieHandler, it is 
possible to use the former everywhere where the latter is expected, but 
not the other way around.


As a side note, in the plugin environment, it might be more natural and 
less invasive to interface the browser cookie store via 
java.net.CookieHandler rather than java.net.CookieManager. FWIW, the 
current plug-in implementation, 
com.sun.deploy.net.cookie.DeployCookieSelector, does extend 
java.net.CookieHandler.


Just my two cents.

Thank you,
-- Vasiliy

Re: Http client API

[Michael McMahon]

Chris,

Thanks for the comments. Some responses below:


On 09/08/12 19:15, Chris Hegarty wrote:

Michael,

Looking good, some comments.

1) Why the use of CookieManager, rather than CookieHandler?  I  would
   expect that CookieHandler would be a cleaner API



CookieHandler is a very low level API, which doesn't offer much 
functionality
beyond managing the cookies yourself, which is what you would have to 
do, if there
were no cookie capabilities in the http api. So, I don't know what we 
would gain from using it.


While CookieManager isn't perfect (and we'd like to suggest a few 
improvements)
it is a higher level API that does take care of cookie management with 
some common

policies pre-defined.


2) What is the impact on the sendHeader, setBody for HEAD requests?



Ah yes, HEAD needs special treatment - though not with respect to the 
methods above
as HEAD is identical to GET except with respect to any returned response 
body.


So, we need to clarify that HttpResponse.getContentLength() returns the 
value of
the content-length header in the case of HEAD responses. Also, I think 
we should

clarify how empty response bodies are treated. Something like:-
- an InputStream that returns EOF on first read
- empty byte[] or ByteBuffer[] with one element that is empty
- HttpResponseHandler calls onBodyPart() once with last=true and an 
empty ByteBuffer

- IteratorByteBuffer where hasNext() returns false on first call.


3) I think HttpClient could be an interface and move the create method
   to a builder/factory, and make it as immutable as possible ( this
   came up a few times now ).



Right. Am looking into that now.


4) The Filter API looks a little funny, in that filter instances are
   added to the client, while the ByteBufferWrapper instances are
   presumably created by the implementation after registering the
   wrapper class with the filter instance. Probably best/cleaner
   to use the same style. It could also be an interface.



ByteBufferWrapper implementations could be generally useful (eg content 
encoders/decoders)
but each Filter will be associated with zero or one specific 
ByteBufferWrapper implementation.


Though, perhaps Filter could return ByteBufferWrapper instances rather 
than the class ...


What do you think the advantage of Filter being an interface is? In that 
case, implementers
would have to implement both methods in all cases. I think there will be 
many cases
where filters are interested in either the request or the response, but 
not both.



5) ByteBufferWrapper seems a little cluttered with implementation detail
   setSource() nor setWrapper()?? Maybe best to just provide an
   interface.



That comment about setSource() is wrong and needs to be removed. I agree
setWrapper() is implementation detail as it only needs to be called by 
the implementation.

Maybe it could be package private...


6) Upgrade handler, similar comment to 4. Why not just register an
   instance.



Danny?


7) Exposing the filter list seems a little wrong, given the getter/
   setter style.



We'll look at that again with the builder changes


8) java.net.httpclient.HttpConnectionCache.CachedConnection should
   probably be an interface.



Yes, we were thinking about doing that.


9) How does the cache handle tunnels? endpoint address/proxy/etc



good point. That needs to be clarified.


10) Missing fluent style return from HttpRequest.setRequestBodyLimit



ok


11) Should sendHeaders be specified to allow a null return value. I'm
thinking about when setSendExpectContinue is set.



Do you mean if a 417 expectation failed is received? Yes, in that case it
probably does make sense to return null. It's a slightly awkward special 
case,

but would have to be documented.

We're trying to handle the normal situation as transparently as possible.
The idea is that a blocking application will just block until
the 100 continue is received, before being allowed to send the body.
An asynchronous application will just not be called back to get the body
until the 100 is received.

Thanks
Michael

-Chris.


On 08/08/12 00:09, Michael McMahon wrote:

Hi,

A new revision of the Http client API planned for jdk 8 can be viewed
at the following link

http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/

We would like to review the api on this mailing list.
So, all comments are welcome.

Thanks
Michael McMahon.

Re: Http client API

[Frank Carver]

On 17 August 2012 11:30, Michael McMahon michael.x.mcma...@oracle.com wrote:
 2) What is the impact on the sendHeader, setBody for HEAD requests?
 Ah yes, HEAD needs special treatment - though not with respect to the methods 
 above as HEAD is identical to GET except with respect to any returned 
 response body.

 So, we need to clarify that HttpResponse.getContentLength() returns the value 
 of the content-length header in the case of HEAD responses.

That worries me.

I would much prefer that HttpResponse.getContentLength() always
returns the actual content length, which in the case of a HEAD would
presumably be zero.

This has the massive benefit that any code which reads and processes a
response can be completely generic and re-usable, does not need to
know anything about the request, and even relatively naive code will
not break if given a bodiless response from a HEAD request.

Code written specifically for a response from a HEAD is highly likely
to just iterate through or cherry-pick the returned headers, including
content-length, transfer-encoding and any other headers which have
implications for how the body should be processed.

My preference would be to leave the headers alone so the original data
is available if anyone wants it, but make sure any special-case
accessor methods always return directly usable values.

Frank.

Re: Http client API

[Michael McMahon]

On 17/08/12 11:59, Frank Carver wrote:

On 17 August 2012 11:30, Michael McMahonmichael.x.mcma...@oracle.com  wrote:

2) What is the impact on the sendHeader, setBody for HEAD requests?

Ah yes, HEAD needs special treatment - though not with respect to the methods 
above as HEAD is identical to GET except with respect to any returned response 
body.

So, we need to clarify that HttpResponse.getContentLength() returns the value 
of the content-length header in the case of HEAD responses.

That worries me.

I would much prefer that HttpResponse.getContentLength() always
returns the actual content length, which in the case of a HEAD would
presumably be zero.

This has the massive benefit that any code which reads and processes a
response can be completely generic and re-usable, does not need to
know anything about the request, and even relatively naive code will
not break if given a bodiless response from a HEAD request.

Code written specifically for a response from a HEAD is highly likely
to just iterate through or cherry-pick the returned headers, including
content-length, transfer-encoding and any other headers which have
implications for how the body should be processed.

My preference would be to leave the headers alone so the original data
is available if anyone wants it, but make sure any special-case
accessor methods always return directly usable values.


I take your point Frank. I agree, it's probably wrong to complicate
the overwhelmingly common situation with this special case.

Maybe we could just add some text to the HttpResponse.getContentLength() 
method.


Note. This method will always return zero in response to a HEAD 
request. The

response headers must be queried directly to get the Content-length
from a HEAD response.

- Michael



Frank.

Re: Http client API

[Michael McMahon]

Mike. Thanks for the comments. I have commented on most of your points.
There are a few I'm not sure about, and maybe others will chime in.

- Michael.


On 08/08/2012 23:33, Mike Duigou wrote:

General::
- It's probably already been mentioned but having the classes in the httpclient package 
and most of them begin with Http seems redundant.
I think I'd agree if type names were always written as fully qualified 
names,

or if it were possible to import partial package names
eg. import java.net.* and then refer to httpclient.Request or 
httpclient.Response
but I don't think that is possible. Maybe we don't need a sub-package 
for httpclient

but it is a convenient grouping from a readability point of view.
What do other folks think?

- Package JavaDoc is missing for both packages.

right we need to add that

- Is the SPI package needed for just one class?

I suspect not. We probably should just bring it into the main package.

- I am unsure if sendRequest should be a method of Client or Request.


That question arose before and there seems to be two differing preferences.
We probably should enumerate the arguments for and against the two.

HttpClientProvider::

- HttpClientProvider JavaDoc uses inconsistent capitalization of HTTP.

- createAsynchronousHttpClient() Since there's only one create method why bother to 
mention that it's Asynchronous?

- It's not clear how loadFromExternal() is different from provider().

- provider enumeration, request alternate providers?
Yes, the provider needs work. I agree with the above. Though I don't 
know about the

necessity to provide multiple alternate providers.



HttpClient::

- There's no way to identify the source of the HttpClient(). ie. it is returned 
by provider() but there's no way to tell what you got. Would be useful for 
debugging to log to the file what provider you received.
I'm not sure I follow the comment above. Does it just relate to 
implementation rather than the API?

- HttpClient createClient() -- is this the same as 
HttpClientProvider.provider().createAsynchronousHttpClient()?

yes, and that needs to be documented.

- There doesn't seem to be a way to be truly thread safe about changes to 
configuration other than to set all configuration options before sharing an 
HttpClient instance and then never changing the configuration. Is changing the 
configuration *after* sharing realistic or should perhaps configuration be 
immutable (perhaps using a Builder pattern for HttpClient instances, ie. 
createClient() becomes clientBuilder()). Seeing that the set methods return 
HttpClient (the same instance unfortunately) it looks like you are considering 
or have considered using a builder style construction.
Right. I agree (as others have suggested) a builder pattern for mutable 
state is a good idea. The current behavior of returning

the same client instance was more about simply a fluent programming style.

- Why expose the connection cache? Seems like just a good way to mess things 
up. Have you considered provider scope for connection cache? ie. all clients 
from the same provider share a cache?
The idea behind connection cache was to allow applications more control 
over opening, closing and caching of TCP
connections. But, it's possibly not 100% right yet.  I figured that its 
scope would be at client level. If there were a higher
level notion of an application in Java SE, then it probably would be 
associated at that level imo.

In what way do you think it would be easy to mess things up?

- setProxy lacks proxy authentication features. (sorry, I just said that to 
annoy you. :-) )
Yes. Authentication is one missing piece of the puzzle. We want to 
implement authentication
as a Filter, but it still needs a public API. But, it will be through 
that API where proxy authentication

happens also.

- Does the proxy default to system properties and respect the nonProxyHosts? ie:
http.proxyHost (default:none)
http.proxyPort (default: 80 if http.proxyHost specified)
http.nonProxyHosts (default:none)
Currently no. Since this is a new API, I'm not sure about the benefit of 
re-using the
legacy properties. Perhaps there could be value in allowing system proxy 
settings to

be used.

- Is the default cookie manager from CookieHandler.getDefault() or is there no 
default?

Currently no default, which means no special treatment of cookie headers,
which I think is reasonable

- Default is not specified for setAllowPipeLineMode() Presumably it is false

right. we need to specify default is false

- There's no discussion of pipeline mode and connection cache.

ok

- sendHeaders/sendRequest : presumably it is an error to use an HttpRequest 
from a different HttpClient.
that was one reason why the sendXXX methods made sense on HttpRequest, 
because otherwise
the possibility for sending a request on the wrong client has to be 
considered.

So, we will have to specify that if we keep it as it is.

- setResponseBodyBufferLimit - like timeout this is a default. 

Re: Use Builder pattern ( was: Re: Http client API)

[Michael McMahon]

On 09/08/12 10:50, Chris Hegarty wrote:

On 09/08/12 00:00, Jed Wesley-Smith wrote:

Michael McMahon michael.x.mcmahon@... writes:


A new revision of the Http client API planned for jdk 8 can be viewed
at the following link

http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/

We would like to review the api on this mailing list.
So, all comments are welcome.


Can you separate the domain objects (in particular HttpClient, 
HttpRequest)
and their set-up (all the mutators) into separate concerns (Builders 
perhaps,

see Guava for instance)?


+1, I agree with your comment.

Wherever possible we should try to use a builder pattern to build 
immutable objects ( or limit its mutability as much as possible ). I 
think Mike made a very similar comment too. Maybe the spi and 
factory/builder could be separated out, I think this would be much 
cleaner.


As you say, you get thread-safety by default, which would appear to be 
a nice property for this API, given its different programming models.


I agree. The various mutable set() type methods should be in builders 
and the resulting HttpClient and HttpRequests

should be immutable. Will look into that now.

Thanks,
Michael.

Re: Http client API

[Kurchi Subhra Hazra]

- HttpClient createClient() -- is this the same as 
HttpClientProvider.provider().createAsynchronousHttpClient()?

yes, and that needs to be documented.
Just on this one, the method name createAsynchronousHttpClient() looks 
like a remnant of our previous API iterations, and I think this

should just be renamed to createHttpClient() or createClient().

- Kurchi

Re: Http client API

[Kurchi Subhra Hazra]

Just as a reminder, for reading the response, we had originally decided to
simply terminate whatever bytebuffer/byte arrays we have with a -1 to 
indicate
EOF. But if returning the number of bytes read results in cleaner code, 
maybe we should do this.




HttpResponse::

- getBodyAsBytes(byte[], int) - I would rather the return the number 
of bytes put into the buffer.
yes. Actually the number of bytes read needs to be returned somewhere. 
It should return

an int or long
- getBodyAsBytes(byte[], int) - Why would I ever want a max size less 
than the size of my byte buffer?
I originally wanted to implement the no-arg getBodyAsBytes() using the 
other method.

Maybe it's better to replace them both with:

long getBodyAsBytes(byte[] buffer) - buffer can't be null, use entire 
buffer, return # bytes read
byte[] getBodyAsBytes() - size limited by 
HttpClient.getResponseBodyBufferLimit()


- getBodyAsBytes -- Document handling for content-length  
Integer.MAX_VALUE
dealt with implicitly from above change and the response body buffer 
limit itself.
- getBodyAsByteBuffers -- handling for not enough space in the array 
needs to be documented. Unused entries nulled out?
situation would only arise through ByteBuffer allocation failure, and 
presumably OOM exception would be thrown.

Is it necessary to document this?

- Perhaps maxlength -  maxBytes to be more clear?

- getBodyAsByteBufferSource -- if the same iterator is always 
returned why not return the iterator rather than an iterable?

The idea was to allow for use of the foreach convenience. eg

IterableByteBuffer  buffers = response.getBodyAsByteBufferSource();
for (ByteBuffer buffer : buffers) {
// handle data in buffer
}


This raises the same concern then about the Iterable being 
non-restartable, which in this case
it clearly can't be. This is a one-shot streaming API. I notice that 
the documentation for

Iterable is rather terse and doesn't say whether
this usage is encouraged or discouraged. So, I'm not sure now. The 
question is if programmers would
really be confused by the fact that the Iterable can only be used to 
return one Iterator instance.



HttpConnectionCache.CachedConnection::
- I would expect this to be abstract and that client providers would 
extend.


- getCache() to return the client provider.


On Aug 7 2012, at 16:09 , Michael McMahon wrote:


Hi,

A new revision of the Http client API planned for jdk 8 can be viewed
at the following link

http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/

We would like to review the api on this mailing list.
So, all comments are welcome.

Thanks
Michael McMahon.

Re: Http client API

[Michael McMahon]

On 09/08/12 01:49, David M. Lloyd wrote:

On 08/07/2012 06:09 PM, Michael McMahon wrote:

Hi,

A new revision of the Http client API planned for jdk 8 can be viewed
at the following link

http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/

We would like to review the api on this mailing list.
So, all comments are welcome.


Why not javax.net.httpclient?  Having it be backportable to earlier 
JDKs would definitely help adoption IMO.



David

The API is dependent on the asynchronous NIO capability that first 
appeared in jdk7. So, that would limit
how far back it could be ported. Technically, I don't see a problem with 
using a javax. package name
but I'm not sure what precedent there is for doing core platform APIs 
under the javax namespace.

What do others think?

- Michael

Re: Http client API

[Mike Duigou]

On Aug 15 2012, at 09:06 , Michael McMahon wrote:

 HttpClientProvider::
 
 - HttpClientProvider JavaDoc uses inconsistent capitalization of HTTP.
 
 - createAsynchronousHttpClient() Since there's only one create method why 
 bother to mention that it's Asynchronous?
 
 - It's not clear how loadFromExternal() is different from provider().
 
 - provider enumeration, request alternate providers?
 Yes, the provider needs work. I agree with the above. Though I don't know 
 about the
 necessity to provide multiple alternate providers.

If there is only ever going to be one provider then why include a provider 
interface. I suspect that HttpClient will have about as many implementations as 
there are JCE providers, ie. less than 10 but it is possible that someone may 
have more than one simultaneously and want to select a particular provider.

 
 
 HttpClient::
 
 - There's no way to identify the source of the HttpClient(). ie. it is 
 returned by provider() but there's no way to tell what you got. Would be 
 useful for debugging to log to the file what provider you received.
 I'm not sure I follow the comment above. Does it just relate to 
 implementation rather than the API?

Yes. Mostly for diagnostic purposes. Since the provider isn't associated with 
the JDK I need to be able to get version information about the provider to 
record to logs, use in bug reports, etc.

 - Why expose the connection cache? Seems like just a good way to mess things 
 up. Have you considered provider scope for connection cache? ie. all clients 
 from the same provider share a cache?
 The idea behind connection cache was to allow applications more control over 
 opening, closing and caching of TCP
 connections. But, it's possibly not 100% right yet.  I figured that its scope 
 would be at client level. If there were a higher
 level notion of an application in Java SE, then it probably would be 
 associated at that level imo.
 In what way do you think it would be easy to mess things up?

Failure to return connections mostly. Resource leakage.
 - I missed the s in setHttpsConfigurator until I looked at the method in 
 more detail. It doesn't really stand out.
 maybe setSSLConfigurator() and change HttpsConfigurator to SSLConfigurator?

It would certainly stand out more though TLS is hopeful used.

 - getFilters() -- this is the only modify-in-place API. Kinda weird. I think 
 having setFilters() would be better.
 Do you mean getFilters() returns a copy and setFilters() would just set the 
 actual list?

Yes.

 - setBody(IterableByteBuffer, boolean isRestartable) -- This seems 
 heinous. Non-restartable Iterables should be avoided. I like the suggestion 
 of additional setBody(IterableByteBuffer) method instead.
 Ok. So, a setBody(IteratorByteBuffer)  then ?

Yes.
 - getBodyAsByteBuffers -- handling for not enough space in the array needs 
 to be documented. Unused entries nulled out?
 situation would only arise through ByteBuffer allocation failure, and 
 presumably OOM exception would be thrown.
 Is it necessary to document this?

Probably not. OOM is an Error rather than an exception to it wouldn't be 
reasonable for developers to try to catch it. 

 - Perhaps maxlength -  maxBytes to be more clear?
 
 - getBodyAsByteBufferSource -- if the same iterator is always returned why 
 not return the iterator rather than an iterable?
 The idea was to allow for use of the foreach convenience. eg
 
 IterableByteBuffer  buffers = response.getBodyAsByteBufferSource();
 for (ByteBuffer buffer : buffers) {
// handle data in buffer
 }
 
 
 This raises the same concern then about the Iterable being non-restartable, 
 which in this case
 it clearly can't be. This is a one-shot streaming API. I notice that the 
 documentation for
 Iterable is rather terse and doesn't say whether
 this usage is encouraged or discouraged. So, I'm not sure now.

One-shot Iterables are definitely discouraged

Mike

Re: Http client API

[Xuelei Fan]

On Aug 14, 2012, at 8:49 PM, Michael McMahon michael.x.mcma...@oracle.com 
wrote:

 Xuelei,
 
 We have no particular requirement on HostnameVerifier. So,
 if I understood you correctly, HostnameVerifier is redundant in new APIs
 and it is possible to control hostname verification through the SSLParameters 
 class
 (and an X509ExtendedTrustManager).
 
 So, we can drop HostnameVerifier from our API. Is that correct?
 
Yes.

Xuelei

 Thanks
 Michael
 
 On 08/08/12 13:10, Xuelei Fan wrote:
 From JDK 7, JSSE introduces a new hostname verifying approach. It is
 call endpoint identification in JSSE context. It can be used to
 replace the HostnameVerifier on SSLSession. A typical user case looks like:
 
 1. implement a X509ExtendedTrustManager. It is required to check the
 endpoint identification in the following methods:
checkClientTrusted(X509Certificate[], String, Socket)
checkClientTrusted(X509Certificate[], String, SSLEngine)
checkServerTrusted(X509Certificate[], String, Socket)
checkServerTrusted(X509Certificate[], String, SSLEngine)
 
 2. initialize a SSLParameters to enable the endpoint identification:
sslParameter.setEndpointIdentificationAlgorithm(https);
 
 3. set the SSLParameters to SSLSocket or SSLEngine, the instance of
 X509ExtendedTrustManager will check the endpoint (hostname) during
 handshaking.
 
 Considering the java.net.httpclient.HttpsConfigurator, it is an
 implementation of HostnameVerifier.  So it would support both
 HostnameVerifier and the above endpoint identification.  I think as may
 be redundant if no compatibility concerns.  I was wondering maybe it is
 OK to detach the HostnameVerifier interface and remove the verify() method.
 
 Maybe, you have other concerns that the HttpsConfigurator.verify()
 method is really needed.
 
 Thanks,
 Xuelei
 
 On 8/8/2012 7:09 AM, Michael McMahon wrote:
 Hi,
 
 A new revision of the Http client API planned for jdk 8 can be viewed
 at the following link
 
 http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/
 
 We would like to review the api on this mailing list.
 So, all comments are welcome.
 
 Thanks
 Michael McMahon.
 

Re: Http client API

[Michael McMahon]

To everyone who has commented on the API so far, thank you!
It is much appreciated. We will be getting back on all the issues raised 
in the coming days.


I am just about to send out a request on jdk8-dev (which covers all JDK 
groups) to solicit

feedback from as many people as possible.

Thanks again,
Michael.

On 08/08/12 00:09, Michael McMahon wrote:

Hi,

A new revision of the Http client API planned for jdk 8 can be viewed
at the following link

http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/

We would like to review the api on this mailing list.
So, all comments are welcome.

Thanks
Michael McMahon.

Re: Http client API

[Alan Bateman]

On 09/08/2012 01:49, David M. Lloyd wrote:

On 08/07/2012 06:09 PM, Michael McMahon wrote:

Hi,

A new revision of the Http client API planned for jdk 8 can be viewed
at the following link

http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/

We would like to review the api on this mailing list.
So, all comments are welcome.


Why not javax.net.httpclient?

I assume this would require submitting it as a standalone JSR.

-Alan.

Use Builder pattern ( was: Re: Http client API)

[Chris Hegarty]

On 09/08/12 00:00, Jed Wesley-Smith wrote:

Michael McMahon michael.x.mcmahon@... writes:


A new revision of the Http client API planned for jdk 8 can be viewed
at the following link

http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/

We would like to review the api on this mailing list.
So, all comments are welcome.


Can you separate the domain objects (in particular HttpClient, HttpRequest)
and their set-up (all the mutators) into separate concerns (Builders perhaps,
see Guava for instance)?


+1, I agree with your comment.

Wherever possible we should try to use a builder pattern to build 
immutable objects ( or limit its mutability as much as possible ). I 
think Mike made a very similar comment too. Maybe the spi and 
factory/builder could be separated out, I think this would be much cleaner.


As you say, you get thread-safety by default, which would appear to be a 
nice property for this API, given its different programming models.


-Chris.


It'd be nice to have this all thread-safe by default, it seems creating an API
that isn't thread-safe is maybe not ideal.

cheers,
Jed Wesley-Smith

Re: Http client API

[Chris Hegarty]

On 09/08/12 06:10, Sean Chou wrote:

Hi Chris,

  That's exactly my concern. I agree this provides best flexibility
and content convenient methods are not proper in this API. So, is there
going to have content specific convenient APIs like java.nio.file.Files
? Although it is a wrapper, it may be useful and intuitive, and easy to
use for those who just want to open a webpage but don't want to know
request and response. And as well doesn't break modularization. Thanks.


I think it is certainly worth some consideration, but exactly where it 
would reside and how it could be specified without breaking modularity 
is a bit of a conundrum. Most of the types that would be useful to have 
in such a wrapper/convenience/utility API will come from modules that 
are not from the same module as the httpclient will most likely be 
targeted to. So any API will have cross module dependencies ( not 
necessarily a problem ), just need to think through the implications.


-Chris


On Wed, Aug 8, 2012 at 5:52 PM, Chris Hegarty chris.hega...@oracle.com
mailto:chris.hega...@oracle.com wrote:

On 08/08/2012 07:25, Sean Chou wrote:


Is it possible to have methods like

public abstract HTMLDocument getResponse(String request)  in class
HttpClient ?


Hi Sean,

I think what you are suggesting is content specific convenience
methods, something akin to URLConnection.getContent(), right? In my
experience, this type of interface can be a little problematic and
not all that widely used.

I'm personally not in favor of such convenience methods. I can see
their potential value, but this would appear to be a low level API
and I don't think it should favor one specific content type over
another.

 From my reading of the API it exposes various methods to access the
response body. It is quite easy to wrap one of these in a content
specific parser. For example, for xml you could do

   XMLInputFactory xif = ...;


xif.createStreamReader(client.__getResponse(request).__getBodyAsInputStream());

  OR
   SAXParserFactory spf = ...;


spf.newSAXParser().parse(__client.getResponse(request).__getBodyAsInputStream(),
handler);

I think this gives the best flexibility without favoring one content
type parser, or one programming model, over another.

Does this make sense? Have I missed the point of your proposal?
Maybe others have a different view...

-Chris.




On Wed, Aug 8, 2012 at 7:09 AM, Michael McMahon
michael.x.mcma...@oracle.com
mailto:michael.x.mcma...@oracle.com
mailto:michael.x.mcmahon@__oracle.com
mailto:michael.x.mcma...@oracle.com wrote:

 Hi,

 A new revision of the Http client API planned for jdk 8 can
be viewed
 at the following link

http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/
http://cr.openjdk.java.net/~__michaelm/httpclient/v0.3/

 http://cr.openjdk.java.net/~__michaelm/httpclient/v0.3/
http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/

 We would like to review the api on this mailing list.
 So, all comments are welcome.

 Thanks
 Michael McMahon.




--
Best Regards,
Sean Chou




--
Best Regards,
Sean Chou

Re: Http client API

[Chris Hegarty]

Wow, Mike. Great feedback.

I just scanned your comments, and agree/understand most of them. I'll 
help Michael to feed them back into the API. Though, there'll probably 
be a few follow up mails to answers your direct questions and seek 
further clarification before the new rev of the API.


Thanks again,
-Chris.

On 08/08/12 23:33, Mike Duigou wrote:

Hi Michael!

I really look forward to using this API! It looks like you have made a lot of 
progress. Sorry for having so many comments on just one round.

Mike

General::
- It's probably already been mentioned but having the classes in the httpclient package 
and most of them begin with Http seems redundant.

- Package JavaDoc is missing for both packages.

- Is the SPI package needed for just one class?

- I am unsure if sendRequest should be a method of Client or Request.


HttpClientProvider::

- HttpClientProvider JavaDoc uses inconsistent capitalization of HTTP.

- createAsynchronousHttpClient() Since there's only one create method why bother to 
mention that it's Asynchronous?

- It's not clear how loadFromExternal() is different from provider().

- provider enumeration, request alternate providers?



HttpClient::

- There's no way to identify the source of the HttpClient(). ie. it is returned 
by provider() but there's no way to tell what you got. Would be useful for 
debugging to log to the file what provider you received.

- HttpClient createClient() -- is this the same as 
HttpClientProvider.provider().createAsynchronousHttpClient()?

- There doesn't seem to be a way to be truly thread safe about changes to 
configuration other than to set all configuration options before sharing an 
HttpClient instance and then never changing the configuration. Is changing the 
configuration *after* sharing realistic or should perhaps configuration be 
immutable (perhaps using a Builder pattern for HttpClient instances, ie. 
createClient() becomes clientBuilder()). Seeing that the set methods return 
HttpClient (the same instance unfortunately) it looks like you are considering 
or have considered using a builder style construction.

- Why expose the connection cache? Seems like just a good way to mess things 
up. Have you considered provider scope for connection cache? ie. all clients 
from the same provider share a cache?

- setProxy lacks proxy authentication features. (sorry, I just said that to 
annoy you. :-) )

- Does the proxy default to system properties and respect the nonProxyHosts? ie:
http.proxyHost (default: none)
http.proxyPort (default: 80 if http.proxyHost specified)
http.nonProxyHosts (default: none)

- Is the default cookie manager from CookieHandler.getDefault() or is there no 
default?

- Default is not specified for setAllowPipeLineMode() Presumably it is false

- There's no discussion of pipeline mode and connection cache.

- sendHeaders/sendRequest : presumably it is an error to use an HttpRequest 
from a different HttpClient.

- setResponseBodyBufferLimit - like timeout this is a default. Perhaps 
setDefaultResponseBodyBufferLimit (long I know).

- What happens with sendHeaders when the request already has a body?

- Impact of closing the OutputStream returned from sendHeaders(). chunked mode 
vs. non-chucked.

- getResponse : java.util.concurrent.TimeoutException has a nice name but is an 
odd exception to be throwing. java.net.SocketTimeoutException seems more 
appropriate.

- I'm curious why setUpgradeHandler() takes a class rather than an instance.

- getHttpsConfigurator() -- since a default is established before first 
invocation why not never return null. ie. implement the default behaviour in 
getHttpsConfigurator() and have the impl call getHttpsConfigurator().

- I missed the s in setHttpsConfigurator until I looked at the method in more 
detail. It doesn't really stand out.

- getFilters() -- this is the only modify-in-place API. Kinda weird. I think 
having setFilters() would be better.



HttpUpgradeHandler::

- perhaps in the spi package?

- Method or methods to identify the UpgradeHandler. ie. getName(), getProtocols.



UpgradeConnection::

- Perhaps Upgrade**d**Connection?

- There's no way to get back the UpdgradeHandler or source client.



SimpleConnectionClass::

- Package private?



HttpRequest::

- copy() vs clone()?

- setFollowRedirects should get a default from HttpClient (or remove defaults 
for timeout and buffer limits). Asymmetry is annoying.

- setBody(IterableByteBuffer, boolean isRestartable) -- This seems heinous. 
Non-restartable Iterables should be avoided. I like the suggestion of additional 
setBody(IterableByteBuffer) method instead.

- setRequestURI() -- eliminate this unless there is a really good reason to 
keep it. Alternately, perhaps copy(URI) instead?

- getMethod() -- missing.



HttpHeaders::

- getValues() how about return empty result when there is no header of that 
name?

- getValues() -- the returned list is unmodifiable, correct?

- contains() -- IAE for null.

- getFirstValue() 

Re: Http client API

[Chris Hegarty]

Michael,

Looking good, some comments.

1) Why the use of CookieManager, rather than CookieHandler?  I  would
   expect that CookieHandler would be a cleaner API

2) What is the impact on the sendHeader, setBody for HEAD requests?

3) I think HttpClient could be an interface and move the create method
   to a builder/factory, and make it as immutable as possible ( this
   came up a few times now ).

4) The Filter API looks a little funny, in that filter instances are
   added to the client, while the ByteBufferWrapper instances are
   presumably created by the implementation after registering the
   wrapper class with the filter instance. Probably best/cleaner
   to use the same style. It could also be an interface.

5) ByteBufferWrapper seems a little cluttered with implementation detail
   setSource() nor setWrapper()?? Maybe best to just provide an
   interface.

6) Upgrade handler, similar comment to 4. Why not just register an
   instance.

7) Exposing the filter list seems a little wrong, given the getter/
   setter style.

8) java.net.httpclient.HttpConnectionCache.CachedConnection should
   probably be an interface.

9) How does the cache handle tunnels? endpoint address/proxy/etc

10) Missing fluent style return from HttpRequest.setRequestBodyLimit

11) Should sendHeaders be specified to allow a null return value. I'm
thinking about when setSendExpectContinue is set.

-Chris.


On 08/08/12 00:09, Michael McMahon wrote:

Hi,

A new revision of the Http client API planned for jdk 8 can be viewed
at the following link

http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/

We would like to review the api on this mailing list.
So, all comments are welcome.

Thanks
Michael McMahon.

Re: Http client API

[Sean Chou]

Is it possible to have methods like

public abstract HTMLDocument getResponse(String request)  in class
HttpClient ?



On Wed, Aug 8, 2012 at 7:09 AM, Michael McMahon 
michael.x.mcma...@oracle.com wrote:

 Hi,

 A new revision of the Http client API planned for jdk 8 can be viewed
 at the following link

 http://cr.openjdk.java.net/~**michaelm/httpclient/v0.3/http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/

 We would like to review the api on this mailing list.
 So, all comments are welcome.

 Thanks
 Michael McMahon.




-- 
Best Regards,
Sean Chou

Re: Http client API

[Chris Hegarty]

On 08/08/2012 07:25, Sean Chou wrote:


Is it possible to have methods like

public abstract HTMLDocument getResponse(String request)  in class
HttpClient ?


Hi Sean,

I think what you are suggesting is content specific convenience methods, 
something akin to URLConnection.getContent(), right? In my experience, 
this type of interface can be a little problematic and not all that 
widely used.


I'm personally not in favor of such convenience methods. I can see their 
potential value, but this would appear to be a low level API and I don't 
think it should favor one specific content type over another.


From my reading of the API it exposes various methods to access the 
response body. It is quite easy to wrap one of these in a content 
specific parser. For example, for xml you could do


  XMLInputFactory xif = ...;

xif.createStreamReader(client.getResponse(request).getBodyAsInputStream());

 OR
  SAXParserFactory spf = ...;

spf.newSAXParser().parse(client.getResponse(request).getBodyAsInputStream(), 
handler);


I think this gives the best flexibility without favoring one content 
type parser, or one programming model, over another.


Does this make sense? Have I missed the point of your proposal? Maybe 
others have a different view...


-Chris.





On Wed, Aug 8, 2012 at 7:09 AM, Michael McMahon
michael.x.mcma...@oracle.com mailto:michael.x.mcma...@oracle.com wrote:

Hi,

A new revision of the Http client API planned for jdk 8 can be viewed
at the following link

http://cr.openjdk.java.net/~__michaelm/httpclient/v0.3/
http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/

We would like to review the api on this mailing list.
So, all comments are welcome.

Thanks
Michael McMahon.




--
Best Regards,
Sean Chou

Re: Http client API

[Alan Bateman]

On 08/08/2012 07:25, Sean Chou wrote:


Is it possible to have methods like

public abstract HTMLDocument getResponse(String request)  in class 
HttpClient ?


I see Chris has replied to this. One other point is that we also need to 
consider JDK modularization and I don't think we should have anything in 
this API that would require the AWT/Swing (or the desktop module as 
know it in the current prototype set of modules).


-Alan

Re: Http client API

[Anthony Vanelverdinghe]

Hi

With the current API (java.net.HttpURLConnection) it 's not possible to 
follow redirects from one protocol to another (http to https  vice versa).
This is a known problem ( 
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4620571 ), but out of 
security concerns this feature was not added.


Will you please reconsider this feature for the new API and possibly:
add extra methods: 
HttpRequest#setFollowRedirectsAccrossProtocols(boolean follows)  
HttpRequest#followRedirectsAcrossProtocols() which would be false by default
or add a system property (like the ones at 
http://docs.oracle.com/javase/7/docs/technotes/guides/net/properties.html )

?

I am not a security expert, but for example Firefox happily follows such 
redirects, even in a single request like: http (request) - https - 
http (response)
The current behavior is also what caused a recent issue with the JavaFX 
installer ( http://javafx-jira.kenai.com/browse/RT-21275 ). The solution 
to this JavaFX issue says the fix enhanced code to follow https 
redirects. So JavaFX seems to already implement this feature.


Thanks for your feedback

  Anthony Vanelverdinghe


Op 8/08/2012 1:09, Michael McMahon schreef:

Hi,

A new revision of the Http client API planned for jdk 8 can be viewed
at the following link

http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/

We would like to review the api on this mailing list.
So, all comments are welcome.

Thanks
Michael McMahon.

Re: Http client API

[Ian Robertston]

Instead of HttpRequest having 

  void setBody(IterableByteBuffer buffers, boolean isRestartable)

what about having two methods:

  void setBody(IterableByteBuffer buffers) // presumed restartable
  void setBody(IteratorByteBuffer buffers) // clearly not restartable

Not only does this avoid a potentially confusing boolean parameter, but it also
avoids forcing people to create dishonest Iterables, where they know the
iterator() method cannot be called more than once.

  - Ian

Re: Http client API

[Mike Duigou]

Hi Michael!

I really look forward to using this API! It looks like you have made a lot of 
progress. Sorry for having so many comments on just one round.

Mike

General::
- It's probably already been mentioned but having the classes in the httpclient 
package and most of them begin with Http seems redundant.

- Package JavaDoc is missing for both packages.

- Is the SPI package needed for just one class?

- I am unsure if sendRequest should be a method of Client or Request.


HttpClientProvider::

- HttpClientProvider JavaDoc uses inconsistent capitalization of HTTP.

- createAsynchronousHttpClient() Since there's only one create method why 
bother to mention that it's Asynchronous?

- It's not clear how loadFromExternal() is different from provider().

- provider enumeration, request alternate providers?



HttpClient::

- There's no way to identify the source of the HttpClient(). ie. it is returned 
by provider() but there's no way to tell what you got. Would be useful for 
debugging to log to the file what provider you received.

- HttpClient createClient() -- is this the same as 
HttpClientProvider.provider().createAsynchronousHttpClient()?

- There doesn't seem to be a way to be truly thread safe about changes to 
configuration other than to set all configuration options before sharing an 
HttpClient instance and then never changing the configuration. Is changing the 
configuration *after* sharing realistic or should perhaps configuration be 
immutable (perhaps using a Builder pattern for HttpClient instances, ie. 
createClient() becomes clientBuilder()). Seeing that the set methods return 
HttpClient (the same instance unfortunately) it looks like you are considering 
or have considered using a builder style construction.

- Why expose the connection cache? Seems like just a good way to mess things 
up. Have you considered provider scope for connection cache? ie. all clients 
from the same provider share a cache?

- setProxy lacks proxy authentication features. (sorry, I just said that to 
annoy you. :-) )

- Does the proxy default to system properties and respect the nonProxyHosts? ie:
http.proxyHost (default: none)
http.proxyPort (default: 80 if http.proxyHost specified)
http.nonProxyHosts (default: none)

- Is the default cookie manager from CookieHandler.getDefault() or is there no 
default?

- Default is not specified for setAllowPipeLineMode() Presumably it is false

- There's no discussion of pipeline mode and connection cache.

- sendHeaders/sendRequest : presumably it is an error to use an HttpRequest 
from a different HttpClient.

- setResponseBodyBufferLimit - like timeout this is a default. Perhaps 
setDefaultResponseBodyBufferLimit (long I know).

- What happens with sendHeaders when the request already has a body?

- Impact of closing the OutputStream returned from sendHeaders(). chunked mode 
vs. non-chucked.

- getResponse : java.util.concurrent.TimeoutException has a nice name but is an 
odd exception to be throwing. java.net.SocketTimeoutException seems more 
appropriate.

- I'm curious why setUpgradeHandler() takes a class rather than an instance.

- getHttpsConfigurator() -- since a default is established before first 
invocation why not never return null. ie. implement the default behaviour in 
getHttpsConfigurator() and have the impl call getHttpsConfigurator().

- I missed the s in setHttpsConfigurator until I looked at the method in more 
detail. It doesn't really stand out.

- getFilters() -- this is the only modify-in-place API. Kinda weird. I think 
having setFilters() would be better. 



HttpUpgradeHandler::

- perhaps in the spi package?

- Method or methods to identify the UpgradeHandler. ie. getName(), getProtocols.



UpgradeConnection::

- Perhaps Upgrade**d**Connection? 

- There's no way to get back the UpdgradeHandler or source client.



SimpleConnectionClass:: 

- Package private?



HttpRequest::

- copy() vs clone()?

- setFollowRedirects should get a default from HttpClient (or remove defaults 
for timeout and buffer limits). Asymmetry is annoying.

- setBody(IterableByteBuffer, boolean isRestartable) -- This seems heinous. 
Non-restartable Iterables should be avoided. I like the suggestion of 
additional setBody(IterableByteBuffer) method instead.

- setRequestURI() -- eliminate this unless there is a really good reason to 
keep it. Alternately, perhaps copy(URI) instead?

- getMethod() -- missing.



HttpHeaders::

- getValues() how about return empty result when there is no header of that 
name?

- getValues() -- the returned list is unmodifiable, correct?

- contains() -- IAE for null.

- getFirstValue() -- IAE for null name. It has to consistently be either IAE or 
NPE.

- getAllHeaderNames() -- unmodifiable list, correct?



HttpResponse::

- getBodyAsBytes(byte[], int) - I would rather the return the number of bytes 
put into the buffer.

- getBodyAsBytes(byte[], int) - Why would I ever want a max size less than the 
size of my byte buffer?

- 

Re: Http client API

[Jed Wesley-Smith]

Michael McMahon michael.x.mcmahon@... writes:

 A new revision of the Http client API planned for jdk 8 can be viewed
 at the following link
 
 http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/
 
 We would like to review the api on this mailing list.
 So, all comments are welcome.

Can you separate the domain objects (in particular HttpClient, HttpRequest)
and their set-up (all the mutators) into separate concerns (Builders perhaps, 
see Guava for instance)?

It'd be nice to have this all thread-safe by default, it seems creating an API 
that isn't thread-safe is maybe not ideal.

cheers,
Jed Wesley-Smith

Re: Http client API

[Sean Chou]

Hi Chris,

 That's exactly my concern. I agree this provides best flexibility and
content convenient methods are not proper in this API. So, is there going
to have content specific convenient APIs like java.nio.file.Files ?
Although it is a wrapper, it may be useful and intuitive, and easy to use
for those who just want to open a webpage but don't want to know request
and response. And as well doesn't break modularization. Thanks.


On Wed, Aug 8, 2012 at 5:52 PM, Chris Hegarty chris.hega...@oracle.comwrote:

 On 08/08/2012 07:25, Sean Chou wrote:


 Is it possible to have methods like

 public abstract HTMLDocument getResponse(String request)  in class
 HttpClient ?


 Hi Sean,

 I think what you are suggesting is content specific convenience methods,
 something akin to URLConnection.getContent(), right? In my experience, this
 type of interface can be a little problematic and not all that widely used.

 I'm personally not in favor of such convenience methods. I can see their
 potential value, but this would appear to be a low level API and I don't
 think it should favor one specific content type over another.

 From my reading of the API it exposes various methods to access the
 response body. It is quite easy to wrap one of these in a content specific
 parser. For example, for xml you could do

   XMLInputFactory xif = ...;

 xif.createStreamReader(client.**getResponse(request).**
 getBodyAsInputStream());

  OR
   SAXParserFactory spf = ...;

 spf.newSAXParser().parse(**client.getResponse(request).**getBodyAsInputStream(),
 handler);

 I think this gives the best flexibility without favoring one content type
 parser, or one programming model, over another.

 Does this make sense? Have I missed the point of your proposal? Maybe
 others have a different view...

 -Chris.




 On Wed, Aug 8, 2012 at 7:09 AM, Michael McMahon
 michael.x.mcma...@oracle.com 
 mailto:michael.x.mcmahon@**oracle.commichael.x.mcma...@oracle.com
 wrote:

 Hi,

 A new revision of the Http client API planned for jdk 8 can be viewed
 at the following link

 
 http://cr.openjdk.java.net/~__**michaelm/httpclient/v0.3/http://cr.openjdk.java.net/~__michaelm/httpclient/v0.3/

 
 http://cr.openjdk.java.net/~**michaelm/httpclient/v0.3/http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/
 

 We would like to review the api on this mailing list.
 So, all comments are welcome.

 Thanks
 Michael McMahon.




 --
 Best Regards,
 Sean Chou




-- 
Best Regards,
Sean Chou