RFR JDK-8087113: Websocket API and implementation

[Andrej Golovnin]

Hi Pavel,

I’m sorry for the late response. I was too busy this week.

> Maybe we could somehow melt what you call CloseReason and CloseCode into a
> single entity? Something like (different name is an option too):
> 
>    public final class CloseReason {
> 
>        static CloseReason normalClosure(CharSequence reason);
>        static CloseReason goingAway(CharSequence reason);
>        // ...
>        static CloseReason unexpectedCondition(CharSequence reason);
> 
>        static CloseReason of(int code);
>        static CloseReason of(int code, CharSequence reason);
>    }
> 
> In an application:
> 
>    ws.sendClose(CloseReason.normalClosure("Too tired"));
>    ws.sendClose(CloseReason.of(2016, "Leap year"));
> 
> This doesn't solve the first problem of not having code description, but seems
> to solve the second one.

I think it is an acceptable solution. At least I can live with it.

> 
>> And I think there should be a way to configure a timeout for sending messages.
> 
> As I understand timeout expiration for sendXXX would be a fatal condition (in
> general.) Simply because by the time a returned CF has completed exceptionally,
> some bytes of a message might have been sent already. (That's generally not the
> case with read timeouts see java.net.Socket#setSoTimeout)
> 
> What would you expect _additionally_ from the implementation (API guarantees) in
> a case where send completes exceptionally with a timeout in contrast with this
> snippet?
>    ws.sendBinary(giganticByteBuffer, true)
>            .orTimeout(30, TimeUnit.SECONDS)
>            .whenComplete((v, e) -> {
>                if (e instanceof TimeoutException) {
>                    try {
>                        ws.abort();
>                    } catch (IOException ignored) { }
>                }
>            });
> 

I would expect to get pre-configured CompletableFutures from the #sendXXX-methods.
So I don’t need to call the #orTimeout()-method on every CompletableFuture returned by
the #sendXXX-methods. But I think the usage the #orTimeout-method is also OK.

BTW, someone should describe in the JavaDocs of CompletableFuture#orTimeout()
what would happen when this method is called multiple times on the same
CompletableFuture with different arguments.

Best regards,
Andrej Golovnin


> BTW, I'm thinking of changing sendXXX methods return type to CompletionStage
> (now it's CompletableFuture), this will cause, for example,
> CompletableFuture#get(long, java.util.concurrent.TimeUnit) not to be directly
> accessible.
> 
>> One minor thing off-topic:
>> While changing my code I noticed that the class ProxySelector$StaticProxySelector
>> and the JavaDocs of the ProxySelector#of-method can be improved a little bit:
>> 
>> diff -r 589795e4cd38 src/java.base/share/classes/java/net/ProxySelector.java
>> --- a/src/java.base/share/classes/java/net/ProxySelector.java	Wed Mar 23 19:33:39 2016 -0700
>> +++ b/src/java.base/share/classes/java/net/ProxySelector.java	Mon Mar 28 19:30:24 2016 +0200
>> @@ -165,7 +165,8 @@
>> 
>>    /**
>>     * Returns a ProxySelector which uses the given proxy address for all HTTP
>> -     * and HTTPS requests. If proxy is {@code null} then proxying is disabled.
>> +     * and HTTPS requests. If the given proxy address is {@code null}
>> +     * then proxying is disabled.
>>     *
>>     * @param proxyAddress
>>     *        The address of the proxy
>> @@ -178,9 +179,9 @@
>>        return new StaticProxySelector(proxyAddress);
>>    }
>> 
>> -    static class StaticProxySelector extends ProxySelector {
>> +    private static final class StaticProxySelector extends ProxySelector {
>>        private static final List<Proxy> NO_PROXY_LIST = List.of(Proxy.NO_PROXY);
>> -        final List<Proxy> list;
>> +        private final List<Proxy> list;
>> 
>>        StaticProxySelector(InetSocketAddress address){
>>            Proxy p;
>> @@ -198,9 +199,9 @@
>>        }
>> 
>>        @Override
>> -        public synchronized List<Proxy> select(URI uri) {
>> -            String scheme = uri.getScheme().toLowerCase();
>> -            if (scheme.equals("http") || scheme.equals("https")) {
>> +        public List<Proxy> select(URI uri) {
>> +            String scheme = uri.getScheme();
>> +            if (scheme.equalsIgnoreCase("http") || scheme.equalsIgnoreCase("https")) {
>>                return list;
>>            } else {
>>                return NO_PROXY_LIST;
> 
> CC'ing Michael.
>