Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[Viktor Klang]

On Tue, Apr 17, 2018 at 6:28 PM, Chris Hegarty 
wrote:

> On 17 Apr 2018, at 17:52, Viktor Klang  wrote:
> > ...
> > There is technical and non-technical effort required. It is non-trivial.
> > That said, we’re making every effort possible to move this forward.
> >
> > Hi Chris,
> >
> > how can we help?
>
> Thank you for asking, but I think we have it under control. I hope
> to have news on this in the next couple of weeks.
>

Ok!


>
> -Chris.




-- 
Cheers,
√

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[Chris Hegarty]

On 17 Apr 2018, at 17:52, Viktor Klang  wrote:
> ...
> There is technical and non-technical effort required. It is non-trivial.
> That said, we’re making every effort possible to move this forward.
> 
> Hi Chris,
> 
> how can we help?

Thank you for asking, but I think we have it under control. I hope
to have news on this in the next couple of weeks.

-Chris.

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[Viktor Klang]

On Tue, Apr 17, 2018 at 9:55 AM, Chris Hegarty 
wrote:

> Simone,
>
> > On 16 Apr 2018, at 18:47, Simone Bordet  wrote:
> >
> >> ...
> >
> > Out of curiosity, is this code implementing the ReactiveStreams TCK
> > (in its Flow declination) ?
>
> The code should be compliant with the RS TCK.
>
> > I ask because I have recently implemented it for Jetty's Reactive
> > HttpClient (https://github.com/jetty-project/jetty-reactive-httpclient)
> > and found a few surprising failures.
>
> If you encounter failures can you please report those to us, since
> we are not aware of such.
>
> > It will be great if this can be done because all ReactiveStreams
> > implementations implement the ReactiveStreams TCK, and so there is an
> > assumed baseline of how these libraries should work that is beneficial
> > for interoperability.
>
> Of course, interoperability is of utmost importance.
>
> > The effort is not much: each Publisher/Processor/Subscriber
> > implementation should just extend a ReactiveStream TCK class
> > overriding a few methods, for example:
> > https://github.com/jetty-project/jetty-reactive-
> httpclient/blob/master/src/test/java/org/eclipse/jetty/
> reactive/client/internal/QueuedSinglePublisherTCKTest.java
>
> There is technical and non-technical effort required. It is non-trivial.
> That said, we’re making every effort possible to move this forward.
>

Hi Chris,

how can we help?


>
> -Chris.
>
>


-- 
Cheers,
√

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[Chris Hegarty]

Simone,

> On 16 Apr 2018, at 18:47, Simone Bordet  wrote:
> 
>> ...
> 
> Out of curiosity, is this code implementing the ReactiveStreams TCK
> (in its Flow declination) ?

The code should be compliant with the RS TCK.

> I ask because I have recently implemented it for Jetty's Reactive
> HttpClient (https://github.com/jetty-project/jetty-reactive-httpclient)
> and found a few surprising failures.

If you encounter failures can you please report those to us, since
we are not aware of such.

> It will be great if this can be done because all ReactiveStreams
> implementations implement the ReactiveStreams TCK, and so there is an
> assumed baseline of how these libraries should work that is beneficial
> for interoperability.

Of course, interoperability is of utmost importance.

> The effort is not much: each Publisher/Processor/Subscriber
> implementation should just extend a ReactiveStream TCK class
> overriding a few methods, for example:
> https://github.com/jetty-project/jetty-reactive-httpclient/blob/master/src/test/java/org/eclipse/jetty/reactive/client/internal/QueuedSinglePublisherTCKTest.java

There is technical and non-technical effort required. It is non-trivial.
That said, we’re making every effort possible to move this forward.

-Chris. 

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[Simone Bordet]

Chris,

On Mon, Apr 16, 2018 at 7:09 PM, Chris Hegarty  wrote:
> Hi,
>
> Here are refreshed links to the latest code in the sandbox:
>
> http://cr.openjdk.java.net/~chegar/httpclient/03/javadoc/api/java.net.http/module-summary.html
> http://cr.openjdk.java.net/~chegar/httpclient/03/webrev/
>
> Includes:
>
> a) Changes to address the two issues raised by Simone during
>the JEP Propose To Target [1] [2].
>
> b) Changes to address review comments from dfuchs, prappo,
>michaelm, and chegar [3].
>
> c) Several bug fixes, additional test coverage, and stability
>improvements.

Out of curiosity, is this code implementing the ReactiveStreams TCK
(in its Flow declination) ?

I ask because I have recently implemented it for Jetty's Reactive
HttpClient (https://github.com/jetty-project/jetty-reactive-httpclient)
and found a few surprising failures.
It will be great if this can be done because all ReactiveStreams
implementations implement the ReactiveStreams TCK, and so there is an
assumed baseline of how these libraries should work that is beneficial
for interoperability.

The effort is not much: each Publisher/Processor/Subscriber
implementation should just extend a ReactiveStream TCK class
overriding a few methods, for example:
https://github.com/jetty-project/jetty-reactive-httpclient/blob/master/src/test/java/org/eclipse/jetty/reactive/client/internal/QueuedSinglePublisherTCKTest.java

The ReactiveStreams TCK does the rest.

Thanks !

-- 
Simone Bordet
---
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless.   Victoria Livschitz

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[Chris Hegarty]

Hi,

Here are refreshed links to the latest code in the sandbox:

http://cr.openjdk.java.net/~chegar/httpclient/03/javadoc/api/java.net.http/module-summary.html
http://cr.openjdk.java.net/~chegar/httpclient/03/webrev/

Includes:

a) Changes to address the two issues raised by Simone during
   the JEP Propose To Target [1] [2].

b) Changes to address review comments from dfuchs, prappo,
   michaelm, and chegar [3].

c) Several bug fixes, additional test coverage, and stability
   improvements.

-Chris.

[1] https://bugs.openjdk.java.net/browse/JDK-8201325
[2] https://bugs.openjdk.java.net/browse/JDK-8201361
[3] 
http://hg.openjdk.java.net/jdk/sandbox/log?revcount=80=desc%28%22review+comment%22%29+and+branch%28%22http-client-branch%22%29+and+date%28%22%3E2018-02-01%22%29


On 21/03/18 20:33, Chris Hegarty wrote:

Hi,

This is a request for review for the HTTP Client implementation - JEP 321.

The javadoc link has been refreshed, based on feedback received from
the CSR [1] ( and a follow on supplementary CSR [2] ):
http://cr.openjdk.java.net/~chegar/httpclient/02/javadoc/api/java.net.http/java/net/http/package-summary.html

The webrev link has been refreshed to include implementation changes
to support the latest API, additional test code coverage and scenarios:
http://cr.openjdk.java.net/~chegar/httpclient/02/webrev/


On 9 Feb 2018, at 14:33, Chris Hegarty  wrote:
Development of the JDK HTTP Client, incubated in JDK 9 and re-incubated
in JDK 10, has continued. It will soon be in a position to be proposed
for inclusion in Java 11, through JEP 321. This will proceed, as usual,
through the JEP 2.0 process.

JEP 321 has recently been updated with the proposed package and module
name, `java.net.http`. The code in the sandbox has been updated to
reflect this.

Javadoc based on the latest code in the sandbox:
http://cr.openjdk.java.net/~chegar/httpclient/00/javadoc/api/java.net.http-summary.html

Webrev based on the latest code in the sandbox:
  http://cr.openjdk.java.net/~chegar/httpclient/00/webrev/

( much of the changes in the webrev are mechanical due to renaming, but
  there are stability fixes and the API documentation has been
  reorganized to improve readability )

More information about the JDK HTTP Client can be found on the
networking groups page.
  http://openjdk.java.net/groups/net/httpclient/



-Chris.

[1] https://bugs.openjdk.java.net/browse/JDK-8197565
[2] https://bugs.openjdk.java.net/browse/JDK-8199938

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[Sebastien Deleuze]

Michael, Chris,

Thanks for your feedback, indeed with PublishingBodySubscriber, it works
also for streaming use cases and it provides the behavior I was trying to
achieve.
So +1 for inclusion of JDK-8201186 in JDK 11.

On Thu, Apr 5, 2018 at 6:12 PM, Chris Hegarty 
wrote:

> Hi Sebastien,
>
> > On 5 Apr 2018, at 11:07, Sebastien Deleuze  wrote:
> >
> > Hi,
> >
> > I am currently implementing a draft support for JDK new HTTP client in
> Spring Framework 5 [1] (using JDK 10 for now) in order to be able to
> leverage it as a WebClient [2] engine.
>
> Great that you are trying this out.
>
> > Our integration tests all pass with regular HTTP/1.1 web services
>
> Good to hear this.
>
> > (we have not tested the HTTP/2 support yet), but not with streaming APIs.
>
> Reactive streams APIs consist of both Publishers and Subscribers, but
> in your specific case you appear to most interested in Publishers. I'll
> assume "streaming APIs” here means "RS Publishers", since the HTTP
> Client quite obvious interoperates with Subscribers.
>
> > Based on my current understanding, it seems the 
> > CompletableFuture
> returned by HttpClient#sendAsync completes only when all the data of the
> response body is available, not asap the status code + headers are
> available like with Jetty or Reactor Netty clients (which prevents to
> consume infinite streams via Reactive Streams API).
>
> As Michael has already said, it can operate either way depending on the
> actual higher-level Java type that the body is being converted to. In
> fact, you can even write your own BodyHandler that returns a Subscriber
> with an already complete body CF. So there is no issue working with
> infinite streams via RS, if that is what you want.
>
> I believe what you are actually seeking is a
> `BodyHandler>`, that makes its HttpResponse
> available once the headers have been received. This has come up just
> recently, in this email thread [1]. A JIRA issue has been filed to track
> the addition of such a BodyHandler [2].
>
> In the interim, until the new handler can been added, it should be
> possible to either write your own ( Daniel has similar,
> `static class PublishingBodySubscriber implements
> BodySubscriber>`, in a test [3] ). Or
> if your library, or reactor, provides functionality similar to
> `io.reactivex.processors.PublishProcessor`, then you can use it as the
> BodyHandler type and write a delegating BodySubscriber that
> completes its body CF immediately with an instance of that processor.
>
> > Is consuming streaming HTTP/1.1 endpoints (like JSON streaming or SSE)
> supported by current implementation?
>
>
> Yes, but of course not out of the box, you need to hook up to a JSON
> Subscriber, etc.
>
> -Chris.
>
> [1] http://mail.openjdk.java.net/pipermail/net-dev/2018-April/011314.html
> [2] https://bugs.openjdk.java.net/browse/JDK-8201186
> [3] http://hg.openjdk.java.net/jdk/sandbox/file/c59f684f1eda/
> test/jdk/java/net/httpclient/ResponsePublisher.java
>

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[Chris Hegarty]

Hi Sebastien,

> On 5 Apr 2018, at 11:07, Sebastien Deleuze  wrote:
> 
> Hi,
> 
> I am currently implementing a draft support for JDK new HTTP client in Spring 
> Framework 5 [1] (using JDK 10 for now) in order to be able to leverage it as 
> a WebClient [2] engine.

Great that you are trying this out.

> Our integration tests all pass with regular HTTP/1.1 web services

Good to hear this.

> (we have not tested the HTTP/2 support yet), but not with streaming APIs.

Reactive streams APIs consist of both Publishers and Subscribers, but
in your specific case you appear to most interested in Publishers. I'll
assume "streaming APIs” here means "RS Publishers", since the HTTP
Client quite obvious interoperates with Subscribers.

> Based on my current understanding, it seems the 
> CompletableFuture returned by HttpClient#sendAsync completes 
> only when all the data of the response body is available, not asap the status 
> code + headers are available like with Jetty or Reactor Netty clients (which 
> prevents to consume infinite streams via Reactive Streams API).

As Michael has already said, it can operate either way depending on the
actual higher-level Java type that the body is being converted to. In
fact, you can even write your own BodyHandler that returns a Subscriber
with an already complete body CF. So there is no issue working with
infinite streams via RS, if that is what you want.

I believe what you are actually seeking is a
`BodyHandler>`, that makes its HttpResponse
available once the headers have been received. This has come up just
recently, in this email thread [1]. A JIRA issue has been filed to track
the addition of such a BodyHandler [2].

In the interim, until the new handler can been added, it should be
possible to either write your own ( Daniel has similar,
`static class PublishingBodySubscriber implements
BodySubscriber>`, in a test [3] ). Or
if your library, or reactor, provides functionality similar to
`io.reactivex.processors.PublishProcessor`, then you can use it as the
BodyHandler type and write a delegating BodySubscriber that
completes its body CF immediately with an instance of that processor.

> Is consuming streaming HTTP/1.1 endpoints (like JSON streaming or SSE) 
> supported by current implementation?


Yes, but of course not out of the box, you need to hook up to a JSON
Subscriber, etc.

-Chris.

[1] http://mail.openjdk.java.net/pipermail/net-dev/2018-April/011314.html
[2] https://bugs.openjdk.java.net/browse/JDK-8201186
[3] 
http://hg.openjdk.java.net/jdk/sandbox/file/c59f684f1eda/test/jdk/java/net/httpclient/ResponsePublisher.java

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[Michael McMahon]

Sebastien,

The answer depends on the particular HttpResponse.BodySubscriber/Handler 
implementation
supplied to the sendAsync() call. Some of the simpler ones which return 
aggregate/completed objects
like String, or byte[] do not appear to stream and the response body is 
not returned until after all data has been read.


But, BodySubscribers and handlers can be designed to return the body 
reading object any time after the response
headers have been received. One example is the 
BodySubscriber which is another standard implementation
we provide. Obviously, in that case, the InputStream does not represent 
the completely read body, but is just

an object used to read the body.

Note, the only difference between those two types of body subscriber is 
when the object returned from
HttpResponse.body() becomes visible. In both cases, the data is streamed 
to the subscriber object as it is
received and according to the normal flow control rules of the 
j.u.concurrent.Flow API.


Hope this helps,

Michael.


On 05/04/2018, 11:07, Sebastien Deleuze wrote:

Hi,

I am currently implementing a draft support for JDK new HTTP client in 
Spring Framework 5 [1] (using JDK 10 for now) in order to be able to 
leverage it as a WebClient [2] engine.


Our integration tests all pass with regular HTTP/1.1 webservices (we 
have not tested the HTTP/2 support yet), but not with streaming APIs. 
Based on my current understanding, it seems 
the CompletableFuture returned by HttpClient#sendAsync 
completes only when all the data of the response body is available, 
not asap the status code + headers are available like with Jetty or 
Reactor Netty clients (which prevents to consume infinite streams via 
Reactive Streams API).


Is consuming streaming HTTP/1.1 endpoints (like JSON streaming or SSE) 
supported by current implementation?


Regards,
Sébastien Deleuze

[1] https://github.com/sdeleuze/jdk-http-webclient/
[2] 
https://docs.spring.io/spring/docs/current/spring-framework-reference/web-reactive.html#webflux-client

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[Viktor Klang]

Hi Simone!

:)

I think James conveyed basically everything I had in mind to respond, but I
felt compelled to address the following:

>Forcing users to implement Subscribers or Processors (because that's
what the API requires, despite having a few utilities that cover the
common cases) is way more difficult for users than just passing around
a Publisher.

Simply because an API has an interface type as an parameter or a return
value does not (in my mind) in any way imply that end-users (think:
application developers) are required, encouraged, forced, or otherwise, to
create their own implementations of said interface types to be able to use
it. What is pretty awesome though is if someone has a need for
functionality not provided elsewhere, is that Reactive Streams has a formal
specification and a—free and readily available—TCK to guide implementors to
implement them correctly.


On Mon, Apr 2, 2018 at 2:09 PM, Simone Bordet 
wrote:

> James,
>
> perhaps I was not clear enough.
>
> The JDK will offer an API based on RS, so that's a user-facing API.
>
> Given that, the question is whether the surface of exposition to users
> when they want to use the JDK HttpClient API should be limited to
> Publishers, or expanded to Publishers, Subscribers and Processors.
> My suggestion was to limit it to Publishers only.
> Forcing users to implement Subscribers or Processors (because that's
> what the API requires, despite having a few utilities that cover the
> common cases) is way more difficult for users than just passing around
> a Publisher.
>
> It is by exposing Publishers only that you get rid of the need for
> users to implement any of the RS interfaces to interoperate with other
> libraries.
>
> I'm not sure I buy your opinion on the RxJava2 and Reactor libraries
> (peace! :)
> Both have plenty APIs to create a Flowable/Flux from Publishers, and
> almost none to handle Subscribers: Publishers are exposed, Subscribers
> hidden.
> I don't see any confusion.
>
> Thanks !
>
> --
> Simone Bordet
> ---
> Finally, no matter how good the architecture and design are,
> to deliver bug-free software with optimal performance and reliability,
> the implementation technique must be flawless.   Victoria Livschitz
>



-- 
Cheers,
√

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[Simone Bordet]

Hi,

On Mon, Apr 2, 2018 at 12:39 PM, Chris Hegarty  wrote:
> I can find no material supporting this assertion on reactive-streams.org,
> or anywhere else for that matter. This is not something that I have, or
> any member of the HTTP Client team has, ever come across. We have had
> review comments and feedback, on this list, from folk in the Reactive
> Streams community ( that greatly improved interoperability ), and this
> has never come up. Without supporting material, I cannot accept this
> assertion.

Fair enough.
I had my impression by using/designing APIs around RS, but I guess
it's a cat that can be skinned in various ways.

> The HTTP Client deliberately does not expose its Publisher of response
> body data. It instead subscribes the given Subscriber, on behalf of the
> invoking code. There can, and must, be a single Subscriber.  This is a
> simple and safe model, expected to be more readily understood by an
> average Java developer ( than exposing a Publisher ). If Publishers are
> better for interoperability with some RS implementations, then this
> could be addressed by a small addition to the API, as proposed later in
> this mail (see below).

I guess this is the guts of what I wanted to discuss.
I would have preferred to see the Publisher exposed, rather than
having user code to pass a Subscriber to the implementation.
Again, I guess it's matter of personal experience, and again the
current way works, so I can't bring strong arguments against it.

> For interoperability
> with other RS implementations, where the Flow adaptation is being done
> through Publishers, I would expect folk to use something like RxJava2's
> `io.reactivex.processors.PublishProcessor`.  In fact, Daniel just wrote
> something similar in a recent test [1], a handler that makes the
> response body available through a Publisher,
> `BodyHandler>`. It is a not a general
> purpose Processor, but a Subscriber that makes its data available
> through a Publisher.

Sure.
Again, my point was more along the lines that providing a Publisher
was more immediately familiar to users of other libraries.
I guess all those libraries have a way to convert a Subscriber to a
Publisher, so no big deal - just feels a little more complicated to
me.

> so it would not take much work to provide an API similar to:
>
> BodyHandlers {
> ..
> public static BodyHandler> ofPublisher() {...}
> ..
> }
>
> Does this satisfy your concern?

Yes, a way to convert the Subscriber to a Publisher via a utility
Processor provided by the JDK that users won't need to implement.

> I accept that a carrier would be a more friendly for possible evolution,
> maybe:
>
> BodyHandlerInfo {
>   int statusCode() {... }
>   HttpHeaders responseHeaders { ... }
> }
>
> Does this satisfy your concern?

Yes.
This class could already hold the HTTP version, and possibly in future
other information (e.g. whether the response has a body or not).

Thanks !

-- 
Simone Bordet
---
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless.   Victoria Livschitz

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[Simone Bordet]

James,

perhaps I was not clear enough.

The JDK will offer an API based on RS, so that's a user-facing API.

Given that, the question is whether the surface of exposition to users
when they want to use the JDK HttpClient API should be limited to
Publishers, or expanded to Publishers, Subscribers and Processors.
My suggestion was to limit it to Publishers only.
Forcing users to implement Subscribers or Processors (because that's
what the API requires, despite having a few utilities that cover the
common cases) is way more difficult for users than just passing around
a Publisher.

It is by exposing Publishers only that you get rid of the need for
users to implement any of the RS interfaces to interoperate with other
libraries.

I'm not sure I buy your opinion on the RxJava2 and Reactor libraries (peace! :)
Both have plenty APIs to create a Flowable/Flux from Publishers, and
almost none to handle Subscribers: Publishers are exposed, Subscribers
hidden.
I don't see any confusion.

Thanks !

-- 
Simone Bordet
---
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless.   Victoria Livschitz

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[Chris Hegarty]

Hi Simone,

Thank you for your reply.

I have inserted inline my replies to specific assertions and comments,
but please continue to read on past these as there two concrete
proposals further down that address your concerns, as I understand
them.


> On 30 Mar 2018, at 14:34, Simone Bordet  > wrote:
> 
> ...
> It is my understanding, and that of the ReactiveStreams (RS) people I
> spoke with, that both the RS API and the higher level API based on RS
> (such as those offered by Reactor and RxJava2) are supposed to expose
> to users only Publishers.

I can find no material supporting this assertion on reactive-streams.org 
,
or anywhere else for that matter. This is not something that I have, or
any member of the HTTP Client team has, ever come across. We have had
review comments and feedback, on this list, from folk in the Reactive
Streams community ( that greatly improved interoperability ), and this
has never come up. Without supporting material, I cannot accept this
assertion.

Publisher and Subscriber are low level building blocks that can be used
by different Reactive Streams (RS) implementations. They were added to
Java SE 9 to provide interoperability between various RS
implementations. The HTTP Client API, being proposed for inclusion in
Java SE 11, can only directly reference other Java SE types, so uses
Publisher and Subscriber directly.

The HTTP Client API provides out-of-the-box Publisher and Subscriber
implementations for various common operations. Beyond that, custom
implementations can be used, or more likely Publishers and Subscribers
from RS implementations. Interoperability with these RS implementations
is of utmost importance.

At the end of a Flow, there must always be a Subscriber. Without it
there is no demand. Therefore, all RS APIs must provide Subscribers of
some sort or other.

> This may be counterintuitive at beginning: the typical case of
> InputStream to read content and OutputStream to write content are both
> represented with Publishers in the RS experience.
> The reason behind only offering Publishers is that Subscribers are
> more difficult to implement by regular developers;

I am surprised by this assertion. Our experience writing Publishers and
Subscribers has shown that the most difficult piece of code to write,
in a scalable thread-safe manner, is the Subscription::request.
Subscriptions are created and returned by a Publisher and usually have
an intimate relationship with their Publisher. I consider them part of
the Publishing-side. If a Publisher is not creating its own Subscription,
but instead returning a Subscription from an upstream Publisher, in a
chain, then some work can be avoided, but this is not always the case.
I believe that this could be the scenario that you are referring to, but
it is not always the case.

The HTTP Client deliberately does not expose its Publisher of response
body data. It instead subscribes the given Subscriber, on behalf of the
invoking code. There can, and must, be a single Subscriber.  This is a
simple and safe model, expected to be more readily understood by an
average Java developer ( than exposing a Publisher ). If Publishers are
better for interoperability with some RS implementations, then this
could be addressed by a small addition to the API, as proposed later in
this mail (see below).

> Processors even
> more so, so it is better that this task is left to library
> implementers such as the JDK, Reactor and RxJava2.

Again, similar to Publisher, these are highly context sensitive. For
some common "pass-through" scenarios then they can be relatively
straight forward, but not so when processing is required. I agree, I
think the average Java developer should not be expected to write these,
for common scenarios.

> It is therefore a surprise that the the HTTP client uses instead
> Subscriber for the response content.
> BodyHandler must return a BodySubscriber, which is-a Flow.Subscriber.

The HTTP Client has had this design for a couple of years now, and the
use of Subscriber ( of response body data ) has not surprised anyone,
to my knowledge. Again, folk in the Reactive Streams community are
aware of this API.

> This will force users that need to write non-trivial response content
> handling to implement a Subscriber, or most of the times a Processor
> (to convert the Subscriber back into a Publisher for consumption by
> other more "regular" RS APIs).

In many cases a simple single Subscriber ( sink ) will be a reasonable
choice. The HTTP Client provides a number of combinators and adapters
that demonstrate the composability of Subscribers. For interoperability
with other RS implementations, where the Flow adaptation is being done
through Publishers, I would expect folk to use something like RxJava2's
`io.reactivex.processors.PublishProcessor`.  In fact, Daniel just wrote
something similar in a recent test [1], a handler that makes the

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[James Roper]

Hi Simone,

There are some RS implementations that offer a user facing API that only
offers their equivalent of a Publisher to end users, but that is immaterial
to RS, since RS is not an end user facing API, but an integration API. End
users should not be implementing their own Publisher/Subscriber instances,
so it doesn't matter about how difficult they are or aren't to implement.
RS compliant implementations should have no problems offering either
subscribers or publishers, this is why the RS TCK offers verification for
both.

I have been involved in the implementation of several RS implementations,
including:

* Akka Streams
* netty-reactive-streams:
https://github.com/playframework/netty-reactive-streams/
* A servlet implementation of reactive streams:
https://github.com/jroper/reactive-streams-servlet
* An iteratees based implemnetation of reactive streams:
https://github.com/playframework/play-iteratees

Plus, at Lightbend we're also working on an API that is intended to be a
proposal for the JDK that offers basic Reactive Streams transformation APIs:

https://github.com/lightbend/reactive-streams-utils/

*None* of the above APIs are publisher biased, in fact some of them are
even subscriber biased, and they present no problems when implementing the
RS APIs. Furthermore, the JDK library proposal also has an implementation
using RxJava2, which demonstrates that RxJava, when using Reactive Streams
as an integration API, is perfectly capable of exposing and delivering
Subscribers as per the spec.

I think the problem here is that some libraries have confused the end user
API with the integration API. They are not meant to be the same thing. When
Lightbend approached the Netflix devs working on RxJava to start the
Reactive Streams effort, it was never the intention that we would create a
common end user API, it was the intention that we would create an API that
would allow our respective streaming libraries to integrate with each
other. Reactive Streams is the result. That RxJava has confused these two
things is disappointing, but I think it very odd that we would take the
constraints of the RxJava and Reactor end user APIs, and force
implementations to conform to how they would like all users to use it, even
though most other RS implementations don't do that. That makes no sense for
an integration API.

Regards,

James

On 31 March 2018 at 00:34, Simone Bordet  wrote:

> Hi,
>
> On Wed, Mar 21, 2018 at 9:33 PM, Chris Hegarty 
> wrote:
> > Hi,
> >
> > This is a request for review for the HTTP Client implementation - JEP
> 321.
>
> It is my understanding, and that of the ReactiveStreams (RS) people I
> spoke with, that both the RS API and the higher level API based on RS
> (such as those offered by Reactor and RxJava2) are supposed to expose
> to users only Publishers.
> This may be counterintuitive at beginning: the typical case of
> InputStream to read content and OutputStream to write content are both
> represented with Publishers in the RS experience.
> The reason behind only offering Publishers is that Subscribers are
> more difficult to implement by regular developers; Processors even
> more so, so it is better that this task is left to library
> implementers such as the JDK, Reactor and RxJava2.
>
> It is therefore a surprise that the the HTTP client uses instead
> Subscriber for the response content.
> BodyHandler must return a BodySubscriber, which is-a Flow.Subscriber.
>
> This will force users that need to write non-trivial response content
> handling to implement a Subscriber, or most of the times a Processor
> (to convert the Subscriber back into a Publisher for consumption by
> other more "regular" RS APIs).
> It also creates an asymmetry between read and write side, so that for
> example it is not possible, without writing a processor, to echo a
> response content as the next request content.
> Another difficult example is piping the response content to a
> WebSocket RS API (that would take a Publisher), and so on for
> basically all RS libraries out there.
>
> I would like to suggest to review this: it is definitely possible to
> have BodyHandler refactored in this way:
>
> public Publisher apply(int statusCode, HttpHeaders responseHeaders,
> Publisher responseContent);
>
> Returning a Publisher that performs transformations on the
> responseContent Publisher is the heart of libraries such as Reactor
> and RxJava2, so this will be super simple for users of the API.
> Utility Publishers that currently discard, convert to string, save to
> file, etc. can be equally trivially implemented as they are now in the
> JDK.
>
> With the occasion, I think that the statusCode parameter and the
> responseHeaders parameter can be replaced by a single HttpResponse
> parameter, which would be better for future maintenance in case other
> information needs to be provided.
> The simple case that comes to mind is the HTTP version of the
> response, which 

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[Simone Bordet]

Hi,

On Wed, Mar 21, 2018 at 9:33 PM, Chris Hegarty  wrote:
> Hi,
>
> This is a request for review for the HTTP Client implementation - JEP 321.

It is my understanding, and that of the ReactiveStreams (RS) people I
spoke with, that both the RS API and the higher level API based on RS
(such as those offered by Reactor and RxJava2) are supposed to expose
to users only Publishers.
This may be counterintuitive at beginning: the typical case of
InputStream to read content and OutputStream to write content are both
represented with Publishers in the RS experience.
The reason behind only offering Publishers is that Subscribers are
more difficult to implement by regular developers; Processors even
more so, so it is better that this task is left to library
implementers such as the JDK, Reactor and RxJava2.

It is therefore a surprise that the the HTTP client uses instead
Subscriber for the response content.
BodyHandler must return a BodySubscriber, which is-a Flow.Subscriber.

This will force users that need to write non-trivial response content
handling to implement a Subscriber, or most of the times a Processor
(to convert the Subscriber back into a Publisher for consumption by
other more "regular" RS APIs).
It also creates an asymmetry between read and write side, so that for
example it is not possible, without writing a processor, to echo a
response content as the next request content.
Another difficult example is piping the response content to a
WebSocket RS API (that would take a Publisher), and so on for
basically all RS libraries out there.

I would like to suggest to review this: it is definitely possible to
have BodyHandler refactored in this way:

public Publisher apply(int statusCode, HttpHeaders responseHeaders,
Publisher responseContent);

Returning a Publisher that performs transformations on the
responseContent Publisher is the heart of libraries such as Reactor
and RxJava2, so this will be super simple for users of the API.
Utility Publishers that currently discard, convert to string, save to
file, etc. can be equally trivially implemented as they are now in the
JDK.

With the occasion, I think that the statusCode parameter and the
responseHeaders parameter can be replaced by a single HttpResponse
parameter, which would be better for future maintenance in case other
information needs to be provided.
The simple case that comes to mind is the HTTP version of the
response, which may be different from that of the request and
indicates server capabilities: this is currently not provided in the
BodyHandler.apply() signature.

So perhaps:

public Publisher apply(HttpResponse response, Publisher
responseContent);

This change would also simplify the maintenance since BodySubscriber
will be removed.

My concern is that driving users of the JDK APIs outside of the
familiar RS APIs offered by Reactor and RxJava2 where only Publishers
are relevant, by forcing them to implement Subscribers and Processors
(heck, I even have an IntelliJ warning telling me that I should not do
that!), will cause confusion and be potential source of bugs - we all
know non-blocking/async programming is hard.

Thanks !

-- 
Simone Bordet
---
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless.   Victoria Livschitz

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[James Roper]

On 28 March 2018 at 18:50, Wenbo Zhu  wrote:

>
>>> Any useful RS support will have to involve some application-level
>>> protocols.
>>>
>>
>> I completely disagree with this statement. As someone who has been
>> involved in/led the implementation of multiple WebSocket clients and
>> servers (including Play Framework, Lagom Framework and Akka HTTP), I can
>> tell you from my experience that after the initial WebSocket handshake,
>> WebSockets can be offered to application developers as just streams of
>> messages using Reactive Streams, without any additional WebSocket protocol
>> exposure, and still be useful for 99% of use cases. It certainly does not
>> have to involve exposing the protocol to the application, most applications
>> care nothing about message fragmentation, they care nothing about close
>> codes or reasons beyond whether the connection was terminated with a
>> success or error, and they care nothing about pings/pongs. Everything that
>> they do care about, Reactive Streams gives a way of expressing.
>>
>
> I don't see the role of RS if the WS wire protocol doesn't support any
> signaling for the receiver to notify the sender to "slow down", except to
> respect the basic TCP flow-control.
>
> I am reasonably familiar with the IETF WS spec (
> https://tools.ietf.org/html/rfc6455) ... and it would be useful if you
> could point out how exactly flow-control is supposed to work with RS on top
> of Websockets.
>

I'm not sure how familiar you are with Reactive Streams, so let me first
start by explaining its backpressure mechanism.

RS backpressure works by a Subscriber having a Subscription object.
Subscription provides a method request(n: Long). A Publisher may only
invoke a Subscribers onNext method as many times as the number that has
been supplied to request. So, if request(8) has been invoked, then the
Publisher is allowed to invoke onNext 8 times with 8 elements. The
outstanding demand is cumulative, so if request(8) is invoked, then after
that request(4) has been invoked, then altogether, the publisher is allowed
to invoke onNext 12 times. So backpressure is implemented in RS by virtue
of a Subscriber letting the outstanding demand reach 0, and not invoking
request again until it's ready to receive more elements.

When a Publisher reaches this state, where outstanding demand is zero, it
will stop consuming data from its source of data. In the case of
WebSockets, this will be a TCP socket. Assuming an NIO based
implementation, this will in practice mean that it will cancel the
selection key for that channels read interest. Now at this point it's worth
pointing out, there will be buffers involved along the way, there may be
partially decoded WebSocket frames sitting in a buffer, there may be more
fully decoded WebSocket frames ready to be emitted but can't because demand
is zero. So, there is buffering done here, deregistering interest in
reading doesn't result in immediate backpressure propagation. But
eventually, the OS receive buffer will fill up, and the OS will then send
an ack with a receive window of zero. This will prevent the remote system
from sending any more data.

Now, how that then impacts the WebSocket endpoint on the remote system
depends on that endpoints implementation. If it's using blocking IO, then,
once the remote OS's send buffer is full, any calls to send new messages
will block. Thus, back pressure will have been propagated to the remote
WebSocket endpoint. If it's using non blocking IO with a backpressure
mechanism, for example, if it's using Reactive Streams, then the subscriber
is going to stop requesting demand. Demand will reach zero, causing the
publisher to be unable to send more data. The publisher will then translate
this to whatever backpressure mechanism its source of data represents,
perhaps its receiving messages from a message queue, it's going to stop
consuming those messages. Thus, backpressure will have been propagated.
Another case is that it might be using non blocking IO with no backpressure
mechanism - this is unfortunately the case for many WebSocket
implementations, including web browsers, and the Java EE WebSocket API. In
that case, the WebSocket implementation there has two options, buffer,
drop, or fail, or a combination of two of those. If it buffers, it will
eventually run out of memory. Probably the most common implementation is to
buffer up to a point, and then fail. In such a case - back pressure has
been propagated, though not gracefully, the result is a failure. But the
important thing to realise here is that the side that is receiving messages
has been able to use backpressure to protect itself from running out of
memory - without a backpressure mechanism, there would have been no tcp
backpressure, and it would have had to have accepted data at the rate
produced by the remote end, potentially buffering and running out of
memory, or having to fail.

So, as you can see, WebSockets don't need an explicit 

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[Chris Hegarty]

Simone,

Thank you for you comments.

On 23/03/18 08:28, Simone Bordet wrote:

Hi,

On Thu, Mar 22, 2018 at 1:57 PM, Chris Hegarty  wrote:

Rather, I think that there should be a built in helper that allows Reactive 
Streams to be added on top,


I fully agree that a Reactive Streams WebSocket abstraction should be
built on top. Where I disagree is that it needs to be done in Java SE, let
alone in JEP 321.


Why is that ?
There have been no problems in having Flow-based APIs for the HTTP
client, so can you expand on why it would be different for WebSocket ?
Not that I would like to have Flow-based APIs in WebSocket (it's
indifferent for me, as long as the low-level API is there), I am just
wondering why the difference.


You have been directly involved in the design of the WebSocket API
as it stands today ( not based on reactive streams Flow ). Are you
now suggesting now that Java SE should provide two programming
models for WebSocket? I think one is more than sufficient.


with the understanding that such a helper will limit the capabilities and 
performance when its used - eg, the range of options for closing, range of 
options for how errors are handled, and perhaps introduce some small 
performance penalties such as additional allocations and/or buffer copies.


That is a whole set of comprises and decisions that would need to
be considered.


Sure. However, such decisions have been made for the HTTP client.
For example, the decision to use Flow-based APIs makes the HTTP client
unsuitable to write an efficient HTTP proxy, as it will require data
copies.


Writing an efficient HTTP proxy is not the primary use-case for
this API. It is being positioned as a replacement for the legacy
HttpURLConnection API. It needs to be approachable by the average
( whatever that is ) Java developer.


What I see incongruent in JEP 321 is that it provides Flow based APIs
for HTTP but not WebSocket, and low-level efficient APIs for
WebSocket, but not for HTTP.


I disagree. WebSocket and HTTP are two distinct protocols, whose
use will likely be very different. I don't see why it is
incongruent to model them in different ways.


If the focus is on the HTTP client, I'd rather see efficient APIs for
the HTTP client since the HTTP proxy use case is quite common.


Answered above.


The design decisions taken for WebSocket (low-level APIs; easily
wrappable in Flow APIs; efficient with respect to data copy and
allocation; based on CompletableFuture) should have been taken for
HTTP with even more importance, given how more popular is HTTP over
WebSocket.


I disagree. The WebSocket API is less approachable by the average
Java developer. It's low-level semantics and use of CompletableFuture,
while appropriate for libraries building on top or using it as a
transport, is acceptable. It is not something that I would expect
the large majority of Java developers ( likely to be retrieving HTTP
resources ) to be as comfortable with.

The substantive point is that the HTTP Client API and the WebSocket
API are targeting two different audiences.


I would like this revision to be taken into consideration, offering an
efficient low-level API for HTTP as well.


The HTTP Client API been based on Flow for for a long time now.
Incubated in JDK 9, and re-incubated in JDK 10. I note, that you
have been following and contributing to JEP 110 and now JEP 321
too ( your contributions have been very valuable, thank you ).

Offering a low-level HTTP API ( akin to that of WebSocket ) will
make the API less approachable to the majority of Java developers.
This is not something that I am comfortable with.

-Chris.

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[Simone Bordet]

Hi,

On Thu, Mar 22, 2018 at 1:57 PM, Chris Hegarty  wrote:
>> Rather, I think that there should be a built in helper that allows Reactive 
>> Streams to be added on top,
>
> I fully agree that a Reactive Streams WebSocket abstraction should be
> built on top. Where I disagree is that it needs to be done in Java SE, let
> alone in JEP 321.

Why is that ?
There have been no problems in having Flow-based APIs for the HTTP
client, so can you expand on why it would be different for WebSocket ?
Not that I would like to have Flow-based APIs in WebSocket (it's
indifferent for me, as long as the low-level API is there), I am just
wondering why the difference.

>> with the understanding that such a helper will limit the capabilities and 
>> performance when its used - eg, the range of options for closing, range of 
>> options for how errors are handled, and perhaps introduce some small 
>> performance penalties such as additional allocations and/or buffer copies.
>
> That is a whole set of comprises and decisions that would need to
> be considered.

Sure. However, such decisions have been made for the HTTP client.
For example, the decision to use Flow-based APIs makes the HTTP client
unsuitable to write an efficient HTTP proxy, as it will require data
copies.

What I see incongruent in JEP 321 is that it provides Flow based APIs
for HTTP but not WebSocket, and low-level efficient APIs for
WebSocket, but not for HTTP.
If the focus is on the HTTP client, I'd rather see efficient APIs for
the HTTP client since the HTTP proxy use case is quite common.

The design decisions taken for WebSocket (low-level APIs; easily
wrappable in Flow APIs; efficient with respect to data copy and
allocation; based on CompletableFuture) should have been taken for
HTTP with even more importance, given how more popular is HTTP over
WebSocket.

I would like this revision to be taken into consideration, offering an
efficient low-level API for HTTP as well.

Thanks !

-- 
Simone Bordet
---
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless.   Victoria Livschitz

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[Chris Hegarty]

James,

Thanks for your reply.

> On 22 Mar 2018, at 00:28, James Roper  wrote:
> 
> Hi Chris,
> 
> I'm still really keen to see an option provided out of the box for using 
> Reactive Streams with the WebSocket API.

What we are delivering in JEP 321 is mainly an HTTP Client API
( you get a small footprint, high performant, low-level, WebSocket
API for free ). It has taken a long time to get to this point in the
project, where we are close to requesting that it be Proposed
To Target for Java SE 11.

> Note that I'm not proposing that anything be changed about the existing API,

Ok, good.

> I understand fully that you want to provide a full powered API that offers 
> the full range of WebSocket features with very high performance, and mapping 
> that to the Reactive Streams API goes against that.

Yes, it does.

> Rather, I think that there should be a built in helper that allows Reactive 
> Streams to be added on top,

I fully agree that a Reactive Streams WebSocket abstraction should be
built on top. Where I disagree is that it needs to be done in Java SE, let
alone in JEP 321.

> with the understanding that such a helper will limit the capabilities and 
> performance when its used - eg, the range of options for closing, range of 
> options for how errors are handled, and perhaps introduce some small 
> performance penalties such as additional allocations and/or buffer copies.

That is a whole set of comprises and decisions that would need to
be considered.

> The use case here is how well this API will interact with other APIs. 
> WebSockets are rarely used in isolation from other technologies. Most non 
> trivial usages of WebSockets will involve plumbing the WebSocket API to some 
> other source or sink of streaming data - for example, a message broker, a 
> database, perhaps a gRPC stream to another service.

I could well image a set of vertical-market specific libraries being built
on top of the WebSocket API, and that is how it should be. In this
specific area, providing the lowest level API is an enabler for others.

> The question is, how difficult will it be to connect those two APIs?

I imagine some work may be required, but certainly much less than
without the WebSocket API.

> Will their backpressure mechanisms map to each other easily?

I see no reason that it should not. Are you aware of any?

>  Will their failure handling and error handling semantics map to each other 
> easily?

I see no reason that they should not. In fact, most higher-level APIs
provide more coarse grain error handling.

> How many lines of boilerplate will it take to hook them up?

I guess that depends on the particular library's functionality, will it
support/handle/expose partial message delivery, buffering, etc. I
don’t see this as boilerplate though.

> If the WebSocket provides an option to use Reactive Streams, then for any 
> other sources/sink of streamed data that support Reactive Streams, 
> application developers will have an option where the answers are guaranteed 
> to be yes without the developer having to do any mapping themselves, and the 
> lines of code to do that will be one.

Sure, if one has a higher-level library that works with Reactive Streams,
and one wants to use WebSocket as a transport, then it would be little
work if Java SE provided a Reactive Streams interface to WebSocket.
There are also vertical-markets using WebSocket, but not using Reactive
Streams.

At this point in JEP 321, I do not want to increase the scope of the project
to include a Reactive Streams interface for WebSocket. This is something
that can, and should, be considered separate to JEP 321. It could follow
in a future release if deemed appropriate, or not, but adding it at this point
will add risk to JEP 321, which I am not willing to do.

-Chris.

Re: RFR [11] 8197564: HTTP Client implementation - JEP 321

[James Roper]

Hi Chris,

I'm still really keen to see an option provided out of the box for using
Reactive Streams with the WebSocket API. Note that I'm not proposing that
anything be changed about the existing API, I understand fully that you
want to provide a full powered API that offers the full range of WebSocket
features with very high performance, and mapping that to the Reactive
Streams API goes against that. Rather, I think that there should be a built
in helper that allows Reactive Streams to be added on top, with the
understanding that such a helper will limit the capabilities and
performance when its used - eg, the range of options for closing, range of
options for how errors are handled, and perhaps introduce some small
performance penalties such as additional allocations and/or buffer copies.

The use case here is how well this API will interact with other APIs.
WebSockets are rarely used in isolation from other technologies. Most non
trivial usages of WebSockets will involve plumbing the WebSocket API to
some other source or sink of streaming data - for example, a message
broker, a database, perhaps a gRPC stream to another service. The question
is, how difficult will it be to connect those two APIs? Will their
backpressure mechanisms map to each other easily? Will their failure
handling and error handling semantics map to each other easily? How many
lines of boilerplate will it take to hook them up? If the WebSocket
provides an option to use Reactive Streams, then for any other sources/sink
of streamed data that support Reactive Streams, application developers will
have an option where the answers are guaranteed to be yes without the
developer having to do any mapping themselves, and the lines of code to do
that will be one.

Regards,

James

On 22 March 2018 at 07:33, Chris Hegarty  wrote:

> Hi,
>
> This is a request for review for the HTTP Client implementation - JEP 321.
>
> The javadoc link has been refreshed, based on feedback received from
> the CSR [1] ( and a follow on supplementary CSR [2] ):
> http://cr.openjdk.java.net/~chegar/httpclient/02/javadoc/
> api/java.net.http/java/net/http/package-summary.html
>
> The webrev link has been refreshed to include implementation changes
> to support the latest API, additional test code coverage and scenarios:
> http://cr.openjdk.java.net/~chegar/httpclient/02/webrev/
>
> > On 9 Feb 2018, at 14:33, Chris Hegarty  wrote:
> > Development of the JDK HTTP Client, incubated in JDK 9 and re-incubated
> > in JDK 10, has continued. It will soon be in a position to be proposed
> > for inclusion in Java 11, through JEP 321. This will proceed, as usual,
> > through the JEP 2.0 process.
> >
> > JEP 321 has recently been updated with the proposed package and module
> > name, `java.net.http`. The code in the sandbox has been updated to
> > reflect this.
> >
> > Javadoc based on the latest code in the sandbox:
> > http://cr.openjdk.java.net/~chegar/httpclient/00/javadoc/
> api/java.net.http-summary.html
> >
> > Webrev based on the latest code in the sandbox:
> >  http://cr.openjdk.java.net/~chegar/httpclient/00/webrev/
> >
> > ( much of the changes in the webrev are mechanical due to renaming, but
> >  there are stability fixes and the API documentation has been
> >  reorganized to improve readability )
> >
> > More information about the JDK HTTP Client can be found on the
> > networking groups page.
> >  http://openjdk.java.net/groups/net/httpclient/
>
>
> -Chris.
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8197565
> [2] https://bugs.openjdk.java.net/browse/JDK-8199938




-- 
*James Roper*
*Senior Octonaut*

Lightbend  – Build reactive apps!
Twitter: @jroper