JENKINS-29059: Display cron run times after warnings if warnings occur.

core/src/main/java/hudson/triggers/TimerTrigger.java

@@ -35,6 +35,7 @@ import hudson.scheduler.Hash;
 import hudson.util.FormValidation;
 import java.text.DateFormat;
 import java.util.Calendar;
+import hudson.util.ResponseObject;
 import org.kohsuke.stapler.AncestorInPath;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.QueryParameter;
@@ -83,15 +84,13 @@ public class TimerTrigger extends Trigger<BuildableItem> {
         public FormValidation doCheckSpec(@QueryParameter String value, @AncestorInPath Item item) {
             try {
                 CronTabList ctl = CronTabList.create(fixNull(value), item != null ? Hash.from(item.getFullName()) : null);
-                String msg = ctl.checkSanity();
-                if(msg!=null)   return FormValidation.warning(msg);
-                Calendar prev = ctl.previous();
-                Calendar next = ctl.next();
-                if (prev != null && next != null) {
-                    DateFormat fmt = DateFormat.getDateTimeInstance(DateFormat.FULL, DateFormat.FULL);
-                    return FormValidation.ok(Messages.TimerTrigger_would_last_have_run_at_would_next_run_at(fmt.format(prev.getTime()), fmt.format(next.getTime())));
+                ResponseObject response = new ResponseObject();
+                response = getResponseForSanity(response, ctl);
+                response = getResponseForNextRun(response, ctl);
+                if (response.hasWarning()) {
+                    return FormValidation.warning(response.getWarningAndMessge());
                 } else {
-                    return FormValidation.warning(Messages.TimerTrigger_no_schedules_so_will_never_run());
+                    return FormValidation.ok(response.getMessage());
                 }
             } catch (ANTLRException e) {
                 if (value.trim().indexOf('\n')==-1 && value.contains("**"))
@@ -99,6 +98,26 @@ public class TimerTrigger extends Trigger<BuildableItem> {
                 return FormValidation.error(e.getMessage());
             }
         }
+
+        private ResponseObject getResponseForSanity(ResponseObject response, CronTabList ctl) {
+            String msg = ctl.checkSanity();
+            if(msg!=null) {
+                return response.withExtraWarning(msg);
+            } else {
+                return response;
+            }
+        }
+
+        private ResponseObject getResponseForNextRun(ResponseObject response, CronTabList ctl) {
+            Calendar prev = ctl.previous();
+            Calendar next = ctl.next();
+            if (prev != null && next != null) {
+                DateFormat fmt = DateFormat.getDateTimeInstance(DateFormat.FULL, DateFormat.FULL);
+                return response.withExtraMessage(Messages.TimerTrigger_would_last_have_run_at_would_next_run_at(fmt.format(prev.getTime()), fmt.format(next.getTime())));
+            } else {
+                return response.withExtraWarning(Messages.TimerTrigger_no_schedules_so_will_never_run());
+            }
+        }
     }
     
     public static class TimerTriggerCause extends Cause {

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

+package hudson.util;
+
+/**
+ * A class used to contain types of string response. Extra messages and warnings
+ * can be appended to the class until a response is required.
+ * The purpose of this class is to allow methods to update the message or
+ * warning response without needing to return the entire response, allowing more
+ * complex responses to be constructed.
+ */
+public class ResponseObject {
+  private final String message;
+  private final String warning;
+
+  public ResponseObject() {
+    message = "";
+    warning = "";
+  }
+
+  public ResponseObject(String message, String warning) {
+    this.message = message;
+    this.warning = warning;
+  }
+
+  public boolean hasMessage() {
+    return !message.isEmpty();
+  }
+
+  public boolean hasWarning() {
+    return !warning.isEmpty();
+  }
+
+  public String getMessage() {
+    return message;
+  }
+
+  public String getWarning() {
+    return warning;
+  }
+  
+  public String getWarningAndMessge() {
+      if (hasWarning() && hasMessage()) {
+          return warning + "; " + message;
+      } else {
+          return warning + message;
+      }
+  }
+
+  public ResponseObject withExtraMessage(String message) {
+    String updatedMessage;
+    if (this.hasMessage()) {
+      updatedMessage = this.message + "; " + message;
+    } else {
+      updatedMessage = message;
+    }
+    return new ResponseObject(updatedMessage, warning);
+  }
+
+  public ResponseObject withExtraWarning(String warning) {
+    String updatedWarning;
+    if (this.hasWarning()) {
+      updatedWarning = this.warning + "; " + warning;
+    } else {
+      updatedWarning = warning;
+    }
+    return new ResponseObject(message, updatedWarning);
+  }
+}

test/src/test/java/hudson/util/ResponseObjectTest.java

+package hudson.util;
+
+import org.junit.Test;
+import static org.junit.Assert.*;
+
+public class ResponseObjectTest {
+
+  @Test
+  public void emptyMessageReportsAsEmpty() throws Exception {
+    // given
+    ResponseObject response = new ResponseObject();
+
+    // when
+    boolean hasMessage = response.hasMessage();
+
+    // then
+    assertFalse(hasMessage);
+  }
+
+  @Test
+  public void emptyWarningReportsAsEmpty() throws Exception {
+    // given
+    ResponseObject response = new ResponseObject();
+
+    // when
+    boolean hasWarning = response.hasWarning();
+
+    // then
+    assertFalse(hasWarning);
+  }
+
+  @Test
+  public void nonEmptyMessageReportsAsNonEmpty() throws Exception {
+    // given
+    ResponseObject response = new ResponseObject("test", "");
+
+    // when
+    boolean hasMessage = response.hasMessage();
+
+    // then
+    assertTrue(hasMessage);
+  }
+
+  @Test
+  public void nonEmptWarningReportsAsNonEmpty() throws Exception {
+    // given
+    ResponseObject response = new ResponseObject("", "test");
+
+    // when
+    boolean hasWarning = response.hasWarning();
+
+    // then
+    assertTrue(hasWarning);
+  }
+
+  @Test
+  public void messageIsReturned() throws Exception {
+    // given
+    String expected = "test123";
+    ResponseObject response = new ResponseObject(expected, "");
+
+    // when
+    String actual = response.getMessage();
+
+    // then
+    assertEquals(expected, actual);
+  }
+
+  @Test
+  public void warningIsReturned() throws Exception {
+    // given
+    String expected = "test123";
+    ResponseObject response = new ResponseObject("", expected);
+
+    // when
+    String actual = response.getWarning();
+
+    // then
+    assertEquals(expected, actual);
+  }
+
+  @Test
+  public void emptyMessageIsUpdatedCorrectly() throws Exception {
+    // given
+    String expected = "test123";
+    ResponseObject response = new ResponseObject();
+
+    // when
+    ResponseObject newResponse = response.withExtraMessage(expected);
+    String actual = newResponse.getMessage();
+
+    // then
+    assertEquals(expected, actual);
+  }
+
+  @Test
+  public void emptyWarningIsUpdatedCorrectly() throws Exception {
+    // given
+    String expected = "test123";
+    ResponseObject response = new ResponseObject();
+
+    // when
+    ResponseObject newResponse = response.withExtraWarning(expected);
+    String actual = newResponse.getWarning();
+
+    // then
+    assertEquals(expected, actual);
+  }
+
+  @Test
+  public void nonEmptyMessageIsUpdatedCorrectly() throws Exception {
+    // given
+    String extra = "123";
+    String initial = "test";
+    ResponseObject response = new ResponseObject(initial, "");
+
+    // when
+    ResponseObject newResponse = response.withExtraMessage(extra);
+    String actual = newResponse.getMessage();
+
+    // then
+    String expected = initial + "; " + extra;
+    assertEquals(expected, actual);
+  }
+
+  @Test
+  public void nonEmptyWarningIsUpdatedCorrectly() throws Exception {
+    // given
+    String extra = "123";
+    String initial = "test";
+    ResponseObject response = new ResponseObject("", initial);
+
+    // when
+    ResponseObject newResponse = response.withExtraWarning(extra);
+    String actual = newResponse.getWarning();
+
+    // then
+    String expected = initial + "; " + extra;
+    assertEquals(expected, actual);
+  }
+
+  @Test
+  public void warningAndMessageAreSeparated() throws Exception {
+    // given
+    String warning = "warning";
+    String message = "message";
+    ResponseObject response = new ResponseObject(message, warning);
+
+    // when
+    String actual = response.getWarningAndMessge();
+
+    // then
+    String expected = warning + "; " + message;
+    assertEquals(expected, actual);
+  }
+
+  @Test
+  public void onlyWarningNotSeparated() throws Exception {
+    // given
+    String warning = "warning";
+    ResponseObject response = new ResponseObject("", warning);
+
+    // when
+    String actual = response.getWarningAndMessge();
+
+    // then
+    assertEquals(warning, actual);
+  }
+
+  @Test
+  public void onlyMessageNotSeparated() throws Exception {
+    // given
+    String message = "message";
+    ResponseObject response = new ResponseObject(message, "");
+
+    // when
+    String actual = response.getWarningAndMessge();
+
+    // then
+    assertEquals(message, actual);
+  }
+}