From f360a59b0545adf26825a9880e3a6c49eabe5d1c Mon Sep 17 00:00:00 2001 From: Jon Wire Date: Fri, 6 May 2022 13:06:55 -0500 Subject: [PATCH 1/8] test: added observeQuery tests; one skipped, intended to show correct behavior ahead of fix --- packages/datastore/__tests__/DataStore.ts | 81 +++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/packages/datastore/__tests__/DataStore.ts b/packages/datastore/__tests__/DataStore.ts index c61f652c215..678167f019a 100644 --- a/packages/datastore/__tests__/DataStore.ts +++ b/packages/datastore/__tests__/DataStore.ts @@ -349,6 +349,87 @@ describe('DataStore observeQuery, with fake-indexeddb and fake sync', () => { } }); + test('can filter items', async done => { + try { + const expecteds = [0, 5]; + + const sub = DataStore.observeQuery(Post, p => + p.title('contains', 'include') + ).subscribe(({ items }) => { + const expected = expecteds.shift() || 0; + expect(items.length).toBe(expected); + + for (const item of items) { + expect(item.title).toMatch('include'); + } + + if (expecteds.length === 0) { + sub.unsubscribe(); + done(); + } + }); + + setTimeout(async () => { + for (let i = 0; i < 10; i++) { + await DataStore.save( + new Post({ + title: `the post ${i} - ${Boolean(i % 2) ? 'include' : 'omit'}`, + }) + ); + } + }, 100); + } catch (error) { + done(error); + } + }); + + // TODO: fix. + // See: https://github.com/aws-amplify/amplify-js/issues/9325 + // The test currently times out, because the mutation is filtered before + // observeQuery can get ahold of it, so the last expected subscription + // message never even arrives. + test.skip('can remove newly-unmatched items out of the snapshot on subsequent saves', async done => { + try { + const expecteds = [0, 5, 4]; + + const sub = DataStore.observeQuery(Post, p => + p.title('contains', 'include') + ).subscribe(({ items }) => { + const expected = expecteds.shift() || 0; + expect(items.length).toBe(expected); + + for (const item of items) { + expect(item.title).toMatch('include'); + } + + if (expecteds.length === 0) { + sub.unsubscribe(); + done(); + } + }); + + setTimeout(async () => { + for (let i = 0; i < 10; i++) { + await DataStore.save( + new Post({ + title: `the post ${i} - ${Boolean(i % 2) ? 'include' : 'omit'}`, + }) + ); + } + setTimeout(async () => { + const itemToEdit = (await DataStore.query(Post)).pop(); + await DataStore.save( + Post.copyOf(itemToEdit, draft => { + draft.title = 'edited post - omit'; + }) + ); + }, 100); + }, 100); + } catch (error) { + done(error); + } + }); + test('publishes preexisting local data AND follows up with subsequent saves', async done => { try { const expecteds = [5, 15]; From 9c239b4340ece5e780303a9752a9dcf5639fac7f Mon Sep 17 00:00:00 2001 From: Jon Wire Date: Wed, 1 Jun 2022 16:16:35 -0500 Subject: [PATCH 2/8] working fix; still needs cleanup --- packages/datastore/__tests__/DataStore.ts | 163 ++++++++++++------ packages/datastore/src/datastore/datastore.ts | 48 +++++- 2 files changed, 161 insertions(+), 50 deletions(-) diff --git a/packages/datastore/__tests__/DataStore.ts b/packages/datastore/__tests__/DataStore.ts index ee0c09a12bd..60e5988e437 100644 --- a/packages/datastore/__tests__/DataStore.ts +++ b/packages/datastore/__tests__/DataStore.ts @@ -13,7 +13,7 @@ import { PersistentModel, PersistentModelConstructor, } from '../src/types'; -import { Comment, Model, Post, Metadata, testSchema } from './helpers'; +import { Comment, Model, Post, Metadata, testSchema, pause } from './helpers'; let initSchema: typeof initSchemaType; let DataStore: typeof DataStoreType; @@ -273,6 +273,8 @@ describe('DataStore observeQuery, with fake-indexeddb and fake sync', () => { Comment: PersistentModelConstructor; Post: PersistentModelConstructor; }); + + await DataStore.start(); await DataStore.clear(); // Fully faking or mocking the sync engine would be pretty significant. @@ -294,6 +296,25 @@ describe('DataStore observeQuery, with fake-indexeddb and fake sync', () => { (DataStore as any).syncPageSize = 1000; }); + afterEach(async () => { + // + // ~~~~ NAUGHTINESS WARNING! ~~~~ + // + // ( cover your eyes ) + // + // this prevents pollutions between tests, especially those that test observe + // behavior. it would seem that, due to the order in which DataStore processes + // observers internally, we need to inject a small async pause to let DataStore + // "settle" before clearing it and starting the next test -- IF NOT, we get + // spooky contamination between tests. + // + await pause(10); + + // and out of an abundance of caution: + await DataStore.start(); + await DataStore.clear(); + }); + test('publishes preexisting local data immediately', async done => { try { for (let i = 0; i < 5; i++) { @@ -388,13 +409,16 @@ describe('DataStore observeQuery, with fake-indexeddb and fake sync', () => { // The test currently times out, because the mutation is filtered before // observeQuery can get ahold of it, so the last expected subscription // message never even arrives. - test.skip('can remove newly-unmatched items out of the snapshot on subsequent saves', async done => { + test('can remove newly-unmatched items out of the snapshot on subsequent saves', async done => { try { - const expecteds = [0, 5, 4]; - + // watch for post snapshots. + // the first "real" snapshot should include all five posts with "include" + // in the title. after the update to change ONE of those posts to "omit" instead, + // we should see a snapshot of 4 posts with the updated post removed. + const expecteds = [0, 4, 3]; const sub = DataStore.observeQuery(Post, p => p.title('contains', 'include') - ).subscribe(({ items }) => { + ).subscribe(async ({ items }) => { const expected = expecteds.shift() || 0; expect(items.length).toBe(expected); @@ -402,13 +426,43 @@ describe('DataStore observeQuery, with fake-indexeddb and fake sync', () => { expect(item.title).toMatch('include'); } - if (expecteds.length === 0) { + if (expecteds.length === 1) { + // after the second snapshot arrives, changes a single post from + // "the post # - include" + // to + // "edited post - omit" + + // because this is intended to trigger a new, post-sync'd snapshot, + // we'll start with a little sanity check: + expect( + ((DataStore as any).sync as any).getModelSyncedStatus({}) + ).toBe(true); + + await pause(1); + const itemToEdit = ( + await DataStore.query(Post, p => p.title('contains', 'include')) + ).pop(); + await DataStore.save( + Post.copyOf(itemToEdit, draft => { + draft.title = 'second edited post - omit'; + }) + ); + } else if (expecteds.length === 0) { sub.unsubscribe(); done(); } }); setTimeout(async () => { + // creates posts like: + // + // "the post 0 - include" + // "the post 1 - omit" + // "the post 2 - include" + // "the post 3 - omit" + // + // etc. + // for (let i = 0; i < 10; i++) { await DataStore.save( new Post({ @@ -416,60 +470,73 @@ describe('DataStore observeQuery, with fake-indexeddb and fake sync', () => { }) ); } - setTimeout(async () => { - const itemToEdit = (await DataStore.query(Post)).pop(); - await DataStore.save( - Post.copyOf(itemToEdit, draft => { - draft.title = 'edited post - omit'; - }) - ); - }, 100); - }, 100); - } catch (error) { - done(error); - } - }); - - test('publishes preexisting local data AND follows up with subsequent saves', async done => { - try { - const expecteds = [5, 15]; - for (let i = 0; i < 5; i++) { + // changes a single post from + // "the post # - include" + // to + // "edited post - omit" + await pause(1); + ((DataStore as any).sync as any).getModelSyncedStatus = (model: any) => + true; + + // the first edit simulates a quick-turnaround update that gets + // applied while the first snapshot is still being generated + const itemToEdit = ( + await DataStore.query(Post, p => p.title('contains', 'include')) + ).pop(); await DataStore.save( - new Post({ - title: `the post ${i}`, + Post.copyOf(itemToEdit, draft => { + draft.title = 'first edited post - omit'; }) ); - } - - const sub = DataStore.observeQuery(Post).subscribe( - ({ items, isSynced }) => { - const expected = expecteds.shift() || 0; - expect(items.length).toBe(expected); - - for (let i = 0; i < expected; i++) { - expect(items[i].title).toEqual(`the post ${i}`); - } + }, 1); + } catch (error) { + done(error); + } + }); - if (expecteds.length === 0) { - sub.unsubscribe(); - done(); - } - } - ); + test('publishes preexisting local data AND follows up with subsequent saves', done => { + (async () => { + try { + const expecteds = [5, 15]; - setTimeout(async () => { - for (let i = 5; i < 15; i++) { + for (let i = 0; i < 5; i++) { await DataStore.save( new Post({ title: `the post ${i}`, }) ); } - }, 100); - } catch (error) { - done(error); - } + + const sub = DataStore.observeQuery(Post).subscribe( + ({ items, isSynced }) => { + const expected = expecteds.shift() || 0; + expect(items.length).toBe(expected); + + for (let i = 0; i < expected; i++) { + expect(items[i].title).toEqual(`the post ${i}`); + } + + if (expecteds.length === 0) { + sub.unsubscribe(); + done(); + } + } + ); + + setTimeout(async () => { + for (let i = 5; i < 15; i++) { + await DataStore.save( + new Post({ + title: `the post ${i}`, + }) + ); + } + }, 100); + } catch (error) { + done(error); + } + })(); }); }); diff --git a/packages/datastore/src/datastore/datastore.ts b/packages/datastore/src/datastore/datastore.ts index d6e58105e83..c9def5361d0 100644 --- a/packages/datastore/src/datastore/datastore.ts +++ b/packages/datastore/src/datastore/datastore.ts @@ -70,7 +70,9 @@ import { registerNonModelClass, sortCompareFunction, DeferredCallbackResolver, + validatePredicate, } from '../util'; +import { has } from 'immer/dist/internal'; setAutoFreeze(true); enablePatches(); @@ -1188,6 +1190,7 @@ class DataStore { const itemsChanged = new Map(); let deletedItemIds: string[] = []; let handle: ZenObservable.Subscription; + let predicate: ModelPredicate; const generateAndEmitSnapshot = (): void => { const snapshot = generateSnapshot(); @@ -1205,6 +1208,28 @@ class DataStore { const { sort } = options || {}; const sortOptions = sort ? { sort } : undefined; + const modelDefinition = getModelDefinition(model); + if (isQueryOne(criteria)) { + predicate = ModelPredicateCreator.createForId( + modelDefinition, + criteria + ); + } else { + if (isPredicatesAll(criteria)) { + // Predicates.ALL means "all records", so no predicate (undefined) + predicate = undefined; + } else { + predicate = ModelPredicateCreator.createFromExisting( + modelDefinition, + criteria + ); + } + } + + const { predicates, type: predicateGroupType } = + ModelPredicateCreator.getPredicates(predicate, false) || {}; + const hasPredicate = !!predicates; + (async () => { try { // first, query and return any locally-available records @@ -1214,10 +1239,29 @@ class DataStore { // observe the model and send a stream of updates (debounced) handle = this.observe( - model, + model // @ts-ignore TODO: fix this TSlint error - criteria + // criteria ).subscribe(({ element, model, opType }) => { + // We need to filter HERE instead of in `observe()` to ensure we see updated for + // items that ~become~ filtered-out as the result of a save. We need to remove + // those items from the existing snapshot. + if ( + hasPredicate && + !validatePredicate(element, predicateGroupType, predicates) + ) { + if ( + opType === 'UPDATE' && + (items.has(element.id) || itemsChanged.has(element.id)) + ) { + // we need to track this as a "deleted item" to calcuate a correct page `limit`. + deletedItemIds.push(element.id); + } else { + // ignore updates for irrelevant/filtered items. + return; + } + } + // Flag items which have been recently deleted // NOTE: Merging of separate operations to the same model instance is handled upstream // in the `mergePage` method within src/sync/merger.ts. The final state of a model instance From 262f5e1934ec2675416e22fda4235ac61b48ec7e Mon Sep 17 00:00:00 2001 From: Jon Wire Date: Fri, 3 Jun 2022 09:40:28 -0500 Subject: [PATCH 3/8] a little cleanup --- packages/datastore/__tests__/DataStore.ts | 21 +++-- packages/datastore/src/datastore/datastore.ts | 76 +++++++++---------- 2 files changed, 47 insertions(+), 50 deletions(-) diff --git a/packages/datastore/__tests__/DataStore.ts b/packages/datastore/__tests__/DataStore.ts index 60e5988e437..55136c9f759 100644 --- a/packages/datastore/__tests__/DataStore.ts +++ b/packages/datastore/__tests__/DataStore.ts @@ -302,7 +302,7 @@ describe('DataStore observeQuery, with fake-indexeddb and fake sync', () => { // // ( cover your eyes ) // - // this prevents pollutions between tests, especially those that test observe + // this prevents pollution between tests, especially those that test observe // behavior. it would seem that, due to the order in which DataStore processes // observers internally, we need to inject a small async pause to let DataStore // "settle" before clearing it and starting the next test -- IF NOT, we get @@ -310,7 +310,7 @@ describe('DataStore observeQuery, with fake-indexeddb and fake sync', () => { // await pause(10); - // and out of an abundance of caution: + // an abundance of caution await DataStore.start(); await DataStore.clear(); }); @@ -404,11 +404,7 @@ describe('DataStore observeQuery, with fake-indexeddb and fake sync', () => { } }); - // TODO: fix. - // See: https://github.com/aws-amplify/amplify-js/issues/9325 - // The test currently times out, because the mutation is filtered before - // observeQuery can get ahold of it, so the last expected subscription - // message never even arrives. + // Fix for: https://github.com/aws-amplify/amplify-js/issues/9325 test('can remove newly-unmatched items out of the snapshot on subsequent saves', async done => { try { // watch for post snapshots. @@ -427,13 +423,14 @@ describe('DataStore observeQuery, with fake-indexeddb and fake sync', () => { } if (expecteds.length === 1) { - // after the second snapshot arrives, changes a single post from + // After the second snapshot arrives, changes a single post from // "the post # - include" // to // "edited post - omit" - // because this is intended to trigger a new, post-sync'd snapshot, - // we'll start with a little sanity check: + // This is intended to trigger a new, after-sync'd snapshot. + // This sanity-checks helps confirms we're testing what we think + // we're testing: expect( ((DataStore as any).sync as any).getModelSyncedStatus({}) ).toBe(true); @@ -454,7 +451,7 @@ describe('DataStore observeQuery, with fake-indexeddb and fake sync', () => { }); setTimeout(async () => { - // creates posts like: + // Creates posts like: // // "the post 0 - include" // "the post 1 - omit" @@ -471,7 +468,7 @@ describe('DataStore observeQuery, with fake-indexeddb and fake sync', () => { ); } - // changes a single post from + // Changes a single post from // "the post # - include" // to // "edited post - omit" diff --git a/packages/datastore/src/datastore/datastore.ts b/packages/datastore/src/datastore/datastore.ts index c9def5361d0..dbd338f66fe 100644 --- a/packages/datastore/src/datastore/datastore.ts +++ b/packages/datastore/src/datastore/datastore.ts @@ -72,7 +72,6 @@ import { DeferredCallbackResolver, validatePredicate, } from '../util'; -import { has } from 'immer/dist/internal'; setAutoFreeze(true); enablePatches(); @@ -1237,53 +1236,54 @@ class DataStore { items.set(item.id, item) ); - // observe the model and send a stream of updates (debounced) - handle = this.observe( - model - // @ts-ignore TODO: fix this TSlint error - // criteria - ).subscribe(({ element, model, opType }) => { - // We need to filter HERE instead of in `observe()` to ensure we see updated for - // items that ~become~ filtered-out as the result of a save. We need to remove - // those items from the existing snapshot. - if ( - hasPredicate && - !validatePredicate(element, predicateGroupType, predicates) - ) { + // Observe the model and send a stream of updates (debounced). + // We need to post-filter results instead of passing criteria through + // to have visibility into items that move from in-set to out-of-set. + // We need to explicitly remove those items from the existing snapshot. + handle = this.observe(model).subscribe( + ({ element, model, opType }) => { if ( - opType === 'UPDATE' && - (items.has(element.id) || itemsChanged.has(element.id)) + hasPredicate && + !validatePredicate(element, predicateGroupType, predicates) ) { - // we need to track this as a "deleted item" to calcuate a correct page `limit`. + if ( + opType === 'UPDATE' && + (items.has(element.id) || itemsChanged.has(element.id)) + ) { + // tracking as a "deleted item" will include the item in + // page limit calculations and ensure it is removed from the + // final items collection, regardless of which collection(s) + // it is currently in. (I mean, it could be in both, right!?) + deletedItemIds.push(element.id); + } else { + // ignore updates for irrelevant/filtered items. + return; + } + } + + // Flag items which have been recently deleted + // NOTE: Merging of separate operations to the same model instance is handled upstream + // in the `mergePage` method within src/sync/merger.ts. The final state of a model instance + // depends on the LATEST record (for a given id). + if (opType === 'DELETE') { deletedItemIds.push(element.id); } else { - // ignore updates for irrelevant/filtered items. - return; + itemsChanged.set(element.id, element); } - } - // Flag items which have been recently deleted - // NOTE: Merging of separate operations to the same model instance is handled upstream - // in the `mergePage` method within src/sync/merger.ts. The final state of a model instance - // depends on the LATEST record (for a given id). - if (opType === 'DELETE') { - deletedItemIds.push(element.id); - } else { - itemsChanged.set(element.id, element); - } + const isSynced = this.sync?.getModelSyncedStatus(model) ?? false; - const isSynced = this.sync?.getModelSyncedStatus(model) ?? false; + const limit = + itemsChanged.size - deletedItemIds.length >= this.syncPageSize; - const limit = - itemsChanged.size - deletedItemIds.length >= this.syncPageSize; + if (limit || isSynced) { + limitTimerRace.resolve(); + } - if (limit || isSynced) { - limitTimerRace.resolve(); + // kicks off every subsequent race as results sync down + limitTimerRace.start(); } - - // kicks off every subsequent race as results sync down - limitTimerRace.start(); - }); + ); // returns a set of initial/locally-available results generateAndEmitSnapshot(); From 1c07f8708b334e563892d31c7d4c99bb88439505 Mon Sep 17 00:00:00 2001 From: Jon Wire Date: Fri, 3 Jun 2022 11:12:29 -0500 Subject: [PATCH 4/8] comment, docstring updates --- packages/datastore/__tests__/DataStore.ts | 12 ++++---- packages/datastore/src/datastore/datastore.ts | 30 ++++++++++++++++++- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/packages/datastore/__tests__/DataStore.ts b/packages/datastore/__tests__/DataStore.ts index 55136c9f759..904606020d7 100644 --- a/packages/datastore/__tests__/DataStore.ts +++ b/packages/datastore/__tests__/DataStore.ts @@ -302,11 +302,13 @@ describe('DataStore observeQuery, with fake-indexeddb and fake sync', () => { // // ( cover your eyes ) // - // this prevents pollution between tests, especially those that test observe - // behavior. it would seem that, due to the order in which DataStore processes - // observers internally, we need to inject a small async pause to let DataStore - // "settle" before clearing it and starting the next test -- IF NOT, we get - // spooky contamination between tests. + // this prevents pollution between tests that may include observe() calls. + // This is a cheap solution let DataStore "settle" before clearing it and + // starting the next test. If we don't do this, we get "spooky" + // contamination between tests. + // + // NOTE: If you know of a better way to isolate these tests, please + // replace this pause! // await pause(10); diff --git a/packages/datastore/src/datastore/datastore.ts b/packages/datastore/src/datastore/datastore.ts index dbd338f66fe..41d7d84b450 100644 --- a/packages/datastore/src/datastore/datastore.ts +++ b/packages/datastore/src/datastore/datastore.ts @@ -1293,6 +1293,13 @@ class DataStore { })(); // TODO: abstract this function into a util file to be able to write better unit tests + + /** + * Combines the `items`, `itemsChanged`, and `deletedItemIds` collections into + * a snapshot in the form of `{ items: T[], isSynced: boolean}`. + * + * SIDE EFFECT: The shared `items` collection is recreated. + */ const generateSnapshot = (): DataStoreSnapshot => { const isSynced = this.sync?.getModelSyncedStatus(model) ?? false; const itemsArray = [ @@ -1316,6 +1323,14 @@ class DataStore { }; }; + /** + * Emits the list of items to the observer. + * + * SIDE EFFECT: `itemsChanged` and `deletedItemIds` are cleared to prepare + * for the next snapshot. + * + * @param snapshot The generated items data to emit. + */ const emitSnapshot = (snapshot: DataStoreSnapshot): void => { // send the generated snapshot to the primary subscription observer.next(snapshot); @@ -1325,6 +1340,12 @@ class DataStore { deletedItemIds = []; }; + /** + * Sorts an `Array` of `T` according to the sort instructions given in the + * original `observeQuery()` call. + * + * @param itemsToSort A array of model type. + */ const sortItems = (itemsToSort: T[]): void => { const modelDefinition = getModelDefinition(model); const pagination = this.processPagination(modelDefinition, options); @@ -1339,7 +1360,14 @@ class DataStore { } }; - // send one last snapshot when the model is fully synced + /** + * Force one last snapshot when the model is fully synced. + * + * This reduces latency for that last snapshot, which will otherwise + * wait for the configured timeout. + * + * @param payload The payload from the Hub event. + */ const hubCallback = ({ payload }): void => { const { event, data } = payload; if ( From 43899d11b84a0e22f4fa11bd3c66c00e7e0d47fc Mon Sep 17 00:00:00 2001 From: Jon Wire Date: Fri, 3 Jun 2022 11:15:25 -0500 Subject: [PATCH 5/8] more docstrings --- packages/datastore/src/datastore/datastore.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/datastore/src/datastore/datastore.ts b/packages/datastore/src/datastore/datastore.ts index 41d7d84b450..f075c95d375 100644 --- a/packages/datastore/src/datastore/datastore.ts +++ b/packages/datastore/src/datastore/datastore.ts @@ -1191,6 +1191,18 @@ class DataStore { let handle: ZenObservable.Subscription; let predicate: ModelPredicate; + /** + * As the name suggests, this geneates a snapshot in the form of + * `{items: T[], isSynced: boolean}` + * and sends it to the observer. + * + * SIDE EFFECT: The underlying generation and emission methods may touch: + * 1. `items` + * 2. `itemsChanged + * 3. `deletedItemIds` + * + * Refer to `generateSnapshot` and `emitSnapshot` for more details. + */ const generateAndEmitSnapshot = (): void => { const snapshot = generateSnapshot(); emitSnapshot(snapshot); @@ -1292,8 +1304,6 @@ class DataStore { } })(); - // TODO: abstract this function into a util file to be able to write better unit tests - /** * Combines the `items`, `itemsChanged`, and `deletedItemIds` collections into * a snapshot in the form of `{ items: T[], isSynced: boolean}`. From ace33602921857e59490f8758f8dfdf879940e7d Mon Sep 17 00:00:00 2001 From: Jon Wire Date: Fri, 3 Jun 2022 11:16:40 -0500 Subject: [PATCH 6/8] fixed formatting in docstring --- packages/datastore/src/datastore/datastore.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/datastore/src/datastore/datastore.ts b/packages/datastore/src/datastore/datastore.ts index f075c95d375..0353194f282 100644 --- a/packages/datastore/src/datastore/datastore.ts +++ b/packages/datastore/src/datastore/datastore.ts @@ -1197,9 +1197,7 @@ class DataStore { * and sends it to the observer. * * SIDE EFFECT: The underlying generation and emission methods may touch: - * 1. `items` - * 2. `itemsChanged - * 3. `deletedItemIds` + * `items`, `itemsChanged`, and `deletedItemIds`. * * Refer to `generateSnapshot` and `emitSnapshot` for more details. */ From eadb5fccfd249d0a5281367d291bbe1bab8c8a51 Mon Sep 17 00:00:00 2001 From: Jon Wire Date: Wed, 8 Jun 2022 18:30:16 -0500 Subject: [PATCH 7/8] replaced naughty test pollution solution with better one --- packages/datastore/__tests__/DataStore.ts | 25 ++++------------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/packages/datastore/__tests__/DataStore.ts b/packages/datastore/__tests__/DataStore.ts index 904606020d7..36be1bf0c89 100644 --- a/packages/datastore/__tests__/DataStore.ts +++ b/packages/datastore/__tests__/DataStore.ts @@ -274,6 +274,10 @@ describe('DataStore observeQuery, with fake-indexeddb and fake sync', () => { Post: PersistentModelConstructor; }); + // This prevents pollution between tests. DataStore may have processes in + // flight that need to settle. If we stampede ahead before we do this, + // we can end up in very goofy states when we try to re-init the schema. + await DataStore.stop(); await DataStore.start(); await DataStore.clear(); @@ -296,27 +300,6 @@ describe('DataStore observeQuery, with fake-indexeddb and fake sync', () => { (DataStore as any).syncPageSize = 1000; }); - afterEach(async () => { - // - // ~~~~ NAUGHTINESS WARNING! ~~~~ - // - // ( cover your eyes ) - // - // this prevents pollution between tests that may include observe() calls. - // This is a cheap solution let DataStore "settle" before clearing it and - // starting the next test. If we don't do this, we get "spooky" - // contamination between tests. - // - // NOTE: If you know of a better way to isolate these tests, please - // replace this pause! - // - await pause(10); - - // an abundance of caution - await DataStore.start(); - await DataStore.clear(); - }); - test('publishes preexisting local data immediately', async done => { try { for (let i = 0; i < 5; i++) { From cd09343caf1d87692ea0d17697bbd5b24135bf83 Mon Sep 17 00:00:00 2001 From: Jon Wire Date: Thu, 9 Jun 2022 14:35:41 -0500 Subject: [PATCH 8/8] added test cases for delete --- packages/datastore/__tests__/DataStore.ts | 80 ++++++++++++++++++++++- 1 file changed, 79 insertions(+), 1 deletion(-) diff --git a/packages/datastore/__tests__/DataStore.ts b/packages/datastore/__tests__/DataStore.ts index 36be1bf0c89..88df077d90d 100644 --- a/packages/datastore/__tests__/DataStore.ts +++ b/packages/datastore/__tests__/DataStore.ts @@ -383,7 +383,7 @@ describe('DataStore observeQuery, with fake-indexeddb and fake sync', () => { }) ); } - }, 100); + }, 1); } catch (error) { done(error); } @@ -520,6 +520,84 @@ describe('DataStore observeQuery, with fake-indexeddb and fake sync', () => { } })(); }); + + test('removes deleted items from the snapshot', done => { + (async () => { + try { + const expecteds = [5, 4]; + + for (let i = 0; i < 5; i++) { + await DataStore.save( + new Post({ + title: `the post ${i}`, + }) + ); + } + + const sub = DataStore.observeQuery(Post).subscribe( + ({ items, isSynced }) => { + const expected = expecteds.shift() || 0; + expect(items.length).toBe(expected); + + for (let i = 0; i < expected; i++) { + expect(items[i].title).toContain(`the post`); + } + + if (expecteds.length === 0) { + sub.unsubscribe(); + done(); + } + } + ); + + setTimeout(async () => { + const itemToDelete = (await DataStore.query(Post)).pop(); + await DataStore.delete(itemToDelete); + }, 1); + } catch (error) { + done(error); + } + })(); + }); + + test('removes deleted items from the snapshot with a predicate', done => { + (async () => { + try { + const expecteds = [5, 4]; + + for (let i = 0; i < 5; i++) { + await DataStore.save( + new Post({ + title: `the post ${i}`, + }) + ); + } + + const sub = DataStore.observeQuery(Post, p => + p.title('beginsWith', 'the post') + ).subscribe(({ items, isSynced }) => { + const expected = expecteds.shift() || 0; + expect(items.length).toBe(expected); + + for (let i = 0; i < expected; i++) { + expect(items[i].title).toContain(`the post`); + } + + if (expecteds.length === 0) { + sub.unsubscribe(); + done(); + } + }); + + setTimeout(async () => { + const itemToDelete = (await DataStore.query(Post)).pop(); + await DataStore.delete(itemToDelete); + }, 1); + } catch (error) { + done(error); + } + })(); + }); }); describe('DataStore tests', () => {