Re: RFR JDK-8087113: Websocket API and implementation

Hi, On Tue, May 10, 2016 at 2:03 PM, Pavel Rappo wrote: > Hi Simone, > >> On 4 Apr 2016, at 20:07, Simone Bordet wrote: >> >> In my experience, once you get rid of exception throwing in async API >> everything becomes much simpler, especially

Re: RFR JDK-8087113: Websocket API and implementation

Hi Simone, > On 4 Apr 2016, at 20:07, Simone Bordet wrote: > > In my experience, once you get rid of exception throwing in async API > everything becomes much simpler, especially "deep" exceptions related > to invalid implementation internal states. Am I right saying

Re: RFR JDK-8087113: Websocket API and implementation

> On 6 May 2016, at 09:16, Felix Yang wrote: > > Hi Pavel, > several comments: > > 1. WebSocket.request(long n) is documented as " > > @throws IllegalArgumentException if n < -1 > " > > It looks meaningless to allow 0. First of all, the way `request` is defined

Re: RFR JDK-8087113: Websocket API and implementation

Hi Pavel, several comments: 1. WebSocket.request(long n) is documented as " @throws IllegalArgumentException if n < -1 " It looks meaningless to allow 0. 2. Some concern on the way of handling close. Consider following scenario. * obtain a ws connection * message communications

Re: RFR JDK-8087113: Websocket API and implementation

Hi, On Tue, May 3, 2016 at 5:23 PM, Pavel Rappo wrote: > > Hello, > > Here's an updated webrev with the latest implementation: > >http://cr.openjdk.java.net/~prappo/8087113/webrev.04/ Can you please summarize what's different from the previous ? I had a very quick

Re: RFR JDK-8087113: Websocket API and implementation

Strictly speaking, not sending a Pong for every Ping is not against the protocol semantics [1]: If an endpoint receives a Ping frame and has not yet sent Pong frame(s) in response to previous Ping frame(s), the endpoint MAY elect to send a Pong frame for only the most recently processed

Re: RFR JDK-8087113: Websocket API and implementation

On 3 May 2016, at 16:23, Pavel Rappo wrote: > Hello, > > Here's an updated webrev with the latest implementation: > > http://cr.openjdk.java.net/~prappo/8087113/webrev.04/ This looks much better, more straight forward. I will ignore any TODO’s and items mentioned in

Re: RFR JDK-8087113: Websocket API and implementation

> On 5 May 2016, at 00:20, Jitendra Kotamraju wrote: > > * I see that there is an issue for autoponging. May be this falls under it. > The default impl of onPing() doesn't send PONG for *every* received PING. Yes, that's correct. The default implementation sends a Pong in

Re: RFR JDK-8087113: Websocket API and implementation

* I see that there is an issue for autoponging. May be this falls under it. The default impl of onPing() doesn't send PONG for *every* received PING. Since it is against the protocol semantics, it forces all applications to override the default onPing() impl. Is that right ? It seems to me that

Re: RFR JDK-8087113: Websocket API and implementation

Hello, Here's an updated webrev with the latest implementation: http://cr.openjdk.java.net/~prappo/8087113/webrev.04/ If you're going to review it, please be advised that there is a number of known API and implementation issues yet to be resolved [1]. More API tests will be provided in the

Re: RFR JDK-8087113: Websocket API and implementation

On Sun, Apr 10, 2016 at 10:19 PM, Andrej Golovnin wrote: >> On Sun, Apr 10, 2016 at 2:00 PM, Andrej Golovnin >> wrote: >>> BTW, someone should describe in the JavaDocs of >>> CompletableFuture#orTimeout() >>> what would happen when this

Re: RFR JDK-8087113: Websocket API and implementation

> On Sun, Apr 10, 2016 at 2:00 PM, Andrej Golovnin > wrote: >> 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. > >

Re: RFR JDK-8087113: Websocket API and implementation

> On 6 Apr 2016, at 21:28, Roger Riggs wrote: > >>> Reader.java: >>> - line 137: Why should the reader force a new task? Should it be left to >>> the subscriber to >>>decide whether its processing is lightweight and can be done immediately >>> or to re-submit to

Re: RFR JDK-8087113: Websocket API and implementation

> On 4 Apr 2016, at 18:16, Anthony Vanelverdinghe > wrote: > >>> - CompletableFuture sendClose(CloseCode code, CharSequence reason) >>> change the type of "reason" to String >> What's the rationale behind this? > Unlike with the sendText methods, I don't see

Re: RFR JDK-8087113: Websocket API and implementation

On 6 Apr 2016, at 09:37, Simone Bordet wrote: > Hi, > > On Tue, Apr 5, 2016 at 11:06 PM, Pavel Rappo wrote: >> Let's suppose we have a ByteBuffer to send. This ByteBuffer contains 1 MB of >> data, the socket send buffer is 16 KB, the network is

Re: RFR JDK-8087113: Websocket API and implementation

Hi, On Tue, Apr 5, 2016 at 11:06 PM, Pavel Rappo wrote: > Let's suppose we have a ByteBuffer to send. This ByteBuffer contains 1 MB of > data, the socket send buffer is 16 KB, the network is not particularly fast. > Suppose then the first pass fills the full buffer's 16Kb

Re: RFR JDK-8087113: Websocket API and implementation

Hi Simone, > On 5 Apr 2016, at 21:16, Simone Bordet wrote: > > Hi, > > Sure, the caller must not block. > But there is no need to dispatch to achieve that when all code is > non-blocking already. Sorry, could you please explain this to me in more detail? I'm not sure

Re: RFR JDK-8087113: Websocket API and implementation

Hi, On Tue, Apr 5, 2016 at 9:37 PM, Chris Hegarty wrote: > I need to get back into the code, but are you counting the calling thread, > the one invoking sendXXX(), as dispatch 1? No, that's why I am proposing *zero* dispatches. > We always need this to > allow the

Re: RFR JDK-8087113: Websocket API and implementation

Simone, On 5 Apr 2016, at 20:25, Simone Bordet wrote: > Hi, > > On Mon, Apr 4, 2016 at 5:35 PM, Chris Hegarty > wrote: On 3 Apr 2016, at 18:43, Simone Bordet wrote: Threading. ---

Re: RFR JDK-8087113: Websocket API and implementation

Hi, On Mon, Apr 4, 2016 at 5:35 PM, Chris Hegarty wrote: >>> On 3 Apr 2016, at 18:43, Simone Bordet wrote: >>> Threading. >>> --- >>> WebSocket.sendXXX() calls >>> MessagePublisher.send(), which dispatches a to >>> MessagePublisher.react(),

Re: RFR JDK-8087113: Websocket API and implementation

Hi Roger, thanks for looking into this. > On 5 Apr 2016, at 17:37, Roger Riggs wrote: > > It would be helpful if the classnames/filenames reflected the participation > in the WebSocket implementation > to keep them distinct from the HTTP 2.0 implementation in the same

Re: RFR JDK-8087113: Websocket API and implementation

Hi Pavel, Initial comments, bottom up. It would be helpful if the classnames/filenames reflected the participation in the WebSocket implementation to keep them distinct from the HTTP 2.0 implementation in the same directory. For example, Writer, Reader, etc. perhaps a 'Ws' prefix would be

Re: RFR JDK-8087113: Websocket API and implementation

Hi, On Mon, Apr 4, 2016 at 8:45 PM, Chris Hegarty wrote: > Isn’t this the same as the new HTTP Client in java.net.http, e.g. > HttpRequest::responseAsync ? Do you think that this should be > changed also? I have not looked at the new HTTP client yet, but I will. In

Re: RFR JDK-8087113: Websocket API and implementation

On 4 Apr 2016, at 17:00, Simone Bordet wrote: > Hi, > > On Mon, Apr 4, 2016 at 4:02 PM, Pavel Rappo wrote: >> I see your point. Yes, working asynchronously kind of suggests that we should >> relay all exceptions through the asynchronous

Re: RFR JDK-8087113: Websocket API and implementation

Hi Pavel Thanks for your considerate response, replies are inline. On 4/04/2016 13:08, Pavel Rappo wrote: Hi Anthony, thanks a lot for looking into this! On 3 Apr 2016, at 17:45, Anthony Vanelverdinghe wrote: Here are my suggestions concerning the public

Re: RFR JDK-8087113: Websocket API and implementation

Hi, On Mon, Apr 4, 2016 at 4:02 PM, Pavel Rappo wrote: > I see your point. Yes, working asynchronously kind of suggests that we should > relay all exceptions through the asynchronous interface (CF in this particular > case), simply because otherwise we would have to

Re: RFR JDK-8087113: Websocket API and implementation

I'm still working my way through the code, but ... On 04/04/16 15:02, Pavel Rappo wrote: Hi Simone, thanks for deep dive into this! On 3 Apr 2016, at 18:43, Simone Bordet wrote: Throwing exceptions. --- Given that the API is making use of CompletableFuture,

Re: RFR JDK-8087113: Websocket API and implementation

Hi Simone, thanks for deep dive into this! > On 3 Apr 2016, at 18:43, Simone Bordet wrote: > > Throwing exceptions. > --- > Given that the API is making use of CompletableFuture, throwing > exceptions instead is in my experience not the right thing to do. > You want to

Re: RFR JDK-8087113: Websocket API and implementation

Hi Anthony, thanks a lot for looking into this! > On 3 Apr 2016, at 17:45, Anthony Vanelverdinghe > wrote: > > Here are my suggestions concerning the public types: > > java.net.http.WebSocket > - order the arguments of > static Builder newBuilder(URI uri,

Re: RFR JDK-8087113: Websocket API and implementation

Hi, On Fri, Mar 25, 2016 at 5:21 PM, Pavel Rappo wrote: > Hi, > > Could you please review my change for JDK-8087113 > >http://cr.openjdk.java.net/~prappo/8087113/webrev.03/ > > This webrev contains initial implementation and basic tests for WebSocket API. >

Re: RFR JDK-8087113: Websocket API and implementation

Hi Pavel Here are my suggestions concerning the public types: java.net.http.WebSocket - order the arguments of static Builder newBuilder(URI uri, HttpClient client, Listener listener) consistently with the 2-argument method: static Builder newBuilder(URI uri, Listener listener, HttpClient

Re: RFR JDK-8087113: Websocket API and implementation

> > > > You see there'a thing. That's why I didn't do it immediately. For example > let's > consider a "Sec-WebSocket-Extensions" (a very good illustrative example.) > RFC 6455 states it very clearly [1]: > > The |Sec-WebSocket-Extensions| header field MAY appear multiple times > in an

Re: RFR JDK-8087113: Websocket API and implementation

Hi Chris, > On 31 Mar 2016, at 17:35, Chris Hegarty wrote: > > I’ve looked at the API a number of times, and overall I’m happy with it > ( modulo the known open issues ). Some initial comments. Thanks a lot for looking into this! > Generally, I prefer javadoc style

Re: RFR JDK-8087113: Websocket API and implementation

On 25 Mar 2016, at 16:21, Pavel Rappo wrote: > Hi, > > Could you please review my change for JDK-8087113 > > http://cr.openjdk.java.net/~prappo/8087113/webrev.03/ I’ve looked at the API a number of times, and overall I’m happy with it ( modulo the known open issues

Re: RFR JDK-8087113: Websocket API and implementation

On 29/03/2016 20:16, Andrej Golovnin wrote: 177 throw new IllegalArgumentException(String.valueOf(n)); >> >>Could you please provide a better message for exception here? > >What would the purpose be? It's an internal implementation detail. If this >exception is

Re: RFR JDK-8087113: Websocket API and implementation

> On 29 Mar 2016, at 20:16, Andrej Golovnin wrote: > > JEP-280 does not apply in this case. But when you rewrite this code to > use the plain String concatenation, then JEP-280 would apply: > > 211 static String toString(Buffer b) { > 212 return

Re: RFR JDK-8087113: Websocket API and implementation

Hi Pavel, >> 215 .append(" >> rem=").append(b.remaining()).append("]").toString(); >> >> Please use ']' instead of "]”. >> >> 222 >> .append("[len=").append(s.length()).append("]").toString(); >> >> Please use ']' instead of "]”. > > I believe since JEP 280

RFR JDK-8087113: Websocket API and implementation

Hi, Could you please review my change for JDK-8087113 http://cr.openjdk.java.net/~prappo/8087113/webrev.03/ This webrev contains initial implementation and basic tests for WebSocket API. Specification conformance & interoperability testing has been performed with Autobahn Testsuite [1]. As