Fix a bug in MemoizationList.

Properly handle the case when source.get() in force() causes forget().
TomasMikula · Feb 23, 2015 · f5d6fb5 · f5d6fb5
1 parent 1f0d7a9
commit f5d6fb5
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 6 deletions.
diff --git a/reactfx/src/main/java/org/reactfx/collection/MemoizationList.java b/reactfx/src/main/java/org/reactfx/collection/MemoizationList.java
@@ -50,6 +50,10 @@ private void prepareNotifications(QuasiListChange<? extends E> change) {
             enqueueNotifications(change);
         }
 
+        private void prepareNotifications(QuasiListModification<? extends E> mod) {
+            enqueueNotifications(mod.asListChange());
+        }
+
         private void publishNotifications() {
             notifyObservers();
         }
@@ -112,19 +116,16 @@ public void force(int from, int to) {
         }
 
         Lists.checkRange(from, to, size());
-        int presentBefore = sparseList.getPresentCountBefore(from);
-        ListChangeAccumulator<E> mods = new ListChangeAccumulator<>();
         for(int i = from; i < to; ++i) {
             if(!sparseList.isPresent(i)) {
                 E elem = source.get(i);
                 if(sparseList.setIfAbsent(i, elem)) {
-                    mods.add(ProperLiveList.elemInsertion(presentBefore + (i - from)));
+                    int presentBefore = sparseList.getPresentCountBefore(i);
+                    memoizedItems.prepareNotifications(ProperLiveList.elemInsertion(presentBefore));
                 }
             }
         }
-        if(!mods.isEmpty()) {
-            memoizedItems.notifyObservers(mods.fetch());
-        }
+        memoizedItems.publishNotifications();
     }
 
     @Override

diff --git a/reactfx/src/test/java/org/reactfx/collection/MemoizationListTest.java b/reactfx/src/test/java/org/reactfx/collection/MemoizationListTest.java
@@ -14,6 +14,7 @@
 
 import org.junit.Test;
 import org.reactfx.Counter;
+import org.reactfx.value.Val;
 
 public class MemoizationListTest {
 
@@ -189,4 +190,23 @@ public void testRecursionWithinForce() {
 
         assertEquals(Arrays.asList(1), memo2Mirror);
     }
+
+    @Test
+    public void testMemoizedItemsChangeWithinForce() {
+        LiveList<Integer> src = new LiveArrayList<>(1, 2, 4, 8, 16, 32);
+        MemoizationList<Integer> memo1 = src.memoize();
+        MemoizationList<Integer> memo2 = memo1.map(Function.identity()).memoize();
+        Val<Integer> memo1Sum = memo1.memoizedItems().reduce((a, b) -> a + b).orElseConst(0);
+        memo1Sum.addListener((obs, oldVal, newVal) -> memo2.forget(0, memo2.size()));
+
+        List<Integer> memo2Mirror = new ArrayList<>();
+        memo2.memoizedItems().observeModifications(mod -> {
+            memo2Mirror.subList(mod.getFrom(), mod.getFrom() + mod.getRemovedSize()).clear();
+            memo2Mirror.addAll(mod.getFrom(), mod.getAddedSubList());
+            // the main part of this test is that it does not throw IndexOutOfBoundsException
+        });
+        memo2.force(3, 6);
+
+        assertEquals(Arrays.asList(32), memo2Mirror);
+    }
 }