Adds error handling to Grid Editor (#15556)

Change-Id: I93551548aad280c4e0193d65a066976d40d65a86

3 files changed, 180 insertions, 87 deletions

diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java
index 5a68bbf4b8..0414e82680 100644
--- a/client/src/com/vaadin/client/connectors/GridConnector.java
+++ b/client/src/com/vaadin/client/connectors/GridConnector.java
@@ -221,14 +221,14 @@ public class GridConnector extends AbstractHasComponentsConnector implements
                 }
 
                 @Override
-                public void confirmBind() {
-                    endRequest();
+                public void confirmBind(final boolean bindSucceeded) {
+                    endRequest(bindSucceeded);
 
                 }
 
                 @Override
-                public void confirmSave() {
-                    endRequest();
+                public void confirmSave(boolean saveSucceeded) {
+                    endRequest(saveSucceeded);
                 }
             });
         }
@@ -288,6 +288,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements
 
             if (serverInitiated) {
                 serverInitiated = false;
+                request.success();
                 return true;
             } else {
                 return false;
@@ -296,10 +297,9 @@ public class GridConnector extends AbstractHasComponentsConnector implements
 
         private void startRequest(EditorRequest<?> request) {
             currentRequest = request;
-            request.startAsync();
         }
 
-        private void endRequest() {
+        private void endRequest(boolean succeeded) {
             assert currentRequest != null;
             /*
              * Clear current request first to ensure the state is valid if
@@ -307,7 +307,11 @@ public class GridConnector extends AbstractHasComponentsConnector implements
              */
             EditorRequest<?> request = currentRequest;
             currentRequest = null;
-            request.complete();
+            if (succeeded) {
+                request.success();
+            } else {
+                request.fail();
+            }
         }
     }
 
diff --git a/client/src/com/vaadin/client/widget/grid/EditorHandler.java b/client/src/com/vaadin/client/widget/grid/EditorHandler.java
index f834143a45..07ec1b231c 100644
--- a/client/src/com/vaadin/client/widget/grid/EditorHandler.java
+++ b/client/src/com/vaadin/client/widget/grid/EditorHandler.java
@@ -36,12 +36,9 @@ public interface EditorHandler<T> {
      * request is callback-based to facilitate usage with remote or otherwise
      * asynchronous data sources.
      * <p>
-     * In any of the EditorHandler methods, an implementation may call
-     * {@link EditorRequest#startAsync()} to signal the caller that the request
-     * is handled asynchronously. In that case, {@link EditorRequest#complete()}
-     * must be called when the request is complete.
-     * <p>
-     * TODO Should have a mechanism for signaling a failed request to the caller
+     * An implementation must call either {@link #success()} or {@link #fail()},
+     * according to whether the operation was a success or failed during
+     * execution, respectively.
      * 
      * @param <T>
      *            the row data type
@@ -56,13 +53,28 @@ public interface EditorHandler<T> {
          *            the row data type
          */
         public interface RequestCallback<T> {
-            public void onResponse(EditorRequest<T> request);
+            /**
+             * The method that must be called when the request has been
+             * processed correctly.
+             * 
+             * @param request
+             *            the original request object
+             */
+            public void onSuccess(EditorRequest<T> request);
+
+            /**
+             * The method that must be called when processing the request has
+             * produced an aborting error.
+             * 
+             * @param request
+             *            the original request object
+             */
+            public void onError(EditorRequest<T> request);
         }
 
         private Grid<T> grid;
         private int rowIndex;
         private RequestCallback<T> callback;
-        private boolean async = false;
         private boolean completed = false;
 
         /**
@@ -122,19 +134,6 @@ public interface EditorHandler<T> {
             return w;
         }
 
-        public boolean isAsync() {
-            return async;
-        }
-
-        /**
-         * Marks this request as asynchronous. If this method is invoked, the
-         * caller must also ensure that {@link #complete()} is invoked once the
-         * request is finished.
-         */
-        public void startAsync() {
-            async = true;
-        }
-
         /**
          * Completes this request. The request can only be completed once. This
          * method should only be called by an EditorHandler implementer if the
@@ -145,26 +144,52 @@ public interface EditorHandler<T> {
          * @throws IllegalStateException
          *             if the request is already completed
          */
-        public void complete() {
+        private void complete() {
             if (completed) {
                 throw new IllegalStateException(
                         "An EditorRequest must be completed exactly once");
             }
             completed = true;
+        }
+
+        /**
+         * Informs Grid that the editor request was a success.
+         */
+        public void success() {
+            complete();
             if (callback != null) {
-                callback.onResponse(this);
+                callback.onSuccess(this);
             }
         }
+
+        /**
+         * Informs Grid that an error occurred while trying to process the
+         * request.
+         */
+        public void fail() {
+            complete();
+            if (callback != null) {
+                callback.onError(this);
+            }
+        }
+
+        /**
+         * Checks whether the request is completed or not.
+         * 
+         * @return <code>true</code> iff the request is completed
+         */
+        public boolean isCompleted() {
+            return completed;
+        }
     }
 
     /**
      * Binds row data to the editor widgets. Called by the editor when it is
      * opened for editing.
      * <p>
-     * An implementation may call {@link EditorRequest#startAsync()
-     * request.startAsync()} to signal the caller that the request is handled
-     * asynchronously. In that case, {@link EditorRequest#complete()} must be
-     * called once the binding is complete.
+     * The implementation <em>must</em> call either
+     * {@link EditorRequest#success()} or {@link EditorRequest#fail()} to signal
+     * a successful or a failed (respectively) bind action.
      * 
      * @param request
      *            the data binding request
@@ -177,10 +202,11 @@ public interface EditorHandler<T> {
      * Called by the editor when editing is cancelled. This method may have an
      * empty implementation in case no special processing is required.
      * <p>
-     * An implementation may call {@link EditorRequest#startAsync()
-     * request.startAsync()} to signal the caller that the request is handled
-     * asynchronously. In that case, {@link EditorRequest#complete()} must be
-     * called once the cancel operation is complete.
+     * In contrast to {@link #bind(EditorRequest)} and
+     * {@link #save(EditorRequest)}, any calls to
+     * {@link EditorRequest#success()} or {@link EditorRequest#fail()} have no
+     * effect on the outcome of the cancel action. The editor is already closed
+     * when this method is called.
      * 
      * @param request
      *            the cancel request
@@ -193,10 +219,9 @@ public interface EditorHandler<T> {
      * Commits changes in the currently active edit to the data source. Called
      * by the editor when changes are saved.
      * <p>
-     * An implementation may call {@link EditorRequest#startAsync()
-     * request.startAsync()} to signal the caller that the request is handled
-     * asynchronously. In that case, {@link EditorRequest#complete()} must be
-     * called once the commit operation is complete.
+     * The implementation <em>must</em> call either
+     * {@link EditorRequest#success()} or {@link EditorRequest#fail()} to signal
+     * a successful or a failed (respectively) save action.
      * 
      * @param request
      *            the save request
diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java
index 18115b2a3b..a215b9df6d 100644
--- a/client/src/com/vaadin/client/widgets/Grid.java
+++ b/client/src/com/vaadin/client/widgets/Grid.java
@@ -961,6 +961,88 @@ public class Grid<T> extends ResizeComposite implements
 
         private HandlerRegistration scrollHandler;
 
+        private Button saveButton;
+        private Button cancelButton;
+
+        private static final int SAVE_TIMEOUT_MS = 5000;
+        private final Timer saveTimeout = new Timer() {
+            @Override
+            public void run() {
+                getLogger().warning(
+                        "Editor save action is taking longer than expected ("
+                                + SAVE_TIMEOUT_MS + "ms). Does your "
+                                + EditorHandler.class.getSimpleName()
+                                + " remember to call success() or fail()?");
+            }
+        };
+
+        private final RequestCallback<T> saveRequestCallback = new RequestCallback<T>() {
+            @Override
+            public void onSuccess(EditorRequest<T> request) {
+                if (state == State.SAVING) {
+                    cleanup();
+                    cancel();
+                }
+            }
+
+            @Override
+            public void onError(EditorRequest<T> request) {
+                if (state == State.SAVING) {
+                    cleanup();
+
+                    // TODO probably not the most correct thing to do...
+                    getLogger().warning(
+                            "An error occurred when trying to save the "
+                                    + "modified row");
+                }
+            }
+
+            private void cleanup() {
+                state = State.ACTIVE;
+                enableButtons(true);
+                saveTimeout.cancel();
+            }
+        };
+
+        private static final int BIND_TIMEOUT_MS = 5000;
+        private final Timer bindTimeout = new Timer() {
+            @Override
+            public void run() {
+                getLogger().warning(
+                        "Editor bind action is taking longer than expected ("
+                                + BIND_TIMEOUT_MS + "ms). Does your "
+                                + EditorHandler.class.getSimpleName()
+                                + " remember to call success() or fail()?");
+            }
+        };
+        private final RequestCallback<T> bindRequestCallback = new RequestCallback<T>() {
+            @Override
+            public void onSuccess(EditorRequest<T> request) {
+                if (state == State.ACTIVATING) {
+                    state = State.ACTIVE;
+                    bindTimeout.cancel();
+
+                    showOverlay(grid.getEscalator().getBody()
+                            .getRowElement(request.getRowIndex()));
+                }
+            }
+
+            @Override
+            public void onError(EditorRequest<T> request) {
+                if (state == State.ACTIVATING) {
+                    state = State.INACTIVE;
+                    bindTimeout.cancel();
+
+                    // TODO show something in the DOM as well?
+                    getLogger().warning(
+                            "An error occurred while trying to show the "
+                                    + "Grid editor");
+                    grid.getEscalator().setScrollLocked(Direction.VERTICAL,
+                            false);
+                }
+            }
+        };
+
         public int getRow() {
             return rowIndex;
         }
@@ -1021,8 +1103,6 @@ public class Grid<T> extends ResizeComposite implements
             EditorRequest<T> request = new EditorRequest<T>(grid, rowIndex,
                     null);
             handler.cancel(request);
-            completeIfSync(request);
-
             state = State.INACTIVE;
         }
 
@@ -1045,18 +1125,11 @@ public class Grid<T> extends ResizeComposite implements
             }
 
             state = State.SAVING;
+            enableButtons(false);
+            saveTimeout.schedule(SAVE_TIMEOUT_MS);
             EditorRequest<T> request = new EditorRequest<T>(grid, rowIndex,
-                    new RequestCallback<T>() {
-                        @Override
-                        public void onResponse(EditorRequest<T> request) {
-                            if (state == State.SAVING) {
-                                state = State.ACTIVE;
-                                cancel();
-                            }
-                        }
-                    });
+                    saveRequestCallback);
             handler.save(request);
-            completeIfSync(request);
         }
 
         /**
@@ -1115,23 +1188,10 @@ public class Grid<T> extends ResizeComposite implements
 
         protected void show() {
             if (state == State.ACTIVATING) {
+                bindTimeout.schedule(BIND_TIMEOUT_MS);
                 EditorRequest<T> request = new EditorRequest<T>(grid, rowIndex,
-                        new RequestCallback<T>() {
-                            @Override
-                            public void onResponse(EditorRequest<T> request) {
-                                if (state == State.ACTIVATING) {
-                                    state = State.ACTIVE;
-                                    showOverlay(grid
-                                            .getEscalator()
-                                            .getBody()
-                                            .getRowElement(
-                                                    request.getRowIndex()));
-                                }
-                            }
-                        });
+                        bindRequestCallback);
                 handler.bind(request);
-                completeIfSync(request);
-
                 grid.getEscalator().setScrollLocked(Direction.VERTICAL, true);
             }
         }
@@ -1225,30 +1285,31 @@ public class Grid<T> extends ResizeComposite implements
                 }
             }
 
-            Button save = new Button();
-            save.setText("Save");
-            save.setStyleName(styleName + "-save");
-            save.addClickHandler(new ClickHandler() {
+            saveButton = new Button();
+            saveButton.setText("Save");
+            saveButton.setStyleName(styleName + "-save");
+            saveButton.addClickHandler(new ClickHandler() {
                 @Override
                 public void onClick(ClickEvent event) {
-                    // TODO should have a mechanism for handling failed save
                     save();
                 }
             });
-            setBounds(save.getElement(), 0, tr.getOffsetHeight() + 5, 50, 25);
-            attachWidget(save, editorOverlay);
-
-            Button cancel = new Button();
-            cancel.setText("Cancel");
-            cancel.setStyleName(styleName + "-cancel");
-            cancel.addClickHandler(new ClickHandler() {
+            setBounds(saveButton.getElement(), 0, tr.getOffsetHeight() + 5, 50,
+                    25);
+            attachWidget(saveButton, editorOverlay);
+
+            cancelButton = new Button();
+            cancelButton.setText("Cancel");
+            cancelButton.setStyleName(styleName + "-cancel");
+            cancelButton.addClickHandler(new ClickHandler() {
                 @Override
                 public void onClick(ClickEvent event) {
                     cancel();
                 }
             });
-            setBounds(cancel.getElement(), 55, tr.getOffsetHeight() + 5, 50, 25);
-            attachWidget(cancel, editorOverlay);
+            setBounds(cancelButton.getElement(), 55, tr.getOffsetHeight() + 5,
+                    50, 25);
+            attachWidget(cancelButton, editorOverlay);
         }
 
         protected void hideOverlay() {
@@ -1309,10 +1370,9 @@ public class Grid<T> extends ResizeComposite implements
             editorOverlay.getStyle().setLeft(-grid.getScrollLeft(), Unit.PX);
         }
 
-        private void completeIfSync(EditorRequest<T> request) {
-            if (!request.isAsync()) {
-                request.complete();
-            }
+        private void enableButtons(boolean enabled) {
+            saveButton.setEnabled(enabled);
+            cancelButton.setEnabled(enabled);
         }
     }
 
@@ -3475,6 +3535,10 @@ public class Grid<T> extends ResizeComposite implements
 
     @Override
     public void setEnabled(boolean enabled) {
+        if (enabled == this.enabled) {
+            return;
+        }
+
         this.enabled = enabled;
         getElement().setTabIndex(enabled ? 0 : -1);
         getEscalator().setScrollLocked(Direction.VERTICAL, !enabled);