Fix Range.restrictTo to work with non-intersecting ranges(#13334)

Also adds a special case for isSubsetOf with two empty ranges.

Change-Id: I9b4c854051bd760bbac05c7a0bc2e5418371e90e

4 files changed, 48 insertions, 5 deletions

diff --git a/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java b/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java
index 2afc8185ca..c6aa7c6291 100644
--- a/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java
+++ b/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java
@@ -589,6 +589,7 @@ public abstract class AbstractRemoteDataSource<T> implements DataSource<T> {
 
     private Range getMinCacheRange() {
         Range availableDataRange = getAvailableRangeForCache();
+
         Range minCacheRange = cacheStrategy.getMinCacheRange(
                 requestedAvailability, cached, availableDataRange);
 
diff --git a/shared/src/com/vaadin/shared/ui/grid/Range.java b/shared/src/com/vaadin/shared/ui/grid/Range.java
index 2054845320..6be9e04cbc 100644
--- a/shared/src/com/vaadin/shared/ui/grid/Range.java
+++ b/shared/src/com/vaadin/shared/ui/grid/Range.java
@@ -177,6 +177,10 @@ public final class Range implements Serializable {
      *         range
      */
     public boolean isSubsetOf(final Range other) {
+        if (isEmpty() && other.isEmpty()) {
+            return true;
+        }
+
         return other.getStart() <= getStart() && getEnd() <= other.getEnd();
     }
 
@@ -411,8 +415,10 @@ public final class Range implements Serializable {
      * @return a bounded range
      */
     public Range restrictTo(Range bounds) {
-        boolean startWithin = getStart() >= bounds.getStart();
-        boolean endWithin = getEnd() <= bounds.getEnd();
+        boolean startWithin = bounds.contains(getStart());
+        boolean endWithin = bounds.contains(getEnd());
+        boolean boundsWithin = getStart() < bounds.getStart()
+                && getEnd() >= bounds.getEnd();
 
         if (startWithin) {
             if (endWithin) {
@@ -423,8 +429,10 @@ public final class Range implements Serializable {
         } else {
             if (endWithin) {
                 return Range.between(bounds.getStart(), getEnd());
-            } else {
+            } else if (boundsWithin) {
                 return bounds;
+            } else {
+                return Range.withLength(getStart(), 0);
             }
         }
     }
diff --git a/shared/tests/src/com/vaadin/shared/ui/grid/RangeTest.java b/shared/tests/src/com/vaadin/shared/ui/grid/RangeTest.java
index ab67b22d0b..e3cae858ee 100644
--- a/shared/tests/src/com/vaadin/shared/ui/grid/RangeTest.java
+++ b/shared/tests/src/com/vaadin/shared/ui/grid/RangeTest.java
@@ -372,6 +372,7 @@ public class RangeTest {
         assertTrue(r2 == r3);
     }
 
+    @Test
     public void restrictTo_notInterstecting() {
         Range r1 = Range.between(5, 10);
         Range r2 = Range.between(15, 20);
@@ -385,6 +386,7 @@ public class RangeTest {
                 r4.isEmpty());
     }
 
+    @Test
     public void restrictTo_startOutside() {
         Range r1 = Range.between(5, 10);
         Range r2 = Range.between(7, 15);
@@ -392,8 +394,11 @@ public class RangeTest {
         Range r3 = r1.restrictTo(r2);
 
         assertEquals(Range.between(7, 10), r3);
+
+        assertEquals(r2.restrictTo(r1), r3);
     }
 
+    @Test
     public void restrictTo_endOutside() {
         Range r1 = Range.between(5, 10);
         Range r2 = Range.between(4, 7);
@@ -401,6 +406,20 @@ public class RangeTest {
         Range r3 = r1.restrictTo(r2);
 
         assertEquals(Range.between(5, 7), r3);
+
+        assertEquals(r2.restrictTo(r1), r3);
+    }
+
+    @Test
+    public void restrictTo_empty() {
+        Range r1 = Range.between(5, 10);
+        Range r2 = Range.between(0, 0);
+
+        Range r3 = r1.restrictTo(r2);
+        assertTrue(r3.isEmpty());
+
+        Range r4 = r2.restrictTo(r1);
+        assertTrue(r4.isEmpty());
     }
 
 }
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStructureTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStructureTest.java
index d9d1acb4c1..9e1a9d5e91 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStructureTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStructureTest.java
@@ -26,10 +26,10 @@ import static org.junit.Assert.fail;
 import java.util.List;
 
 import org.junit.Test;
-import org.openqa.selenium.By;
 import org.openqa.selenium.NoSuchElementException;
 import org.openqa.selenium.WebElement;
 
+import com.vaadin.testbench.By;
 import com.vaadin.testbench.TestBenchElement;
 import com.vaadin.testbench.elements.GridElement;
 import com.vaadin.testbench.elements.GridElement.GridCellElement;
@@ -188,7 +188,8 @@ public class GridStructureTest extends GridBasicFeaturesTest {
     public void testItemSetChangeEvent() throws Exception {
         openTestURL();
 
-        final By newRow = By.xpath("//td[text()='newcell: 0']");
+        final org.openqa.selenium.By newRow = By
+                .xpath("//td[text()='newcell: 0']");
 
         assertTrue("Unexpected initial state", !isElementPresent(newRow));
 
@@ -404,6 +405,20 @@ public class GridStructureTest extends GridBasicFeaturesTest {
         assertEquals("Grid scrolled unexpectedly", cellContent, cell.getText());
     }
 
+    @Test
+    public void testScrollAndRemoveAll() {
+        setDebug(true);
+        openTestURL();
+
+        getGridElement().scrollToRow(500);
+        selectMenuPath("Component", "Body rows", "Remove all rows");
+
+        assertFalse("Error notification was present after removing all rows",
+                isElementPresent(NotificationElement.class));
+
+        assertFalse(getGridElement().isElementPresent(By.vaadin("#cell[0][0]")));
+    }
+
     private void assertPrimaryStylename(String stylename) {
         assertTrue(getGridElement().getAttribute("class").contains(stylename));