Null-handling refactoring

- Remove unnecessary usages of null.
- Use Optional instead of null values where appropriate.
- Add comments to explain nulls with special meanings.
- Annotate fields and parameters that can be null with @checkfornull.
- Validate constructor arguments with Preconditions.checkNotNull.

Further refactoring
- Add class to read time-shifts, previously handled by TimestampsReader.
- Varint.read reads directly from an InputStream, instead of needing a
  Varint.ByteReader.

src/main/java/hudson/plugins/timestamper/TimestampNote.java

@@ -102,6 +102,6 @@ public ConsoleAnnotator<Object> annotate(Object context, MarkupText text,
       Timestamp timestamp = getTimestamp(build);
       formatter.markup(text, timestamp);
     }
-    return null;
+    return null; // each time-stamp note affects one line only
   }
 }

src/main/java/hudson/plugins/timestamper/TimestamperBuildWrapper.java

@@ -23,6 +23,7 @@
  */
 package hudson.plugins.timestamper;
 
+import static com.google.common.base.Preconditions.checkNotNull;
 import hudson.Extension;
 import hudson.Launcher;
 import hudson.console.LineTransformationOutputStream;
@@ -42,6 +43,8 @@
 
 import org.kohsuke.stapler.DataBoundConstructor;
 
+import com.google.common.base.Optional;
+
 /**
  * Build wrapper that decorates the build's logger to record time-stamps as each
  * line is logged.
@@ -84,9 +87,9 @@ public OutputStream decorateLogger(AbstractBuild build, OutputStream logger) {
     if (Boolean.getBoolean(TimestampNote.getSystemProperty())) {
       return new TimestampNotesOutputStream(logger);
     }
-    MessageDigest digest = null;
+    Optional<MessageDigest> digest = Optional.absent();
     try {
-      digest = MessageDigest.getInstance("SHA-1");
+      digest = Optional.of(MessageDigest.getInstance("SHA-1"));
     } catch (NoSuchAlgorithmException ex) {
       LOGGER.log(Level.WARNING, ex.getMessage(), ex);
     }
@@ -118,7 +121,7 @@ private static class TimestampNotesOutputStream extends
      *          the delegate output stream
      */
     TimestampNotesOutputStream(OutputStream delegate) {
-      this.delegate = delegate;
+      this.delegate = checkNotNull(delegate);
     }
 
     /**

src/main/java/hudson/plugins/timestamper/TimestamperConfig.java

@@ -31,6 +31,7 @@
 
 import java.text.SimpleDateFormat;
 
+import javax.annotation.CheckForNull;
 import javax.servlet.http.HttpServletRequest;
 
 import jenkins.model.GlobalConfiguration;
@@ -77,12 +78,14 @@ public TimestampFormatter get() {
    * The chosen format for displaying the system clock time, as recognised by
    * {@link SimpleDateFormat}.
    */
+  @CheckForNull
   private String timestampFormat;
 
   /**
    * The chosen format for displaying the elapsed time, as recognised by
    * {@link DurationFormatUtils}.
    */
+  @CheckForNull
   private String elapsedTimeFormat;
 
   /**
@@ -107,7 +110,7 @@ public String getSystemTimeFormat() {
    * @param timestampFormat
    *          the system clock time format in {@link SimpleDateFormat} pattern
    */
-  public void setSystemTimeFormat(String timestampFormat) {
+  public void setSystemTimeFormat(@CheckForNull String timestampFormat) {
     this.timestampFormat = timestampFormat;
   }
 
@@ -126,7 +129,7 @@ public String getElapsedTimeFormat() {
    * @param elapsedTimeFormat
    *          the elapsed time format in {@link DurationFormatUtils} pattern
    */
-  public void setElapsedTimeFormat(String elapsedTimeFormat) {
+  public void setElapsedTimeFormat(@CheckForNull String elapsedTimeFormat) {
     this.elapsedTimeFormat = elapsedTimeFormat;
   }
 

src/main/java/hudson/plugins/timestamper/action/TimestampsAction.java

@@ -23,6 +23,7 @@
  */
 package hudson.plugins.timestamper.action;
 
+import static com.google.common.base.Preconditions.checkNotNull;
 import hudson.console.ConsoleNote;
 import hudson.model.Action;
 import hudson.model.Run;
@@ -75,25 +76,23 @@ public final class TimestampsAction implements Action {
    *          the build to inspect
    */
   TimestampsAction(Run<?, ?> build) {
-    this.build = build;
+    this.build = checkNotNull(build);
   }
 
   /**
    * {@inheritDoc}
    */
   @Override
   public String getIconFileName() {
-    // Do not display this action.
-    return null;
+    return null; // do not display this action
   }
 
   /**
    * {@inheritDoc}
    */
   @Override
   public String getDisplayName() {
-    // Do not display this action.
-    return null;
+    return null; // do not display this action
   }
 
   /**
@@ -173,11 +172,10 @@ private void unrecognisedPrecision(String precision) {
 
   private void writeConsoleNotes(PrintWriter writer, int precision)
       throws IOException {
-    DataInputStream dataInputStream = null;
+    DataInputStream dataInputStream = new DataInputStream(
+        new BufferedInputStream(build.getLogInputStream()));
     boolean threw = true;
     try {
-      dataInputStream = new DataInputStream(new BufferedInputStream(
-          build.getLogInputStream()));
       while (true) {
         dataInputStream.mark(1);
         int currentByte = dataInputStream.read();

src/main/java/hudson/plugins/timestamper/annotator/ConsoleLogParserImpl.java

@@ -64,10 +64,9 @@ class ConsoleLogParserImpl implements ConsoleLogParser {
   public ConsoleLogParser.Result seek(Run<?, ?> build) throws IOException {
     ConsoleLogParser.Result result = new ConsoleLogParser.Result();
     result.atNewLine = true;
-    InputStream inputStream = null;
+    InputStream inputStream = new BufferedInputStream(build.getLogInputStream());
     boolean threw = true;
     try {
-      inputStream = new BufferedInputStream(build.getLogInputStream());
       long posFromStart = pos;
       if (pos < 0) {
         posFromStart = build.getLogText().length() + pos;

src/main/java/hudson/plugins/timestamper/annotator/TimestampAnnotator.java

@@ -23,6 +23,7 @@
  */
 package hudson.plugins.timestamper.annotator;
 
+import static com.google.common.base.Preconditions.checkNotNull;
 import hudson.MarkupText;
 import hudson.console.ConsoleAnnotator;
 import hudson.model.Run;
@@ -34,6 +35,10 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import javax.annotation.CheckForNull;
+
+import com.google.common.base.Optional;
+
 /**
  * Inserts formatted time-stamps into the annotated console output.
  * 
@@ -50,6 +55,7 @@ public final class TimestampAnnotator extends ConsoleAnnotator<Object> {
 
   private final ConsoleLogParser logParser;
 
+  @CheckForNull
   private TimestampsReader timestampsReader;
 
   /**
@@ -61,8 +67,8 @@ public final class TimestampAnnotator extends ConsoleAnnotator<Object> {
    *          the console log parser
    */
   TimestampAnnotator(TimestampFormatter formatter, ConsoleLogParser logParser) {
-    this.formatter = formatter;
-    this.logParser = logParser;
+    this.formatter = checkNotNull(formatter);
+    this.logParser = checkNotNull(logParser);
   }
 
   /**
@@ -71,45 +77,33 @@ public final class TimestampAnnotator extends ConsoleAnnotator<Object> {
   @Override
   public ConsoleAnnotator<Object> annotate(Object context, MarkupText text) {
     if (!(context instanceof Run<?, ?>)) {
-      return null;
+      return null; // do not annotate the following lines
     }
     Run<?, ?> build = (Run<?, ?>) context;
 
     try {
       if (timestampsReader == null) {
         ConsoleLogParserImpl.Result logPosition = logParser.seek(build);
         if (logPosition.endOfFile) {
-          return null;
+          return null; // do not annotate the following lines
         }
         timestampsReader = new TimestampsReader(build);
         timestampsReader.skip(logPosition.lineNumber);
-        Timestamp timestamp = timestampsReader.read();
-        return markup(text, logPosition.atNewLine ? timestamp : null);
+        Optional<Timestamp> timestamp = timestampsReader.read();
+        if (logPosition.atNewLine && timestamp.isPresent()) {
+          formatter.markup(text, timestamp.get());
+        }
+        return this;
       }
-      Timestamp timestamp = timestampsReader.read();
-      if (timestamp != null) {
-        return markup(text, timestamp);
+      Optional<Timestamp> timestamp = timestampsReader.read();
+      if (timestamp.isPresent()) {
+        formatter.markup(text, timestamp.get());
+        return this;
       }
     } catch (IOException ex) {
       LOGGER.log(Level.WARNING,
           "Error reading timestamps for " + build.getFullDisplayName(), ex);
     }
-    return null;
-  }
-
-  /**
-   * Add a time-stamp to the given text.
-   * 
-   * @param text
-   *          the text to modify
-   * @param timestamp
-   *          the time-stamp
-   * @return {@code this}
-   */
-  private TimestampAnnotator markup(MarkupText text, Timestamp timestamp) {
-    if (timestamp != null) {
-      formatter.markup(text, timestamp);
-    }
-    return this;
+    return null; // do not annotate the following lines
   }
 }

src/main/java/hudson/plugins/timestamper/annotator/TimestampAnnotatorFactory.java

@@ -29,6 +29,8 @@
 import hudson.plugins.timestamper.TimestamperConfig;
 import hudson.plugins.timestamper.format.TimestampFormatter;
 
+import javax.annotation.CheckForNull;
+
 import org.kohsuke.stapler.Stapler;
 import org.kohsuke.stapler.StaplerRequest;
 
@@ -61,10 +63,9 @@ public ConsoleAnnotator<Object> newInstance(Object context) {
    * @param request
    * @return the offset in bytes
    */
-  private static long getOffset(StaplerRequest request) {
-    // Rare case where a Jenkins slave is put offline, while build job is still
-    // running
-    if (null == request) {
+  private static long getOffset(@CheckForNull StaplerRequest request) {
+    if (request == null) {
+      // JENKINS-16778: The request can be null when the slave goes off-line.
       return 0;
     }
     String path = request.getPathInfo();

src/main/java/hudson/plugins/timestamper/format/TimestampFormatterImpl.java

@@ -23,12 +23,14 @@
  */
 package hudson.plugins.timestamper.format;
 
+import static com.google.common.base.Preconditions.checkNotNull;
 import hudson.MarkupText;
 import hudson.plugins.timestamper.Timestamp;
 
 import java.io.Serializable;
 import java.util.Date;
 
+import javax.annotation.CheckForNull;
 import javax.annotation.concurrent.Immutable;
 import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
@@ -53,6 +55,7 @@ public final class TimestampFormatterImpl implements TimestampFormatter {
    * Function that converts a time-stamp to a formatted string representation of
    * that time-stamp.
    */
+  @CheckForNull
   private final Function<Timestamp, String> formatTimestamp;
 
   /**
@@ -67,6 +70,7 @@ public final class TimestampFormatterImpl implements TimestampFormatter {
    */
   public TimestampFormatterImpl(String systemTimeFormat,
       String elapsedTimeFormat, HttpServletRequest request) {
+
     if (request == null) {
       // JENKINS-16778: The request can be null when the slave goes off-line.
       formatTimestamp = null;
@@ -122,7 +126,7 @@ private static class SystemTimeFormatFunction implements
     private final String systemTimeFormat;
 
     SystemTimeFormatFunction(String systemTimeFormat) {
-      this.systemTimeFormat = systemTimeFormat;
+      this.systemTimeFormat = checkNotNull(systemTimeFormat);
     }
 
     @Override
@@ -143,7 +147,7 @@ private static class ElapsedTimeFormatFunction implements
     private final String elapsedTimeFormat;
 
     ElapsedTimeFormatFunction(String elapsedTimeFormat) {
-      this.elapsedTimeFormat = elapsedTimeFormat;
+      this.elapsedTimeFormat = checkNotNull(elapsedTimeFormat);
     }
 
     @Override

src/main/java/hudson/plugins/timestamper/io/DumpTimestamps.java

@@ -31,6 +31,7 @@
 import java.util.List;
 
 import com.google.common.base.Joiner;
+import com.google.common.io.CountingInputStream;
 import com.google.common.io.Files;
 
 /**
@@ -57,7 +58,7 @@ public static void main(String... args) throws IOException {
     File timestamperDir = new File(Joiner.on(' ').join(args));
     System.out.println("timestamps");
     dump(TimestampsWriter.timestampsFile(timestamperDir), 1, System.out);
-    File timeShiftsFile = TimestampsReader.timeShiftsFile(timestamperDir);
+    File timeShiftsFile = TimeShiftsReader.timeShiftsFile(timestamperDir);
     if (timeShiftsFile.isFile()) {
       System.out.println("timeshifts");
       dump(timeShiftsFile, 2, System.out);
@@ -67,11 +68,11 @@ public static void main(String... args) throws IOException {
   private static void dump(File file, int columns, PrintStream output)
       throws IOException {
     final byte[] fileContents = Files.toByteArray(file);
-    TimestampsReader.InputStreamByteReader byteReader = new TimestampsReader.InputStreamByteReader(
+    CountingInputStream inputStream = new CountingInputStream(
         new ByteArrayInputStream(fileContents));
     List<Long> values = new ArrayList<Long>();
-    while (byteReader.bytesRead < fileContents.length) {
-      values.add(Varint.read(byteReader));
+    while (inputStream.getCount() < fileContents.length) {
+      values.add(Varint.read(inputStream));
       if (values.size() == columns) {
         output.println(Joiner.on('\t').join(values));
         values.clear();