fix: persist queue entries after updating optimistic ids (#389)

* fix: persist queue entries after updating optimistic ids

* fix: lint

packages/offix-client-boost/integration_test/test/offline.test.js

@@ -91,7 +91,7 @@ describe('Offline mutations', function () {
       }
       await client.offlineMutate(mutationOptions).catch(err => { });
 
-      const queue = client.queue.queue
+      const queue = client.queue.entries
       expect(queue.length).to.equal(1)
       const op = queue[0].operation.op
       expect(op.mutation).to.deep.equal(mutationOptions.mutation)
@@ -160,7 +160,7 @@ describe('Offline mutations', function () {
         });
       } catch (ignore) { }
 
-      const queue = client.queue.queue
+      const queue = client.queue.entries
       expect(queue.length).to.equal(2)
       expect(queue[0].operation.op.context.operationName).to.equal('updateTask');
       expect(queue[1].operation.op.context.operationName).to.equal('deleteTask');

packages/offix-client/integration_test/test/offline.test.js

@@ -123,7 +123,7 @@ describe('Offline mutations', function () {
       }
       await client.offlineMutate(mutationOptions).catch(err => { });
 
-      const queue = client.queue.queue
+      const queue = client.queue.entries
       expect(queue.length).to.equal(1)
       const op = queue[0].operation.op
       expect(op.mutation).to.deep.equal(mutationOptions.mutation)
@@ -192,7 +192,7 @@ describe('Offline mutations', function () {
         });
       } catch (ignore) { }
 
-      const queue = client.queue.queue
+      const queue = client.queue.entries
       expect(queue.length).to.equal(2)
       expect(queue[0].operation.op.context.operationName).to.equal('updateTask');
       expect(queue[1].operation.op.context.operationName).to.equal('deleteTask');

packages/offix-client/src/ApolloOfflineClient.ts

@@ -96,7 +96,7 @@ export class ApolloOfflineClient extends ApolloClient<NormalizedCacheObject> {
         addOptimisticResponse(this, operation);
       },
       onOperationSuccess: (operation: ApolloQueueEntryOperation, result: FetchResult) => {
-        replaceClientGeneratedIDsInQueue(this.scheduler.queue.queue, operation, result);
+        replaceClientGeneratedIDsInQueue(this.scheduler.queue, operation, result);
         removeOptimisticResponse(this, operation);
       },
       onOperationFailure: (operation: ApolloQueueEntryOperation, error) => {

packages/offix-client/src/apollo/optimisticResponseHelpers.ts

@@ -1,4 +1,4 @@
-import { ApolloQueueEntryOperation, ApolloQueueEntry } from "./ApolloOfflineTypes";
+import { ApolloQueueEntryOperation, ApolloOfflineQueue } from "./ApolloOfflineTypes";
 import { CacheUpdates, isClientGeneratedId } from "offix-cache";
 import { FetchResult } from "apollo-link";
 import ApolloClient from "apollo-client";
@@ -43,7 +43,7 @@ export function restoreOptimisticResponse(
  * we need to update all references in the queue to the client generated ID
  * with the actual ID returned from the server.
  */
-export function replaceClientGeneratedIDsInQueue(queue: ApolloQueueEntry[], operation: ApolloQueueEntryOperation, result: FetchResult) {
+export function replaceClientGeneratedIDsInQueue(queue: ApolloOfflineQueue, operation: ApolloQueueEntryOperation, result: FetchResult) {
 
   const op = operation.op;
   const operationName = op.context.operationName as string;
@@ -62,12 +62,13 @@ export function replaceClientGeneratedIDsInQueue(queue: ApolloQueueEntry[], oper
   }
 
   if (isClientGeneratedId(optimisticId)) {
-    queue.forEach((entry) => {
+    queue.entries.forEach((entry) => {
       // replace all instances of the optimistic id in the queue with
       // the new id that came back from the server
       traverse(entry.operation.op.variables).forEach(function(val) {
         if (this.isLeaf && val && val === optimisticId) {
           this.update(resultId);
+          queue.updateOperation(entry.operation);
         }
       });
     });

packages/offix-client/test/conflictBase.test.ts

@@ -62,7 +62,7 @@ it("base should be correctly calculated with regular id field", async function()
       returnType: "Task"
     });
   } catch (e) {
-    const operationInQueue = client.queue.queue[0].operation.op;
+    const operationInQueue = client.queue.entries[0].operation.op;
     expect(operationInQueue.context.conflictBase).toEqual(newTask);
   }
 });
@@ -123,7 +123,7 @@ it("base should be correctly calculated with custom id field", async function()
       idField: "uuid"
     });
   } catch (e) {
-    const operationInQueue = client.queue.queue[0].operation.op;
+    const operationInQueue = client.queue.entries[0].operation.op;
     expect(operationInQueue.context.conflictBase).toEqual(newTask);
   }
 });
@@ -183,7 +183,7 @@ it("base should be correctly calculated with if custom id is non stanard", async
       idField: "uuid"
     });
   } catch (e) {
-    const operationInQueue = client.queue.queue[0].operation.op;
+    const operationInQueue = client.queue.entries[0].operation.op;
     expect(operationInQueue.context.conflictBase).toEqual(newTask);
   }
 });

packages/offix-client/test/optimisticResponseHelpers.test.ts

@@ -1,5 +1,6 @@
 import { replaceClientGeneratedIDsInQueue } from "../src/apollo/optimisticResponseHelpers";
 import { DocumentNode } from "graphql";
+import { ApolloOfflineQueue, ApolloQueueEntryOperation } from "../src";
 
 test("Process id without change", () => {
   const finalId = "test:1";
@@ -19,7 +20,13 @@ test("Process id without change", () => {
       qid: "someId"
     }
   };
-  const queue = [entry];
+  const queue = {
+    entries: [entry],
+    updateOperation(operation: ApolloQueueEntryOperation) {
+      //no op
+    }
+  } as any as ApolloOfflineQueue;
+
   replaceClientGeneratedIDsInQueue(queue, entry.operation, { data: { test: { id: "notApplied:1" } } });
   expect(exampleOperation.variables.id).toBe(finalId);
 });
@@ -42,7 +49,12 @@ test("Process with change", () => {
       qid: "someId"
     }
   };
-  const queue = [entry];
+  const queue = {
+    entries: [entry],
+    updateOperation(operation: ApolloQueueEntryOperation) {
+      //no op
+    }
+  } as any as ApolloOfflineQueue;
   replaceClientGeneratedIDsInQueue(queue, entry.operation, { data: { test: { id: "applied:1" } } });
   expect(exampleOperation.variables.id).toBe("applied:1");
 });
@@ -83,7 +95,12 @@ test("Can handle cases where variables is a nested object", () => {
       }
     }
   };
-  const queue = [op0, op1];
+  const queue = {
+    entries: [op0, op1],
+    updateOperation(operation: ApolloQueueEntryOperation) {
+      //no op
+    }
+  } as any as ApolloOfflineQueue;
 
   const op0Result = { data: { someOperation: { id: "applied:1" } } };
   replaceClientGeneratedIDsInQueue(queue, op0.operation, op0Result);
@@ -127,9 +144,63 @@ test("Can handle cases optimistic value is referenced in other keys (example: re
       }
     }
   };
-  const queue = [op0, op1];
+  const queue = {
+    entries: [op0, op1],
+    updateOperation(operation: ApolloQueueEntryOperation) {
+      //no op
+    }
+  } as any as ApolloOfflineQueue;
 
   const op0Result = { data: { createExample: { id: "applied:1" } } };
   replaceClientGeneratedIDsInQueue(queue, op0.operation, op0Result);
   expect(op1.operation.op.variables.anotherCreateExampleInput.parentId).toBe("applied:1");
 });
+
+test("queue.updateOperation is called for each updated item", () => {
+  const optimisticId = "client:1";
+  const op0 = {
+    operation: {
+      qid: "queue:1",
+      op: {
+        mutation: {} as DocumentNode,
+        variables: {
+          someOperationInput: {
+            id: optimisticId
+          }
+        },
+        optimisticResponse: { someOperation: { id: optimisticId } },
+        context: {
+          operationName: "someOperation"
+        }
+      }
+    }
+  };
+  const op1 = {
+    operation: {
+      qid: "queue:2",
+      op: {
+        mutation: {} as DocumentNode,
+        variables: {
+          anotherOperationInput: {
+            id: optimisticId
+          }
+        },
+        optimisticResponse: { anotherOperation: { id: optimisticId } },
+        context: {
+          operationName: "anotherOperation"
+        }
+      }
+    }
+  };
+  const updateOperation = jest.fn();
+  const queue = {
+    entries: [op0, op1],
+    updateOperation
+  } as any as ApolloOfflineQueue;
+
+  const op0Result = { data: { someOperation: { id: "applied:1" } } };
+  replaceClientGeneratedIDsInQueue(queue, op0.operation, op0Result);
+  expect(op0.operation.op.variables.someOperationInput.id).toBe("applied:1");
+  expect(op1.operation.op.variables.anotherOperationInput.id).toBe("applied:1");
+  expect(updateOperation).toHaveBeenCalledTimes(2);
+});

packages/offix-scheduler/src/queue/OfflineQueue.ts

@@ -49,7 +49,7 @@ export interface QueueEntryHandlers {
  * - updating client IDs with server IDs (explained below)
  */
 export class OfflineQueue<T> {
-  public queue: Array<QueueEntry<T>> = [];
+  public entries: Array<QueueEntry<T>> = [];
 
   // listeners that can be added by the user to handle various events coming from the offline queue
   public listeners: Array<OfflineQueueListener<T>> = [];
@@ -81,7 +81,7 @@ export class OfflineQueue<T> {
     };
 
     // enqueue and persist
-    this.queue.push(entry);
+    this.entries.push(entry);
     // notify listeners
     this.onOperationEnqueued(entry.operation);
 
@@ -106,7 +106,7 @@ export class OfflineQueue<T> {
   }
 
   public async dequeueOperation(entry: QueueEntry<T>) {
-    this.queue = this.queue.filter(e => e !== entry);
+    this.entries = this.entries.filter(e => e !== entry);
     if (this.store.initialized) {
       try {
         await this.store.removeEntry(entry.operation);
@@ -118,8 +118,18 @@ export class OfflineQueue<T> {
     }
   }
 
+  public async updateOperation(operation: QueueEntryOperation<T>) {
+    const { qid } = operation;
+    const index = this.entries.findIndex((entry) => { return entry.operation.qid === qid; });
+    if (index === -1) {
+      throw new Error(`error updating item in queue could not find entry for operation ${operation}`);
+    }
+    this.entries[index].operation = operation;
+    await this.store.saveEntry(operation);
+  }
+
   public async forwardOperations() {
-    for (const entry of this.queue) {
+    for (const entry of this.entries) {
       await this.forwardOperation(entry);
     }
   }
@@ -146,7 +156,7 @@ export class OfflineQueue<T> {
         for (const entry of offlineEntries) {
           this.onOperationRequeued(entry.operation);
         }
-        this.queue = offlineEntries;
+        this.entries = offlineEntries;
       } catch (error) {
         // TODO should we be logging something here?
         // TODO integration tests covering this
@@ -212,7 +222,7 @@ export class OfflineQueue<T> {
       this.onOperationSuccess(entry.operation, result);
     }
     this.dequeueOperation(entry);
-    if (this.queue.length === 0) {
+    if (this.entries.length === 0) {
       this.queueCleared();
     }
   }

packages/offix-scheduler/src/store/OfflineStore.ts

@@ -37,8 +37,11 @@ export class OfflineStore<T> {
   public async saveEntry(entry: QueueEntryOperation<T>) {
     const serialized = this.serializer.serializeForStorage(entry);
     const offlineKey = this.getOfflineKey(entry.qid);
-    this.arrayOfKeys.push(offlineKey);
-    await this.storage.setItem(this.offlineMetaKey, this.arrayOfKeys);
+    // only add the offline key to the arrray if it's not already there
+    if (!this.arrayOfKeys.includes(offlineKey)) {
+      this.arrayOfKeys.push(offlineKey);
+      await this.storage.setItem(this.offlineMetaKey, this.arrayOfKeys);
+    }
     await this.storage.setItem(offlineKey, serialized);
   }
 

packages/offix-scheduler/test/OfflineStore.test.ts

@@ -50,6 +50,36 @@ it("offlineStore.saveEntry stores data", async () => {
   expect(offlineData[0].operation).toEqual(entry);
 });
 
+it("offlineStore.saveEntry overwrites existing data", async () => {
+  const offlineStore = new OfflineStore(storage as PersistentStore<PersistedData>, mockSerializer);
+  await offlineStore.init();
+
+  const entry: QueueEntryOperation<any> = {
+    op: {
+      hello: "world"
+    },
+    qid: "123"
+  };
+
+  await offlineStore.saveEntry(entry);
+
+  let offlineData = await offlineStore.getOfflineData();
+  expect(offlineData.length).toBe(1);
+  expect(offlineData[0].operation).toEqual(entry);
+
+  const updatedEntry: QueueEntryOperation<any> = {
+    op: {
+      hello: "universe"
+    },
+    qid: "123"
+  };
+
+  await offlineStore.saveEntry(updatedEntry);
+  offlineData = await offlineStore.getOfflineData();
+  expect(offlineData.length).toBe(1);
+  expect(offlineData[0].operation).toEqual(updatedEntry);
+});
+
 it("offlineStore.removeEntry removes data", async () => {
   const offlineStore = new OfflineStore(storage as PersistentStore<PersistedData>, mockSerializer);
   await offlineStore.init();