Ignore component removal if the component has no parent (#4537)

Fixes #4528
vaadin · Aug 28, 2018 · f47daba · f47daba
1 parent db73c7b
commit f47daba
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 16 deletions.
diff --git a/flow-server/src/main/java/com/vaadin/flow/component/HasComponents.java b/flow-server/src/main/java/com/vaadin/flow/component/HasComponents.java
@@ -17,6 +17,8 @@
 
 import java.util.Objects;
 
+import org.slf4j.LoggerFactory;
+
 import com.vaadin.flow.dom.Element;
 
 /**
@@ -62,7 +64,13 @@ default void remove(Component... components) {
         for (Component component : components) {
             Objects.requireNonNull(component,
                     "Component to remove cannot be null");
-            if (getElement().equals(component.getElement().getParent())) {
+            Element parent = component.getElement().getParent();
+            if (parent == null) {
+                LoggerFactory.getLogger(HasComponents.class).debug(
+                        "Remove of a component with no parent does nothing.");
+                return;
+            }
+            if (getElement().equals(parent)) {
                 getElement().removeChild(component.getElement());
             } else {
                 throw new IllegalArgumentException("The given component ("
@@ -84,7 +92,7 @@ default void removeAll() {
     /**
      * Adds the given component as child of this component at the specific
      * index.
-     * 
+     *
      * @param index
      *            the index, where the component will be added. If {@code index}
      *            is negative, the component will be added as the first child of
@@ -109,7 +117,7 @@ default void addComponentAtIndex(int index, Component component) {
 
     /**
      * Adds the given component as the first child of this component.
-     * 
+     *
      * @param component
      *            the component to add, value should not be null
      */

diff --git a/flow-server/src/test/java/com/vaadin/flow/component/HasComponentsTest.java b/flow-server/src/test/java/com/vaadin/flow/component/HasComponentsTest.java
@@ -44,9 +44,8 @@ public void insertComponentAtIndex() {
         innerComponent.setId("insert-component-index");
         component.addComponentAtIndex(2, innerComponent);
         checkChildren(4, component);
-        Assert.assertEquals(innerComponent.getId(),
-                component.getElement().getChild(2).getComponent().get()
-                        .getId());
+        Assert.assertEquals(innerComponent.getId(), component.getElement()
+                .getChild(2).getComponent().get().getId());
     }
 
     @Test
@@ -67,9 +66,45 @@ public void insertComponentIndexGreaterThanChildrenNumber() {
         innerComponent.setId("insert-component-index-greater");
         component.addComponentAtIndex(100, innerComponent);
         checkChildren(4, component);
-        Assert.assertEquals(innerComponent.getId(), component.getElement()
-                .getChild((int) (component.getChildren().count() - 1))
-                .getComponent().get().getId());
+        Assert.assertEquals(innerComponent.getId(),
+                component.getElement()
+                        .getChild((int) (component.getChildren().count() - 1))
+                        .getComponent().get().getId());
+    }
+
+    @Test
+    public void remove_removeComponentWithNoParent() {
+        TestComponent component = createTestStructure();
+        TestComponent innerComponent = new TestComponent();
+
+        // No any exception is thrown
+        component.remove(innerComponent);
+    }
+
+    @Test
+    public void remove_removeComponentWithCorrectParent() {
+        TestComponent component = createTestStructure();
+        TestComponent innerComponent = new TestComponent();
+
+        long size = component.getChildren().count();
+
+        component.add(innerComponent);
+
+        component.remove(innerComponent);
+
+        Assert.assertEquals(size, component.getChildren().count());
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void remove_removeComponentWithDifferentParent() {
+        TestComponent component = createTestStructure();
+
+        TestComponent another = createTestStructure();
+        TestComponent innerComponent = new TestComponent();
+
+        another.add(innerComponent);
+
+        component.remove(innerComponent);
     }
 
     private TestComponent createTestStructure() {

diff --git a/flow-server/src/test/java/com/vaadin/flow/component/UITest.java b/flow-server/src/test/java/com/vaadin/flow/component/UITest.java
@@ -307,13 +307,6 @@ public void removeComponent() {
         ComponentTest.assertChildren(ui);
     }
 
-    @Test(expected = IllegalArgumentException.class)
-    public void removeNotChildComponent() {
-        UI ui = new UI();
-        Text text = new Text("foo");
-        ui.remove(text);
-    }
-
     @Test
     public void setSession_attachEventIsFired()
             throws InvalidRouteConfigurationException {