Fixed TreeRangeMap.SubRangeMap.putCoalescing

The short circuiting on an attempt to put an empty range caused the contract of putCoalescing to be violated. Fixes #3886 RELNOTES=n/a ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=308881996
google · Apr 28, 2020 · 24fe5d9 · 24fe5d9
1 parent 1f55c42
commit 24fe5d9
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 2 deletions.
diff --git a/android/guava-tests/test/com/google/common/collect/TreeRangeMapTest.java b/android/guava-tests/test/com/google/common/collect/TreeRangeMapTest.java
@@ -489,6 +489,20 @@ public void testPutCoalescingEmpty() {
     assertEquals(ImmutableMap.of(Range.closedOpen(0, 2), 1), rangeMap.asMapOfRanges());
   }
 
+  public void testPutCoalescingSubmapEmpty() {
+    RangeMap<Integer, Integer> rangeMap = TreeRangeMap.create();
+    rangeMap.put(Range.closedOpen(0, 1), 1);
+    rangeMap.put(Range.closedOpen(1, 2), 1);
+    assertEquals(
+        ImmutableMap.of(Range.closedOpen(0, 1), 1, Range.closedOpen(1, 2), 1),
+        rangeMap.asMapOfRanges());
+
+    RangeMap<Integer, Integer> subRangeMap = rangeMap.subRangeMap(Range.closedOpen(0, 2));
+    subRangeMap.putCoalescing(Range.closedOpen(1, 1), 1); // empty range coalesces connected ranges
+    assertEquals(ImmutableMap.of(Range.closedOpen(0, 2), 1), subRangeMap.asMapOfRanges());
+    assertEquals(ImmutableMap.of(Range.closedOpen(0, 2), 1), rangeMap.asMapOfRanges());
+  }
+
   public void testPutCoalescingComplex() {
     // {[0..1): 1, [1..3): 1, [3..5): 1, [7..10): 2, [12..15): 2, [18..19): 3}
     RangeMap<Integer, Integer> rangeMap = TreeRangeMap.create();

diff --git a/android/guava/src/com/google/common/collect/TreeRangeMap.java b/android/guava/src/com/google/common/collect/TreeRangeMap.java
@@ -431,7 +431,7 @@ public void put(Range<K> range, V value) {
 
     @Override
     public void putCoalescing(Range<K> range, V value) {
-      if (entriesByLowerBound.isEmpty() || range.isEmpty() || !subRange.encloses(range)) {
+      if (entriesByLowerBound.isEmpty() || !subRange.encloses(range)) {
         put(range, value);
         return;
       }

diff --git a/guava-tests/test/com/google/common/collect/TreeRangeMapTest.java b/guava-tests/test/com/google/common/collect/TreeRangeMapTest.java
@@ -490,6 +490,20 @@ public void testPutCoalescingEmpty() {
     assertEquals(ImmutableMap.of(Range.closedOpen(0, 2), 1), rangeMap.asMapOfRanges());
   }
 
+  public void testPutCoalescingSubmapEmpty() {
+    RangeMap<Integer, Integer> rangeMap = TreeRangeMap.create();
+    rangeMap.put(Range.closedOpen(0, 1), 1);
+    rangeMap.put(Range.closedOpen(1, 2), 1);
+    assertEquals(
+        ImmutableMap.of(Range.closedOpen(0, 1), 1, Range.closedOpen(1, 2), 1),
+        rangeMap.asMapOfRanges());
+
+    RangeMap<Integer, Integer> subRangeMap = rangeMap.subRangeMap(Range.closedOpen(0, 2));
+    subRangeMap.putCoalescing(Range.closedOpen(1, 1), 1); // empty range coalesces connected ranges
+    assertEquals(ImmutableMap.of(Range.closedOpen(0, 2), 1), subRangeMap.asMapOfRanges());
+    assertEquals(ImmutableMap.of(Range.closedOpen(0, 2), 1), rangeMap.asMapOfRanges());
+  }
+
   public void testPutCoalescingComplex() {
     // {[0..1): 1, [1..3): 1, [3..5): 1, [7..10): 2, [12..15): 2, [18..19): 3}
     RangeMap<Integer, Integer> rangeMap = TreeRangeMap.create();

diff --git a/guava/src/com/google/common/collect/TreeRangeMap.java b/guava/src/com/google/common/collect/TreeRangeMap.java
@@ -508,7 +508,7 @@ public void put(Range<K> range, V value) {
 
     @Override
     public void putCoalescing(Range<K> range, V value) {
-      if (entriesByLowerBound.isEmpty() || range.isEmpty() || !subRange.encloses(range)) {
+      if (entriesByLowerBound.isEmpty() || !subRange.encloses(range)) {
         put(range, value);
         return;
       }