[FIXED JENKINS-16778] Allow for a null stapler request

jenkinsci · Mar 17, 2013 · 4029ac4 · 4029ac4
1 parent 1671078
commit 4029ac4
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 39 deletions.
diff --git a/src/main/java/hudson/plugins/timestamper/TimestampFormatter.java b/src/main/java/hudson/plugins/timestamper/TimestampFormatter.java
@@ -67,6 +67,12 @@ public final class TimestampFormatter implements Serializable {
    */
   public TimestampFormatter(String systemTimeFormat, String elapsedTimeFormat,
       HttpServletRequest request) {
+    if (request == null) {
+      // JENKINS-16778: The request can be null when the slave goes off-line.
+      formatTimestamp = null;
+      return;
+    }
+
     String cookieValue = null;
     Cookie[] cookies = request.getCookies();
     if (cookies != null) {
@@ -97,6 +103,9 @@ public TimestampFormatter(String systemTimeFormat, String elapsedTimeFormat,
    *          the time-stamp to format
    */
   public void markup(MarkupText text, Timestamp timestamp) {
+    if (formatTimestamp == null) {
+      return;
+    }
     String timestampString = formatTimestamp.apply(timestamp);
     // Wrap the time-stamp in a span element, which is used to detect the
     // time-stamp when inspecting the page with Javascript.

diff --git a/src/test/java/hudson/plugins/timestamper/TimestampFormatterTest.java b/src/test/java/hudson/plugins/timestamper/TimestampFormatterTest.java
@@ -35,8 +35,6 @@
 
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
 import java.util.TimeZone;
 
 import javax.servlet.http.Cookie;
@@ -64,33 +62,47 @@ public class TimestampFormatterTest {
   @Parameters
   public static Collection<Object[]> data() {
     return Arrays.asList(new Object[][] {
-        { Collections.singletonList("system"), "00:00:42 ", "08:00:42 " },
-        { Collections.singletonList("elapsed"), "00.123 ", "00.123 " },
-        { Collections.singletonList("none"), "", "" },
-        { Collections.emptyList(), "00:00:42 ", "08:00:42 " },
-        { null, "00:00:42 ", "08:00:42 " } });
+        { request("system"), span("00:00:42 "), span("08:00:42 ") },
+        { request("elapsed"), span("00.123 "), span("00.123 ") },
+        { request("none"), span(""), span("") },
+        { request(), span("00:00:42 "), span("08:00:42 ") },
+        { request((String[]) null), span("00:00:42 "), span("08:00:42 ") },
+        { null, "", "" } });
   }
 
-  private List<String> cookieValues;
+  private static HttpServletRequest request(String... cookieValues) {
+    HttpServletRequest request = mock(HttpServletRequest.class);
+    Cookie[] cookies = null;
+    if (cookieValues != null) {
+      cookies = new Cookie[cookieValues.length];
+      for (int i = 0; i < cookieValues.length; i++) {
+        cookies[i] = new Cookie("jenkins-timestamper", cookieValues[i]);
+      }
+    }
+    when(request.getCookies()).thenReturn(cookies);
+    return request;
+  }
 
-  private String timestampString;
+  private HttpServletRequest request;
 
-  private String timestampStringInDifferentTimezone;
+  private String prefix;
+
+  private String prefixInDifferentTimezone;
 
   private TimeZone systemDefaultTimeZone;
 
   private boolean serialize;
 
   /**
-   * @param cookieValues
-   * @param timestampString
-   * @param timestampStringInDifferentTimezone
+   * @param request
+   * @param prefix
+   * @param prefixInDifferentTimezone
    */
-  public TimestampFormatterTest(List<String> cookieValues,
-      String timestampString, String timestampStringInDifferentTimezone) {
-    this.cookieValues = cookieValues;
-    this.timestampString = timestampString;
-    this.timestampStringInDifferentTimezone = timestampStringInDifferentTimezone;
+  public TimestampFormatterTest(HttpServletRequest request, String prefix,
+      String prefixInDifferentTimezone) {
+    this.request = request;
+    this.prefix = prefix;
+    this.prefixInDifferentTimezone = prefixInDifferentTimezone;
   }
 
   /**
@@ -114,26 +126,24 @@ public void tearDown() {
    */
   @Test
   public void testMarkup() {
-    assertThat(markup("line").toString(true),
-        is(span(timestampString) + "line"));
+    assertThat(markup("line").toString(true), is(prefix + "line"));
   }
 
   /**
    */
   @Test
   public void testMarkupAfterSerialization() {
     serialize = true;
-    assertThat(markup("line").toString(true),
-        is(span(timestampString) + "line"));
+    assertThat(markup("line").toString(true), is(prefix + "line"));
   }
 
   /**
    */
   @Test
   public void testMarkupElapsedTimeWithDifferentTimeZone() {
     TimeZone.setDefault(TimeZone.getTimeZone("GMT+8"));
-    assertThat(markup("line").toString(true),
-        is(span(timestampStringInDifferentTimezone) + "line"));
+    assertThat(markup("line").toString(true), is(prefixInDifferentTimezone
+        + "line"));
   }
 
   /**
@@ -142,16 +152,16 @@ public void testMarkupElapsedTimeWithDifferentTimeZone() {
   public void testMarkupElapsedTimeWithDifferentTimeZoneAfterSerialization() {
     serialize = true;
     TimeZone.setDefault(TimeZone.getTimeZone("GMT+8"));
-    assertThat(markup("line").toString(true),
-        is(span(timestampStringInDifferentTimezone) + "line"));
+    assertThat(markup("line").toString(true), is(prefixInDifferentTimezone
+        + "line"));
   }
 
   /**
    */
   @Test
   public void testMarkupThenAntTargetNote() {
     assertThat(annotate(markup("target:"), new AntTargetNote()).toString(true),
-        is(span(timestampString) + "<b class=ant-target>target</b>:"));
+        is(prefix + "<b class=ant-target>target</b>:"));
   }
 
   /**
@@ -160,15 +170,15 @@ public void testMarkupThenAntTargetNote() {
   public void testMarkupThenAntTargetNoteAfterSerialization() {
     serialize = true;
     assertThat(annotate(markup("target:"), new AntTargetNote()).toString(true),
-        is(span(timestampString) + "<b class=ant-target>target</b>:"));
+        is(prefix + "<b class=ant-target>target</b>:"));
   }
 
   /**
    */
   @Test
   public void testAntTargetNoteThenMarkup() {
     assertThat(markup(annotate("target:", new AntTargetNote())).toString(true),
-        is(span(timestampString) + "<b class=ant-target>target</b>:"));
+        is(prefix + "<b class=ant-target>target</b>:"));
   }
 
   /**
@@ -177,23 +187,14 @@ public void testAntTargetNoteThenMarkup() {
   public void testAntTargetNoteThenMarkupAfterSerialization() {
     serialize = true;
     assertThat(markup(annotate("target:", new AntTargetNote())).toString(true),
-        is(span(timestampString) + "<b class=ant-target>target</b>:"));
+        is(prefix + "<b class=ant-target>target</b>:"));
   }
 
   private MarkupText markup(String text) {
     return markup(new MarkupText(text));
   }
 
   private MarkupText markup(MarkupText markupText) {
-    HttpServletRequest request = mock(HttpServletRequest.class);
-    Cookie[] cookies = null;
-    if (cookieValues != null) {
-      cookies = new Cookie[cookieValues.size()];
-      for (int i = 0; i < cookieValues.size(); i++) {
-        cookies[i] = new Cookie("jenkins-timestamper", cookieValues.get(i));
-      }
-    }
-    when(request.getCookies()).thenReturn(cookies);
     TimestampFormatter formatter = new TimestampFormatter("HH:mm:ss ", "ss.S ",
         request);
     if (serialize) {