diff --git a/packages/offix-client-boost/integration_test/test/offline.test.js b/packages/offix-client-boost/integration_test/test/offline.test.js index dd820df1a..5c16fc0a3 100644 --- a/packages/offix-client-boost/integration_test/test/offline.test.js +++ b/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'); diff --git a/packages/offix-client/integration_test/test/offline.test.js b/packages/offix-client/integration_test/test/offline.test.js index e8235cd0c..2f1a29d0a 100644 --- a/packages/offix-client/integration_test/test/offline.test.js +++ b/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'); diff --git a/packages/offix-client/src/ApolloOfflineClient.ts b/packages/offix-client/src/ApolloOfflineClient.ts index b937f7616..8faafe7af 100644 --- a/packages/offix-client/src/ApolloOfflineClient.ts +++ b/packages/offix-client/src/ApolloOfflineClient.ts @@ -96,7 +96,7 @@ export class ApolloOfflineClient extends ApolloClient { 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) => { diff --git a/packages/offix-client/src/apollo/optimisticResponseHelpers.ts b/packages/offix-client/src/apollo/optimisticResponseHelpers.ts index 557c01dea..9260b5ae8 100644 --- a/packages/offix-client/src/apollo/optimisticResponseHelpers.ts +++ b/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); } }); }); diff --git a/packages/offix-client/test/conflictBase.test.ts b/packages/offix-client/test/conflictBase.test.ts index 8b428e511..8697061c1 100644 --- a/packages/offix-client/test/conflictBase.test.ts +++ b/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); } }); diff --git a/packages/offix-client/test/optimisticResponseHelpers.test.ts b/packages/offix-client/test/optimisticResponseHelpers.test.ts index 0a48c2e96..1a6e08ed0 100644 --- a/packages/offix-client/test/optimisticResponseHelpers.test.ts +++ b/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); +}); diff --git a/packages/offix-scheduler/src/queue/OfflineQueue.ts b/packages/offix-scheduler/src/queue/OfflineQueue.ts index 58121bdba..06e80f97f 100644 --- a/packages/offix-scheduler/src/queue/OfflineQueue.ts +++ b/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 { - public queue: Array> = []; + public entries: Array> = []; // listeners that can be added by the user to handle various events coming from the offline queue public listeners: Array> = []; @@ -81,7 +81,7 @@ export class OfflineQueue { }; // enqueue and persist - this.queue.push(entry); + this.entries.push(entry); // notify listeners this.onOperationEnqueued(entry.operation); @@ -106,7 +106,7 @@ export class OfflineQueue { } public async dequeueOperation(entry: QueueEntry) { - 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 { } } + public async updateOperation(operation: QueueEntryOperation) { + 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 { 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 { this.onOperationSuccess(entry.operation, result); } this.dequeueOperation(entry); - if (this.queue.length === 0) { + if (this.entries.length === 0) { this.queueCleared(); } } diff --git a/packages/offix-scheduler/src/store/OfflineStore.ts b/packages/offix-scheduler/src/store/OfflineStore.ts index 2641b21e8..b88478d58 100644 --- a/packages/offix-scheduler/src/store/OfflineStore.ts +++ b/packages/offix-scheduler/src/store/OfflineStore.ts @@ -37,8 +37,11 @@ export class OfflineStore { public async saveEntry(entry: QueueEntryOperation) { 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); } diff --git a/packages/offix-scheduler/test/OfflineStore.test.ts b/packages/offix-scheduler/test/OfflineStore.test.ts index f3d08d137..b4535e05f 100644 --- a/packages/offix-scheduler/test/OfflineStore.test.ts +++ b/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, mockSerializer); + await offlineStore.init(); + + const entry: QueueEntryOperation = { + 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 = { + 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, mockSerializer); await offlineStore.init();