Refactor critical notifications handling. (#16592)

Change-Id: I235804a80b1d70a564a54953b9255416a7386fe6

2 files changed, 91 insertions, 34 deletions

diff --git a/server/src/com/vaadin/server/VaadinService.java b/server/src/com/vaadin/server/VaadinService.java
index 2aaab31dd1..be4dd045be 100644
--- a/server/src/com/vaadin/server/VaadinService.java
+++ b/server/src/com/vaadin/server/VaadinService.java
@@ -1574,30 +1574,11 @@ public abstract class VaadinService implements Serializable {
             String message, String details, String url) {
         String returnString = "";
         try {
-            if (message == null) {
-                message = details;
-            } else if (details != null) {
-                message += "<br/><br/>" + details;
-            }
-
             JsonObject appError = Json.createObject();
-            if (caption == null) {
-                appError.put("caption", Json.createNull());
-            } else {
-                appError.put("caption", caption);
-            }
-
-            if (message == null) {
-                appError.put("message", Json.createNull());
-            } else {
-                appError.put("message", message);
-            }
-
-            if (url == null) {
-                appError.put("url", Json.createNull());
-            } else {
-                appError.put("url", url);
-            }
+            putValueOrJsonNull(appError, "caption", caption);
+            putValueOrJsonNull(appError, "url", url);
+            putValueOrJsonNull(appError, "message",
+                    createCriticalNotificationMessage(message, details));
 
             JsonObject meta = Json.createObject();
             meta.put("appError", appError);
@@ -1617,6 +1598,24 @@ public abstract class VaadinService implements Serializable {
         return "for(;;);[" + returnString + "]";
     }
 
+    private static String createCriticalNotificationMessage(String message, String details) {
+        if(message == null) {
+            return details;
+        } else if(details != null) {
+            return message + "<br/><br/>" + details;
+        }
+
+        return message;
+    }
+
+    private static void putValueOrJsonNull(JsonObject json, String key, String value) {
+        if(value == null) {
+            json.put(key, Json.createNull());
+        } else {
+            json.put(key, value);
+        }
+    }
+
     /**
      * @deprecated As of 7.0. Will likely change or be removed in a future
      *             version
diff --git a/server/tests/src/com/vaadin/server/VaadinServiceTest.java b/server/tests/src/com/vaadin/server/VaadinServiceTest.java
index c2ca8b0e57..4b655e7855 100644
--- a/server/tests/src/com/vaadin/server/VaadinServiceTest.java
+++ b/server/tests/src/com/vaadin/server/VaadinServiceTest.java
@@ -15,6 +15,9 @@
  */
 package com.vaadin.server;
 
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
+
 import javax.servlet.ServletConfig;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpSessionBindingEvent;
@@ -39,6 +42,11 @@ public class VaadinServiceTest {
         }
     }
 
+    private String createCriticalNotification(String caption, String message, String details, String url) {
+        return VaadinService
+                .createCriticalNotificationJSON(caption, message, details, url);
+    }
+
     @Test
     public void testFireSessionDestroy() throws ServletException {
         ServletConfig servletConfig = new MockServletConfig();
@@ -68,16 +76,66 @@ public class VaadinServiceTest {
     }
 
     @Test
-    public void testCriticalNotificationNullHandling() {
-        for (String caption : new String[] { "some caption", null }) {
-            for (String message : new String[] { "some message", null }) {
-                for (String details : new String[] { "some details", null }) {
-                    for (String url : new String[] { "some url", null }) {
-                        VaadinService.createCriticalNotificationJSON(caption,
-                                message, details, url);
-                    }
-                }
-            }
-        }
+    public void captionIsSetToACriticalNotification() {
+        String notification =
+                createCriticalNotification("foobar", "message", "details", "url");
+
+        assertThat(notification, containsString("\"caption\":\"foobar\""));
+    }
+
+    @Test
+    public void nullCaptionIsSetToACriticalNotification() {
+        String notification =
+                createCriticalNotification(null, "message", "details", "url");
+
+        assertThat(notification, containsString("\"caption\":null"));
+    }
+
+    @Test
+    public void messageWithDetailsIsSetToACriticalNotification() {
+        String notification =
+                createCriticalNotification("caption", "foo", "bar", "url");
+
+        assertThat(notification, containsString("\"message\":\"foo<br/><br/>bar\""));
+    }
+
+    @Test
+    public void nullMessageIsReplacedByDetailsInACriticalNotification() {
+        String notification =
+                createCriticalNotification("caption", null, "foobar", "url");
+
+        assertThat(notification, containsString("\"message\":\"foobar\""));
+    }
+
+    @Test
+    public void nullMessageIsSetToACriticalNotification() {
+        String notification =
+                createCriticalNotification("caption", null, null, "url");
+
+        assertThat(notification, containsString("\"message\":null"));
+    }
+
+    @Test
+    public void messageSetToACriticalNotification() {
+        String notification =
+                createCriticalNotification("caption", "foobar", null, "url");
+
+        assertThat(notification, containsString("\"message\":\"foobar\""));
+    }
+
+    @Test
+    public void urlIsSetToACriticalNotification() {
+        String notification =
+                createCriticalNotification("caption", "message", "details", "foobar");
+
+        assertThat(notification, containsString("\"url\":\"foobar\""));
+    }
+
+    @Test
+    public void nullUrlIsSetToACriticalNotification() {
+        String notification =
+                createCriticalNotification("caption", "message", "details", null);
+
+        assertThat(notification, containsString("\"url\":null"));
     }
 }