Fix issues from API review for 8.2 (#10342)

* Rename HierarchicalDataCommunicator#getMapper to getHierarchyMapper

* Make rpc field in Notification private

* Change DropIndexCalculator.ALWAYS_DROP_TO_END to a generic static method

* Move EditorImpl#editRow documentation to the interface level

* Correct GridDragEndEvent, GridDragStartEvent constructor javadocs

* Revert SharedState.registeredEventListeners to a Set

* Rename GridDropTarget dropAllowedOnSortedGridRows

* Rename ColumnState.contentMode to tooltipContentMode

14 files changed, 68 insertions, 71 deletions

diff --git a/server/src/main/java/com/vaadin/data/provider/HierarchicalDataCommunicator.java b/server/src/main/java/com/vaadin/data/provider/HierarchicalDataCommunicator.java
index f45bae9fea..ffdcf042c9 100644
--- a/server/src/main/java/com/vaadin/data/provider/HierarchicalDataCommunicator.java
+++ b/server/src/main/java/com/vaadin/data/provider/HierarchicalDataCommunicator.java
@@ -323,11 +323,11 @@ public class HierarchicalDataCommunicator<T> extends DataCommunicator<T> {
     }
 
     /**
-     * Returns active {@code HierarchyMapper}
+     * Returns the {@code HierarchyMapper} used by this data communicator.
      *
-     * @return the mapper
+     * @return the hierarchy mapper used by this data communicator
      */
-    protected HierarchyMapper<T, ?> getMapper() {
+    protected HierarchyMapper<T, ?> getHierarchyMapper() {
         return mapper;
     }
 }
diff --git a/server/src/main/java/com/vaadin/event/EventRouter.java b/server/src/main/java/com/vaadin/event/EventRouter.java
index b4dd43d9f4..f381e1875d 100644
--- a/server/src/main/java/com/vaadin/event/EventRouter.java
+++ b/server/src/main/java/com/vaadin/event/EventRouter.java
@@ -120,9 +120,10 @@ public class EventRouter implements MethodEventSource {
                 .addRegisteredEventListener(state, eventIdentifier);
 
         return () -> {
-            registration.remove();
-
             listenerList.remove(listenerMethod);
+            if (!hasListeners(eventType)) {
+                registration.remove();
+            }
         };
     }
 
@@ -313,5 +314,4 @@ public class EventRouter implements MethodEventSource {
         }
         return listeners;
     }
-
 }
diff --git a/server/src/main/java/com/vaadin/ui/Grid.java b/server/src/main/java/com/vaadin/ui/Grid.java
index d79fad7ec2..8074c35f0d 100644
--- a/server/src/main/java/com/vaadin/ui/Grid.java
+++ b/server/src/main/java/com/vaadin/ui/Grid.java
@@ -1414,7 +1414,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
          * @param cellDescriptionGenerator
          *            the cell description generator to set, or {@code null} to
          *            remove a previously set generator
-         * @param contentMode
+         * @param tooltipContentMode
          *            the content mode for tooltips
          * @return this column
          *
@@ -1422,9 +1422,9 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
          */
         public Column<T, V> setDescriptionGenerator(
                 DescriptionGenerator<T> cellDescriptionGenerator,
-                ContentMode contentMode) {
+                ContentMode tooltipContentMode) {
             this.descriptionGenerator = cellDescriptionGenerator;
-            getState().contentMode = contentMode;
+            getState().tooltipContentMode = tooltipContentMode;
             getGrid().getDataCommunicator().reset();
             return this;
         }
diff --git a/server/src/main/java/com/vaadin/ui/Notification.java b/server/src/main/java/com/vaadin/ui/Notification.java
index 6171c53562..82ee3b2f57 100644
--- a/server/src/main/java/com/vaadin/ui/Notification.java
+++ b/server/src/main/java/com/vaadin/ui/Notification.java
@@ -69,14 +69,6 @@ import com.vaadin.shared.ui.notification.NotificationState;
  */
 public class Notification extends AbstractExtension {
 
-    /**
-     * The server RPC.
-     *
-     * @since 8.2
-     */
-    protected NotificationServerRpc rpc = () -> fireEvent(
-            new CloseEvent(Notification.this));
-
     public enum Type {
         HUMANIZED_MESSAGE("humanized"), WARNING_MESSAGE(
                 "warning"), ERROR_MESSAGE("error"), TRAY_NOTIFICATION("tray"),
@@ -129,6 +121,14 @@ public class Notification extends AbstractExtension {
     public static final int DELAY_NONE = 0;
 
     /**
+     * The server RPC.
+     *
+     * @since 8.2
+     */
+    private NotificationServerRpc rpc = () -> fireEvent(
+            new CloseEvent(Notification.this));
+
+    /**
      * Creates a "humanized" notification message.
      *
      * The caption is rendered as plain text with HTML automatically escaped.
diff --git a/server/src/main/java/com/vaadin/ui/components/grid/DropIndexCalculator.java b/server/src/main/java/com/vaadin/ui/components/grid/DropIndexCalculator.java
index 5b2e4ead0d..08645d5b23 100644
--- a/server/src/main/java/com/vaadin/ui/components/grid/DropIndexCalculator.java
+++ b/server/src/main/java/com/vaadin/ui/components/grid/DropIndexCalculator.java
@@ -31,11 +31,14 @@ import java.io.Serializable;
 public interface DropIndexCalculator<T> extends Serializable {
 
     /**
-     * Calculator for always dropping items to the end of the target grid,
-     * regardless of drop position.
+     * Returns a calculator for always dropping items to the end of the target
+     * grid, regardless of drop position.
+     *
+     * @return the created drop index calculator
      */
-    @SuppressWarnings("rawtypes")
-    static DropIndexCalculator ALWAYS_DROP_TO_END = (event -> Integer.MAX_VALUE);
+    static <T> DropIndexCalculator<T> alwaysDropToEnd() {
+        return (GridDropEvent<T> event) -> Integer.MAX_VALUE;
+    }
 
     /**
      * Called when Items are dropped onto a target grid.
diff --git a/server/src/main/java/com/vaadin/ui/components/grid/Editor.java b/server/src/main/java/com/vaadin/ui/components/grid/Editor.java
index 33d6fc0c4f..8238f2d15c 100644
--- a/server/src/main/java/com/vaadin/ui/components/grid/Editor.java
+++ b/server/src/main/java/com/vaadin/ui/components/grid/Editor.java
@@ -104,12 +104,22 @@ public interface Editor<T> extends Serializable {
     public void cancel();
 
     /**
-     * Edits the selected row
+     * Opens the editor interface for the provided row. Scrolls the Grid to
+     * bring the row to view if it is not already visible.
      *
-     * @param rowNumber
-     *          the row to edit
+     * Note that any cell content rendered by a WidgetRenderer will not be
+     * visible in the editor row.
      *
+     * @see #setEnabled(boolean)
      * @since 8.2
+     *
+     * @param rowNumber
+     *            the row number of the edited item
+     * @throws IllegalStateException
+     *             if the editor is not enabled or already editing a different
+     *             item in buffered mode
+     * @throws IllegalArgumentException
+     *             if the {@code rowNumber} is not in the backing data provider
      */
     public void editRow(int rowNumber);
 
diff --git a/server/src/main/java/com/vaadin/ui/components/grid/EditorImpl.java b/server/src/main/java/com/vaadin/ui/components/grid/EditorImpl.java
index 571594028e..ae89a41cab 100644
--- a/server/src/main/java/com/vaadin/ui/components/grid/EditorImpl.java
+++ b/server/src/main/java/com/vaadin/ui/components/grid/EditorImpl.java
@@ -267,22 +267,6 @@ public class EditorImpl<T> extends AbstractGridExtension<T>
         rpc.cancel();
     }
 
-    /**
-     * Opens the editor interface for the provided row. Scrolls the Grid to
-     * bring the row to view if it is not already visible.
-     *
-     * Note that any cell content rendered by a WidgetRenderer will not be
-     * visible in the editor row.
-     *
-     * @param rowNumber
-     *             the row number of the edited item
-     * @throws IllegalStateException
-     *             if the editor is not enabled or already editing a different item
-     *             in buffered mode
-     * @throws IllegalArgumentException
-     *             if the {@code rowNumber} is not in the backing data provider
-     * @see #setEnabled(boolean)
-     */
     @Override
     public void editRow(int rowNumber)
             throws IllegalStateException, IllegalArgumentException {
diff --git a/server/src/main/java/com/vaadin/ui/components/grid/GridDragEndEvent.java b/server/src/main/java/com/vaadin/ui/components/grid/GridDragEndEvent.java
index 76de717978..703e63913e 100644
--- a/server/src/main/java/com/vaadin/ui/components/grid/GridDragEndEvent.java
+++ b/server/src/main/java/com/vaadin/ui/components/grid/GridDragEndEvent.java
@@ -43,7 +43,7 @@ public class GridDragEndEvent<T> extends DragEndEvent<Grid<T>> {
      * @param dropEffect
      *            Drop effect from {@code DataTransfer.dropEffect} object.
      * @param draggedItems
-     *            Set of items having been dragged.
+     *            List of items having been dragged.
      */
     public GridDragEndEvent(Grid<T> source, DropEffect dropEffect,
             List<T> draggedItems) {
diff --git a/server/src/main/java/com/vaadin/ui/components/grid/GridDragStartEvent.java b/server/src/main/java/com/vaadin/ui/components/grid/GridDragStartEvent.java
index 0372235111..37507aa13a 100644
--- a/server/src/main/java/com/vaadin/ui/components/grid/GridDragStartEvent.java
+++ b/server/src/main/java/com/vaadin/ui/components/grid/GridDragStartEvent.java
@@ -43,7 +43,7 @@ public class GridDragStartEvent<T> extends DragStartEvent<Grid<T>> {
      * @param effectAllowed
      *            Allowed effect from {@code DataTransfer.effectAllowed} object.
      * @param draggedItems
-     *            Set of items being dragged.
+     *            List of items being dragged.
      */
     public GridDragStartEvent(Grid<T> source, EffectAllowed effectAllowed,
             List<T> draggedItems) {
diff --git a/server/src/main/java/com/vaadin/ui/components/grid/GridDropTarget.java b/server/src/main/java/com/vaadin/ui/components/grid/GridDropTarget.java
index 632a787107..c2e1cb2411 100644
--- a/server/src/main/java/com/vaadin/ui/components/grid/GridDropTarget.java
+++ b/server/src/main/java/com/vaadin/ui/components/grid/GridDropTarget.java
@@ -41,7 +41,7 @@ public class GridDropTarget<T> extends DropTargetExtension<Grid<T>> {
 
     private Registration sortListenerRegistration;
     private DropMode cachedDropMode;
-    private boolean dropAllowedOnSortedGridRows = true;
+    private boolean dropAllowedOnRowsWhenSorted = true;
 
     /**
      * Extends a Grid and makes it's rows drop targets for HTML5 drag and drop.
@@ -88,18 +88,18 @@ public class GridDropTarget<T> extends DropTargetExtension<Grid<T>> {
      * in this case.
      * <p>
      * <em>NOTE: {@link DropMode#ON_GRID} is used automatically when the grid
-     * has been sorted and {@link #setDropAllowedOnSortedGridRows(boolean)} is
+     * has been sorted and {@link #setDropAllowedOnRowsWhenSorted(boolean)} is
      * {@code false} - since the drop location would not necessarily match the
      * correct row because of the sorting. During the sorting, any calls to this
      * method don't have any effect until the sorting has been removed, or
-     * {@link #setDropAllowedOnSortedGridRows(boolean)} is set back to
+     * {@link #setDropAllowedOnRowsWhenSorted(boolean)} is set back to
      * {@code true}.</em>
      *
      * @param dropMode
      *            Drop mode that describes the allowed drop locations within the
      *            Grid's row.
      * @see GridDropEvent#getDropLocation()
-     * @see #setDropAllowedOnSortedGridRows(boolean)
+     * @see #setDropAllowedOnRowsWhenSorted(boolean)
      */
     public void setDropMode(DropMode dropMode) {
         if (dropMode == null) {
@@ -147,10 +147,10 @@ public class GridDropTarget<T> extends DropTargetExtension<Grid<T>> {
      *            drops on sorted grid rows
      * @since 8.2
      */
-    public void setDropAllowedOnSortedGridRows(
+    public void setDropAllowedOnRowsWhenSorted(
             boolean dropAllowedOnSortedGridRows) {
-        if (this.dropAllowedOnSortedGridRows != dropAllowedOnSortedGridRows) {
-            this.dropAllowedOnSortedGridRows = dropAllowedOnSortedGridRows;
+        if (this.dropAllowedOnRowsWhenSorted != dropAllowedOnSortedGridRows) {
+            this.dropAllowedOnRowsWhenSorted = dropAllowedOnSortedGridRows;
 
             if (!dropAllowedOnSortedGridRows) {
 
@@ -194,8 +194,8 @@ public class GridDropTarget<T> extends DropTargetExtension<Grid<T>> {
      *         the grid
      * @since 8.2
      */
-    public boolean isDropAllowedOnSortedGridRows() {
-        return dropAllowedOnSortedGridRows;
+    public boolean isDropAllowedOnRowsWhenSorted() {
+        return dropAllowedOnRowsWhenSorted;
     }
 
     /**
diff --git a/server/src/main/java/com/vaadin/ui/components/grid/GridRowDragger.java b/server/src/main/java/com/vaadin/ui/components/grid/GridRowDragger.java
index 960fe3ec39..09e2b22c9b 100644
--- a/server/src/main/java/com/vaadin/ui/components/grid/GridRowDragger.java
+++ b/server/src/main/java/com/vaadin/ui/components/grid/GridRowDragger.java
@@ -190,7 +190,7 @@ public class GridRowDragger<T> implements Serializable {
         gridDragSource = new GridDragSource<>(source);
 
         gridDropTarget = new GridDropTarget<>(target, dropMode);
-        gridDropTarget.setDropAllowedOnSortedGridRows(false);
+        gridDropTarget.setDropAllowedOnRowsWhenSorted(false);
 
         gridDragSource.addGridDragStartListener(event -> {
             draggedItems = event.getDraggedItems();
diff --git a/server/src/test/java/com/vaadin/tests/components/grid/GridDropTargetTest.java b/server/src/test/java/com/vaadin/tests/components/grid/GridDropTargetTest.java
index d5de0ad808..cc5b31de0f 100644
--- a/server/src/test/java/com/vaadin/tests/components/grid/GridDropTargetTest.java
+++ b/server/src/test/java/com/vaadin/tests/components/grid/GridDropTargetTest.java
@@ -24,14 +24,14 @@ public class GridDropTargetTest {
     @Test
     public void dropAllowedOnSortedGridRows_defaultValue_isTrue() {
         Assert.assertTrue("Default drop allowed should be backwards compatible",
-                target.isDropAllowedOnSortedGridRows());
+                target.isDropAllowedOnRowsWhenSorted());
     }
 
     @Test
     public void dropAllowedOnSortedGridRows_notAllowed_changesDropModeWhenSorted() {
         Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
 
-        target.setDropAllowedOnSortedGridRows(false);
+        target.setDropAllowedOnRowsWhenSorted(false);
 
         Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
 
@@ -64,11 +64,11 @@ public class GridDropTargetTest {
 
         Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
 
-        target.setDropAllowedOnSortedGridRows(false);
+        target.setDropAllowedOnRowsWhenSorted(false);
 
         Assert.assertEquals(DropMode.ON_GRID, target.getDropMode());
 
-        target.setDropAllowedOnSortedGridRows(true);
+        target.setDropAllowedOnRowsWhenSorted(true);
 
         Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
     }
@@ -77,7 +77,7 @@ public class GridDropTargetTest {
     public void dropAllowedOnSortedGridRows_notAllowedBackToAllowed_changesBackToUserDefinedMode() {
         Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
 
-        target.setDropAllowedOnSortedGridRows(false);
+        target.setDropAllowedOnRowsWhenSorted(false);
 
         Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
 
@@ -85,7 +85,7 @@ public class GridDropTargetTest {
 
         Assert.assertEquals(DropMode.ON_GRID, target.getDropMode());
 
-        target.setDropAllowedOnSortedGridRows(true);
+        target.setDropAllowedOnRowsWhenSorted(true);
 
         Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
 
@@ -98,19 +98,19 @@ public class GridDropTargetTest {
     public void dropAllowedOnSortedGridRows_swappingAllowedDropOnSortedOffAndOn() {
         Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
 
-        target.setDropAllowedOnSortedGridRows(false);
+        target.setDropAllowedOnRowsWhenSorted(false);
 
         Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
 
-        target.setDropAllowedOnSortedGridRows(false);
+        target.setDropAllowedOnRowsWhenSorted(false);
 
         Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
 
-        target.setDropAllowedOnSortedGridRows(true);
+        target.setDropAllowedOnRowsWhenSorted(true);
 
         Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
 
-        target.setDropAllowedOnSortedGridRows(true);
+        target.setDropAllowedOnRowsWhenSorted(true);
 
         Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
     }
@@ -119,7 +119,7 @@ public class GridDropTargetTest {
     public void dropAllowedOnSortedGridRows_changingDropModeWhileSorted_replacesPreviouslyCachedButDoesntOverride() {
         Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
 
-        target.setDropAllowedOnSortedGridRows(false);
+        target.setDropAllowedOnRowsWhenSorted(false);
 
         Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
 
@@ -131,7 +131,7 @@ public class GridDropTargetTest {
 
         Assert.assertEquals(DropMode.ON_GRID, target.getDropMode());
         Assert.assertFalse("Changing drop mode should not have any effect here",
-                target.isDropAllowedOnSortedGridRows());
+                target.isDropAllowedOnRowsWhenSorted());
 
         grid.clearSortOrder();
 
@@ -145,7 +145,7 @@ public class GridDropTargetTest {
 
         Assert.assertEquals(DropMode.ON_GRID, target.getDropMode());
 
-        target.setDropAllowedOnSortedGridRows(true);
+        target.setDropAllowedOnRowsWhenSorted(true);
 
         Assert.assertEquals(DropMode.ON_TOP_OR_BETWEEN, target.getDropMode());
     }
diff --git a/server/src/test/java/com/vaadin/tests/server/component/grid/GridRowDraggerOneGridTest.java b/server/src/test/java/com/vaadin/tests/server/component/grid/GridRowDraggerOneGridTest.java
index 9888f42807..08b647cdde 100644
--- a/server/src/test/java/com/vaadin/tests/server/component/grid/GridRowDraggerOneGridTest.java
+++ b/server/src/test/java/com/vaadin/tests/server/component/grid/GridRowDraggerOneGridTest.java
@@ -15,8 +15,8 @@ import com.vaadin.data.provider.ListDataProvider;
 import com.vaadin.shared.ui.grid.DropLocation;
 import com.vaadin.ui.Grid;
 import com.vaadin.ui.components.grid.DropIndexCalculator;
-import com.vaadin.ui.components.grid.GridRowDragger;
 import com.vaadin.ui.components.grid.GridDropEvent;
+import com.vaadin.ui.components.grid.GridRowDragger;
 import com.vaadin.ui.components.grid.SourceDataProviderUpdater;
 
 public class GridRowDraggerOneGridTest {
@@ -143,7 +143,7 @@ public class GridRowDraggerOneGridTest {
     public void alwaysDropToEndCalculator() {
         source.setItems("0", "1", "2");
 
-        dragger.setDropIndexCalculator(DropIndexCalculator.ALWAYS_DROP_TO_END);
+        dragger.setDropIndexCalculator(DropIndexCalculator.alwaysDropToEnd());
 
         drop("1", DropLocation.ABOVE, "0");
 
@@ -208,7 +208,7 @@ public class GridRowDraggerOneGridTest {
     public void dropOnSortedGrid_byDefault_dropsToTheEnd() {
         Assert.assertFalse(
                 "Default drops on sorted grid rows should not be allowed",
-                dragger.getGridDropTarget().isDropAllowedOnSortedGridRows());
+                dragger.getGridDropTarget().isDropAllowedOnRowsWhenSorted());
 
         source.setItems("0", "1", "2", "3", "4");
 
diff --git a/server/src/test/java/com/vaadin/tests/server/component/grid/GridRowDraggerTwoGridsTest.java b/server/src/test/java/com/vaadin/tests/server/component/grid/GridRowDraggerTwoGridsTest.java
index 127365be36..6401867e3b 100644
--- a/server/src/test/java/com/vaadin/tests/server/component/grid/GridRowDraggerTwoGridsTest.java
+++ b/server/src/test/java/com/vaadin/tests/server/component/grid/GridRowDraggerTwoGridsTest.java
@@ -15,8 +15,8 @@ import com.vaadin.data.provider.ListDataProvider;
 import com.vaadin.shared.ui.grid.DropLocation;
 import com.vaadin.ui.Grid;
 import com.vaadin.ui.components.grid.DropIndexCalculator;
-import com.vaadin.ui.components.grid.GridRowDragger;
 import com.vaadin.ui.components.grid.GridDropEvent;
+import com.vaadin.ui.components.grid.GridRowDragger;
 import com.vaadin.ui.components.grid.SourceDataProviderUpdater;
 
 public class GridRowDraggerTwoGridsTest {
@@ -186,7 +186,7 @@ public class GridRowDraggerTwoGridsTest {
         source.setItems("0");
         target.setItems("1", "2");
 
-        dragger.setDropIndexCalculator(DropIndexCalculator.ALWAYS_DROP_TO_END);
+        dragger.setDropIndexCalculator(DropIndexCalculator.alwaysDropToEnd());
 
         drop("1", DropLocation.ABOVE, "0");
 
@@ -255,7 +255,7 @@ public class GridRowDraggerTwoGridsTest {
     public void dropOnSortedGrid_byDefault_dropsToTheEnd() {
         Assert.assertFalse(
                 "Default drops on sorted grid rows should not be allowed",
-                dragger.getGridDropTarget().isDropAllowedOnSortedGridRows());
+                dragger.getGridDropTarget().isDropAllowedOnRowsWhenSorted());
 
         source.setItems("0", "1", "2");
         target.setItems("4", "5");