Merge pull request #1824 from dexie/issue1823

Repro and fix #1823

src/live-query/cache/adjust-optimistic-request-from-failures.ts

@@ -0,0 +1,38 @@
+import { delArrayItem, isArray } from '../../functions/utils';
+import { TblQueryCache } from '../../public/types/cache';
+import {
+  DBCoreMutateRequest,
+  DBCoreMutateResponse,
+} from '../../public/types/dbcore';
+
+export function adjustOptimisticFromFailures(
+  tblCache: TblQueryCache,
+  req: DBCoreMutateRequest,
+  res: DBCoreMutateResponse
+): DBCoreMutateRequest {
+  if (res.numFailures === 0) return req;
+  if (req.type === 'deleteRange') {
+    // numFailures > 0 means the deleteRange operation failed in its whole.
+    return null;
+  }
+
+  const numBulkOps = req.keys
+    ? req.keys.length
+    : 'values' in req && req.values
+    ? req.values.length
+    : 1;
+  if (res.numFailures === numBulkOps) {
+    // Same number of failures as the number of ops. This means that all ops failed.
+    return null;
+  }
+
+  const clone: DBCoreMutateRequest = { ...req };
+
+  if (isArray(clone.keys)) {
+    clone.keys = clone.keys.filter((_, i) => !(i in res.failures));
+  }
+  if ('values' in clone && isArray(clone.values)) {
+    clone.values = clone.values.filter((_, i) => !(i in res.failures));
+  }
+  return clone;
+}

src/live-query/cache/cache-middleware.ts

@@ -4,10 +4,11 @@ import { deepClone, delArrayItem, setByKeyPath } from '../../functions/utils';
 import DexiePromise, { PSD } from '../../helpers/promise';
 import { ObservabilitySet } from '../../public/types/db-events';
 import {
-  DBCore, DBCoreQueryRequest,
+  DBCore, DBCoreMutateRequest, DBCoreMutateResponse, DBCoreQueryRequest,
   DBCoreQueryResponse
 } from '../../public/types/dbcore';
 import { Middleware } from '../../public/types/middleware';
+import { adjustOptimisticFromFailures } from './adjust-optimistic-request-from-failures';
 import { applyOptimisticOps } from './apply-optimistic-ops';
 import { cache } from './cache';
 import { findCompatibleQuery } from './find-compatible-query';
@@ -127,7 +128,7 @@ export const cacheMiddleware: Middleware<DBCore> = {
         const primKey = downTable.schema.primaryKey;
         const tableMW = {
           ...downTable,
-          mutate(req) {
+          mutate(req: DBCoreMutateRequest): Promise<DBCoreMutateResponse> {
             if (
               primKey.outbound || // Non-inbound tables are harded to apply optimistic updates on because we can't know primary key of results
               PSD.trans.db._options.cache === 'disabled' // User has opted-out from caching
@@ -157,7 +158,8 @@ export const cacheMiddleware: Middleware<DBCore> = {
                     return valueWithKey;
                   })
                 };
-                tblCache.optimisticOps.push(reqWithResolvedKeys);
+                const adjustedReq = adjustOptimisticFromFailures(tblCache, reqWithResolvedKeys, res);
+                tblCache.optimisticOps.push(adjustedReq);
                 // Signal subscribers after the observability middleware has complemented req.mutatedParts with the new keys.
                 // We must queue the task so that we get the req.mutatedParts updated by observability middleware first.
                 // If we refactor the dependency between observability middleware and this middleware we might not need to queue the task.
@@ -168,6 +170,17 @@ export const cacheMiddleware: Middleware<DBCore> = {
               tblCache.optimisticOps.push(req);
               // Signal subscribers that there are mutated parts
               signalSubscribersLazily(req.mutatedParts);
+              promise.then((res) => {
+                if (res.numFailures > 0) {
+                  // In case the operation failed, we need to remove it from the optimisticOps array.
+                  delArrayItem(tblCache.optimisticOps, req);
+                  const adjustedReq = adjustOptimisticFromFailures(tblCache, req, res);
+                  if (adjustedReq) {
+                    tblCache.optimisticOps.push(adjustedReq);
+                  }
+                  signalSubscribersLazily(req.mutatedParts); // Signal the rolling back of the operation.
+                }
+              });
               promise.catch(()=> {
                 // In case the operation failed, we need to remove it from the optimisticOps array.
                 delArrayItem(tblCache.optimisticOps, req);

test/tests-live-query.js

@@ -239,6 +239,35 @@ promisedTest("subscribe and error occur", async ()=> {
   subscription.unsubscribe();
 });
 
+promisedTest("optimistic updates that eventually fail must be reverted (Issue #1823)", async ()=>{
+  const log = [];
+  let subscription = liveQuery(
+    ()=>db.items.toArray()
+  ).subscribe({
+    next: result => {
+      log.push(result);
+      console.log("optimistic result (from #1823 test)", result);
+    },
+  });
+
+  await db.transaction('rw', db.items, async ()=>{
+    // Simple test a catched failing operation
+    await db.items.add(
+      {id: 1, iWillFail: true} // Contraint error (key 1 already exists)
+    ).catch(()=>{});
+    // Test another code path in adjustOptimisticFromFailures() where some operations succeed and some not.
+    await db.items.bulkAdd([
+      {id: 2, iWillFail: true}, // Constraint error (key 2 already exists)
+      {id: 99, iWillSucceed: true}
+    ]).catch(()=>{});
+  });
+  // Wait for a successful readonly transaction to complete after the write transaction.
+  // This will make sure that the liveQuery has been updated with the final result.
+  await db.transaction('r', db.items, ()=>db.items.toArray());
+  subscription.unsubscribe();
+  deepEqual(log.at(-1), [{id: 1},{id:2},{id:3},{id: 99, iWillSucceed: true}], "Last log entry contains the correct result. There might be optimistic updates before though.");
+});
+
 /* Use cases to cover:
 
   Queries