Merge pull request #3489 from oleg-nenashev/bug/JENKINS-51889

[JENKINS-51889,JENKINS-51955,JENKINS-51971] - Stop using Guava’s NullOutputStream, document setChannel API

core/src/main/java/hudson/slaves/SlaveComputer.java

@@ -23,7 +23,6 @@
  */
 package hudson.slaves;
 
-import com.google.common.io.NullOutputStream;
 import hudson.AbortException;
 import hudson.FilePath;
 import hudson.Functions;
@@ -387,7 +386,15 @@ public class SlaveComputer extends Computer {
 
     private final Object channelLock = new Object();
 
-    public void setChannel(InputStream in, OutputStream out, TaskListener taskListener, Channel.Listener listener) throws IOException, InterruptedException {
+    /**
+     * Creates a {@link Channel} from the given stream and sets that to this agent.
+     *
+     * Same as {@link #setChannel(InputStream, OutputStream, OutputStream, Channel.Listener)}, but for
+     * {@link TaskListener}.
+     */
+    public void setChannel(@Nonnull InputStream in, @Nonnull OutputStream out,
+                           @Nonnull TaskListener taskListener,
+                           @CheckForNull Channel.Listener listener) throws IOException, InterruptedException {
         setChannel(in,out,taskListener.getLogger(),listener);
     }
 
@@ -410,7 +417,9 @@ public class SlaveComputer extends Computer {
      *      By the time this method is called, the cause of the termination is reported to the user,
      *      so the implementation of the listener doesn't need to do that again.
      */
-    public void setChannel(InputStream in, OutputStream out, OutputStream launchLog, Channel.Listener listener) throws IOException, InterruptedException {
+    public void setChannel(@Nonnull InputStream in, @Nonnull OutputStream out,
+                           @CheckForNull OutputStream launchLog,
+                           @CheckForNull Channel.Listener listener) throws IOException, InterruptedException {
         ChannelBuilder cb = new ChannelBuilder(nodeName,threadPoolForRemoting)
             .withMode(Channel.Mode.NEGOTIATE)
             .withHeaderStream(launchLog);
@@ -437,7 +446,7 @@ public class SlaveComputer extends Computer {
      *      Gets a notification when the channel closes, to perform clean up. Can be {@code null}.
      *      By the time this method is called, the cause of the termination is reported to the user,
      *      so the implementation of the listener doesn't need to do that again.
-     * @since TODO
+     * @since 2.127
      */
     @Restricted(Beta.class)
     public void setChannel(@Nonnull ChannelBuilder cb,
@@ -451,7 +460,6 @@ public class SlaveComputer extends Computer {
         if (headerStream == null) {
             LOGGER.log(Level.WARNING, "No header stream defined when setting channel for computer {0}. " +
                     "Launch log won't be printed", this);
-            headerStream = new NullOutputStream();
         }
         Channel channel = cb.build(commandTransport);
         setChannel(channel, headerStream, listener);
@@ -570,14 +578,18 @@ public class SlaveComputer extends Computer {
 
     /**
      * Sets up the connection through an existing channel.
-     * @param channel the channel to use; <strong>warning:</strong> callers are expected to have called {@link ChannelConfigurator} already
+     * @param channel the channel to use; <strong>warning:</strong> callers are expected to have called {@link ChannelConfigurator} already.
+     * @param launchLog Launch log. If not {@code null}, will receive launch log messages
+     * @param listener Channel event listener to be attached (if not {@code null})
      * @since 1.444
      */
-    public void setChannel(Channel channel, OutputStream launchLog, Channel.Listener listener) throws IOException, InterruptedException {
+    public void setChannel(@Nonnull Channel channel,
+                           @CheckForNull OutputStream launchLog,
+                           @CheckForNull Channel.Listener listener) throws IOException, InterruptedException {
         if(this.channel!=null)
             throw new IllegalStateException("Already connected");
 
-        final TaskListener taskListener = new StreamTaskListener(launchLog);
+        final TaskListener taskListener = launchLog != null ? new StreamTaskListener(launchLog) : TaskListener.NULL;
         PrintStream log = taskListener.getLogger();
 
         channel.setProperty(SlaveComputer.class, this);

core/src/main/java/hudson/util/StreamTaskListener.java

@@ -44,6 +44,9 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 import org.kohsuke.stapler.framework.io.WriterOutputStream;
 
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
+
 // TODO: AbstractTaskListener is empty now, but there are dependencies on that e.g. Ruby Runtime - JENKINS-48116)
 // The change needs API deprecation policy or external usages cleanup.
 
@@ -56,7 +59,9 @@ import org.kohsuke.stapler.framework.io.WriterOutputStream;
  * @author Kohsuke Kawaguchi
  */
 public class StreamTaskListener extends AbstractTaskListener implements TaskListener, Closeable {
+    @Nonnull
     private PrintStream out;
+    @CheckForNull
     private Charset charset;
 
     /**
@@ -66,15 +71,15 @@ public class StreamTaskListener extends AbstractTaskListener implements TaskList
      *      or use {@link #fromStdout()} or {@link #fromStderr()}.
      */
     @Deprecated
-    public StreamTaskListener(PrintStream out) {
+    public StreamTaskListener(@Nonnull PrintStream out) {
         this(out,null);
     }
 
-    public StreamTaskListener(OutputStream out) {
+    public StreamTaskListener(@Nonnull OutputStream out) {
         this(out,null);
     }
 
-    public StreamTaskListener(OutputStream out, Charset charset) {
+    public StreamTaskListener(@Nonnull OutputStream out, @CheckForNull Charset charset) {
         try {
             if (charset == null)
                 this.out = (out instanceof PrintStream) ? (PrintStream)out : new PrintStream(out, false);
@@ -87,18 +92,18 @@ public class StreamTaskListener extends AbstractTaskListener implements TaskList
         }
     }
 
-    public StreamTaskListener(File out) throws IOException {
+    public StreamTaskListener(@Nonnull File out) throws IOException {
         this(out,null);
     }
 
-    public StreamTaskListener(File out, Charset charset) throws IOException {
+    public StreamTaskListener(@Nonnull File out, @CheckForNull Charset charset) throws IOException {
         // don't do buffering so that what's written to the listener
         // gets reflected to the file immediately, which can then be
         // served to the browser immediately
         this(Files.newOutputStream(asPath(out)), charset);
     }
 
-    private static Path asPath(File out) throws IOException {
+    private static Path asPath(@Nonnull File out) throws IOException {
         try {
             return out.toPath();
         } catch (InvalidPathException e) {
@@ -115,7 +120,7 @@ public class StreamTaskListener extends AbstractTaskListener implements TaskList
      * @throws IOException if the file could not be opened.
      * @since 1.651
      */
-    public StreamTaskListener(File out, boolean append, Charset charset) throws IOException {
+    public StreamTaskListener(@Nonnull File out, boolean append, @CheckForNull Charset charset) throws IOException {
         // don't do buffering so that what's written to the listener
         // gets reflected to the file immediately, which can then be
         // served to the browser immediately
@@ -127,7 +132,7 @@ public class StreamTaskListener extends AbstractTaskListener implements TaskList
         );
     }
 
-    public StreamTaskListener(Writer w) throws IOException {
+    public StreamTaskListener(@Nonnull Writer w) throws IOException {
         this(new WriterOutputStream(w));
     }
 
@@ -155,7 +160,7 @@ public class StreamTaskListener extends AbstractTaskListener implements TaskList
 
     @Override
     public Charset getCharset() {
-        return charset;
+        return charset != null ? charset : Charset.defaultCharset();
     }
 
     private void writeObject(ObjectOutputStream out) throws IOException {