From e191abb0da9e4660e925991490e967c178380aef Mon Sep 17 00:00:00 2001
From: Henrik Paul <henrik@vaadin.com>
Date: Wed, 25 Mar 2015 18:04:45 +0200
Subject: [PATCH] Fixed some faulty asserts in Grid's detail row creation
 (#17293)

Change-Id: I8e9998524c02ca1e2f9d3391fa27bacc53655c7f
---
 .../vaadin/data/RpcDataProviderExtension.java | 36 +++++++++++++------
 .../server/GridDetailsServerTest.java         | 15 ++++++++
 2 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/server/src/com/vaadin/data/RpcDataProviderExtension.java b/server/src/com/vaadin/data/RpcDataProviderExtension.java
index 217b2fe392..e645ec60f7 100644
--- a/server/src/com/vaadin/data/RpcDataProviderExtension.java
+++ b/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.
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridDetailsServerTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridDetailsServerTest.java
index cfaf79ea05..4ea64073f3 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridDetailsServerTest.java
+++ b/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"));
+    }
 }
-- 
2.39.5