Fixed some faulty asserts in Grid's detail row creation (#17293)

Change-Id: I8e9998524c02ca1e2f9d3391fa27bacc53655c7f

server/src/com/vaadin/data/RpcDataProviderExtension.java

@@ -663,8 +663,10 @@ public class RpcDataProviderExtension extends AbstractExtension {
             assert itemId != null : "itemId was null";
             Integer newRowIndex = Integer.valueOf(rowIndex);
 
-            assert !visibleDetailsComponents.containsKey(itemId) : "itemId "
-                    + "already has a component. Should be destroyed first.";
+            if (visibleDetailsComponents.containsKey(itemId)) {
+                // Don't overwrite existing components
+                return;
+            }
 
             RowReference rowReference = new RowReference(grid);
             rowReference.set(itemId);
@@ -695,14 +697,8 @@ public class RpcDataProviderExtension extends AbstractExtension {
                         + "itemId is empty even though we just created a "
                         + "component for it (" + itemId + ")";
             } else {
-                assert !emptyDetails.containsKey(itemId) : "Bookkeeping has "
-                        + "already itemId marked as empty (itemId: " + itemId
-                        + ", old index: " + emptyDetails.get(itemId)
-                        + ", new index: " + newRowIndex + ")";
-                assert !emptyDetails.containsValue(newRowIndex) : "Bookkeeping"
-                        + " already had another itemId for this empty index "
-                        + "(index: " + newRowIndex + ", new itemId: " + itemId
-                        + ")";
+                assert assertItemIdHasNotMovedAndNothingIsOverwritten(itemId,
+                        newRowIndex);
                 emptyDetails.put(itemId, newRowIndex);
             }
 
@@ -712,6 +708,26 @@ public class RpcDataProviderExtension extends AbstractExtension {
              */
         }
 
+        private boolean assertItemIdHasNotMovedAndNothingIsOverwritten(
+                Object itemId, Integer newRowIndex) {
+
+            Integer oldRowIndex = emptyDetails.get(itemId);
+            if (!SharedUtil.equals(oldRowIndex, newRowIndex)) {
+
+                assert !emptyDetails.containsKey(itemId) : "Unexpected "
+                        + "change of empty details row index for itemId "
+                        + itemId + " from " + oldRowIndex + " to "
+                        + newRowIndex;
+
+                assert !emptyDetails.containsValue(newRowIndex) : "Bookkeeping"
+                        + " already had another itemId for this empty index "
+                        + "(index: " + newRowIndex + ", new itemId: " + itemId
+                        + ")";
+            }
+
+            return true;
+        }
+
         /**
          * Destroys correctly a details component, by the request of the client
          * side.

uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridDetailsServerTest.java

@@ -288,4 +288,19 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
         assertFalse("Details should be not empty with details component",
                 getGridElement().getDetails(0).getText().isEmpty());
     }
+
+    @Test
+    public void noAssertErrorsOnEmptyDetailsAndScrollDown() {
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
+        scrollGridVerticallyTo(500);
+        assertFalse(logContainsText("AssertionError"));
+    }
+
+    @Test
+    public void noAssertErrorsOnPopulatedDetailsAndScrollDown() {
+        selectMenuPath(DETAILS_GENERATOR_WATCHING);
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
+        scrollGridVerticallyTo(500);
+        assertFalse(logContainsText("AssertionError"));
+    }
 }