Fix Editor creating fields before client Grid can attach them (#16214)

This patch includes some race condition handling. Change-Id: I6ab3cf15a67de722181b2718ab85b6b4a6bcb997
vaadin · Jan 20, 2015 · 74976a7 · 74976a7
1 parent b9360a2
commit 74976a7
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 59 deletions.
diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java
@@ -210,8 +210,13 @@ public CustomEditorHandler() {
 
                 @Override
                 public void bind(final int rowIndex) {
-                    serverInitiated = true;
+                    // call this finally to avoid issues with editing on init
-                    GridConnector.this.getWidget().editRow(rowIndex);
+                    Scheduler.get().scheduleFinally(new ScheduledCommand() {
+                        @Override
+                        public void execute() {
+                            GridConnector.this.getWidget().editRow(rowIndex);
+                        }
+                    });
                 }
 
                 @Override
@@ -223,7 +228,6 @@ public void cancel(int rowIndex) {
                 @Override
                 public void confirmBind(final boolean bindSucceeded) {
                     endRequest(bindSucceeded);
-
                 }
 
                 @Override
@@ -235,18 +239,14 @@ public void confirmSave(boolean saveSucceeded) {
 
         @Override
         public void bind(EditorRequest<JsonObject> request) {
-            if (!handleServerInitiated(request)) {
+            startRequest(request);
-                startRequest(request);
+            rpc.bind(request.getRowIndex());
-                rpc.bind(request.getRowIndex());
-            }
         }
 
         @Override
         public void save(EditorRequest<JsonObject> request) {
-            if (!handleServerInitiated(request)) {
+            startRequest(request);
-                startRequest(request);
+            rpc.save(request.getRowIndex());
-                rpc.save(request.getRowIndex());
-            }
         }
 
         @Override
@@ -296,11 +296,13 @@ private boolean handleServerInitiated(EditorRequest<?> request) {
         }
 
         private void startRequest(EditorRequest<?> request) {
+            assert currentRequest == null : "Earlier request not yet finished";
+
             currentRequest = request;
         }
 
         private void endRequest(boolean succeeded) {
-            assert currentRequest != null;
+            assert currentRequest != null : "Current request was null";
             /*
              * Clear current request first to ensure the state is valid if
              * another request is made in the callback.
@@ -406,6 +408,7 @@ public GridState getState() {
     protected void init() {
         super.init();
 
+        // All scroll RPC calls are executed finally to avoid issues on init
         registerRpc(GridClientRpc.class, new GridClientRpc() {
             @Override
             public void scrollToStart() {

diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java
@@ -944,7 +944,7 @@ protected static class Editor<T> {
         public static final int KEYCODE_HIDE = KeyCodes.KEY_ESCAPE;
 
         protected enum State {
-            INACTIVE, ACTIVATING, ACTIVE, SAVING
+            INACTIVE, ACTIVATING, BINDING, ACTIVE, SAVING
         }
 
         private Grid<T> grid;
@@ -1018,7 +1018,7 @@ public void run() {
         private final RequestCallback<T> bindRequestCallback = new RequestCallback<T>() {
             @Override
             public void onSuccess(EditorRequest<T> request) {
-                if (state == State.ACTIVATING) {
+                if (state == State.BINDING) {
                     state = State.ACTIVE;
                     bindTimeout.cancel();
 
@@ -1029,7 +1029,7 @@ public void onSuccess(EditorRequest<T> request) {
 
             @Override
             public void onError(EditorRequest<T> request) {
-                if (state == State.ACTIVATING) {
+                if (state == State.BINDING) {
                     state = State.INACTIVE;
                     bindTimeout.cancel();
 
@@ -1188,6 +1188,7 @@ public void setEnabled(boolean enabled) {
 
         protected void show() {
             if (state == State.ACTIVATING) {
+                state = State.BINDING;
                 bindTimeout.schedule(BIND_TIMEOUT_MS);
                 EditorRequest<T> request = new EditorRequest<T>(grid, rowIndex,
                         bindRequestCallback);

diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java
@@ -2740,14 +2740,19 @@ public void itemClick(String rowKey, String columnId,
 
             @Override
             public void bind(int rowIndex) {
-                boolean success;
+                boolean success = false;
                 try {
                     Object id = getContainerDataSource().getIdByIndex(rowIndex);
-                    doEditItem(id);
+                    if (editedItemId == null) {
-                    success = true;
+                        editedItemId = id;
+                    }
+
+                    if (editedItemId.equals(id)) {
+                        success = true;
+                        doEditItem();
+                    }
                 } catch (Exception e) {
                     handleError(e);
-                    success = false;
                 }
                 getEditorRpc().confirmBind(success);
             }
@@ -2764,13 +2769,12 @@ public void cancel(int rowIndex) {
 
             @Override
             public void save(int rowIndex) {
-                boolean success;
+                boolean success = false;
                 try {
                     saveEditor();
                     success = true;
                 } catch (Exception e) {
                     handleError(e);
-                    success = false;
                 }
                 getEditorRpc().confirmSave(success);
             }
@@ -4474,31 +4478,31 @@ public Field<?> getEditorField(Object propertyId) {
      * @param itemId
      *            the id of the item to edit
      * @throws IllegalStateException
-     *             if the editor is not enabled
+     *             if the editor is not enabled or already editing an item
      * @throws IllegalArgumentException
      *             if the {@code itemId} is not in the backing container
      * @see #setEditorEnabled(boolean)
      */
     public void editItem(Object itemId) throws IllegalStateException,
             IllegalArgumentException {
-        doEditItem(itemId);
-
-        getEditorRpc().bind(getContainerDataSource().indexOfId(itemId));
-    }
-
-    protected void doEditItem(Object itemId) {
         if (!isEditorEnabled()) {
             throw new IllegalStateException("Item editor is not enabled");
-        }
+        } else if (editedItemId != null) {
-
+            throw new IllegalStateException("Editing item + " + itemId
-        Item item = getContainerDataSource().getItem(itemId);
+                    + " failed. Item editor is already editing item "
-        if (item == null) {
+                    + editedItemId);
+        } else if (!getContainerDataSource().containsId(itemId)) {
             throw new IllegalArgumentException("Item with id " + itemId
                     + " not found in current container");
         }
+        editedItemId = itemId;
+        getEditorRpc().bind(getContainerDataSource().indexOfId(itemId));
+    }
+
+    protected void doEditItem() {
+        Item item = getContainerDataSource().getItem(editedItemId);
 
         editorFieldGroup.setItemDataSource(item);
-        editedItemId = itemId;
 
         for (Column column : getColumns()) {
             Object propertyId = column.getPropertyId();