Re: HTTP client API

[Anthony Vanelverdinghe]

Hi Tobias

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


Below are some quick replies to some of your feedback.

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


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


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


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


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


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


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

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


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


Hope this helps,
Anthony

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

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

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


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


Hi Michael and others -


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



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

(a) increase the usefulness or

(b) decrease the semantic weight

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



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


 *

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

 o

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

 o

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

RE: Ping: RFR (M): 8167481: cleanup of headers and includes for native libnet

[Langer, Christoph]

Thanks, Chris. I pushed it...

> -Original Message-
> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> Sent: Mittwoch, 26. Oktober 2016 15:39
> To: Langer, Christoph 
> Cc: net-dev@openjdk.java.net
> Subject: Re: Ping: RFR (M): 8167481: cleanup of headers and includes for 
> native
> libnet
> 
> 
> > On 20 Oct 2016, at 16:30, Langer, Christoph 
> wrote:
> >
> > Chris,
> >
> > I created a new version that addresses your points:
> > http://cr.openjdk.java.net/~clanger/webrevs/8167481.1/
> 
> Looks good to me.
> 
> > I reordered the headers according to your suggestions ( 1) system, 2)
> platform specific includes, 3) JNI includes, and finally, 4) generated 
> headers ),
> added the @Native and removed the Windows stuff in net_util_md.h. To make
> reodering of system includes possible, I had to add back some system headers
> in a few places and could not always maintain the alphabetical order.
> 
> Ok, thanks.
> 
> > I think you should run it through PRT and then hopefully give me your final
> blessing…
> 
> I ran your patch through our internal build and test system, and there are no
> surprises.  Consider is Reviewed, at least by me.
> 
> -Chris.
> 
> > Thanks & Best regards
> > Christoph
> >
> >> -Original Message-
> >> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> >> Sent: Dienstag, 18. Oktober 2016 20:56
> >> To: Langer, Christoph ; net-
> d...@openjdk.java.net
> >> Subject: Re: Ping: RFR (M): 8167481: cleanup of headers and includes for
> native
> >> libnet
> >>
> >> Christoph,
> >>
> >>> On 17 Oct 2016, at 09:40, Langer, Christoph 
> >> wrote:
> >>> …
> >>> Hi,
> >>>
> >>> as I already mentioned, here is another proposal for cleanup in libnet:
> >>> https://bugs.openjdk.java.net/browse/JDK-8167481
> >>> http://cr.openjdk.java.net/~clanger/webrevs/8167481.0/
> >>
> >> Overall, this looks good. Specific comments below ...
> >>
> >>> This time I touched the includes. Generally I reordered the includes to
> include
> >> “net_util.h” first, then any JNI includes, such as “java_net_InetAddress.h”
> and
> >> finally all standard header includes in alphabetical order. For platform
> specific
> >> includes, I use the order Linux, AIX, Solaris, BSD. I removed all includes 
> >> that
> >> don’t seem to be necessary to get the respective file compiled. Is that the
> >> correct approach that is desired?
> >>
> >> It is good to be consistent. My personal preference is to order the 
> >> includes;
> >> 1) system, 2) platform specific includes, 3) JNI includes, and finally, 4)
> >> generated headers ( e.g. “java_net_InetAddress.h” ).
> >>
> >>> To make this work, I had to remove the definitions “IPv4” and “IPv6” in
> >> net_util.h and replace their usages with “java_net_InetAddress_IPv4” resp.
> >> “java_net_InetAddress_IPv6” from JNI which seems to be the correct thing
> to
> >> do anyway.
> >>
> >> Right. This is a good change. Trivially, and it is not strictly necessary 
> >> but
> >> good for readability, would be to add @Native to InetAddress.IPv4 and
> >> InetAddress.IPv6.
> >>
> >>> For Windows I also cleaned up (removed) the definition of SOCKADDR_IN6
> in
> >> net_util_md.h and replaced all occurences with sockaddr_in6. It seems that
> the
> >> capital letters version SOCKADDR_IN6 is a remainder of a very old Microsoft
> >> runtime (_MSC_VER < 1310) which corresponds to MSVC 7.0, which is even
> >> older than Visual Studio 2003. So I’m wondering if I should remove the
> whole
> >> section in windows net_util_md.h now (lines 47 – 177)?
> >>
> >> I think it should be ok to remove this.
> >>
> >>> For Windows it seems that we can also completely get rid of
> >> src/java.base/windows/native/libnet/icmp.h as the icmp stuff is all brought
> by
> >> the #include . Would you agree to that?
> >>
> >> Ok.
> >>
> >>> So far I successfully ran builds in my local environments for Windows,
> Linux,
> >> AIX, Solaris and Apple. I’ve now added the changeset to our local 
> >> regression
> >> testing environment for OpenJDK. If the change is to your like, I would 
> >> like to
> >> ask you to also run it through JPRT to see if I missed some dependency.
> >>
> >> I ran your patch through our internal build and test system and there
> >> are no surprises.
> >>
> >> -Chris.
> >>
> >>> Thanks and best regards
> >>> Christoph
> >