From 2b7d3ae72bcaf9ee0007cc6162e9de5c99521524 Mon Sep 17 00:00:00 2001 From: David Fahlander Date: Tue, 9 Jul 2024 19:20:00 +0200 Subject: [PATCH] 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. --- src/helpers/rangeset.ts | 4 ++ src/live-query/cache/apply-optimistic-ops.ts | 53 +++++++++++--------- src/public/types/rangeset.d.ts | 5 +- test/tests-live-query.js | 45 +++++++++-------- 4 files changed, 61 insertions(+), 46 deletions(-) diff --git a/src/helpers/rangeset.ts b/src/helpers/rangeset.ts index d7222bce7..ec4e0fee2 100644 --- 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 { return getRangeSetIterator(this); diff --git a/src/live-query/cache/apply-optimistic-ops.ts b/src/live-query/cache/apply-optimistic-ops.ts index 9b9324a71..6088ff2fa 100644 --- 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,11 +60,12 @@ 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)) @@ -67,10 +73,9 @@ export function applyOptimisticOps( 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 index fadc72fa3..4a7709c55 100644 --- 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; } diff --git a/test/tests-live-query.js b/test/tests-live-query.js index 46d88bb36..45a2459c9 100644 --- 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"},