Re: RFR [11] 8208391: Need to differentiate response and connect timeouts in HTTP Client API

2018-08-09 Thread Jaikiran Pai
Thanks for the updates, Chris. This looks fine to me (I'm not a reviewer).

-Jaikiran


On 07/08/18 3:22 PM, Chris Hegarty wrote:
>> On 4 Aug 2018, at 14:08, Jaikiran Pai  wrote:
>>
>> ...
>>
>> Do you think this can be reworded a bit? Although I understand what's
>> being said here, the wording doesn't seem right. Maybe something like:
>> 
>>  *  In the case where a new connection needs to be
>> established, if
>>  * the connection cannot be established within the given {@code
>>  * duration}, then {@link HttpClient#send(java.net.http.HttpRequest,
>>  * java.net.http.HttpResponse.BodyHandler) HttpClient::send}
>> throws a
>>  * {@link HttpConnectTimeoutException}, or
>>  * {@link HttpClient#sendAsync(java.net.http.HttpRequest,
>>  * java.net.http.HttpResponse.BodyHandler) HttpClient::sendAsync}
>>  * completes exceptionally with an {@code
>> HttpConnectTimeoutException}.
> Agreed. This wording avoids the previous awkwardness.
>
>> src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestImpl.java
>>
>> -this.timeout = null;
>> +this.timeout = Duration.ofSeconds(30);
>>
>> Is this an intentional change of default value for the timeout? Is that
>> something that needs to be documented?
> Accidental, left over from a previous hacking session. Removed.
>
> Updated webrev:
>   http://cr.openjdk.java.net/~chegar/8208391/webrev.01/
>
>> One other thing - maybe not directly related to this single patch, but
>> as you are aware, recently as part of [1], a new system property (and a
>> security property) was introduced to optionally include host info in the
>> exception messages thrown for socket exceptions
> I am aware of this.
>
>> . Does the HttpClient
>> honour that system property in the exceptions it throws?
> The HTPT Client does not have any special handing for this property.
> It may not be necessary, at least not for low-level NIO exceptions, once
> the exception, or its cause, is preserved.
>
>> I don't see it
>> being used in this patch for the timeout exceptions. I haven't checked
>> the code, outside of this patch, to see if it's dealt with in other
>> parts of this API.
>  Separately, I will look into the possibility of where such an extension
> can be used.
>
> -Chris.
>



Re: RFR [11] 8208391: Need to differentiate response and connect timeouts in HTTP Client API

2018-08-08 Thread Michael McMahon

The updated webrev looks fine to me, Chris.

Thanks,
Michael

On 07/08/2018, 10:52, Chris Hegarty wrote:

On 4 Aug 2018, at 14:08, Jaikiran Pai  wrote:

...

Do you think this can be reworded a bit? Although I understand what's
being said here, the wording doesn't seem right. Maybe something like:

  *  In the case where a new connection needs to be
established, if
  * the connection cannot be established within the given {@code
  * duration}, then {@link HttpClient#send(java.net.http.HttpRequest,
  * java.net.http.HttpResponse.BodyHandler) HttpClient::send}
throws a
  * {@link HttpConnectTimeoutException}, or
  * {@link HttpClient#sendAsync(java.net.http.HttpRequest,
  * java.net.http.HttpResponse.BodyHandler) HttpClient::sendAsync}
  * completes exceptionally with an {@code
HttpConnectTimeoutException}.

Agreed. This wording avoids the previous awkwardness.


src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestImpl.java

-this.timeout = null;
+this.timeout = Duration.ofSeconds(30);

Is this an intentional change of default value for the timeout? Is that
something that needs to be documented?

Accidental, left over from a previous hacking session. Removed.

Updated webrev:
   http://cr.openjdk.java.net/~chegar/8208391/webrev.01/


One other thing - maybe not directly related to this single patch, but
as you are aware, recently as part of [1], a new system property (and a
security property) was introduced to optionally include host info in the
exception messages thrown for socket exceptions

I am aware of this.


. Does the HttpClient
honour that system property in the exceptions it throws?

The HTPT Client does not have any special handing for this property.
It may not be necessary, at least not for low-level NIO exceptions, once
the exception, or its cause, is preserved.


I don't see it
being used in this patch for the timeout exceptions. I haven't checked
the code, outside of this patch, to see if it's dealt with in other
parts of this API.

  Separately, I will look into the possibility of where such an extension
can be used.

-Chris.



Re: RFR [11] 8208391: Need to differentiate response and connect timeouts in HTTP Client API

2018-08-07 Thread Chris Hegarty


> On 4 Aug 2018, at 14:08, Jaikiran Pai  wrote:
> 
> ...
> 
> Do you think this can be reworded a bit? Although I understand what's
> being said here, the wording doesn't seem right. Maybe something like:
> 
>  *  In the case where a new connection needs to be
> established, if
>  * the connection cannot be established within the given {@code
>  * duration}, then {@link HttpClient#send(java.net.http.HttpRequest,
>  * java.net.http.HttpResponse.BodyHandler) HttpClient::send}
> throws a
>  * {@link HttpConnectTimeoutException}, or
>  * {@link HttpClient#sendAsync(java.net.http.HttpRequest,
>  * java.net.http.HttpResponse.BodyHandler) HttpClient::sendAsync}
>  * completes exceptionally with an {@code
> HttpConnectTimeoutException}.

Agreed. This wording avoids the previous awkwardness.

> src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestImpl.java
> 
> -this.timeout = null;
> +this.timeout = Duration.ofSeconds(30);
> 
> Is this an intentional change of default value for the timeout? Is that
> something that needs to be documented?

Accidental, left over from a previous hacking session. Removed.

Updated webrev:
  http://cr.openjdk.java.net/~chegar/8208391/webrev.01/

> One other thing - maybe not directly related to this single patch, but
> as you are aware, recently as part of [1], a new system property (and a
> security property) was introduced to optionally include host info in the
> exception messages thrown for socket exceptions

I am aware of this.

> . Does the HttpClient
> honour that system property in the exceptions it throws?

The HTPT Client does not have any special handing for this property.
It may not be necessary, at least not for low-level NIO exceptions, once
the exception, or its cause, is preserved.

> I don't see it
> being used in this patch for the timeout exceptions. I haven't checked
> the code, outside of this patch, to see if it's dealt with in other
> parts of this API.

 Separately, I will look into the possibility of where such an extension
can be used.

-Chris.



Re: RFR [11] 8208391: Need to differentiate response and connect timeouts in HTTP Client API

2018-08-04 Thread Jaikiran Pai
Hi Chris,

src/java.net.http/share/classes/java/net/http/HttpClient.java

+ *  In the case where a new connection needs to be
established, if
+ * the connection cannot be established within the given {@code
+ * duration}, then an {@link HttpConnectTimeoutException} is thrown
+ * from {@link HttpClient#send(java.net.http.HttpRequest,
+ * java.net.http.HttpResponse.BodyHandler) HttpClient::send}, or
+ * {@link HttpClient#sendAsync(java.net.http.HttpRequest,
+ * java.net.http.HttpResponse.BodyHandler) HttpClient::sendAsync}
+ * completes exceptionally with an {@code
HttpConnectTimeoutException}.

Do you think this can be reworded a bit? Although I understand what's
being said here, the wording doesn't seem right. Maybe something like:

 *  In the case where a new connection needs to be
established, if
 * the connection cannot be established within the given {@code
 * duration}, then {@link HttpClient#send(java.net.http.HttpRequest,
 * java.net.http.HttpResponse.BodyHandler) HttpClient::send}
throws a
 * {@link HttpConnectTimeoutException}, or
 * {@link HttpClient#sendAsync(java.net.http.HttpRequest,
 * java.net.http.HttpResponse.BodyHandler) HttpClient::sendAsync}
 * completes exceptionally with an {@code
HttpConnectTimeoutException}.



src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestImpl.java

-    this.timeout = null;
+    this.timeout = Duration.ofSeconds(30);

Is this an intentional change of default value for the timeout? Is that
something that needs to be documented?

One other thing - maybe not directly related to this single patch, but
as you are aware, recently as part of [1], a new system property (and a
security property) was introduced to optionally include host info in the
exception messages thrown for socket exceptions. Does the HttpClient
honour that system property in the exceptions it throws? I don't see it
being used in this patch for the timeout exceptions. I haven't checked
the code, outside of this patch, to see if it's dealt with in other
parts of this API.

[1] https://bugs.openjdk.java.net/browse/JDK-8204233

-Jaikiran


On 04/08/18 5:15 PM, Chris Hegarty wrote:
> Hi, This is a code review to add connection specific timeout support
> to the new HTTP Client, as discussed recently over on another thread
> [1]. The connection timeout duration is proposed to be added at the
> level of the client, and if specified applies to all requests sent by
> that client. The connect timeout mechanism is orthogonal to the
> HttpRequest `timeout`, but can be used to complement it. For example,
> HttpClient client = HttpClient.newBuilder()
> .connectTimeout(Duration.ofSeconds(20)) .build(); HttpRequest request
> = HttpRequest.newBuilder() .uri(URI.create("https://foo.com/";))
> .timeout(Duration.ofMinutes(2)) .header("Content-Type",
> "application/json")
> .POST(BodyPublishers.ofFile(Paths.get("file.json"))) .build();
> client.sendAsync(request, BodyHandlers.ofString()) sends the request
> with a 20 second connect timeout and a response timeout of 2 minutes.
> The connect timeout, as implemented in the webrev below, covers the
> DNS lookup, the TCP SYN handshake, and the TLS handshake. The proposed
> spec wording deliberately avoids adding such low-level details.
> Outline of the specification changes: To HttpClient.Builder add: /** *
> Sets the connect timeout duration for this client. * *  In the case
> where a new connection needs to be established, if * the connection
> cannot be established within the given {@code * duration}, then an
> {@link HttpConnectTimeoutException} is thrown * from {@link
> HttpClient#send(java.net.http.HttpRequest, *
> java.net.http.HttpResponse.BodyHandler) HttpClient::send}, or * {@link
> HttpClient#sendAsync(java.net.http.HttpRequest, *
> java.net.http.HttpResponse.BodyHandler) HttpClient::sendAsync} *
> completes exceptionally with an {@code HttpConnectTimeoutException}. *
> If a new connection does not need to be established, for example * if
> a connection can be reused from a previous request, then this *
> timeout duration has no effect. * * @param duration the duration to
> allow the underlying connection to be * established * @return this
> builder * @throws IllegalArgumentException if the duration is
> non-positive */ public Builder connectTimeout(Duration duration); To
> HttpClient add an accessor: /** * Returns an {@code Optional}
> containing the connect timeout duration * for this client. If
> the {@linkplain Builder#connectTimeout(Duration) * connect timeout
> duration} was not set in the client's builder, then the * {@code
> Optional} is empty. * * @return an {@code Optional} containing this
> client's connect timeout * duration */ public abstract
> Optional connectTimeout(); Add new Exception type: /** *
> Thrown when a connection, over which an {@code HttpRequest} is
> intended to be * sent, is no

RFR [11] 8208391: Need to differentiate response and connect timeouts in HTTP Client API

2018-08-04 Thread Chris Hegarty
Hi,

This is a code review to add connection specific timeout support to the
new HTTP Client, as discussed recently over on another thread [1].

The connection timeout duration is proposed to be added at the level of
the client, and if specified applies to all requests sent by that
client. The connect timeout mechanism is orthogonal to the HttpRequest
`timeout`, but can be used to complement it. For example,

HttpClient client = HttpClient.newBuilder()
 .connectTimeout(Duration.ofSeconds(20))
 .build();
HttpRequest request = HttpRequest.newBuilder()
 .uri(URI.create("https://foo.com/";))
 .timeout(Duration.ofMinutes(2))
 .header("Content-Type", "application/json")
 .POST(BodyPublishers.ofFile(Paths.get("file.json")))
 .build();
client.sendAsync(request, BodyHandlers.ofString())

  sends the request with a 20 second connect timeout and a response
timeout of 2 minutes.

The connect timeout, as implemented in the webrev below, covers the
DNS lookup, the TCP SYN handshake, and the TLS handshake. The proposed
spec wording deliberately avoids adding such low-level details.

Outline of the specification changes:

To HttpClient.Builder add:

  /**
   * Sets the connect timeout duration for this client.
   *
   *  In the case where a new connection needs to be established, if
   * the connection cannot be established within the given {@code
   * duration}, then an {@link HttpConnectTimeoutException} is thrown
   * from {@link HttpClient#send(java.net.http.HttpRequest,
   * java.net.http.HttpResponse.BodyHandler) HttpClient::send}, or
   * {@link HttpClient#sendAsync(java.net.http.HttpRequest,
   * java.net.http.HttpResponse.BodyHandler) HttpClient::sendAsync}
   * completes exceptionally with an {@code HttpConnectTimeoutException}.
   * If a new connection does not need to be established, for example
   * if a connection can be reused from a previous request, then this
   * timeout duration has no effect.
   *
   * @param duration the duration to allow the underlying connection to be
   * established
   * @return this builder
   * @throws IllegalArgumentException if the duration is non-positive
   */
  public Builder connectTimeout(Duration duration);

To HttpClient add an accessor:

  /**
   * Returns an {@code Optional} containing the connect timeout duration
   * for this client. If the {@linkplain Builder#connectTimeout(Duration)
   * connect timeout duration} was not set in the client's builder, then the
   * {@code Optional} is empty.
   *
   * @return an {@code Optional} containing this client's connect timeout
   * duration
   */
   public abstract Optional connectTimeout();

Add new Exception type:

  /**
   * Thrown when a connection, over which an {@code HttpRequest} is intended to 
be
   * sent, is not successfully established within a specified time period.
   */
   public class HttpConnectTimeoutException extends HttpTimeoutException {
 ...
   }

Webrev:
  http://cr.openjdk.java.net/~chegar/8208391/webrev.00/

-Chris

[1] http://mail.openjdk.java.net/pipermail/net-dev/2018-August/011683.html


JEP 110 HTTP client API

2016-12-05 Thread Michael McMahon

Hi,

I just want to confirm the plans for the HTTP client API in JDK 9 that 
were hinted at previously. There is a new feature called "incubator 
modules" which has just been proposed for JDK 9. The idea is to 
accommodate new APIs in JDK which are not quite ready to be standardized 
as part of Java SE. There is more information at the link below [1]. 
But, the main implications are:


- the API package namespace must be under jdk.incubator
- the API is not a standard part of the platform and is subject to 
change before being standardized in a future release of JDK.

- compile time and run time warnings are issued when using the feature.
- the feature is not visible by default but must be opted into with a 
command line option.


The plan is for the new httpclient API to be included with JDK 9 using 
this feature. Therefore, the package java.net.http will be renamed as 
jdk.incubator.http in JDK 9, and the implementation will reside in its 
own module also under the jdk.incubator namespace.


The aim is to gain experience with the API, and standardize it for JDK 
10 by moving it back from jdk.incubator.http to java.net.http together 
with any other API changes as a result of that experience.


Bug id 8169768 [2] has been filed to handle the change and the plan is 
to integrate the work that has been done in the sandbox forest using the 
JEP 11 guidelines, into jdk9 dev, by the end of this week. We will 
publish a webrev of the changes in the next day or two. I will update 
the JEP 110 description [3] to reflect these changes in the next day or two.


Thanks,

Michael.


[1] (Incubator modules JEP 11) 
https://bugs.openjdk.java.net/browse/JDK-8169768


[2] (bug to update JEP 110) https://bugs.openjdk.java.net/browse/JDK-8169768

[3] (JEP 110) https://bugs.openjdk.java.net/browse/JDK-8042950


RE: HTTP client API

2016-11-29 Thread 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 
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

2016-11-29 Thread 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 
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 mentioned in JEP 110 
<http://openjdk.java.net/jeps/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 
<http://code-o-matic.blogspot.co.uk/2011/01/note-on-api-design-call-stack-as-source.html>], 
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

Re: HTTP client API

2016-11-21 Thread Chris Hegarty

Tobias,

If you look at the code in the sandbox [*], the notion of a default
client has been removed. Having global static default instances is
problematic. Http Clients are lightweight, easy to configure and
pass around, if that is what you want.

-Chris.

[*] hg clone http://hg.openjdk.java.net/jdk9/sandbox; cd sandbox
bash common/bin/hgforest.sh update http-client-branch


On 21/11/16 11:28, Tobias Thierer wrote:

Replying to my own first email in this thread to add another question /
concern about flexibility of the HTTP Client API:

Have you considered offering applications a way to globally replace the
HTTP Client implementation with their own (e.g. via a method
HttpClient.setDefault() to go with the existing method
HttpClient.getDefault())?
This feature seems to currently be missing from the API while even the
old HttpURLConnection API supported this (via
URL.setURLStreamHandlerFactory()
<https://docs.oracle.com/javase/8/docs/api/java/net/URL.html#setURLStreamHandlerFactory-java.net.URLStreamHandlerFactory->).

I'm aware that such global (process-wide) configuration options have
disadvantages as well, but it would be consistent with other swappable
Java APIs/SPIs such as SSLSocketFactory, XML parsers, CookieHandler,
etc.  The advantage would be that applications / application frameworks
could swap out the HttpClient implementation used by lower level
libraries / applications that they're bundling in binary form - e.g. to
globally provide a fake implementation to use during testing, instrument
HTTP Client usage (e.g. log all URLs accessed or count the number of
bytes transferred), to adhere to requirements in some corporate
networks, or to otherwise decouple the version/implementation of the
HTTP Client from the platform / OpenJDK version. What's your view?

Tobias


On Mon, Oct 24, 2016 at 8:33 PM, Tobias Thierer mailto:tobi...@google.com>> wrote:

Hi Michael and others -


Thanks for publishing the latest HTTP client API docs
<http://cr.openjdk.java.net/~michaelm/httpclient/api/>(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

<http://cr.openjdk.java.net/~michaelm/httpclient/api/java/net/http/HttpRequest.BodyProcessor.html>and
HttpResponse.BodyProcessor

<http://cr.openjdk.java.net/~michaelm/httpclient/api/java/net/http/HttpResponse.BodyProcessor.html>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

<http://cr.openjdk.java.net/~michaelm/httpclient/api/java/net/http/HttpResponse.BodyProcessor.html#discard-U->(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.

  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 chall

Re: HTTP client API

2016-11-21 Thread 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 the subscriber. But, th

Re: HTTP client API

2016-11-01 Thread 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

2016-10-31 Thread 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
<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().

At the same time, the first and third goal mentioned in JEP 110
<http://openjdk.java.net/jeps/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
<http://code-o-matic.blogspot.co.uk/2011/01/note-on-api-design-call-stack-as-source.html>],
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)
<http://cr.openjdk.java.net/~michaelm/httpclient/api.1/java/util/concurrent/Flow.Subscription.html#request-long->
specifies
that *n* specifically *denotes the number of future calls* to onNext()
<http://cr.openjdk.java.net/~michaelm/httpclient/api.1/java/util/concurrent/Flow.Subscriber.html#onNext-T->.
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?

Re: HTTP client API

2016-10-28 Thread 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 
<http://cr.openjdk.java.net/%7Emichaelm/httpclient/api/>(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 
<http://cr.openjdk.java.net/%7Emichaelm/httpclient/api/java/net/http/HttpRequest.BodyProcessor.html>and 
HttpResponse.BodyProcessor 
<http://cr.openjdk.java.net/%7Emichaelm/httpclient/api/java/net/http/HttpResponse.BodyProcessor.html>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

<http://cr.openjdk.java.net/%7Emichaelm/httpclient/api/java/net/http/HttpResponse.BodyProcessor.html#discard-U->(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

Re: HTTP client API

2016-10-27 Thread 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 
<http://cr.openjdk.java.net/%7Emichaelm/httpclient/api/>(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 
<http://cr.openjdk.java.net/%7Emichaelm/httpclient/api/java/net/http/HttpRequest.BodyProcessor.html>and 
HttpResponse.BodyProcessor 
<http://cr.openjdk.java.net/%7Emichaelm/httpclient/api/java/net/http/HttpResponse.BodyProcessor.html>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

2016-09-23 Thread 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

2016-09-22 Thread 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

2016-09-21 Thread 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 
mailto: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

2016-09-21 Thread 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

2016-09-12 Thread 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 
mailto: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.

Thanks,

   

Re: HTTP client API

2016-09-08 Thread 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. Optionalextends T>>


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 
mailto: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.

Thanks,

Michael

[1] http://cr.openjdk.java.net/~michaelm/httpclient/api/
<http://cr.openjdk.java.net/%7Emichaelm/httpclient/api/>

[2]

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

<http://cr.openjdk.java.net/%7Emichaelm/httpclient/specdiffout/package-summary.html>






Re: HTTP client API

2016-09-07 Thread 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 
mailto: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.

Thanks,

Michael

[1] http://cr.openjdk.java.net/~michaelm/httpclient/api/
<http://cr.openjdk.java.net/%7Emichaelm/httpclient/api/>

[2]

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

<http://cr.openjdk.java.net/%7Emichaelm/httpclient/specdiffout/package-summary.html>




Re: HTTP client API

2016-09-03 Thread 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

2016-08-26 Thread 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

2016-08-24 Thread Chris Hegarty

On 23/08/16 22:19, Pavel Rappo wrote:

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


And yes, 'http-client-branch' is the correct branch.

-Chris.




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

2016-08-23 Thread 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

2016-08-23 Thread 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

2016-08-23 Thread 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
>
>


HTTP client API

2016-08-22 Thread Michael McMahon
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




Http client API review (final)

2012-11-30 Thread Michael McMahon

Hi,

I have just posted another draft of the http client API for review.
This will be the final round of review before we submit it to the CCC.

It can be seen at:

http://cr.openjdk.java.net/~michaelm/httpclient/CCCv1.1/javadoc/

Probably the biggest change is that we removed the multiple overloaded
set/getBody() methods on HttpRequestBuilder and HttpResponse and replaced
then with a hierarchy of types defined in a new package 
java.net.httpclient.content.


But, there are many other smaller changes too. If you have been 
following the work

at the java.net project then you will have seen all of this before.
We would like to finalise the CCC request by Wednesday Dec 5.
So, comments would be most welcome as soon as possible.

Thanks,
Michael


Re: Review of new Http client API

2012-11-02 Thread Michael McMahon

Richard,

It is not part of JDK 8 yet, but you can download and build the source
against JDK7 in Netbeans. We have been running it as a java.net project
at http://java.net/projects/http-client

If you'd prefer me to send you a pre-built jar file I could do that.
However, the API has moved on a bit since the javadoc that you probably 
have.

I could probably send you the javadoc as well though.

- Michael

On 02/11/2012 10:52, Richard Bair wrote:

Hi Michael,

Is there a build of JDK that already has this code in it? The best API review I 
think will come from actually trying to build something with it, which I'd be 
happy to give a try at.

Thanks
Richard

On Aug 14, 2012, at 5:01 AM, Michael McMahon  
wrote:


Hi,

(apologies for sending this again)
We have just published a draft of a proposed new Http client API [1] for JDK 8.

This message has been cc'd to jdk8-dev so that as many people as possible
know about it, but the discussion will be on the net-dev list 
(net-dev@openjdk.java.net).
So, folks will have to join that list [2], in order to take part.

Thanks,
Michael.

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

[2] http://mail.openjdk.java.net/mailman/listinfo/net-dev




revision 0.4 Http client API

2012-09-02 Thread Michael McMahon

Hello,

I have just posted the javadoc for v 0.4 of the Http client API [1]
This draft takes into account almost all of the feedback from
the last round of reviews. There are a small number of issues
still outstanding however.

For those who are not aware, the development work done so
far has been done as a java.net project. The home page
for the project is at [2]. The source and latest version of the api
is always available there. We will continue to use this repository
and project page up to the point where we integrate the work
with jdk8.

Thanks,
Michael


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

[2] http://java.net/projects/http-client


Re: Review of new Http client API

2012-08-23 Thread Chris Hegarty



On 23/08/2012 16:34, Paul Sandoz wrote:

...

OK, i would be inclined to separate out the instance used for building from the 
instance passed around, so one cannot muck around with the state of the latter.


Agreed, HttpResponse could use a builder pattern.


Then user code may do:

  AsyncHttpRequest request.async()
   .onHeaders(r ->  dumpHeaders(r))
   .onError((r,t) ->  handleError(r,t));
   .onBodyPart((r,bb,c) ->  transformBody(r,bb,t));
  client.sendRequest(request);



If these calls are on* calls are optional and one is not interested in when the 
headers have been received one could omit the onHeaders call, infact all those 
methods could be optional. That certainly simplifies things.


Right.


  

  void dumpHeaders(HttpResponse r) {
  System.out.println(r);
  }
  void handleError(HttpResponse r, t) {
  throw t;
  }
  void transformBody(HttpResponse r, bb, boolean complete) {
  System.out.println("Hello there!");
  }



FWIW you could use method references:

AsyncHttpRequest request.async()
   .onHeaders(whateverthatclassiscalled::dumpHeaders)


Ok, great.

-Chris.


   ...

Paul.



Some experimentation is necessary to find a good balance here.

-Chris.


Re: Review of new Http client API

2012-08-23 Thread Paul Sandoz

On Aug 23, 2012, at 5:16 PM, Michael McMahon  
wrote:

> Paul,
> 
> Thanks for looking at it. Yes, this is an area that needs some more work.
> The current thinking is along the lines that Chris just posted and I hope to
> have another version of the API to look at tomorrow.
> 
> What you suggest seems like an unusual usage of Future<>  as we have tried to 
> provide
> a mode of operation where applications can get a Future
> which would work in the conventional way of returning the result "in the 
> future".

Agreed, it fit well with the underlying asynchronous support in Jersey, which 
was already using the Future, and it bugged me using callback interfaces with 
two methods, where most of the time the error would be swallowed. If there was 
a listener concept for Future in the JDK I would have used that instead.

I think the approach Chris shows easily allows for a default handler when one 
is not supplied.


> But, it raises a question in that while we currently have callback interfaces 
> for both
> headers and data, we only have a Future based interface for headers (but not 
> data).
> 

Indeed!

Paul.


> - Michael
> 
> On 23/08/12 15:20, Paul Sandoz wrote:
>> Hi,
>> 
>> A potential simplification of the HttpResponseHeadersHandler interface is to 
>> turn it into a functional interface:
>> 
>>   HttpResponseHandler onHeaders(Future  dresp) throws 
>> InterruptedException, ExecutionException;
>> 
>> [Chris, i am not sure if an interface with two methods, one default, is 
>> classified as a functional interface.]
>> 
>> - mirrors the pull-based asynchronous approach
>> 
>> - dresp.isDone() always returns true
>> 
>> - the Future encapsulates the underling exception, if any
>> 
>> - harder to swallow errors, since the exception from drep.get() will 
>> propagate if not caught.
>> 
>> - a return of a null HttpResponseHandler means "not interested in the body".
>> 
>> FWIW the use of Future is the approach i chose for the Jersey client.
>> 
>> HttpResponseHandler would also be a functional interface:
>> 
>> void onBodyPart(Future  bb) throws InterruptedException, 
>> ExecutionException
>> 
>> - there is no inheritance relationship between HttpResponseHeadersHandler 
>> and HttpResponseHandler.
>> 
>> - a "bb" with a capacity of 0 indicates the last part.
>> 
>> - the HttpResponse is not required as a parameter because the implementation 
>> can obtain it from the onHeaders method.
>> 
>> If the use of Future is a bit extreme for some :-) then things can still be 
>> simplified by following the above approach with an additional, and optional, 
>> functional interface to handle errors when HttpClient.sendRequest is called.
>> 
>> --
>> 
>> Rather than setting the bytes on the HttpRequest with numerous methods i 
>> wonder if it is better to have a functional interfaces for both OutputStream 
>> and the NIO equivalent:
>> 
>>   interface EntityWriter  { // Oh for disjunct types!
>> /**
>>  * @return true if there is more to write
>>  */
>> boolean write(T t) throws IOException;
>>   }
>> 
>> I believe the above can support all the existing functionality currently 
>> expressed as methods, including the Iterable/Iterator. There can be 
>> instances of EntityWriter for common functionality:
>> 
>>   EntityWriters.fromBytes(byte[] b, ...);
>> 
>> The same might be applicable to HttpResponse with an EntityReader:
>> 
>>   interface EntityReader  {
>> U read(T t) throws IOException;
>>   }
>> 
>> Of course i might be missing something obvious here in terms of optimisation 
>> currently performed by the implementation!
>> 
>> --
>> 
>> It somewhat bugs me that blocking and asynchronous pull/push functionality 
>> is all defined using the same artifacts. But, my imagination is currently is 
>> failing me on how to improve on such matters. Perhaps something better may 
>> come out of fluent-based API?
>> 
>> Paul.
>> 
>> On Aug 14, 2012, at 2:01 PM, Michael McMahon  
>> wrote:
>> 
>>> Hi,
>>> 
>>> (apologies for sending this again)
>>> We have just published a draft of a proposed new Http client API [1] for 
>>> JDK 8.
>>> 
>>> This message has been cc'd to jdk8-dev so that as many people as possible
>>> know about it, but the discussion will be on the net-dev list 
>>> (net-dev@openjdk.java.net).
>>> So, folks will have to join that list [2], in order to take part.
>>> 
>>> Thanks,
>>> Michael.
>>> 
>>> [1] http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/
>>> 
>>> [2] http://mail.openjdk.java.net/mailman/listinfo/net-dev
> 



Re: Review of new Http client API

2012-08-23 Thread Paul Sandoz

On Aug 23, 2012, at 5:05 PM, Chris Hegarty  wrote:

> Paul,
> 
> All good feedback, and I will leave it to Michael to reply to the specifics. 
> On thought I had on separation of modes is something list this:
> 
>  interface HeaderHandler {
>  onHeaders(HttpResponse);
>  }
>  interface ErrorHandler {
>  onError(HttpResponse, Throwable);
>  }
>  interface BodyHandler {
>  onBodyPart(HttpResponse, ByteBuffer, boolean);
>  }
> 
>  class HttpRequest {
>  
>  AsyncHttpRequest async(HttpRequest);
>  
>  }
> 
>  class AsyncHttpRequest extends HttpRequest {
>  // no public constructors
>  
>  AsyncHttpRequest onHeaders(HeaderHandler);
>  AsyncHttpRequest onError(ErrorHandler);
>  AsyncHttpRequest onBodyPart(BodyHandler);
>  }
> 
>  class HttpClient {
>  
>  OutputStream sendHeaders(HttpRequest request, long contentLength)
>  Future sendRequest(HttpRequest req)
>  void sendRequest(AsyncHttpRequest req)
>  }
> 

OK, i would be inclined to separate out the instance used for building from the 
instance passed around, so one cannot muck around with the state of the latter.


> Then user code may do:
> 
>  AsyncHttpRequest request.async()
>   .onHeaders(r -> dumpHeaders(r))
>   .onError((r,t) -> handleError(r,t));
>   .onBodyPart((r,bb,c) -> transformBody(r,bb,t));
>  client.sendRequest(request);
> 

If these calls are on* calls are optional and one is not interested in when the 
headers have been received one could omit the onHeaders call, infact all those 
methods could be optional. That certainly simplifies things.


>  
> 
>  void dumpHeaders(HttpResponse r) {
>  System.out.println(r);
>  }
>  void handleError(HttpResponse r, t) {
>  throw t;
>  }
>  void transformBody(HttpResponse r, bb, boolean complete) {
>  System.out.println("Hello there!");
>  }
> 

FWIW you could use method references:

   AsyncHttpRequest request.async()
  .onHeaders(whateverthatclassiscalled::dumpHeaders)
  ...

Paul.


> Some experimentation is necessary to find a good balance here.
> 
> -Chris.


Re: Review of new Http client API

2012-08-23 Thread Michael McMahon

Paul,

Thanks for looking at it. Yes, this is an area that needs some more work.
The current thinking is along the lines that Chris just posted and I hope to
have another version of the API to look at tomorrow.

What you suggest seems like an unusual usage of Future<>  as we have 
tried to provide

a mode of operation where applications can get a Future
which would work in the conventional way of returning the result "in the 
future".
But, it raises a question in that while we currently have callback 
interfaces for both
headers and data, we only have a Future based interface for headers (but 
not data).


- Michael

On 23/08/12 15:20, Paul Sandoz wrote:

Hi,

A potential simplification of the HttpResponseHeadersHandler interface is to 
turn it into a functional interface:

   HttpResponseHandler onHeaders(Future  dresp) throws 
InterruptedException, ExecutionException;

[Chris, i am not sure if an interface with two methods, one default, is 
classified as a functional interface.]

- mirrors the pull-based asynchronous approach

- dresp.isDone() always returns true

- the Future encapsulates the underling exception, if any

- harder to swallow errors, since the exception from drep.get() will propagate 
if not caught.

- a return of a null HttpResponseHandler means "not interested in the body".

FWIW the use of Future is the approach i chose for the Jersey client.

HttpResponseHandler would also be a functional interface:

 void onBodyPart(Future  bb) throws InterruptedException, 
ExecutionException

- there is no inheritance relationship between HttpResponseHeadersHandler and 
HttpResponseHandler.

- a "bb" with a capacity of 0 indicates the last part.

- the HttpResponse is not required as a parameter because the implementation 
can obtain it from the onHeaders method.

If the use of Future is a bit extreme for some :-) then things can still be 
simplified by following the above approach with an additional, and optional, 
functional interface to handle errors when HttpClient.sendRequest is called.

--

Rather than setting the bytes on the HttpRequest with numerous methods i wonder 
if it is better to have a functional interfaces for both OutputStream and the 
NIO equivalent:

   interface EntityWriter  { // Oh for disjunct types!
 /**
  * @return true if there is more to write
  */
 boolean write(T t) throws IOException;
   }

I believe the above can support all the existing functionality currently 
expressed as methods, including the Iterable/Iterator. There can be instances 
of EntityWriter for common functionality:

   EntityWriters.fromBytes(byte[] b, ...);

The same might be applicable to HttpResponse with an EntityReader:

   interface EntityReader  {
 U read(T t) throws IOException;
   }

Of course i might be missing something obvious here in terms of optimisation 
currently performed by the implementation!

--

It somewhat bugs me that blocking and asynchronous pull/push functionality is 
all defined using the same artifacts. But, my imagination is currently is 
failing me on how to improve on such matters. Perhaps something better may come 
out of fluent-based API?

Paul.

On Aug 14, 2012, at 2:01 PM, Michael McMahon  
wrote:


Hi,

(apologies for sending this again)
We have just published a draft of a proposed new Http client API [1] for JDK 8.

This message has been cc'd to jdk8-dev so that as many people as possible
know about it, but the discussion will be on the net-dev list 
(net-dev@openjdk.java.net).
So, folks will have to join that list [2], in order to take part.

Thanks,
Michael.

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

[2] http://mail.openjdk.java.net/mailman/listinfo/net-dev




Re: Review of new Http client API

2012-08-23 Thread Chris Hegarty

Paul,

All good feedback, and I will leave it to Michael to reply to the 
specifics. On thought I had on separation of modes is something list this:


  interface HeaderHandler {
  onHeaders(HttpResponse);
  }
  interface ErrorHandler {
  onError(HttpResponse, Throwable);
  }
  interface BodyHandler {
  onBodyPart(HttpResponse, ByteBuffer, boolean);
  }

  class HttpRequest {
  
  AsyncHttpRequest async(HttpRequest);
  
  }

  class AsyncHttpRequest extends HttpRequest {
  // no public constructors
  
  AsyncHttpRequest onHeaders(HeaderHandler);
  AsyncHttpRequest onError(ErrorHandler);
  AsyncHttpRequest onBodyPart(BodyHandler);
  }

  class HttpClient {
  
  OutputStream sendHeaders(HttpRequest request, long contentLength)
  Future sendRequest(HttpRequest req)
  void sendRequest(AsyncHttpRequest req)
  }

Then user code may do:

  AsyncHttpRequest request.async()
   .onHeaders(r -> dumpHeaders(r))
   .onError((r,t) -> handleError(r,t));
   .onBodyPart((r,bb,c) -> transformBody(r,bb,t));
  client.sendRequest(request);

  

  void dumpHeaders(HttpResponse r) {
  System.out.println(r);
  }
  void handleError(HttpResponse r, t) {
  throw t;
  }
  void transformBody(HttpResponse r, bb, boolean complete) {
  System.out.println("Hello there!");
  }

Some experimentation is necessary to find a good balance here.

-Chris.

On 23/08/2012 15:20, Paul Sandoz wrote:

Hi,

A potential simplification of the HttpResponseHeadersHandler interface is to 
turn it into a functional interface:

   HttpResponseHandler onHeaders(Future  dresp) throws 
InterruptedException, ExecutionException;

[Chris, i am not sure if an interface with two methods, one default, is 
classified as a functional interface.]

- mirrors the pull-based asynchronous approach

- dresp.isDone() always returns true

- the Future encapsulates the underling exception, if any

- harder to swallow errors, since the exception from drep.get() will propagate 
if not caught.

- a return of a null HttpResponseHandler means "not interested in the body".

FWIW the use of Future is the approach i chose for the Jersey client.

HttpResponseHandler would also be a functional interface:

 void onBodyPart(Future  bb) throws InterruptedException, 
ExecutionException

- there is no inheritance relationship between HttpResponseHeadersHandler and 
HttpResponseHandler.

- a "bb" with a capacity of 0 indicates the last part.

- the HttpResponse is not required as a parameter because the implementation 
can obtain it from the onHeaders method.

If the use of Future is a bit extreme for some :-) then things can still be 
simplified by following the above approach with an additional, and optional, 
functional interface to handle errors when HttpClient.sendRequest is called.

--

Rather than setting the bytes on the HttpRequest with numerous methods i wonder 
if it is better to have a functional interfaces for both OutputStream and the 
NIO equivalent:

   interface EntityWriter  { // Oh for disjunct types!
 /**
  * @return true if there is more to write
  */
 boolean write(T t) throws IOException;
   }

I believe the above can support all the existing functionality currently 
expressed as methods, including the Iterable/Iterator. There can be instances 
of EntityWriter for common functionality:

   EntityWriters.fromBytes(byte[] b, ...);

The same might be applicable to HttpResponse with an EntityReader:

   interface EntityReader  {
 U read(T t) throws IOException;
   }

Of course i might be missing something obvious here in terms of optimisation 
currently performed by the implementation!

--

It somewhat bugs me that blocking and asynchronous pull/push functionality is 
all defined using the same artifacts. But, my imagination is currently is 
failing me on how to improve on such matters. Perhaps something better may come 
out of fluent-based API?

Paul.

On Aug 14, 2012, at 2:01 PM, Michael McMahon  
wrote:


Hi,

(apologies for sending this again)
We have just published a draft of a proposed new Http client API [1] for JDK 8.

This message has been cc'd to jdk8-dev so that as many people as possible
know about it, but the discussion will be on the net-dev list 
(net-dev@openjdk.java.net).
So, folks will have to join that list [2], in order to take part.

Thanks,
Michael.

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

[2] http://mail.openjdk.java.net/mailman/listinfo/net-dev




Re: Review of new Http client API

2012-08-23 Thread Paul Sandoz
Hi,

A potential simplification of the HttpResponseHeadersHandler interface is to 
turn it into a functional interface:

  HttpResponseHandler onHeaders(Future dresp) throws 
InterruptedException, ExecutionException;

[Chris, i am not sure if an interface with two methods, one default, is 
classified as a functional interface.]

- mirrors the pull-based asynchronous approach

- dresp.isDone() always returns true

- the Future encapsulates the underling exception, if any

- harder to swallow errors, since the exception from drep.get() will propagate 
if not caught.

- a return of a null HttpResponseHandler means "not interested in the body".

FWIW the use of Future is the approach i chose for the Jersey client.

HttpResponseHandler would also be a functional interface:

void onBodyPart(Future bb) throws InterruptedException, 
ExecutionException

- there is no inheritance relationship between HttpResponseHeadersHandler and 
HttpResponseHandler.

- a "bb" with a capacity of 0 indicates the last part.

- the HttpResponse is not required as a parameter because the implementation 
can obtain it from the onHeaders method.

If the use of Future is a bit extreme for some :-) then things can still be 
simplified by following the above approach with an additional, and optional, 
functional interface to handle errors when HttpClient.sendRequest is called.

--

Rather than setting the bytes on the HttpRequest with numerous methods i wonder 
if it is better to have a functional interfaces for both OutputStream and the 
NIO equivalent:

  interface EntityWriter { // Oh for disjunct types!
/**
 * @return true if there is more to write
 */
boolean write(T t) throws IOException;
  }

I believe the above can support all the existing functionality currently 
expressed as methods, including the Iterable/Iterator. There can be instances 
of EntityWriter for common functionality:

  EntityWriters.fromBytes(byte[] b, ...);

The same might be applicable to HttpResponse with an EntityReader:

  interface EntityReader {
U read(T t) throws IOException;
  }

Of course i might be missing something obvious here in terms of optimisation 
currently performed by the implementation!

--

It somewhat bugs me that blocking and asynchronous pull/push functionality is 
all defined using the same artifacts. But, my imagination is currently is 
failing me on how to improve on such matters. Perhaps something better may come 
out of fluent-based API?

Paul.

On Aug 14, 2012, at 2:01 PM, Michael McMahon  
wrote:

> Hi,
> 
> (apologies for sending this again)
> We have just published a draft of a proposed new Http client API [1] for JDK 
> 8.
> 
> This message has been cc'd to jdk8-dev so that as many people as possible
> know about it, but the discussion will be on the net-dev list 
> (net-dev@openjdk.java.net).
> So, folks will have to join that list [2], in order to take part.
> 
> Thanks,
> Michael.
> 
> [1] http://cr.openjdk.java.net/~michaelm/httpclient/v0.3/
> 
> [2] http://mail.openjdk.java.net/mailman/listinfo/net-dev



Re: Review of new Http client API

2012-08-23 Thread Michael McMahon
t) ->  {
// handle data in buf
}
);

It seems fairly readable still, I think.

Another thing that this usage points to, is the usefulness of being able
to hang some user context
off of the HttpResponse or HttpRequest objects. That would be the only
way to share some user state
between the two handlers above, in this Lambda style.

https://github.com/spullara/java-future-jdk8

Another consideration might be to make sure that it is compatible with
an implementation that is using SPDY under the covers for connectivity
as I suspect that HTTP as a wire protocol has peaked though the HTTP
semantics will survive.

Right. This is important. One area where there will be changes is with
pipe-lining.
We need to ensure that our pipe-lining API is not restricted to only
Http 1.1 pipe-lining
Are you aware of other areas that could have an impact on the API?

Thanks
Michael.


Sam

On Tue, Aug 14, 2012 at 5:01 AM, Michael McMahon
  wrote:

Hi,

(apologies for sending this again)
We have just published a draft of a proposed new Http client API [1]
for JDK
8.

This message has been cc'd to jdk8-dev so that as many people as
possible
know about it, but the discussion will be on the net-dev list
(net-dev@openjdk.java.net).
So, folks will have to join that list [2], in order to take part.

Thanks,
Michael.

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

[2]http://mail.openjdk.java.net/mailman/listinfo/net-dev






Re: Review of new Http client API

2012-08-22 Thread Chris Hegarty
ea where there will be changes is with
pipe-lining.
We need to ensure that our pipe-lining API is not restricted to only
Http 1.1 pipe-lining
Are you aware of other areas that could have an impact on the API?

Thanks
Michael.


Sam

On Tue, Aug 14, 2012 at 5:01 AM, Michael McMahon
 wrote:

Hi,

(apologies for sending this again)
We have just published a draft of a proposed new Http client API [1]
for JDK
8.

This message has been cc'd to jdk8-dev so that as many people as
possible
know about it, but the discussion will be on the net-dev list
(net-dev@openjdk.java.net).
So, folks will have to join that list [2], in order to take part.

Thanks,
Michael.

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

[2]http://mail.openjdk.java.net/mailman/listinfo/net-dev







Re: Review of new Http client API

2012-08-22 Thread Michael McMahon

On 22/08/12 15:29, Chris Hegarty wrote:

Michael what you have looks good.

But, I think what Sam is suggesting ( or maybe not, but I like it ;-) 
), is something like this. (I'd need to think more about what effect 
this has on the different modes, async/blocking )


 class HttpResponse {
   HttpResponse onHeaders(Block);
   HttpResponse onError(BiBlock);
   HttpResponse onBodyPart(BiBlock);
 }


Right. I see what you mean in terms of making the setting of the callbacks
fluent. I assume that Block and BiBlock are types associated with Lambda 
somehow,

but otherwise this is unfamiliar territory to me 

 response.onHeaders(r -> headers(r))
 .onError((r,t) -> error(t))
 .onBodyPart((r,bb) -> body(r, bb));


So,  headers() and body() would be methods on HttpResponse ... Right ?
What is error()?

- Michael.
Alternatively, I believe something like this would also be compatible 
with lambda (since there is a default implementation for on Error):


  interface HTTPResponseHandler {
public void onHeaders(HttpResponse resp);

public void onError(HttpRequest request, Throwable exception)
default { throw exception; }
 }

-Chris.


On 21/08/2012 14:57, Michael McMahon wrote:

Sam,

Thanks for the comments. Some discussion below.


On 17/08/12 00:13, Sam Pullara wrote:

I suggest that you make it a more fluent API rather than having
multiple callback methods in your callback interface. As it stands it
isn't compatible with lambdas. You might take some inspiration for the
asynchronous callbacks from my work porting Twitter's Future/Promise
to JDK8:


I agree with the above. In a previous version of the API the main
callback was lambda compatible.
Originally we used HttpResponse to encapsulate everything related to a
response
including errors. But, some preferred to keep HttpResponse aligned to an
actual response
from a server in all cases. There might be other ways to get around that
by combining
HttpResponseHeadersHandler.{onError(), onHeaders()} back into a single
method.
Maybe, drop the onError() method and add the exception/throwable as a
parameter to onHeaders()

But, we also wanted to provide notification of body data (through the
sub-interface HttpResponseHandler).
Keeping the two interfaces distinct meant that applications could get
asynchronous notification of
the response headers, but then possibly read the response body in a
blocking manner.
Or alternatively, applications can use the handler to be notified of
both headers and body.

So, if we revert HttpResponseHeadersHandler back to having a single
method, the sub-interface
now would have two methods (instead of three).

One way around that could be to have two unrelated interfaces:

interface HttpResponseHeadersHandler {
public void onHeaders(HttpResponse response, Exception e);
}

interface HttpResponseBodyHandler {
public void onBodyPart(HttpResponse resp, ByteBuffer buffer, boolean 
last);

}

// Then a HttpResponseBodyHandler would be added to
HttpClient.sendRequest() as below:

public void sendRequest(HttpRequest, HttpResponseHeadersHandler,
HttpResponseBodyHandler);


Both of the interfaces would be lambda compatible (again) though at the
cost
of having to specify two separate handlers. So, the following might 
be how

it could be used (and using a builder for HttpClient)

HttpClient client = HttpClient.createBuilder()
.setAsynchronousChannelGroup (..)
.setCookieManager(..)
.setDefaultTimeout(..)
.setProxy(...)
.addFilter(...)
.buildClient();

HttpRequest request = client.createRequest(new 
URI("http://www.foo.com/";))

.setBody("Hello world".getBytes())
.setMethod(HttpMethod.POST);

client.sendRequest (
request,

// handle headers
(HttpResponse response, Exception e) -> {
if (response.getResponseCode() != 200) {
// handle error response
}
// handle normal case
},

// handle body
(HttpResponse response, ByteBuffer buf, boolean last) -> {
// handle data in buf
}
);

It seems fairly readable still, I think.

Another thing that this usage points to, is the usefulness of being able
to hang some user context
off of the HttpResponse or HttpRequest objects. That would be the only
way to share some user state
between the two handlers above, in this Lambda style.

https://github.com/spullara/java-future-jdk8

Another consideration might be to make sure that it is compatible with
an implementation that is using SPDY under the covers for connectivity
as I suspect that HTTP as a wire protocol has peaked though the HTTP
semantics will survive.

Right. This is important. One area where there will be changes is with
pipe-lining.
We need to ensure that our pipe-lining API is not restricted to only
Http 1.1 pipe-lining
Are you aware of other areas that could have an impact on the API?

Thanks
Michael.


Sam

On Tue, Aug 14, 2012 at 5:01 AM, Michael McMahon
 wrote:

Hi,

(apologies for sending this again)
We have just published a draft of a proposed new Http client API [1]
for JDK
8.

Re: Review of new Http client API

2012-08-22 Thread Chris Hegarty

Michael what you have looks good.

But, I think what Sam is suggesting ( or maybe not, but I like it ;-) ), 
is something like this. (I'd need to think more about what effect this 
has on the different modes, async/blocking )


 class HttpResponse {
   HttpResponse onHeaders(Block);
   HttpResponse onError(BiBlock);
   HttpResponse onBodyPart(BiBlock);
 }

 response.onHeaders(r -> headers(r))
 .onError((r,t) -> error(t))
 .onBodyPart((r,bb) -> body(r, bb));

Alternatively, I believe something like this would also be compatible 
with lambda (since there is a default implementation for on Error):


  interface HTTPResponseHandler {
public void onHeaders(HttpResponse resp);

public void onError(HttpRequest request, Throwable exception)
default { throw exception; }
 }

-Chris.


On 21/08/2012 14:57, Michael McMahon wrote:

Sam,

Thanks for the comments. Some discussion below.


On 17/08/12 00:13, Sam Pullara wrote:

I suggest that you make it a more fluent API rather than having
multiple callback methods in your callback interface. As it stands it
isn't compatible with lambdas. You might take some inspiration for the
asynchronous callbacks from my work porting Twitter's Future/Promise
to JDK8:


I agree with the above. In a previous version of the API the main
callback was lambda compatible.
Originally we used HttpResponse to encapsulate everything related to a
response
including errors. But, some preferred to keep HttpResponse aligned to an
actual response
from a server in all cases. There might be other ways to get around that
by combining
HttpResponseHeadersHandler.{onError(), onHeaders()} back into a single
method.
Maybe, drop the onError() method and add the exception/throwable as a
parameter to onHeaders()

But, we also wanted to provide notification of body data (through the
sub-interface HttpResponseHandler).
Keeping the two interfaces distinct meant that applications could get
asynchronous notification of
the response headers, but then possibly read the response body in a
blocking manner.
Or alternatively, applications can use the handler to be notified of
both headers and body.

So, if we revert HttpResponseHeadersHandler back to having a single
method, the sub-interface
now would have two methods (instead of three).

One way around that could be to have two unrelated interfaces:

interface HttpResponseHeadersHandler {
public void onHeaders(HttpResponse response, Exception e);
}

interface HttpResponseBodyHandler {
public void onBodyPart(HttpResponse resp, ByteBuffer buffer, boolean last);
}

// Then a HttpResponseBodyHandler would be added to
HttpClient.sendRequest() as below:

public void sendRequest(HttpRequest, HttpResponseHeadersHandler,
HttpResponseBodyHandler);


Both of the interfaces would be lambda compatible (again) though at the
cost
of having to specify two separate handlers. So, the following might be how
it could be used (and using a builder for HttpClient)

HttpClient client = HttpClient.createBuilder()
.setAsynchronousChannelGroup (..)
.setCookieManager(..)
.setDefaultTimeout(..)
.setProxy(...)
.addFilter(...)
.buildClient();

HttpRequest request = client.createRequest(new URI("http://www.foo.com/";))
.setBody("Hello world".getBytes())
.setMethod(HttpMethod.POST);

client.sendRequest (
request,

// handle headers
(HttpResponse response, Exception e) -> {
if (response.getResponseCode() != 200) {
// handle error response
}
// handle normal case
},

// handle body
(HttpResponse response, ByteBuffer buf, boolean last) -> {
// handle data in buf
}
);

It seems fairly readable still, I think.

Another thing that this usage points to, is the usefulness of being able
to hang some user context
off of the HttpResponse or HttpRequest objects. That would be the only
way to share some user state
between the two handlers above, in this Lambda style.

https://github.com/spullara/java-future-jdk8

Another consideration might be to make sure that it is compatible with
an implementation that is using SPDY under the covers for connectivity
as I suspect that HTTP as a wire protocol has peaked though the HTTP
semantics will survive.

Right. This is important. One area where there will be changes is with
pipe-lining.
We need to ensure that our pipe-lining API is not restricted to only
Http 1.1 pipe-lining
Are you aware of other areas that could have an impact on the API?

Thanks
Michael.


Sam

On Tue, Aug 14, 2012 at 5:01 AM, Michael McMahon
 wrote:

Hi,

(apologies for sending this again)
We have just published a draft of a proposed new Http client API [1]
for JDK
8.

This message has been cc'd to jdk8-dev so that as many people as
possible
know about it, but the discussion will be on the net-dev list
(net-dev@openjdk.java.net).
So, folks will have to join that list [2], in order to take part.

Thanks,
Michael.

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

[2]http://mail.openjdk.java.net/mailman/listinfo/net-dev





Re: Review of new Http client API

2012-08-21 Thread Michael McMahon

Sam,

Thanks for the comments. Some discussion below.


On 17/08/12 00:13, Sam Pullara wrote:

I suggest that you make it a more fluent API rather than having
multiple callback methods in your callback interface. As it stands it
isn't compatible with lambdas. You might take some inspiration for the
asynchronous callbacks from my work porting Twitter's Future/Promise
to JDK8:


I agree with the above. In a previous version of the API the main 
callback was lambda compatible.
Originally we used HttpResponse to encapsulate everything related to a 
response
including errors. But, some preferred to keep HttpResponse aligned to an 
actual response
from a server in all cases. There might be other ways to get around that 
by combining
HttpResponseHeadersHandler.{onError(), onHeaders()} back into a single 
method.
Maybe, drop the onError() method and add the exception/throwable as a 
parameter to onHeaders()


But, we also wanted to provide notification of body data (through the 
sub-interface HttpResponseHandler).
Keeping the two interfaces distinct meant that applications could get 
asynchronous notification of
the response headers, but then possibly read the response body in a 
blocking manner.
Or alternatively, applications can use the handler to be notified of 
both headers and body.


So, if we revert HttpResponseHeadersHandler back to having a single 
method, the sub-interface

now would have two methods (instead of three).

One way around that could be to have two unrelated interfaces:

interface HttpResponseHeadersHandler  {
public void onHeaders(HttpResponse response, Exception e);
}

interface HttpResponseBodyHandler {
public void onBodyPart(HttpResponse resp, ByteBuffer buffer, boolean last);
}

// Then a HttpResponseBodyHandler would be added to HttpClient.sendRequest() as 
below:

public void sendRequest(HttpRequest, HttpResponseHeadersHandler, 
HttpResponseBodyHandler);


Both of the interfaces would be lambda compatible (again) though at the cost
of having to specify two separate handlers. So, the following might be how
it could be used (and using a builder for HttpClient)

HttpClient client = HttpClient.createBuilder()
.setAsynchronousChannelGroup (..)
.setCookieManager(..)
.setDefaultTimeout(..)
.setProxy(...)
.addFilter(...)
.buildClient();

HttpRequest request = client.createRequest(new URI("http://www.foo.com/";))
.setBody("Hello world".getBytes())
.setMethod(HttpMethod.POST);

client.sendRequest (
request,

// handle headers
(HttpResponse response, Exception e) ->  {
if (response.getResponseCode() != 200) {
// handle error response
}
// handle normal case
},

// handle body
(HttpResponse response, ByteBuffer buf, boolean last) ->  {
// handle data in buf
}
);

It seems fairly readable still, I think.

Another thing that this usage points to, is the usefulness of being able 
to hang some user context
off of the HttpResponse or HttpRequest objects. That would be the only 
way to share some user state

between the two handlers above, in this Lambda style.

https://github.com/spullara/java-future-jdk8

Another consideration might be to make sure that it is compatible with
an implementation that is using SPDY under the covers for connectivity
as I suspect that HTTP as a wire protocol has peaked though the HTTP
semantics will survive.
Right. This is important. One area where there will be changes is with 
pipe-lining.
We need to ensure that our pipe-lining API is not restricted to only 
Http 1.1 pipe-lining

Are you aware of other areas that could have an impact on the API?

Thanks
Michael.


Sam

On Tue, Aug 14, 2012 at 5:01 AM, Michael McMahon
  wrote:

Hi,

(apologies for sending this again)
We have just published a draft of a proposed new Http client API [1] for JDK
8.

This message has been cc'd to jdk8-dev so that as many people as possible
know about it, but the discussion will be on the net-dev list
(net-dev@openjdk.java.net).
So, folks will have to join that list [2], in order to take part.

Thanks,
Michael.

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

[2]http://mail.openjdk.java.net/mailman/listinfo/net-dev





Re: Http client API

2012-08-20 Thread 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

2012-08-17 Thread Michael McMahon

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

On 17 August 2012 11:30, Michael McMahon  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

2012-08-17 Thread Frank Carver
On 17 August 2012 11:30, Michael McMahon  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

2012-08-17 Thread 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

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

2012-08-16 Thread Michael McMahon

On 08/08/12 20:09, Ian Robertston wrote:

Instead of HttpRequest having

   void setBody(Iterable  buffers, boolean isRestartable)

what about having two methods:

   void setBody(Iterable  buffers) // presumed restartable
   void setBody(Iterator  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


Ian,

Thanks for the comment. I agree this is probably the way to do it.
Having two separate methods gives scope to explain the difference
clearly in the javadoc.

- Michael.


Re: Http client API

2012-08-15 Thread Michael McMahon

On 15/08/12 16:59, Mike Duigou wrote:

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.


JCE is slightly different in that different providers potentially 
implement different algorithms. What I was thinking is that there would
be a default provider, and if someone wants to use a different 
implementation then they just need some
way to specify an alternate to the default. I think it is unlikely that 
there would be more than one alternate implementation.


On the Iterable vs Iterator question, I take your point.

Thanks,
Michael



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(Iterable, boolean isRestartable) -- This seems heinous. 
Non-restartable Iterables should be avoided. I like the suggestion of additional 
setBody(Iterable) method instead.

Ok. So, a setBody(Iterator)  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

Iterable   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

2012-08-15 Thread Alan Bateman

On 15/08/2012 16:59, Mike Duigou wrote:

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


:

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.

I haven't studied this API in much detail yet but I assume that 
HttpClientProvider is just a factory for HttpClients and when you invoke 
HttpClient.createClient it just delegates to the system-wise provider to 
create the HttpClient. Once the javadoc is fleshed out more then I 
assume that it will document how the system-wise default provider is 
chosen. I assume it just uses ServiceLoader with a system property or 
some means to override. This approach would be consistent with how 
several other provider interfaces work in the JDK. The approach also 
doesn't preclude using multiple providers at the same time although if 
you want a specific HttpClient then I assume it means getting a 
reference to the provider and invoking its factory method directly. It's 
the same thing with the NIO provider interfaces although the number of 
provider implementations there will be a less than what I expect here.


-Alan.


Re: Http client API

2012-08-15 Thread 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(Iterable, boolean isRestartable) -- This seems 
>> heinous. Non-restartable Iterables should be avoided. I like the suggestion 
>> of additional setBody(Iterable) method instead.
> Ok. So, a setBody(Iterator)  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
> 
> Iterable  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

2012-08-15 Thread 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

2012-08-15 Thread 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

Iterable  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

2012-08-15 Thread 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: Use Builder pattern ( was: Re: Http client API)

2012-08-15 Thread Michael McMahon

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

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

Michael 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

2012-08-15 Thread Michael McMahon
ion 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

2012-08-14 Thread Xuelei Fan
On Aug 14, 2012, at 8:49 PM, Michael McMahon  
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: cross protocol redirects ( was:Re: Http client API )

2012-08-14 Thread Michael McMahon

On 08/08/12 21:35, Chris Hegarty wrote:

Great suggestion Anthony,

This is something that comes up from time to time. With the clear 
distinction between java.net.HttpURLConnection and 
javax.net.ssl.HttpsURLConnection API's then it was a little difficult 
to do in the existing API, but there is a clear opportunity with the 
new API to avoid this issue in the future.


Kurchi just informed me (off-list) that the current prototype 
implementation in the java.net project [1], supports cross protocol 
redirects. Though, this may be by accident! We need to do some further 
investigating to determine if the security concerns related to 4620571 
are still valid. If so, and we cannot continue with automatic cross 
protocol redirects, then an explicit API ( like you suggested ) should 
be added.



Chris,

That behavior isn't accidental. It's one reason why SSL configuration is 
a "property" of HttpClient rather than

defined in a sub-class like HttpsClient.

I agree the security concern needs to be understood (though I'm not sure 
I see a problem right now).
The exact behavior of these classes isn't fully defined yet, in the 
context of a security manager.


- Michael.



Re: Http client API

2012-08-14 Thread Michael McMahon

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?

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.




Review of new Http client API

2012-08-14 Thread Michael McMahon

Hi,

(apologies for sending this again)
We have just published a draft of a proposed new Http client API [1] for 
JDK 8.


This message has been cc'd to jdk8-dev so that as many people as possible
know about it, but the discussion will be on the net-dev list 
(net-dev@openjdk.java.net).

So, folks will have to join that list [2], in order to take part.

Thanks,
Michael.

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

[2] http://mail.openjdk.java.net/mailman/listinfo/net-dev


Review of new Http client API

2012-08-13 Thread Michael McMahon

Hi,

We have just published a draft of a proposed new Http client API [1] for 
JDK 8.


This message has been bcc'd to jdk8-dev so that as many people as possible
know about it, but the discussion will be on the net-dev list 
(net-dev@openjdk.java.net).

So, folks will have to join that list [2], in order to take part.

Thanks,
Michael.

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

[2] http://mail.openjdk.java.net/mailman/listinfo/net-dev


Re: Http client API

2012-08-13 Thread 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

2012-08-09 Thread 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

2012-08-09 Thread Chris Hegarty
() -- 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?

- getBodyAsBytes -- Document handling for content-length > Integer.MAX_VALUE

- getBodyAsByteBuffers -- handling for not enough space in the array needs to 
be documented. Unused entries nulled out?

- Perhaps maxlength -> maxBytes to be more clear?

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



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

2012-08-09 Thread 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 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
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



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

2012-08-09 Thread Chris Hegarty

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

Michael 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

2012-08-09 Thread 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.



Re: Http client API

2012-08-08 Thread 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 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
>> > <mailto:michael.x.mcmahon@**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

2012-08-08 Thread David M. Lloyd

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.


--
- DML




Re: Http client API

2012-08-08 Thread Jed Wesley-Smith
Michael 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

2012-08-08 Thread Mike Duigou
(byte[], int) - Why would I ever want a max size less than the 
size of my byte buffer?

- getBodyAsBytes -- Document handling for content-length > Integer.MAX_VALUE

- getBodyAsByteBuffers -- handling for not enough space in the array needs to 
be documented. Unused entries nulled out?

- Perhaps maxlength -> maxBytes to be more clear?

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



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

2012-08-08 Thread Chris Hegarty


Oh, my head hurts! ;-) There are already three setBody methods.

I agree, a boolean like this can be confusing. A 
setBodyRestartable(Itr), and setBodyNonRestarable(Itr) may be a possible 
solution here. But ( what I think you are suggesting too ), Iterable 
implementations should do the right thing, they should always be 
"restartable".


Maybe/Simply remove the restartable boolean param and put a note in the 
spec that Iterable implementations that for whatever reason cannot be 
restarted should perform some kind of caching. So we only support 
"honest" Iterables.


Unless I'm missing some valid use-case that demands this non-restartable 
API.


-Chris.



On 08/08/12 20:09, Ian Robertston wrote:

Instead of HttpRequest having

   void setBody(Iterable buffers, boolean isRestartable)

what about having two methods:

   void setBody(Iterable buffers) // presumed restartable
   void setBody(Iterator 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



cross protocol redirects ( was:Re: Http client API )

2012-08-08 Thread Chris Hegarty

Great suggestion Anthony,

This is something that comes up from time to time. With the clear 
distinction between java.net.HttpURLConnection and 
javax.net.ssl.HttpsURLConnection API's then it was a little difficult to 
do in the existing API, but there is a clear opportunity with the new 
API to avoid this issue in the future.


Kurchi just informed me (off-list) that the current prototype 
implementation in the java.net project [1], supports cross protocol 
redirects. Though, this may be by accident! We need to do some further 
investigating to determine if the security concerns related to 4620571 
are still valid. If so, and we cannot continue with automatic cross 
protocol redirects, then an explicit API ( like you suggested ) should 
be added.


Thanks,
-Chris.

[1] http://java.net/projects/http-client/

On 08/08/12 19:23, Anthony Vanelverdinghe wrote:

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

2012-08-08 Thread Ian Robertston
Instead of HttpRequest having 

  void setBody(Iterable buffers, boolean isRestartable)

what about having two methods:

  void setBody(Iterable buffers) // presumed restartable
  void setBody(Iterator 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

2012-08-08 Thread 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

2012-08-08 Thread Xuelei Fan
>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

2012-08-08 Thread 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

2012-08-08 Thread 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
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

2012-08-07 Thread 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


Http client API

2012-08-07 Thread Michael McMahon

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.