Resolve #2011 and #2012 (#2033)

Chech input array for duplicate primary keys before updating the liveQuery cache. This commit also introduces RangeSet.hasKey() method (to simplify seeking of a key in a rangeset), corrects the typing of IntervalTreeNode and removes some unused imports.
dexie · Jul 9, 2024 · 2b7d3ae · 2b7d3ae
1 parent df2f963
commit 2b7d3ae
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 46 deletions.
diff --git a/src/helpers/rangeset.ts b/src/helpers/rangeset.ts
@@ -48,6 +48,10 @@ props(RangeSet.prototype, {
     keys.forEach(key => addRange(this, key, key));
     return this;
   },
+  hasKey(key: IndexableType) {
+    const node = getRangeSetIterator(this).next(key).value;
+    return node && cmp(node.from, key) <= 0 && cmp(node.to, key) >= 0;
+  },
 
   [iteratorSymbol](): Iterator<IntervalTreeNode, undefined, IndexableType | undefined> {
     return getRangeSetIterator(this);

diff --git a/src/live-query/cache/apply-optimistic-ops.ts b/src/live-query/cache/apply-optimistic-ops.ts
@@ -1,9 +1,8 @@
 import { cmp } from '../../functions/cmp';
-import { deepClone, isArray } from '../../functions/utils';
-import { RangeSet, rangesOverlap } from '../../helpers/rangeset';
+import { isArray } from '../../functions/utils';
+import { RangeSet } from '../../helpers/rangeset';
 import { CacheEntry } from '../../public/types/cache';
 import {
-  DBCoreIndex,
   DBCoreMutateRequest,
   DBCoreQueryRequest,
   DBCoreTable,
@@ -29,19 +28,25 @@ export function applyOptimisticOps(
 
   let finalResult = ops.reduce((result, op) => {
     let modifedResult = result;
-    const includedValues =
-      op.type === 'add' || op.type === 'put'
-        ? op.values.filter((v) => {
-              const key = extractIndex(v);
-              return multiEntry && isArray(key) // multiEntry index work like plain index unless key is array
-                ? key.some((k) => isWithinRange(k, queryRange)) // multiEntry and array key
-                : isWithinRange(key, queryRange); // multiEntry but not array key
-            }).map(v => {
-              v = deepClone(v);// v might come from user so we can't just freeze it.
-              if (immutable) Object.freeze(v);
-              return v;
-            })
-        : [];
+    const includedValues: any[] = [];
+    if (op.type === 'add' || op.type === 'put') {
+      const includedPKs = new RangeSet(); // For ignoring duplicates
+      for (let i = op.values.length - 1; i >= 0; --i) {
+        // backwards to prioritize last value of same PK
+        const value = op.values[i];
+        const pk = extractPrimKey(value);
+        if (includedPKs.hasKey(pk)) continue;
+        const key = extractIndex(value);
+        if (
+          multiEntry && isArray(key)
+            ? key.some((k) => isWithinRange(k, queryRange))
+            : isWithinRange(key, queryRange)
+        ) {
+          includedPKs.addKey(pk);
+          includedValues.push(value);
+        }
+      }
+    }
     switch (op.type) {
       case 'add':
         modifedResult = result.concat(
@@ -55,22 +60,22 @@ export function applyOptimisticOps(
           op.values.map((v) => extractPrimKey(v))
         );
         modifedResult = result
-          .filter((item) => {
-            const key = req.values ? extractPrimKey(item) : item;
-            return !rangesOverlap(new RangeSet(key), keySet);
-          })
+          .filter(
+            // Remove all items that are being replaced
+            (item) => !keySet.hasKey(req.values ? extractPrimKey(item) : item)
+          )
           .concat(
+            // Add all items that are being put (sorting will be done later)
             req.values
               ? includedValues
               : includedValues.map((v) => extractPrimKey(v))
           );
         break;
       case 'delete':
         const keysToDelete = new RangeSet().addKeys(op.keys);
-        modifedResult = result.filter((item) => {
-          const key = req.values ? extractPrimKey(item) : item;
-          return !rangesOverlap(new RangeSet(key), keysToDelete);
-        });
+        modifedResult = result.filter(
+          (item) => !keysToDelete.hasKey(req.values ? extractPrimKey(item) : item)
+        );
 
         break;
       case 'deleteRange':

diff --git a/src/public/types/rangeset.d.ts b/src/public/types/rangeset.d.ts
@@ -4,8 +4,8 @@ export type IntervalTree = IntervalTreeNode | EmptyRange;
 export interface IntervalTreeNode {
   from: IndexableType; // lower bound
   to: IndexableType; // upper bound
-  l: IntervalTreeNode | null; // left
-  r: IntervalTreeNode | null; // right
+  l?: IntervalTreeNode | null; // left
+  r?: IntervalTreeNode | null; // right
   d: number; // depth
 }
 export interface EmptyRange {
@@ -16,6 +16,7 @@ export interface RangeSetPrototype {
   add(rangeSet: IntervalTree | {from: IndexableType, to: IndexableType}): RangeSet;
   addKey(key: IndexableType): RangeSet;
   addKeys(keys: IndexableType[]): RangeSet;
+  hasKey(key: IndexableType): boolean;
   [Symbol.iterator](): Iterator<IntervalTreeNode, undefined, IndexableType | undefined>;
 }
 

diff --git a/test/tests-live-query.js b/test/tests-live-query.js
@@ -271,7 +271,6 @@ promisedTest("optimistic updates that eventually fail must be reverted (Issue #1
 let abbaKey = 0;
 let lastFriendId = 0;
 let barbarFriendId = 0;
-let fruitCount = 0; // A bug in Safari <= 13.1 makes it unable to count on the name index (adds 1 extra)
 const bulkFriends = [];
 for (let i=0; i<51; ++i) {
   bulkFriends.push({name: `name${i}`, age: i});
@@ -294,7 +293,7 @@ const mutsAndExpects = () => [
       itemsStartsWithAPrimKeys: [-1],
       itemsStartsWithAOffset3: [],
       itemsStartsWithAKeys: ["A"],
-      itemsStartsWithACount: fruitCount + 1
+      itemsStartsWithACount: 1
     }
   ],
   // addAuto
@@ -342,7 +341,7 @@ const mutsAndExpects = () => [
       itemsStartsWithAPrimKeys: [],
       itemsStartsWithAOffset3: [],
       itemsStartsWithAKeys: [],
-      itemsStartsWithACount: fruitCount
+      itemsStartsWithACount: 0
     }
   ],
   [
@@ -355,7 +354,7 @@ const mutsAndExpects = () => [
       itemsStartsWithAPrimKeys: [-1],
       itemsStartsWithAOffset3: [],
       itemsStartsWithAKeys: ["A"],
-      itemsStartsWithACount: fruitCount + 1
+      itemsStartsWithACount: 1
     }
   ],
   // add again
@@ -367,7 +366,7 @@ const mutsAndExpects = () => [
       itemsStartsWithAPrimKeys: [-1, 4, 6, 5],
       itemsStartsWithAOffset3: [{id: 5, name: "Assot"}], // offset 3
       itemsStartsWithAKeys: ["A", "Abbot", "Ambros", "Assot"],
-      itemsStartsWithACount: fruitCount + 4
+      itemsStartsWithACount: 4
     }
   ],
   // delete:
@@ -381,7 +380,7 @@ const mutsAndExpects = () => [
       itemsStartsWithA: [{id: 4, name: "Abbot"}, {id: 6, name: "Ambros"}, {id: 5, name: "Assot"}],
       itemsStartsWithAPrimKeys: [4, 6, 5],
       itemsStartsWithAKeys: ["Abbot", "Ambros", "Assot"],
-      itemsStartsWithACount: fruitCount + 3
+      itemsStartsWithACount: 3
     },
     // Allowed extras:
     // If hooks is listened to we'll get an even more correct update of the itemsStartsWithAOffset3 query
@@ -400,7 +399,7 @@ const mutsAndExpects = () => [
     }, {
       // Things that optionally can be matched in result (if no hooks specified):
       itemsStartsWithAPrimKeys: [4, 6, 5], 
-      itemsStartsWithACount: fruitCount + 3,
+      itemsStartsWithACount: 3,
       itemsStartsWithAOffset3: []
     }
   ],
@@ -413,7 +412,7 @@ const mutsAndExpects = () => [
       itemsStartsWithA: [{id: 4, name: "Abbot"}, {id: 6, name: "Ambros"}],
       itemsStartsWithAPrimKeys: [4, 6],
       itemsStartsWithAKeys: ["Abbot", "Ambros"],
-      itemsStartsWithACount: fruitCount + 2
+      itemsStartsWithACount: 2
     }, {
       itemsStartsWithAOffset3: [] // This is
     }
@@ -543,20 +542,26 @@ const mutsAndExpects = () => [
       ]
     }
   ],
+  // Issue 2011 / 2012
+  [
+    () => db.items.bulkPut([
+      {id: 6, name: "One"},
+      {id: 6, name: "Two"},
+      {id: 6, name: "Three"}
+    ]),
+    {
+      itemsToArray: [{id:1},{id:2},{id:3},{id:4,name:"Abbot"},{id:6,name:"Three"}],
+      itemsStartsWithA: [{id: 4, name: "Abbot"}],
+      itemsStartsWithAPrimKeys: [4],
+      itemsStartsWithAKeys: ["Abbot"],
+      itemsStartsWithACount: 1
+    },[
+      "itemsStartsWithAOffset3" // Should not be updated but need to be ignored because otherwise it fails in dexie-syncable's integration tests that expects it to update to another empty array
+    ]
+  ],
 ]
 
 promisedTest("Full use case matrix", async ()=>{
-  // A bug in Safari <= 13.1 makes it unable to count on the name index (adds 1 extra)
-  fruitCount = await db.items.where('name').startsWith('A').count();
-  if (fruitCount > 0) console.log("fruitCount: " + fruitCount);
-
-  if (isIE) {
-    // The IE implementation becomes shaky here.
-    // Maybe becuase we launch several parallel queries to IDB.
-    ok(true, "Skipping this test for IE - too shaky for the CI");
-    return;
-  }
-
   const queries = {
     itemsToArray: () => db.items.toArray(),
     itemsGet1And2: () => Promise.all(db.items.get(1), db.items.get(-1)),
@@ -591,7 +596,7 @@ promisedTest("Full use case matrix", async ()=>{
     itemsStartsWithAPrimKeys: [],
     itemsStartsWithAOffset3: [],
     itemsStartsWithAKeys: [],
-    itemsStartsWithACount: fruitCount,
+    itemsStartsWithACount: 0,
 
     outboundToArray: [
       {num: 1, name: "A"},