pr review requested changes

packages/datastore/__tests__/graphql.ts

@@ -4,7 +4,9 @@ import {
 	buildGraphQLOperation,
 	buildSubscriptionGraphQLOperation,
 	TransformerMutationType,
+	predicateToGraphQLFilter,
 } from '../src/sync/utils';
+import { PredicatesGroup } from '../src/types';
 import { newSchema } from './schema';
 
 const postSelectionSet = `
@@ -170,3 +172,108 @@ describe('DataStore GraphQL generation', () => {
 		}
 	);
 });
+
+describe('DataStore PredicateGroups to GraphQL filter', () => {
+	test('Single field', () => {
+		const group: PredicatesGroup<any> = {
+			type: 'and',
+			predicates: [{ field: 'someField', operator: 'eq', operand: 'value' }],
+		};
+
+		const groupExpected = { and: [{ someField: { eq: 'value' } }] };
+
+		const gqlResult = predicateToGraphQLFilter(group);
+
+		// stringifying to normalize whitespace and escape chars
+		expect(JSON.stringify(gqlResult)).toStrictEqual(
+			JSON.stringify(groupExpected)
+		);
+	});
+
+	test('Multiple field', () => {
+		const group: PredicatesGroup<any> = {
+			type: 'and',
+			predicates: [
+				{ field: 'someField', operator: 'eq', operand: 'value' },
+				{ field: 'someOtherField', operator: 'gt', operand: 'value2' },
+			],
+		};
+
+		const groupExpected = {
+			and: [
+				{ someField: { eq: 'value' } },
+				{ someOtherField: { gt: 'value2' } },
+			],
+		};
+
+		const gqlResult = predicateToGraphQLFilter(group);
+
+		expect(JSON.stringify(gqlResult)).toStrictEqual(
+			JSON.stringify(groupExpected)
+		);
+	});
+
+	test('Nested field', () => {
+		const group: PredicatesGroup<any> = {
+			type: 'and',
+			predicates: [
+				{ field: 'someField', operator: 'eq', operand: 'value' },
+				{
+					type: 'or',
+					predicates: [
+						{ field: 'someOtherField', operator: 'gt', operand: 'value2' },
+						{ field: 'orField', operator: 'contains', operand: 'str' },
+					],
+				},
+			],
+		};
+
+		const groupExpected = {
+			and: [
+				{ someField: { eq: 'value' } },
+				{
+					or: [
+						{ someOtherField: { gt: 'value2' } },
+						{ orField: { contains: 'str' } },
+					],
+				},
+			],
+		};
+
+		const gqlResult = predicateToGraphQLFilter(group);
+
+		expect(JSON.stringify(gqlResult)).toStrictEqual(
+			JSON.stringify(groupExpected)
+		);
+	});
+
+	test('Nested not', () => {
+		const group: PredicatesGroup<any> = {
+			type: 'not',
+			predicates: [
+				{
+					type: 'or',
+					predicates: [
+						{ field: 'someOtherField', operator: 'gt', operand: 'value2' },
+						{ field: 'orField', operator: 'contains', operand: 'str' },
+					],
+				},
+			],
+		};
+
+		const groupExpected = {
+			not: {
+				or: [
+					{ someOtherField: { gt: 'value2' } },
+					{ orField: { contains: 'str' } },
+				],
+			},
+		};
+
+		const gqlResult = predicateToGraphQLFilter(group);
+
+		expect(JSON.stringify(gqlResult)).toStrictEqual(
+			JSON.stringify(groupExpected)
+		);
+	});
+});

packages/datastore/src/datastore/datastore.ts

@@ -546,7 +546,7 @@ class DataStore {
 	private syncPageSize: number;
 	private syncExpressions: SyncExpression<any>[];
 	private syncPredicates: WeakMap<SchemaModel, ModelPredicate<any>>;
-	private syncModelsUpdated: Set<String> = null;
+	private syncModelsUpdated: Set<string> = new Set<string>();
 
 	getModuleName() {
 		return 'DataStore';
@@ -1147,7 +1147,10 @@ class DataStore {
 		});
 	}
 
-	private createFromCondition(modelDefinition: SchemaModel, condition) {
+	private createFromCondition(
+		modelDefinition: SchemaModel,
+		condition: ProducerModelPredicate<PersistentModel>
+	) {
 		try {
 			return ModelPredicateCreator.createFromExisting(
 				modelDefinition,
@@ -1159,7 +1162,9 @@ class DataStore {
 		}
 	}
 
-	private async unwrapPromise(conditionProducer) {
+	private async unwrapPromise<T extends PersistentModel>(
+		conditionProducer
+	): Promise<ProducerModelPredicate<T>> {
 		try {
 			const condition = await conditionProducer();
 			return condition;
@@ -1175,7 +1180,7 @@ class DataStore {
 		entries: [SchemaModel, ModelPredicate<any>][]
 	): WeakMap<SchemaModel, ModelPredicate<any>> {
 		return entries.reduce((map, [modelDefinition, predicate]) => {
-			if (map.get(modelDefinition)) {
+			if (map.has(modelDefinition)) {
 				const { name } = modelDefinition;
 				logger.warn(
 					`You can only utilize one Sync Expression per model. 

packages/datastore/src/storage/adapter/IndexedDBAdapter.ts

@@ -30,7 +30,6 @@ import {
 	sortCompareFunction,
 } from '../../util';
 import { Adapter } from './index';
-import { tsIndexSignature } from '@babel/types';
 
 const logger = new Logger('DataStore');
 

packages/datastore/src/sync/index.ts

@@ -105,7 +105,7 @@ export class SyncEngine {
 		conflictHandler: ConflictHandler,
 		errorHandler: ErrorHandler,
 		private readonly syncPredicates: WeakMap<SchemaModel, ModelPredicate<any>>,
-		private readonly syncModelsUpdated: Set<String> // Iterate over and compare
+		private readonly syncModelsUpdated: ReadonlySet<string>
 	) {
 		const MutationEvent = this.modelClasses[
 			'MutationEvent'
@@ -721,14 +721,15 @@ export class SyncEngine {
 					ownSymbol
 				);
 			} else {
-				const syncPredicateUpdated =
-					this.syncModelsUpdated && this.syncModelsUpdated.has(model);
+				const syncPredicateUpdated = this.syncModelsUpdated.has(model);
 
 				[[savedModel]] = await this.storage.save(
 					(this.modelClasses.ModelMetadata as PersistentModelConstructor<
 						any
 					>).copyOf(modelMetadata, draft => {
 						draft.fullSyncInterval = fullSyncInterval;
+						// perform a base sync if the syncPredicate changed in between calls to DataStore.start
+						// ensures that the local store contains all the data specified by the syncExpression
 						if (syncPredicateUpdated) {
 							draft.lastSync = null;
 							draft.lastFullSync = null;

packages/datastore/src/sync/utils.ts

@@ -2,6 +2,8 @@ import { ModelInstanceCreator } from '../datastore/datastore';
 import {
 	AuthorizationRule,
 	GraphQLCondition,
+	GraphQLFilter,
+	GraphQLField,
 	isEnumFieldType,
 	isGraphQLScalarType,
 	isPredicateObj,
@@ -391,23 +393,34 @@ export function predicateToGraphQLCondition(
 
 export function predicateToGraphQLFilter(
 	predicatesGroup: PredicatesGroup<any>
-): GraphQLCondition {
-	const result = {};
+): GraphQLFilter {
+	const result: GraphQLFilter = {};
 
 	if (!predicatesGroup || !Array.isArray(predicatesGroup.predicates)) {
 		return result;
 	}
 
 	const { type, predicates } = predicatesGroup;
+	const isList = type === 'and' || type === 'or';
 
-	result[type] = predicates.map(p => {
-		if (isPredicateObj(p)) {
-			const { field, operator, operand } = p;
+	result[type] = isList ? [] : {};
 
-			return { [field]: { [operator]: operand } };
-		} else {
-			result[p.type] = predicateToGraphQLCondition(p);
+	const appendToFilter = value =>
+		isList ? result[type].push(value) : (result[type] = value);
+
+	predicates.forEach(predicate => {
+		if (isPredicateObj(predicate)) {
+			const { field, operator, operand } = predicate;
+
+			const gqlField: GraphQLField = {
+				[field]: { [operator]: operand },
+			};
+
+			appendToFilter(gqlField);
+			return;
 		}
+
+		appendToFilter(predicateToGraphQLFilter(predicate));
 	});
 
 	return result;

packages/datastore/src/types.ts

@@ -326,20 +326,34 @@ export enum QueryOne {
 	FIRST,
 	LAST,
 }
+export type GraphQLField = {
+	[field: string]: {
+		[operator: string]: string | number | [number, number];
+	};
+};
 
 export type GraphQLCondition = Partial<
-	| {
-			[field: string]: {
-				[operator: string]: string | number | [number, number];
-			};
-	  }
+	| GraphQLField
 	| {
 			and: [GraphQLCondition];
 			or: [GraphQLCondition];
 			not: GraphQLCondition;
 	  }
 >;
 
+export type GraphQLFilter = Partial<
+	| GraphQLField
+	| {
+			and: GraphQLFilter[];
+	  }
+	| {
+			or: GraphQLFilter[];
+	  }
+	| {
+			not: GraphQLFilter;
+	  }
+>;
+
 //#endregion
 
 //#region Pagination
@@ -434,22 +448,21 @@ export type DataStoreConfig = {
 		maxRecordsToSync?: number; // merge
 		syncPageSize?: number;
 		fullSyncInterval?: number;
-		syncExpressions?: SyncExpression<any>[];
+		syncExpressions?: SyncExpression<PersistentModel>[];
 	};
 	conflictHandler?: ConflictHandler; // default : retry until client wins up to x times
 	errorHandler?: (error: SyncError) => void; // default : logger.warn
 	maxRecordsToSync?: number; // merge
 	syncPageSize?: number;
 	fullSyncInterval?: number;
-	syncExpressions?: SyncExpression<any>[];
+	syncExpressions?: SyncExpression<PersistentModel>[];
 };
 
 export type SyncExpression<T extends PersistentModel> = Promise<{
 	modelConstructor: PersistentModelConstructor<T>;
 	conditionProducer:
 		| ProducerModelPredicate<T>
-		| (() => ProducerModelPredicate<T>)
-		| (() => Promise<ProducerModelPredicate<T>>);
+		| (() => ProducerModelPredicate<T>);
 }>;
 
 /*
@@ -472,25 +485,21 @@ export async function syncExpression<T extends PersistentModel, P>(
 		condition: P | ModelPredicate<T>
 	) => P extends ModelPredicate<T>
 		? ModelPredicate<T>
-		: ProducerModelPredicate<T> | Promise<ProducerModelPredicate<T>>
+		: ProducerModelPredicate<T>
 ): Promise<{
 	modelConstructor: PersistentModelConstructor<T>;
 	conditionProducer: (
 		condition: P | ModelPredicate<T>
 	) => P extends ModelPredicate<T>
 		? ModelPredicate<T>
-		: ProducerModelPredicate<T> | Promise<ProducerModelPredicate<T>>;
+		: ProducerModelPredicate<T>;
 }> {
 	return {
 		modelConstructor,
 		conditionProducer,
 	};
 }
 
-type SyncConditionProducer<T extends PersistentModel> =
-	| (() => Promise<ProducerModelPredicate<T>>)
-	| (() => ProducerModelPredicate<T>);
-
 export type SyncConflict = {
 	modelConstructor: PersistentModelConstructor<any>;
 	localModel: PersistentModel;