issue-23 fix default size calculations on remove via Buffer.update (a…

…nd Cache.update)

src/classes/buffer.ts

@@ -263,44 +263,52 @@ export class Buffer<Data> {
     generator: (index: number, data: Data) => Item<Data>,
     indexToTrack: number,
     fixRight: boolean
-  ): number {
+  ): { trackedIndex: number, toRemove: Item<Data>[] } {
     if (!this.size || Number.isNaN(this.firstIndex)) {
-      return NaN;
+      return { trackedIndex: NaN, toRemove: [] };
     }
-    let _indexToTrack = indexToTrack;
+    let trackedIndex = indexToTrack;
     let index = fixRight ? this.lastIndex : this.firstIndex;
     const items: Item<Data>[] = [];
     const diff = fixRight ? -1 : 1;
-    const initialIndexList = this.items.map(({ $index }) => $index);
-    (fixRight ? this.items.reverse() : this.items).forEach(item => {
+    const limit = this.size - 1;
+    const beforeMap = new Map<number, Item>(); // need to persist original $indexes
+    const updateArray = Array.prototype[fixRight ? 'unshift' : 'push'];
+
+    for (let i = fixRight ? limit : 0; fixRight ? i >= 0 : i <= limit; i += diff) {
+      const item = this.items[i];
+      beforeMap.set(item.$index, item);
       const result = predicate(item);
+
       // if predicate result is falsy or empty array -> delete
       if (!result || (Array.isArray(result) && !result.length)) {
         item.toRemove = true;
-        _indexToTrack += item.$index >= indexToTrack ? (fixRight ? 1 : 0) : (fixRight ? 0 : -1);
+        trackedIndex += item.$index >= indexToTrack ? (fixRight ? 1 : 0) : (fixRight ? 0 : -1);
         this.shiftExtremum(-1, fixRight);
-        return;
+        continue;
       }
+
       // if predicate result is truthy but not array -> leave
       if (!Array.isArray(result)) {
         item.updateIndex(index);
-        items.push(item);
+        updateArray.call(items, item);
         index += diff;
-        return;
+        continue;
       }
+
       // if predicate result is non-empty array -> insert/replace
       if (item.$index < indexToTrack) {
-        _indexToTrack += fixRight ? 0 : result.length - 1;
+        trackedIndex += fixRight ? 0 : result.length - 1;
       } else if (item.$index > indexToTrack) {
-        _indexToTrack += fixRight ? 1 - result.length : 0;
+        trackedIndex += fixRight ? 1 - result.length : 0;
       }
       let toRemove = true;
       const newItems: Item<Data>[] = [];
       (fixRight ? [...result].reverse() : result).forEach((data, i) => {
         let newItem: Item<Data>;
         if (item.data === data) {
           if (indexToTrack === item.$index) {
-            _indexToTrack = index + i * diff;
+            trackedIndex = index + i * diff;
           }
           item.updateIndex(index + i * diff);
           newItem = item;
@@ -309,26 +317,31 @@ export class Buffer<Data> {
           newItem = generator(index + i * diff, data);
           newItem.toInsert = true;
         }
-        newItems.push(newItem);
+        updateArray.call(newItems, newItem);
       });
       item.toRemove = toRemove;
-      items.push(...newItems);
+      updateArray.call(items, ...newItems);
       index += diff * result.length;
       if (result.length > 1) {
         this.shiftExtremum(result.length - 1, fixRight);
       }
-    });
-    this.items = fixRight ? items.reverse() : items;
-    this.cache.updateSubset(initialIndexList, this.items, fixRight);
+    }
+
+    const toRemove = this.items.filter(item => item.toRemove);
+    const itemsBefore = Array.from(beforeMap)
+      .map(([$index, { size, toRemove }]) => ({ $index, size, toRemove }))
+      .sort((a, b) => a.$index - b.$index);
+    this.items = items;
+    this.cache.updateSubset(itemsBefore, items, fixRight);
 
     if (this.finiteAbsMinIndex === this.finiteAbsMaxIndex) {
-      _indexToTrack = NaN;
-    } else if (_indexToTrack > this.finiteAbsMaxIndex) {
-      _indexToTrack = this.finiteAbsMaxIndex;
-    } else if (_indexToTrack < this.finiteAbsMinIndex) {
-      _indexToTrack = this.finiteAbsMinIndex;
+      trackedIndex = NaN;
+    } else if (trackedIndex > this.finiteAbsMaxIndex) {
+      trackedIndex = this.finiteAbsMaxIndex;
+    } else if (trackedIndex < this.finiteAbsMinIndex) {
+      trackedIndex = this.finiteAbsMinIndex;
     }
-    return _indexToTrack;
+    return { trackedIndex, toRemove };
   }
 
   cacheItem(item: Item<Data>): void {

src/classes/buffer/cache.ts

@@ -4,6 +4,12 @@ import { Settings } from '../settings';
 import { Logger } from '../logger';
 import { SizeStrategy } from '../../inputs/index';
 
+interface ItemUpdate {
+  $index: number;
+  size: number;
+  toRemove?: boolean;
+}
+
 export class ItemCache<Data = unknown> {
   $index: number;
   nodeId: string;
@@ -155,19 +161,20 @@ export class Cache<Data = unknown> {
    * Destructively updates Set based on subset (before-after) changes.
    * Replaces current Set with a new one with new regular $indexes.
    * Maintains min/max indexes. Maintains default item size on remove only.
-   * Inserted and replaced items will be taken into account on Cache.add async calls after render.
    *
-   * @param {number[]} before Initial subset of indexes to be replaced by "after". Must be incremental.
-   * @param {Item<Data>[]} after Transformed subset that replaces "before". Must be be $index-incremental.
+   * @param {ItemUpdate[]} before Initial subset of items to be replaced by "after".
+   * Each element is an object with { $index, size, toRemove } props. Must be $index-incremental.
+   * Items to be removed must have toRemove flag: before[].toRemove = true.
+   * @param {Item<Data>[]} after Transformed subset that replaces "before". Must be $index-incremental.
    * Must contain at least 1 $index from "before" or be empty.
    * @param {boolean} fixRight This is to fix right indexes during subset collapsing. Acts only if "after" is empty.
    */
-  updateSubset(before: number[], after: Item<Data>[], fixRight?: boolean): void {
+  updateSubset(before: ItemUpdate[], after: Item<Data>[], fixRight?: boolean): void {
     if (!this.size || !before.length) {
       return;
     }
-    const minB = before[0], maxB = before[before.length - 1];
-    let leftDiff: number, rightDiff: number, found;
+    const minB = before[0].$index, maxB = before[before.length - 1].$index;
+    let leftDiff: number, rightDiff: number;
     if (after.length) {
       const minA = after[0].$index, maxA = after[after.length - 1].$index;
       leftDiff = minA - minB;
@@ -191,11 +198,9 @@ export class Cache<Data = unknown> {
     after.forEach(item => // subset items
       items.set(item.$index, new ItemCache<Data>(item, this.saveData))
     );
-    before.forEach(index => { // removed items immediately affect the default size
-      if (!after.some(({ $index }) => index === $index) && (found = this.get(index))) {
-        this.defaultSize.setRemoved(found);
-      }
-    });
+    before // to maintain default size on remove
+      .filter(item => item.toRemove)
+      .forEach(item => this.defaultSize.setRemoved(item));
     this.minIndex += leftDiff;
     this.maxIndex += rightDiff;
     this.items = items;

src/classes/buffer/defaultSize.ts

@@ -114,12 +114,16 @@ export class DefaultSize {
       return false;
     }
     const oldValue = this.get();
-    if (this.sizeStrategy === SizeStrategy.Average) {
-      this.recalculateAverageSize(cacheSize);
+    if (!cacheSize) {
+      this.reset(true);
     } else {
-      this.recalculateFrequentSize();
+      if (this.sizeStrategy === SizeStrategy.Average) {
+        this.recalculateAverageSize(cacheSize);
+      } else {
+        this.recalculateFrequentSize();
+      }
+      this.recalculation.reset();
     }
-    this.recalculation.reset();
     return this.get() !== oldValue;
   }
 

src/processes/adapter/update.ts

@@ -26,11 +26,10 @@ export default class Update extends BaseAdapterProcessFactory(AdapterProcess.upd
       logger.log(() => 'no items in Buffer');
       return false;
     }
-    const before = [...buffer.items];
     const { item: firstItem, index: firstIndex, diff: firstItemDiff } =
       viewport.getEdgeVisibleItem(buffer.items, Direction.backward);
 
-    const trackedIndex = buffer.updateItems(
+    const { trackedIndex, toRemove } = buffer.updateItems(
       params.predicate,
       (index, data) => new Item(index, data, routines),
       firstIndex,
@@ -43,21 +42,23 @@ export default class Update extends BaseAdapterProcessFactory(AdapterProcess.upd
       delta = - buffer.getSizeByIndex(trackedIndex) + firstItemDiff;
     }
 
-    const itemsToRemove = before.filter(({ toRemove }) => toRemove);
-    itemsToRemove.forEach(item => item.hide());
-    logger.log(() => itemsToRemove.length
-      ? 'items to remove: [' + itemsToRemove.map(({ $index }) => $index).join(',') + ']'
+    toRemove.forEach(item => item.hide());
+    logger.log(() => toRemove.length
+      ? 'items to remove: [' + toRemove.map(({ $index }) => $index).join(',') + ']'
       : 'no items to remove'
     );
+    if (toRemove.length) { // insertions will be processed on render
+      buffer.checkDefaultSize();
+    }
 
-    const itemsToRender = buffer.items.filter(({ toInsert }) => toInsert);
-    logger.log(() => itemsToRender.length
-      ? 'items to render: [' + itemsToRender.map(({ $index }) => $index).join(',') + ']'
+    const toRender = buffer.items.filter(({ toInsert }) => toInsert);
+    logger.log(() => toRender.length
+      ? 'items to render: [' + toRender.map(({ $index }) => $index).join(',') + ']'
       : 'no items to render'
     );
 
-    fetch.update(trackedIndex, delta, itemsToRender, itemsToRemove);
-    return !!itemsToRemove.length || !!itemsToRender.length;
+    fetch.update(trackedIndex, delta, toRender, toRemove);
+    return !!toRemove.length || !!toRender.length;
   }
 
 }

tests/buffer.spec.ts

@@ -56,7 +56,7 @@ const checkIndexTrackingOnUpdate = (params: BufferUpdateTrackConfig) => () => {
   const absMinBefore = debug && buffer.finiteAbsMinIndex, absMaxBefore = debug && buffer.finiteAbsMaxIndex;
   const minBefore = debug && buffer.minIndex, maxBefore = debug && buffer.maxIndex;
 
-  const trackedIndex = buffer.updateItems(predicate, cb, index, fixRight);
+  const { trackedIndex } = buffer.updateItems(predicate, cb, index, fixRight);
 
   if (debug) {
     const trackedItem = buffer.get(trackedIndex);

tests/cache.spec.ts

@@ -99,7 +99,7 @@ describe('Cache Spec', () => {
   describe('Update', () => {
     const MIN = 1, COUNT = 7;
     const items = generateBufferItems(MIN, COUNT);
-    const subset = items.slice(2, 5).map(({ $index }) => $index); // [3, 4, 5]
+    const subset = items.slice(2, 5); // [3, 4, 5]
     let cache: Cache<Data>;
 
     const checkUpdate = (list: IndexIdList) => {
@@ -248,7 +248,10 @@ describe('Cache Spec', () => {
       }
       const cache = new Cache(averageSizeSettings as never, loggerMock as never);
       const items = generateBufferItems(1, length);
-      items.forEach(item => cache.add(item));
+      items.forEach(item => {
+        item.toRemove = toRemove && toRemove.includes(item.$index);
+        cache.add(item);
+      });
       cache.recalculateDefaultSize();
       return { items, cache, average: { after, before } };
     };
@@ -299,7 +302,7 @@ describe('Cache Spec', () => {
         const toRemove = [5, 8, 10];
         const { items, cache, average } = prepareAverage({ length: 10, toRemove });
         const listAfter = [{ 1: 1 }, { 2: 2 }, { 3: 3 }, { 4: 4 }, { 5: 6 }, { 6: 7 }, { 7: 9 }];
-        const subset = items.slice(toRemove[0], toRemove[toRemove.length - 1] + 1).map(({ $index }) => $index);
+        const subset = items.slice(toRemove[0], toRemove[toRemove.length - 1] + 1);
 
         expect(cache.getDefaultSize()).toBe(average.before);
 
@@ -308,7 +311,6 @@ describe('Cache Spec', () => {
         expect(cache.getDefaultSize()).toBe(average.after);
       });
 
-
       it('should maintain average size on remove via update (explicit)', () => {
         const INDEX_TO_REMOVE = 10;
         const cache = new Cache(averageSizeSettings as never, loggerMock as never);
@@ -321,8 +323,7 @@ describe('Cache Spec', () => {
         cache.recalculateDefaultSize();
         expect(cache.getDefaultSize()).toBe(25);
 
-        const before = items.filter(({ $index }) => [10, 11, 12, 13, 14, 15].includes($index))
-          .map(({ $index }) => $index);
+        const before = items.filter(({ $index }) => [10, 11, 12, 13, 14, 15].includes($index));
         const after = make([{ 10: 11 }, { 11: 12 }, { 12: 13 }, { 13: 14 }, { 14: 15 }]);
         after.forEach(item => item.size = item.data.size = 20);