Collection of various minor cleanups. #428
Conversation
Correct misspellings. Enhance annotations. Remove some unused pieces that are not part of the public API.
jeffret-b
added
the
chore
| this.nioChannelHub = nioChannelHub; | ||
| /** | ||
| * The {@link NioChannelHub} to use or {@code null} | ||
| */ |
There was a problem hiding this comment.
Is this supposed to be removed with no effect?
There was a problem hiding this comment.
Good question. I believe that NioChannelHub was only used by older JNLP implementations which have since been removed. See line result.add(new JnlpProtocol4Handler(clientDatabase, threadPool, ioHub, context, needClientAuth, preferNio)); which doesn't use the nioChannelHub field, so it appears to be unused now.
There was a problem hiding this comment.
@res0nance, as Matt comments, the nioChannelHub field isn't used. I haven't dug into the details of when this might have been used, though I'm not certain it ever was. The value is set on this line, but nothing ever uses it. As part of this PR, I've retained the public API, but removed the unused internal pieces.
There was a problem hiding this comment.
Very nice cleanup! As a side note, it'd be really neat if we could extract the generic networking/buffering code into its own library (think something like a trimmed down Netty or Mina) as it could potentially allow for more reuse of that code (and more eyes testing it). On the other hand, we also have Stapler which shows that separating out libraries isn't always useful.
| try(InputStream istsream = url.openStream(); OutputStream ostream = new DigestOutputStream(new NullOutputStream(), md)) { | ||
| Util.copy(istsream, ostream); | ||
| try(InputStream istream = url.openStream(); OutputStream ostream = new DigestOutputStream(new NullOutputStream(), md)) { | ||
| Util.copy(istream, ostream); |
There was a problem hiding this comment.
In Java 11, we'll be able to replace these with istream.transferTo(ostream) which is nice :)
| this.nioChannelHub = nioChannelHub; | ||
| /** | ||
| * The {@link NioChannelHub} to use or {@code null} | ||
| */ |
There was a problem hiding this comment.
Good question. I believe that NioChannelHub was only used by older JNLP implementations which have since been removed. See line result.add(new JnlpProtocol4Handler(clientDatabase, threadPool, ioHub, context, needClientAuth, preferNio)); which doesn't use the nioChannelHub field, so it appears to be unused now.
|
@jvz, I don't think there's much value in a separate library containing the generic networking/buffering code. What would be nice is separating that code out (essentially removing it) and then replacing it with a proven, reliable messaging system. Remoting over Kafka is one such example. |
Another round of minor cleanups across the Remoting project.
Correct misspellings. Enhance annotations. Remove some unused pieces that are not part of the public API.