From 5ebfdecb1b4eb016b674f7a01257959a50047eb3 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Johannes=20Dahlstr=C3=B6m?= Date: Mon, 8 Jun 2015 17:21:58 +0300 Subject: [PATCH] Better handle exceptions when opening Grid editor (#17935) Change-Id: I68103db75c422b042988c6662da268ff9d11a306 --- server/src/com/vaadin/ui/Grid.java | 35 ++++++++---- .../server/component/grid/GridEditorTest.java | 8 ++- .../grid/GridEditorConverterNotFound.java | 56 +++++++++++++++++++ .../grid/GridEditorConverterNotFoundTest.java | 42 ++++++++++++++ 4 files changed, 129 insertions(+), 12 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/components/grid/GridEditorConverterNotFound.java create mode 100644 uitest/src/com/vaadin/tests/components/grid/GridEditorConverterNotFoundTest.java 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)); + } +} -- 2.39.5