Prevent editor from being canceled while it is being saved (#19458)

Change-Id: I062e097134035856f6c120584f48a5f3601bd9ac

server/src/main/java/com/vaadin/ui/Grid.java

@@ -4509,6 +4509,10 @@ public class Grid extends AbstractFocusable implements SelectionNotifier,
 
     private Object editedItemId = null;
     private boolean editorActive = false;
+    /**
+     * True while the editor is storing the field values, i.e. commiting the field group.
+     */
+    private boolean editorSaving = false;
     private FieldGroup editorFieldGroup = new CustomFieldGroup();
 
     private CellStyleGenerator cellStyleGenerator;
@@ -6845,7 +6849,12 @@ public class Grid extends AbstractFocusable implements SelectionNotifier,
      * @see FieldGroup#commit()
      */
     public void saveEditor() throws CommitException {
-        editorFieldGroup.commit();
+        try {
+            editorSaving = true;
+            editorFieldGroup.commit();
+        } finally {
+            editorSaving = false;
+        }
     }
 
     /**
@@ -6853,6 +6862,13 @@ public class Grid extends AbstractFocusable implements SelectionNotifier,
      * possible unsaved changes in the editor fields.
      */
     public void cancelEditor() {
+        if (editorSaving) {
+            // If the editor is already saving the values, it's too late to
+            // cancel it. This prevents item set changes from propagating during
+            // save, causing discard to be run during commit.
+            return;
+        }
+
         if (isEditorActive()) {
             getEditorRpc().cancel(
                     getContainerDataSource().indexOfId(editedItemId));

server/src/test/java/com/vaadin/tests/server/component/grid/ItemSetChangeDuringEditorCommit.java

@@ -0,0 +1,106 @@
+package com.vaadin.tests.server.component.grid;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.vaadin.data.fieldgroup.FieldGroup;
+import com.vaadin.data.fieldgroup.FieldGroup.CommitException;
+import com.vaadin.data.util.IndexedContainer;
+import com.vaadin.server.ServerRpcManager.RpcInvocationException;
+import com.vaadin.server.ServerRpcMethodInvocation;
+import com.vaadin.shared.ui.grid.EditorServerRpc;
+import com.vaadin.tests.util.MockUI;
+import com.vaadin.ui.Grid;
+import com.vaadin.ui.TextField;
+import com.vaadin.ui.UI;
+
+public class ItemSetChangeDuringEditorCommit {
+
+    private static class IndexedContainerImpl extends IndexedContainer {
+
+        public IndexedContainerImpl() {
+        }
+
+        @Override
+        public void fireItemSetChange() {
+            super.fireItemSetChange();
+        }
+    }
+
+    @Test
+    public void itemSetChangeDoesNotInterruptCommit()
+            throws RpcInvocationException, CommitException {
+        UI ui = new MockUI();
+        final IndexedContainerImpl indexedContainer = new IndexedContainerImpl();
+        indexedContainer.addContainerProperty("firstName", String.class,
+                "first");
+        indexedContainer.addContainerProperty("lastName", String.class, "last");
+        indexedContainer.addItem();
+        indexedContainer.addItem();
+
+        Grid grid = new Grid();
+        ui.setContent(grid);
+        grid.setContainerDataSource(indexedContainer);
+        grid.setEditorEnabled(true);
+        grid.getEditorFieldGroup()
+                .addCommitHandler(new FieldGroup.CommitHandler() {
+                    @Override
+                    public void preCommit(FieldGroup.CommitEvent commitEvent)
+                            throws FieldGroup.CommitException {
+                    }
+
+                    @Override
+                    public void postCommit(FieldGroup.CommitEvent commitEvent)
+                            throws FieldGroup.CommitException {
+                        indexedContainer.fireItemSetChange();
+                    }
+                });
+
+        editItem(grid, 0);
+        ((TextField) grid.getEditorFieldGroup().getField("firstName"))
+                .setValue("New first");
+        ((TextField) grid.getEditorFieldGroup().getField("lastName"))
+                .setValue("New last");
+        grid.saveEditor();
+
+        Assert.assertEquals("New first", indexedContainer
+                .getContainerProperty(grid.getEditedItemId(), "firstName")
+                .getValue());
+        Assert.assertEquals("New last", indexedContainer
+                .getContainerProperty(grid.getEditedItemId(), "lastName")
+                .getValue());
+
+        grid.cancelEditor();
+        Assert.assertFalse(grid.isEditorActive());
+
+        editItem(grid, 0);
+        Assert.assertEquals("New first",
+                ((TextField) grid.getEditorFieldGroup().getField("firstName"))
+                        .getValue());
+        Assert.assertEquals("New last",
+                ((TextField) grid.getEditorFieldGroup().getField("lastName"))
+                        .getValue());
+        saveEditor(grid, 0);
+    }
+
+    private void editItem(Grid grid, int itemIndex)
+            throws RpcInvocationException {
+        ServerRpcMethodInvocation invocation = new ServerRpcMethodInvocation(
+                grid.getConnectorId(), EditorServerRpc.class, "bind", 1);
+        invocation.setParameters(new Object[] { itemIndex });
+        grid.getRpcManager(EditorServerRpc.class.getName())
+                .applyInvocation(invocation);
+        Assert.assertTrue(grid.isEditorActive());
+
+    }
+
+    private void saveEditor(Grid grid, int itemIndex)
+            throws RpcInvocationException {
+        ServerRpcMethodInvocation invocation = new ServerRpcMethodInvocation(
+                grid.getConnectorId(), EditorServerRpc.class, "save", 1);
+        invocation.setParameters(new Object[] { itemIndex });
+        grid.getRpcManager(EditorServerRpc.class.getName())
+                .applyInvocation(invocation);
+
+    }
+}