feat(datastore): allow sync query to complete when non-applicable dat…

…a present (#10471)  #### Description of changes  Integ tests: https://app.circleci.com/pipelines/github/aws-amplify/amplify-js/15284/workflows/2e4117d4-cb80-4c59-85fa-301be1b9b5e0 #### Issue #, if available  #### Description of how you validated changes #### Checklist  - [ ] PR description included - [ ] `yarn test` passes - [ ] Tests are [changed or added](https://github.com/aws-amplify/amplify-js/blob/main/CONTRIBUTING.md#steps-towards-contributions) - [ ] Relevant documentation is changed or added (and PR referenced) By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
aws-amplify · Oct 14, 2022 · f8e8ff4 · f8e8ff4
1 parent a68de60
commit f8e8ff4
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 162 deletions.
diff --git a/.vscode/launch.json b/.vscode/launch.json
@@ -15,7 +15,7 @@
 				"--inspect-brk",
 				"${workspaceRoot}/node_modules/.bin/jest",
 				// Optionally specify a single test file to run/debug:
-				"storage.test.ts",
+				"sync.test.ts",
 				"--runInBand",
 				"--testTimeout",
 				"600000", // 10 min timeout so jest doesn't error while we're stepping through code

diff --git a/package.json b/package.json
@@ -27,9 +27,7 @@
 		"publish:1.0-stable": "lerna publish --conventional-commits --yes --dist-tag=stable-1.0 --message 'chore(release): Publish [ci skip]' --no-verify-access",
 		"publish:ui-components/main": "lerna publish --canary --force-publish \"*\" --yes --dist-tag=ui-preview --preid=ui-preview --exact --no-verify-access",
 		"publish:verdaccio": "lerna publish --no-push --canary minor --dist-tag=unstable --preid=unstable --exact --force-publish --yes --no-verify-access",
-		"publish:geo/main": "lerna publish --canary --force-publish \"*\" --yes --dist-tag=geo --preid=geo --exact --no-verify-access",
-		"publish:ds-custom-pk": "lerna publish --canary --force-publish \"*\" --yes --dist-tag=custom-pk --preid=custom-pk --exact --no-verify-access",
-		"temp-ds-safe-push": "yarn build --scope @aws-amplify/datastore && yarn test --scope @aws-amplify/datastore && git push origin"
+		"publish:geo/main": "lerna publish --canary --force-publish \"*\" --yes --dist-tag=geo --preid=geo --exact --no-verify-access"
 	},
 	"husky": {
 		"hooks": {

diff --git a/packages/datastore/__tests__/__snapshots__/sync.test.ts.snap b/packages/datastore/__tests__/__snapshots__/sync.test.ts.snap
@@ -38,7 +38,7 @@ Object {
 }
 `;
 
-exports[`Sync jitteredRetry should return partial data and send Hub event when datastorePartialData is set 1`] = `
+exports[`Sync jitteredRetry should return partial data and send Hub event 1`] = `
 Object {
   "data": Object {
     "syncPosts": Object {
@@ -62,31 +62,6 @@ Object {
 }
 `;
 
-exports[`Sync jitteredRetry should throw error and NOT return data or send Hub event when datastorePartialData is not set 1`] = `
-Object {
-  "data": Object {
-    "syncPosts": Object {
-      "items": Array [
-        Object {
-          "id": "1",
-          "title": "Item 1",
-        },
-        null,
-        Object {
-          "id": "3",
-          "title": "Item 3",
-        },
-      ],
-    },
-  },
-  "errors": Array [
-    Object {
-      "message": "Item 2 error",
-    },
-  ],
-}
-`;
-
 exports[`Sync jitteredRetry should throw error if no data is returned 1`] = `
 Object {
   "data": null,

diff --git a/packages/datastore/__tests__/sync.test.ts b/packages/datastore/__tests__/sync.test.ts
@@ -54,7 +54,6 @@ describe('Sync', () => {
 		});
 
 		it('should return all data', async () => {
-			window.sessionStorage.setItem('datastorePartialData', 'true');
 			const resolveResponse = {
 				data: {
 					syncPosts: {
@@ -87,7 +86,6 @@ describe('Sync', () => {
 		});
 
 		it('custom pk: should return all data', async () => {
-			window.sessionStorage.setItem('datastorePartialData', 'true');
 			const resolveResponse = {
 				data: {
 					syncPosts: {
@@ -119,8 +117,7 @@ describe('Sync', () => {
 			expect(data).toMatchSnapshot();
 		});
 
-		it('should return partial data and send Hub event when datastorePartialData is set', async () => {
-			window.sessionStorage.setItem('datastorePartialData', 'true');
+		it('should return partial data and send Hub event', async () => {
 			const rejectResponse = {
 				data: {
 					syncPosts: {
@@ -168,7 +165,7 @@ describe('Sync', () => {
 			expect(data).toMatchSnapshot();
 
 			expect(hubDispatchMock).toHaveBeenCalledWith('datastore', {
-				event: 'syncQueriesPartialSyncError',
+				event: 'nonApplicableDataReceived',
 				data: {
 					errors: [
 						{
@@ -180,58 +177,7 @@ describe('Sync', () => {
 			});
 		});
 
-		it('should throw error and NOT return data or send Hub event when datastorePartialData is not set', async () => {
-			const rejectResponse = {
-				data: {
-					syncPosts: {
-						items: [
-							{
-								id: '1',
-								title: 'Item 1',
-							},
-							null,
-							{
-								id: '3',
-								title: 'Item 3',
-							},
-						],
-					},
-				},
-				errors: [
-					{
-						message: 'Item 2 error',
-					},
-				],
-			};
-
-			const hubDispatchMock = jest.fn();
-			const coreMocks = {
-				Hub: {
-					dispatch: hubDispatchMock,
-					listen: jest.fn(),
-				},
-			};
-
-			const SyncProcessor = jitteredRetrySyncProcessorSetup({
-				rejectResponse,
-				coreMocks,
-			});
-
-			try {
-				await SyncProcessor.jitteredRetry({
-					query: defaultQuery,
-					variables: defaultVariables,
-					opName: defaultOpName,
-					modelDefinition: defaultModelDefinition,
-					authMode: defaultAuthMode,
-				});
-			} catch (e) {
-				expect(e).toMatchSnapshot();
-			}
-		});
-
 		it('should throw error if no data is returned', async () => {
-			window.sessionStorage.setItem('datastorePartialData', 'true');
 			const rejectResponse = {
 				data: null,
 				errors: [
@@ -317,7 +263,6 @@ describe('Sync', () => {
 		});
 
 		it('should send user agent suffix with graphql request', async () => {
-			window.sessionStorage.setItem('datastorePartialData', 'true');
 			const resolveResponse = {
 				data: {
 					syncPosts: {
@@ -377,7 +322,6 @@ describe('Sync', () => {
 			jest.resetModules();
 			jest.resetAllMocks();
 			errorHandler.mockClear();
-			window.sessionStorage.setItem('datastorePartialData', 'true');
 		});
 
 		test('bad record', async () => {

diff --git a/packages/datastore/src/sync/processors/sync.ts b/packages/datastore/src/sync/processors/sync.ts
@@ -169,16 +169,6 @@ class SyncProcessor {
 		};
 	}
 
-	// Partial data private feature flag. Not a public API. This will be removed in a future release.
-	private partialDataFeatureFlagEnabled() {
-		try {
-			const flag = sessionStorage.getItem('datastorePartialData');
-			return Boolean(flag);
-		} catch (e) {
-			return false;
-		}
-	}
-
 	private async jitteredRetry<T>({
 		query,
 		variables,
@@ -223,82 +213,79 @@ class SyncProcessor {
 						throw new NonRetryableError(clientOrForbiddenErrorMessage);
 					}
 
-					const hasItems = Boolean(
-						error &&
-							error.data &&
-							error.data[opName] &&
-							error.data[opName].items
-					);
-					if (this.partialDataFeatureFlagEnabled()) {
-						if (hasItems) {
-							const result = error;
-							result.data[opName].items = result.data[opName].items.filter(
-								item => item !== null
-							);
-							if (error.errors) {
-								await Promise.all(
-									error.errors.map(async err => {
-										try {
-											await this.errorHandler({
-												recoverySuggestion:
-													'Ensure app code is up to date, auth directives exist and are correct on each model, and that server-side data has not been invalidated by a schema change. If the problem persists, search for or create an issue: https://github.com/aws-amplify/amplify-js/issues',
-												localModel: null,
-												message: err.message,
-												model: modelDefinition.name,
-												operation: opName,
-												errorType: getSyncErrorType(err),
-												process: ProcessName.sync,
-												remoteModel: null,
-												cause: err,
-											});
-										} catch (e) {
-											logger.error('Sync error handler failed with:', e);
-										}
-									})
-								);
-								Hub.dispatch('datastore', {
-									event: 'syncQueriesPartialSyncError',
-									data: {
-										errors: error.errors,
-										modelName: modelDefinition.name,
-									},
-								});
-							}
-
-							return result;
-						} else {
-							throw error;
-						}
-					}
+					const hasItems = Boolean(error?.data?.[opName]?.items);
 
-					// If the error is unauthorized, filter out unauthorized items and return accessible items
 					const unauthorized =
-						error &&
-						error.errors &&
+						error?.errors &&
 						(error.errors as [any]).some(
 							err => err.errorType === 'Unauthorized'
 						);
-					if (unauthorized) {
-						const result = error;
 
-						if (hasItems) {
-							result.data[opName].items = result.data[opName].items.filter(
-								item => item !== null
-							);
-						} else {
-							result.data[opName] = {
-								...opResultDefaults,
-								...result.data[opName],
-							};
-						}
+					const otherErrors =
+						error?.errors &&
+						(error.errors as [any]).filter(
+							err => err.errorType !== 'Unauthorized'
+						);
+
+					const result = error;
+
+					if (hasItems) {
+						result.data[opName].items = result.data[opName].items.filter(
+							item => item !== null
+						);
+					}
+
+					if (hasItems && otherErrors?.length) {
+						await Promise.all(
+							otherErrors.map(async err => {
+								try {
+									await this.errorHandler({
+										recoverySuggestion:
+											'Ensure app code is up to date, auth directives exist and are correct on each model, and that server-side data has not been invalidated by a schema change. If the problem persists, search for or create an issue: https://github.com/aws-amplify/amplify-js/issues',
+										localModel: null,
+										message: err.message,
+										model: modelDefinition.name,
+										operation: opName,
+										errorType: getSyncErrorType(err),
+										process: ProcessName.sync,
+										remoteModel: null,
+										cause: err,
+									});
+								} catch (e) {
+									logger.error('Sync error handler failed with:', e);
+								}
+							})
+						);
+						Hub.dispatch('datastore', {
+							event: 'nonApplicableDataReceived',
+							data: {
+								errors: otherErrors,
+								modelName: modelDefinition.name,
+							},
+						});
+					}
+
+					if (unauthorized) {
 						logger.warn(
 							'queryError',
 							`User is unauthorized to query ${opName}, some items could not be returned.`
 						);
+
+						result.data = result.data || {};
+
+						result.data[opName] = {
+							...opResultDefaults,
+							...result.data[opName],
+						};
+
 						return result;
-					} else {
-						throw error;
 					}
+
+					if (result.data?.[opName].items?.length) {
+						return result;
+					}
+
+					throw error;
 				}
 			},
 			[query, variables]