From 32c7ab60276bb9df80bf5b542d918a816c387878 Mon Sep 17 00:00:00 2001 From: Jongeun Lee Date: Thu, 9 Jun 2022 11:57:15 +0800 Subject: [PATCH 1/3] fix: graphql query transformer drop the condition when where argument has false or 0 value on object field - Change test style - Remove unnecessary assertion Co-authored-by: Antoine Cormouls --- spec/ParseGraphQLServer.spec.js | 120 ++++++++++++++++++++++++++++++ src/GraphQL/transformers/query.js | 2 +- 2 files changed, 121 insertions(+), 1 deletion(-) diff --git a/spec/ParseGraphQLServer.spec.js b/spec/ParseGraphQLServer.spec.js index adf004932b..c172c94199 100644 --- a/spec/ParseGraphQLServer.spec.js +++ b/spec/ParseGraphQLServer.spec.js @@ -9516,6 +9516,126 @@ describe('ParseGraphQLServer', () => { } }); + it('should support where argument on object field that contains false boolean value or 0 number value', async () => { + try { + const someObjectFieldValue = { + foo: { bar: 'baz', qux: true, quux: 100 }, + number: 10, + }; + + const object = new Parse.Object('SomeClass'); + await object.save({ + someObjectField: someObjectFieldValue, + }); + + const whereWithQuxFalse = { + someObjectField: { + notEqualTo: { key: 'foo.bar', value: 'bat' }, + greaterThan: { key: 'number', value: 9 }, + lessThan: { key: 'number', value: 11 }, + equalTo: { key: 'foo.qux', value: false }, + }, + }; + const whereWithQuxTrue = { + someObjectField: { + ...whereWithQuxFalse.someObjectField, + equalTo: { key: 'foo.qux', value: true }, + }, + }; + const whereWithQuux0 = { + someObjectField: { + notEqualTo: { key: 'foo.bar', value: 'bat' }, + greaterThan: { key: 'number', value: 9 }, + lessThan: { key: 'number', value: 11 }, + equalTo: { key: 'foo.quux', value: 0 }, + }, + }; + const whereWithQuux100 = { + someObjectField: { + notEqualTo: { key: 'foo.bar', value: 'bat' }, + greaterThan: { key: 'number', value: 9 }, + lessThan: { key: 'number', value: 11 }, + equalTo: { key: 'foo.quux', value: 100 }, + }, + }; + const queryResult = await apolloClient.query({ + query: gql` + query GetSomeObject( + $id: ID! + $whereWithQuxFalse: SomeClassWhereInput + $whereWithQuxTrue: SomeClassWhereInput + $whereWithQuux0: SomeClassWhereInput + $whereWithQuux100: SomeClassWhereInput + ) { + someClass(id: $id) { + id + someObjectField + } + someClasses(where: $whereWithQuxFalse) { + edges { + node { + id + someObjectField + } + } + } + someClassesWithQuxTrue: someClasses(where: $whereWithQuxTrue) { + edges { + node { + id + someObjectField + } + } + } + someClassesWithQuux0: someClasses(where: $whereWithQuux0) { + edges { + node { + id + someObjectField + } + } + } + someClassesWithQuux100: someClasses(where: $whereWithQuux100) { + edges { + node { + id + someObjectField + } + } + } + } + `, + variables: { + id: object.id, + whereWithQuxFalse, + whereWithQuxTrue, + whereWithQuux0, + whereWithQuux100, + }, + }); + + const { + someClass: getResult, + someClasses, + someClassesWithQuxTrue, + someClassesWithQuux0, + someClassesWithQuux100, + } = queryResult.data; + + const { someObjectField } = getResult; + expect(someObjectField).toEqual(someObjectFieldValue); + + // Checks class query results + expect(someClasses.edges.length).toEqual(0); + expect(someClassesWithQuxTrue.edges.length).toEqual(1); + + expect(someClassesWithQuux0.edges.length).toEqual(0); + expect(someClassesWithQuux100.edges.length).toEqual(1); + } catch (e) { + handleError(e); + } + }); + it('should support object composed queries', async () => { try { const someObjectFieldValue1 = { diff --git a/src/GraphQL/transformers/query.js b/src/GraphQL/transformers/query.js index a91ee208ad..bf4946f125 100644 --- a/src/GraphQL/transformers/query.js +++ b/src/GraphQL/transformers/query.js @@ -108,7 +108,7 @@ const transformQueryConstraintInputToParse = ( * } * } */ - if (fieldValue.key && fieldValue.value && parentConstraints && parentFieldName) { + if (fieldValue.key && fieldValue.value !== undefined && parentConstraints && parentFieldName) { delete parentConstraints[parentFieldName]; parentConstraints[`${parentFieldName}.${fieldValue.key}`] = { ...parentConstraints[`${parentFieldName}.${fieldValue.key}`], From ee40bda9368231c5bbf93b73f6676dddb302ebfb Mon Sep 17 00:00:00 2001 From: Jongeun Lee Date: Mon, 27 Jun 2022 11:19:11 +0800 Subject: [PATCH 2/3] fix: casting a value type of nested filed name --- spec/ParseGraphQLServer.spec.js | 110 +++++++++--------- .../Postgres/PostgresStorageAdapter.js | 49 +++++--- 2 files changed, 91 insertions(+), 68 deletions(-) diff --git a/spec/ParseGraphQLServer.spec.js b/spec/ParseGraphQLServer.spec.js index c172c94199..205de6263c 100644 --- a/spec/ParseGraphQLServer.spec.js +++ b/spec/ParseGraphQLServer.spec.js @@ -9518,60 +9518,68 @@ describe('ParseGraphQLServer', () => { it('should support where argument on object field that contains false boolean value or 0 number value', async () => { try { - const someObjectFieldValue = { - foo: { bar: 'baz', qux: true, quux: 100 }, - number: 10, + const someObjectFieldValue1 = { + foo: { bar: true, baz: 100 }, }; - const object = new Parse.Object('SomeClass'); - await object.save({ - someObjectField: someObjectFieldValue, + const someObjectFieldValue2 = { + foo: { bar: false, baz: 0 }, + }; + + const object1 = new Parse.Object('SomeClass'); + await object1.save({ + someObjectField: someObjectFieldValue1, + }); + const object2 = new Parse.Object('SomeClass'); + await object2.save({ + someObjectField: someObjectFieldValue2, }); - const whereWithQuxFalse = { + const whereToObject1 = { someObjectField: { - notEqualTo: { key: 'foo.bar', value: 'bat' }, - greaterThan: { key: 'number', value: 9 }, - lessThan: { key: 'number', value: 11 }, - equalTo: { key: 'foo.qux', value: false }, + equalTo: { key: 'foo.bar', value: true }, + notEqualTo: { key: 'foo.baz', value: 0 }, }, }; - const whereWithQuxTrue = { + const whereToObject2 = { someObjectField: { - ...whereWithQuxFalse.someObjectField, - equalTo: { key: 'foo.qux', value: true }, + notEqualTo: { key: 'foo.bar', value: true }, + equalTo: { key: 'foo.baz', value: 0 }, }, }; - const whereWithQuux0 = { + + const whereToAll = { someObjectField: { - notEqualTo: { key: 'foo.bar', value: 'bat' }, - greaterThan: { key: 'number', value: 9 }, - lessThan: { key: 'number', value: 11 }, - equalTo: { key: 'foo.quux', value: 0 }, + lessThan: { key: 'foo.baz', value: 101 }, }, }; - const whereWithQuux100 = { + + const whereToNone = { someObjectField: { - notEqualTo: { key: 'foo.bar', value: 'bat' }, - greaterThan: { key: 'number', value: 9 }, - lessThan: { key: 'number', value: 11 }, - equalTo: { key: 'foo.quux', value: 100 }, + notEqualTo: { key: 'foo.bar', value: true }, + equalTo: { key: 'foo.baz', value: 1 }, }, }; + const queryResult = await apolloClient.query({ query: gql` query GetSomeObject( - $id: ID! - $whereWithQuxFalse: SomeClassWhereInput - $whereWithQuxTrue: SomeClassWhereInput - $whereWithQuux0: SomeClassWhereInput - $whereWithQuux100: SomeClassWhereInput + $id1: ID! + $id2: ID! + $whereToObject1: SomeClassWhereInput + $whereToObject2: SomeClassWhereInput + $whereToAll: SomeClassWhereInput + $whereToNone: SomeClassWhereInput ) { - someClass(id: $id) { + obj1: someClass(id: $id1) { id someObjectField } - someClasses(where: $whereWithQuxFalse) { + obj2: someClass(id: $id2) { + id + someObjectField + } + onlyObj1: someClasses(where: $whereToObject1) { edges { node { id @@ -9579,7 +9587,7 @@ describe('ParseGraphQLServer', () => { } } } - someClassesWithQuxTrue: someClasses(where: $whereWithQuxTrue) { + onlyObj2: someClasses(where: $whereToObject2) { edges { node { id @@ -9587,7 +9595,7 @@ describe('ParseGraphQLServer', () => { } } } - someClassesWithQuux0: someClasses(where: $whereWithQuux0) { + all: someClasses(where: $whereToAll) { edges { node { id @@ -9595,7 +9603,7 @@ describe('ParseGraphQLServer', () => { } } } - someClassesWithQuux100: someClasses(where: $whereWithQuux100) { + none: someClasses(where: $whereToNone) { edges { node { id @@ -9606,31 +9614,27 @@ describe('ParseGraphQLServer', () => { } `, variables: { - id: object.id, - whereWithQuxFalse, - whereWithQuxTrue, - whereWithQuux0, - whereWithQuux100, + id1: object1.id, + id2: object2.id, + whereToObject1, + whereToObject2, + whereToAll, + whereToNone, }, }); - const { - someClass: getResult, - someClasses, - someClassesWithQuxTrue, - someClassesWithQuux0, - someClassesWithQuux100, - } = queryResult.data; + const { obj1, obj2, onlyObj1, onlyObj2, all, none } = queryResult.data; - const { someObjectField } = getResult; - expect(someObjectField).toEqual(someObjectFieldValue); + expect(obj1.someObjectField).toEqual(someObjectFieldValue1); + expect(obj2.someObjectField).toEqual(someObjectFieldValue2); // Checks class query results - expect(someClasses.edges.length).toEqual(0); - expect(someClassesWithQuxTrue.edges.length).toEqual(1); - - expect(someClassesWithQuux0.edges.length).toEqual(0); - expect(someClassesWithQuux100.edges.length).toEqual(1); + expect(onlyObj1.edges.length).toEqual(1); + expect(onlyObj1.edges[0].node.someObjectField).toEqual(someObjectFieldValue1); + expect(onlyObj2.edges.length).toEqual(1); + expect(onlyObj2.edges[0].node.someObjectField).toEqual(someObjectFieldValue2); + expect(all.edges.length).toEqual(2); + expect(none.edges.length).toEqual(0); } catch (e) { handleError(e); } diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index 764cfe2d78..13f348ea4d 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -91,6 +91,22 @@ const toPostgresValue = value => { return value; }; +const toPostgresValueCastType = value => { + const postgresValue = toPostgresValue(value); + let castType; + switch (typeof postgresValue) { + case 'number': + castType = 'double precision'; + break; + case 'boolean': + castType = 'boolean'; + break; + default: + castType = undefined; + } + return castType; +}; + const transformValue = value => { if (typeof value === 'object' && value.__type === 'Pointer') { return value.objectId; @@ -191,6 +207,10 @@ const transformDotFieldToComponents = fieldName => { if (index === 0) { return `"${cmpt}"`; } + if (parseInt(cmpt).toString() === cmpt) { + return `${cmpt}`; + } + return `'${cmpt}'`; }); }; @@ -369,9 +389,12 @@ const buildWhereClause = ({ schema, query, index, caseInsensitive }): WhereClaus ); } else { if (fieldName.indexOf('.') >= 0) { - const constraintFieldName = transformDotField(fieldName); + const castType = toPostgresValueCastType(fieldValue.$ne); + const constraintFieldName = castType + ? `CAST ((${transformDotField(fieldName)}) AS ${castType})` + : transformDotField(fieldName); patterns.push( - `(${constraintFieldName} <> $${index} OR ${constraintFieldName} IS NULL)` + `(${constraintFieldName} <> $${index + 1} OR ${constraintFieldName} IS NULL)` ); } else if (typeof fieldValue.$ne === 'object' && fieldValue.$ne.$relativeTime) { throw new Parse.Error( @@ -401,8 +424,12 @@ const buildWhereClause = ({ schema, query, index, caseInsensitive }): WhereClaus index += 1; } else { if (fieldName.indexOf('.') >= 0) { + const castType = toPostgresValueCastType(fieldValue.$eq); + const constraintFieldName = castType + ? `CAST ((${transformDotField(fieldName)}) AS ${castType})` + : transformDotField(fieldName); values.push(fieldValue.$eq); - patterns.push(`${transformDotField(fieldName)} = $${index++}`); + patterns.push(`${constraintFieldName} = $${index++}`); } else if (typeof fieldValue.$eq === 'object' && fieldValue.$eq.$relativeTime) { throw new Parse.Error( Parse.Error.INVALID_JSON, @@ -771,20 +798,11 @@ const buildWhereClause = ({ schema, query, index, caseInsensitive }): WhereClaus Object.keys(ParseToPosgresComparator).forEach(cmp => { if (fieldValue[cmp] || fieldValue[cmp] === 0) { const pgComparator = ParseToPosgresComparator[cmp]; - let postgresValue = toPostgresValue(fieldValue[cmp]); let constraintFieldName; + let postgresValue = toPostgresValue(fieldValue[cmp]); + if (fieldName.indexOf('.') >= 0) { - let castType; - switch (typeof postgresValue) { - case 'number': - castType = 'double precision'; - break; - case 'boolean': - castType = 'boolean'; - break; - default: - castType = undefined; - } + const castType = toPostgresValueCastType(fieldValue[cmp]); constraintFieldName = castType ? `CAST ((${transformDotField(fieldName)}) AS ${castType})` : transformDotField(fieldName); @@ -823,6 +841,7 @@ const buildWhereClause = ({ schema, query, index, caseInsensitive }): WhereClaus } } values = values.map(transformValue); + return { pattern: patterns.join(' AND '), values, sorts }; }; From f8a8d7673e395ffbbcf1043f661bba1c104ba8d0 Mon Sep 17 00:00:00 2001 From: Jongeun Lee Date: Mon, 27 Jun 2022 17:09:09 +0800 Subject: [PATCH 3/3] remove the codes not related this PR and blank line --- src/Adapters/Storage/Postgres/PostgresStorageAdapter.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index 13f348ea4d..eb668868c6 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -207,10 +207,6 @@ const transformDotFieldToComponents = fieldName => { if (index === 0) { return `"${cmpt}"`; } - if (parseInt(cmpt).toString() === cmpt) { - return `${cmpt}`; - } - return `'${cmpt}'`; }); }; @@ -841,7 +837,6 @@ const buildWhereClause = ({ schema, query, index, caseInsensitive }): WhereClaus } } values = values.map(transformValue); - return { pattern: patterns.join(' AND '), values, sorts }; };