Better handle exceptions when opening Grid editor (#17935)

Change-Id: I68103db75c422b042988c6662da268ff9d11a306

4 files changed, 129 insertions, 12 deletions

diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java
index 2c442d6e43..88a3195857 100644
--- a/server/src/com/vaadin/ui/Grid.java
+++ b/server/src/com/vaadin/ui/Grid.java
@@ -3868,7 +3868,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
 
             @Override
             public void bind(int rowIndex) {
-                boolean success = false;
+                Exception exception = null;
                 try {
                     Object id = getContainerDataSource().getIdByIndex(rowIndex);
                     if (editedItemId == null) {
@@ -3876,13 +3876,20 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
                     }
 
                     if (editedItemId.equals(id)) {
-                        success = true;
                         doEditItem();
                     }
                 } catch (Exception e) {
-                    handleError(e);
+                    exception = e;
+                }
+
+                if (exception != null) {
+                    handleError(exception);
+                    doCancelEditor();
+                    getEditorRpc().confirmBind(false);
+                } else {
+                    doEditItem();
+                    getEditorRpc().confirmBind(true);
                 }
-                getEditorRpc().confirmBind(success);
             }
 
             @Override
@@ -5702,13 +5709,20 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
         }
 
         Field<?> editor = editorFieldGroup.getField(propertyId);
-        if (editor == null) {
-            editor = editorFieldGroup.buildAndBind(propertyId);
-        }
 
-        if (editor.getParent() != Grid.this) {
-            assert editor.getParent() == null;
-            editor.setParent(this);
+        try {
+            if (editor == null) {
+                editor = editorFieldGroup.buildAndBind(propertyId);
+            }
+        } finally {
+            if (editor == null) {
+                editor = editorFieldGroup.getField(propertyId);
+            }
+
+            if (editor != null && editor.getParent() != Grid.this) {
+                assert editor.getParent() == null;
+                editor.setParent(this);
+            }
         }
         return editor;
     }
@@ -5803,6 +5817,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
     protected void doCancelEditor() {
         editedItemId = null;
         editorFieldGroup.discard();
+        editorFieldGroup.setItemDataSource(null);
     }
 
     void resetEditor() {
diff --git a/server/tests/src/com/vaadin/tests/server/component/grid/GridEditorTest.java b/server/tests/src/com/vaadin/tests/server/component/grid/GridEditorTest.java
index 135d7d398c..b70f17779a 100644
--- a/server/tests/src/com/vaadin/tests/server/component/grid/GridEditorTest.java
+++ b/server/tests/src/com/vaadin/tests/server/component/grid/GridEditorTest.java
@@ -184,12 +184,16 @@ public class GridEditorTest {
                 .getEditorField();
         field.setValue("New Name");
 
+        Property<?> datasource = field.getPropertyDataSource();
+
         grid.cancelEditor();
         assertFalse(grid.isEditorActive());
         assertNull(grid.getEditedItemId());
         assertFalse(field.isModified());
-        assertEquals(DEFAULT_NAME, field.getValue());
-        assertEquals(DEFAULT_NAME, field.getPropertyDataSource().getValue());
+        assertEquals("", field.getValue());
+        assertEquals(DEFAULT_NAME, datasource.getValue());
+        assertNull(field.getPropertyDataSource());
+        assertNull(grid.getEditorFieldGroup().getItemDataSource());
     }
 
     @Test(expected = IllegalArgumentException.class)
diff --git a/uitest/src/com/vaadin/tests/components/grid/GridEditorConverterNotFound.java b/uitest/src/com/vaadin/tests/components/grid/GridEditorConverterNotFound.java
new file mode 100644
index 0000000000..e5532b10ad
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/grid/GridEditorConverterNotFound.java
@@ -0,0 +1,56 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.grid;
+
+import com.vaadin.server.ErrorHandler;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUIWithLog;
+import com.vaadin.ui.Grid;
+
+public class GridEditorConverterNotFound extends AbstractTestUIWithLog {
+
+    class Foo {
+    }
+
+    @Override
+    protected void setup(VaadinRequest request) {
+
+        Grid grid = new Grid();
+
+        grid.addColumn("foo", Foo.class);
+        grid.addRow(new Foo());
+        grid.setEditorEnabled(true);
+        grid.setErrorHandler(new ErrorHandler() {
+
+            @Override
+            public void error(com.vaadin.server.ErrorEvent event) {
+                log(event.getThrowable().toString());
+            }
+        });
+
+        addComponent(grid);
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 17935;
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Grid should gracefully handle bind failures when opening editor";
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/components/grid/GridEditorConverterNotFoundTest.java b/uitest/src/com/vaadin/tests/components/grid/GridEditorConverterNotFoundTest.java
new file mode 100644
index 0000000000..468ea8da3a
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/grid/GridEditorConverterNotFoundTest.java
@@ -0,0 +1,42 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.grid;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest;
+
+public class GridEditorConverterNotFoundTest extends GridBasicFeaturesTest {
+
+    @Override
+    protected Class<?> getUIClass() {
+        // Use the correct UI with helpers from GridBasicFeatures
+        return GridEditorConverterNotFound.class;
+    }
+
+    @Test
+    public void testConverterNotFound() {
+        openTestURL();
+
+        $(GridElement.class).first().getCell(0, 0).doubleClick();
+
+        assertEquals("1. com.vaadin.data.Buffered$SourceException",
+                getLogRow(0));
+    }
+}