Add Notification.close() to hide a notification from the server (#10483)

Also fixes the problem that notifications were never removed on the server side

Fixes #2114, fixes #10481

4 files changed, 152 insertions, 16 deletions

diff --git a/client/src/main/java/com/vaadin/client/ui/notification/NotificationConnector.java b/client/src/main/java/com/vaadin/client/ui/notification/NotificationConnector.java
index 038d70dbf3..a0c49af8a3 100644
--- a/client/src/main/java/com/vaadin/client/ui/notification/NotificationConnector.java
+++ b/client/src/main/java/com/vaadin/client/ui/notification/NotificationConnector.java
@@ -36,6 +36,8 @@ import com.vaadin.ui.Notification;
 @Connect(value = Notification.class)
 public class NotificationConnector extends AbstractExtensionConnector {
 
+    private VNotification notification;
+
     @Override
     public NotificationState getState() {
         return (NotificationState) super.getState();
@@ -44,21 +46,33 @@ public class NotificationConnector extends AbstractExtensionConnector {
     @Override
     protected void extend(ServerConnector target) {
         NotificationState state = getState();
-        VNotification n = VNotification.showNotification(
-                target.getConnection(),
-                state.caption, state.description,
-                state.htmlContentAllowed, getResourceUrl("icon"),
-                state.styleName, state.position, state.delay);
+        notification = VNotification.showNotification(target.getConnection(),
+                state.caption, state.description, state.htmlContentAllowed,
+                getResourceUrl("icon"), state.styleName, state.position,
+                state.delay);
 
-        n.addCloseHandler(new CloseHandler<PopupPanel>() {
+        notification.addCloseHandler(new CloseHandler<PopupPanel>() {
 
             @Override
             public void onClose(CloseEvent<PopupPanel> event) {
-              NotificationServerRpc rpc =
-                      getRpcProxy(NotificationServerRpc.class);
-              rpc.closed();
+                if (getParent() == null) {
+                    // Unregistered already
+                    return;
+                }
+                NotificationServerRpc rpc = getRpcProxy(
+                        NotificationServerRpc.class);
+                rpc.closed();
+                notification = null;
             }
         });
     }
 
+    @Override
+    public void onUnregister() {
+        super.onUnregister();
+        if (notification != null) {
+            notification.hide();
+            notification = null;
+        }
+    }
 }
diff --git a/server/src/main/java/com/vaadin/ui/Notification.java b/server/src/main/java/com/vaadin/ui/Notification.java
index 24327a8cc4..9a3bc8198a 100644
--- a/server/src/main/java/com/vaadin/ui/Notification.java
+++ b/server/src/main/java/com/vaadin/ui/Notification.java
@@ -20,6 +20,7 @@ import java.io.Serializable;
 import java.lang.reflect.Method;
 
 import com.vaadin.event.ConnectorEvent;
+import com.vaadin.event.HasUserOriginated;
 import com.vaadin.server.AbstractExtension;
 import com.vaadin.server.Page;
 import com.vaadin.server.Resource;
@@ -125,8 +126,9 @@ public class Notification extends AbstractExtension {
      *
      * @since 8.2
      */
-    private NotificationServerRpc rpc = () -> fireEvent(
-            new CloseEvent(Notification.this));
+    private NotificationServerRpc rpc = () -> {
+        close(true);
+    };
 
     /**
      * Creates a "humanized" notification message.
@@ -391,6 +393,37 @@ public class Notification extends AbstractExtension {
         extend(page.getUI());
     }
 
+    /**
+     * Closes (hides) the notification.
+     * <p>
+     * If the notification is not shown, does nothing.
+     *
+     * @since
+     */
+    public void close() {
+        close(false);
+    }
+
+    /**
+     * Closes (hides) the notification.
+     * <p>
+     * If the notification is not shown, does nothing.
+     *
+     * @param userOriginated
+     *            <code>true</code> if the notification was closed because the
+     *            user clicked on it, <code>false</code> if the notification was
+     *            closed from the server
+     * @since
+     */
+    protected void close(boolean userOriginated) {
+        if (!isAttached()) {
+            return;
+        }
+
+        remove();
+        fireEvent(new CloseEvent(this, userOriginated));
+    }
+
     @Override
     protected NotificationState getState() {
         return (NotificationState) super.getState();
@@ -498,13 +531,21 @@ public class Notification extends AbstractExtension {
      *
      * @since 8.2
      */
-    public static class CloseEvent extends ConnectorEvent {
+    public static class CloseEvent extends ConnectorEvent
+            implements HasUserOriginated {
+
+        private boolean userOriginated;
 
         /**
          * @param source
          */
         public CloseEvent(Notification source) {
+            this(source, true);
+        }
+
+        public CloseEvent(Notification source, boolean userOriginated) {
             super(source);
+            this.userOriginated = userOriginated;
         }
 
         /**
@@ -515,6 +556,11 @@ public class Notification extends AbstractExtension {
         public Notification getNotification() {
             return (Notification) getSource();
         }
+
+        @Override
+        public boolean isUserOriginated() {
+            return userOriginated;
+        }
     }
 
     /**
diff --git a/uitest/src/main/java/com/vaadin/tests/elements/notification/NotificationGetTypeAndDescription.java b/uitest/src/main/java/com/vaadin/tests/elements/notification/NotificationGetTypeAndDescription.java
index 6cd99cb53d..4ac410885c 100644
--- a/uitest/src/main/java/com/vaadin/tests/elements/notification/NotificationGetTypeAndDescription.java
+++ b/uitest/src/main/java/com/vaadin/tests/elements/notification/NotificationGetTypeAndDescription.java
@@ -1,14 +1,21 @@
 package com.vaadin.tests.elements.notification;
 
+import java.util.ArrayList;
+import java.util.List;
+
+import com.vaadin.annotations.PreserveOnRefresh;
+import com.vaadin.annotations.Widgetset;
 import com.vaadin.server.VaadinRequest;
-import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.tests.components.AbstractTestUIWithLog;
 import com.vaadin.ui.Button;
 import com.vaadin.ui.Button.ClickEvent;
 import com.vaadin.ui.Button.ClickListener;
 import com.vaadin.ui.Notification;
 import com.vaadin.ui.Notification.Type;
 
-public class NotificationGetTypeAndDescription extends AbstractTestUI {
+@Widgetset("com.vaadin.DefaultWidgetSet")
+@PreserveOnRefresh
+public class NotificationGetTypeAndDescription extends AbstractTestUIWithLog {
 
     private static final Type[] types = { Type.WARNING_MESSAGE,
             Type.ERROR_MESSAGE, Type.HUMANIZED_MESSAGE,
@@ -34,6 +41,19 @@ public class NotificationGetTypeAndDescription extends AbstractTestUI {
         btn.setId("showid");
         btn.addClickListener(event -> Notification.show("test"));
         addComponent(btn);
+
+        Button hide = new Button("Hide all notifications");
+        hide.setId("hide");
+        hide.addClickListener(event -> {
+            List<Notification> notifications = new ArrayList<>();
+            getAllChildrenIterable(getUI()).forEach(conn -> {
+                if (conn instanceof Notification) {
+                    notifications.add((Notification) conn);
+                }
+            });
+            notifications.forEach(Notification::close);
+        });
+        addComponent(hide);
     }
 
     @Override
@@ -55,8 +75,12 @@ public class NotificationGetTypeAndDescription extends AbstractTestUI {
 
         @Override
         public void buttonClick(ClickEvent event) {
-            Notification.show(captions[index], descriptions[index],
-                    types[index]);
+            Notification n = Notification.show(captions[index],
+                    descriptions[index], types[index]);
+            n.addCloseListener(e -> {
+                log("Notification (" + descriptions[index] + ") closed "
+                        + (e.isUserOriginated() ? "by user" : "from server"));
+            });
         }
 
     }
diff --git a/uitest/src/test/java/com/vaadin/tests/elements/notification/NotificationCloseEventTest.java b/uitest/src/test/java/com/vaadin/tests/elements/notification/NotificationCloseEventTest.java
new file mode 100644
index 0000000000..e8db79d95c
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/elements/notification/NotificationCloseEventTest.java
@@ -0,0 +1,52 @@
+package com.vaadin.tests.elements.notification;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.NotificationElement;
+import com.vaadin.tests.tb3.SingleBrowserTest;
+
+public class NotificationCloseEventTest extends SingleBrowserTest {
+
+    @Override
+    protected Class<?> getUIClass() {
+        return NotificationGetTypeAndDescription.class;
+    }
+
+    @Test
+    public void testCloseByUser() {
+        openTestURL();
+        ButtonElement error = $(ButtonElement.class).caption("error").first();
+        error.click();
+        $(NotificationElement.class).get(0).close();
+        Assert.assertEquals("1. Notification (error) closed by user",
+                getLogRow(0));
+    }
+
+    @Test
+    public void testCloseByServer() {
+        openTestURL();
+        ButtonElement warning = $(ButtonElement.class).caption("warning")
+                .first();
+        warning.click();
+        ButtonElement close = $(ButtonElement.class)
+                .caption("Hide all notifications").first();
+        close.click();
+
+        Assert.assertEquals("1. Notification (warning) closed from server",
+                getLogRow(0));
+    }
+
+    @Test
+    public void notificationsStayAwayAfterRefresh() {
+        openTestURL();
+        ButtonElement warning = $(ButtonElement.class).caption("warning")
+                .first();
+        warning.click();
+        $(NotificationElement.class).first().close();
+        openTestURL();
+
+        Assert.assertEquals(0, $(NotificationElement.class).all().size());
+    }
+}