Merge pull request #3242 from angrykoala/escape-properties-and-relationships

angrykoala · web-flow · commit f224da6efac6 · 2023-04-27T15:03:19.000+01:00
Fix escaping for relationships
diff --git a/.changeset/fluffy-suns-invite.md b/.changeset/fluffy-suns-invite.md
@@ -0,0 +1,6 @@
+---
+"@neo4j/graphql": major
+"@neo4j/cypher-builder": minor
+---
+
+Escape properties and relationships if needed, using | and & as part of the label is no longer supported
diff --git a/packages/cypher-builder/src/expressions/map/MapExpr.test.ts b/packages/cypher-builder/src/expressions/map/MapExpr.test.ts
@@ -53,7 +53,7 @@ describe("Map Expression", () => {
         const queryResult = new TestClause(map).build();
 
         expect(queryResult.cypher).toMatchInlineSnapshot(
-            `"{ key: $param0, value2: \\"Override\\", value3: \\"another value\\" }"`
+            `"{ key: $param0, \`value2\`: \\"Override\\", \`value3\`: \\"another value\\" }"`
         );
 
         expect(queryResult.params).toMatchInlineSnapshot(`
diff --git a/packages/cypher-builder/src/expressions/map/MapProjection.ts b/packages/cypher-builder/src/expressions/map/MapProjection.ts
@@ -23,6 +23,7 @@ import type { Variable } from "../../references/Variable";
 import { serializeMap } from "../../utils/serialize-map";
 import { MapExpr } from "./MapExpr";
 import { isString } from "../../utils/is-string";
+import { escapeProperty } from "../../utils/escape";
 
 /** Represents a Map projection
  * @see [Cypher Documentation](https://neo4j.com/docs/cypher-manual/current/syntax/maps/#cypher-map-projection)
@@ -87,7 +88,7 @@ export class MapProjection implements CypherCompilable {
         const variableStr = this.variable.getCypher(env);
         const extraValuesStr = serializeMap(env, this.extraValues, true);
 
-        const projectionStr = this.projection.map((p) => `.${p}`).join(", ");
+        const projectionStr = this.projection.map((p) => `.${escapeProperty(p)}`).join(", ");
 
         const commaStr = extraValuesStr && projectionStr ? ", " : "";
 
diff --git a/packages/cypher-builder/src/utils/serialize-map.test.ts b/packages/cypher-builder/src/utils/serialize-map.test.ts
@@ -24,17 +24,18 @@ import { serializeMap } from "./serialize-map";
 describe("serializeMap", () => {
     const env = new CypherEnvironment();
     const map = new Map<string, Cypher.Expr>([
-        ["test1", new Cypher.Literal(10)],
+        ["test", new Cypher.Literal(10)],
+        ["test$", new Cypher.Literal(10)],
         ["expr", Cypher.reverse(new Cypher.Literal([1]))],
     ]);
 
     test("serialize a map of expressions", () => {
         const result = serializeMap(env, map);
-        expect(result).toBe("{ test1: 10, expr: reverse([1]) }");
+        expect(result).toBe("{ test: 10, `test$`: 10, expr: reverse([1]) }");
     });
 
     test("serialize a map of expressions without curly braces", () => {
         const result = serializeMap(env, map, true);
-        expect(result).toBe("test1: 10, expr: reverse([1])");
+        expect(result).toBe("test: 10, `test$`: 10, expr: reverse([1])");
     });
 });
diff --git a/packages/cypher-builder/src/utils/serialize-map.ts b/packages/cypher-builder/src/utils/serialize-map.ts
@@ -19,13 +19,14 @@
 
 import type { Expr } from "../types";
 import type { CypherEnvironment } from "../Environment";
+import { escapeProperty } from "./escape";
 
 export function serializeMap(env: CypherEnvironment, map: Map<string, Expr>, omitCurlyBraces = false): string {
     const serializedFields: string[] = [];
 
     for (const [key, value] of map.entries()) {
         if (value) {
-            const fieldStr = `${key}: ${value.getCypher(env)}`;
+            const fieldStr = `${escapeProperty(key)}: ${value.getCypher(env)}`;
             serializedFields.push(fieldStr);
         }
     }
diff --git a/packages/cypher-builder/src/utils/stringify-object.ts b/packages/cypher-builder/src/utils/stringify-object.ts
@@ -17,12 +17,14 @@
  * limitations under the License.
  */
 
+import { escapeProperty } from "./escape";
+
 /** Serializes object into a string for Cypher objects */
 export function stringifyObject(fields: Record<string, string | undefined | null>): string {
     return `{ ${Object.entries(fields)
         .filter(([, value]) => Boolean(value))
         .map(([key, value]): string | undefined => {
-            return `${key}: ${value}`;
+            return `${escapeProperty(key)}: ${value}`;
         })
         .join(", ")} }`;
 }
diff --git a/packages/cypher-builder/src/utils/utils.ts b/packages/cypher-builder/src/utils/utils.ts
@@ -19,4 +19,4 @@
 
 // Note: This file exists for exported utils to the user
 
-export { escapeLabel } from "./escape";
+export { escapeLabel, escapeType, escapeProperty } from "./escape";
diff --git a/packages/graphql/src/schema/get-alias-meta.ts b/packages/graphql/src/schema/get-alias-meta.ts
@@ -34,7 +34,7 @@ function getAliasMeta(directive: DirectiveNode): AliasMeta | undefined {
     const property = (stmtArg.value as StringValueNode).value;
 
     return {
-        property: Cypher.utils.escapeLabel(property),
+        property: Cypher.utils.escapeProperty(property),
         propertyUnescaped: property,
     };
 }
diff --git a/packages/graphql/src/translate/create-connect-or-create-and-params.ts b/packages/graphql/src/translate/create-connect-or-create-and-params.ts
@@ -330,7 +330,7 @@ function getAutogeneratedParams(node: Node | Relationship): Record<string, Cyphe
 function getCypherParameters(onCreateParams: Record<string, any> = {}, node?: Node): Record<string, Cypher.Param<any>> {
     const params = Object.entries(onCreateParams).reduce((acc, [key, value]) => {
         const nodeField = node?.constrainableFields.find((f) => f.fieldName === key);
-        const nodeFieldName = nodeField?.dbPropertyName || nodeField?.fieldName;
+        const nodeFieldName = nodeField?.dbPropertyNameUnescaped || nodeField?.fieldName;
         const fieldName = nodeFieldName || key;
         const valueOrArray = nodeField?.typeMeta.array ? asArray(value) : value;
         acc[fieldName] = valueOrArray;
diff --git a/packages/graphql/tests/tck/aggregations/alias-directive.test.ts b/packages/graphql/tests/tck/aggregations/alias-directive.test.ts
@@ -32,7 +32,7 @@ describe("Cypher Aggregations Many with Alias directive", () => {
             type Movie {
                 id: ID! @alias(property: "_id")
                 title: String! @alias(property: "_title")
-                imdbRating: Int! @alias(property: "_imdbRating")
+                imdbRating: Int! @alias(property: "_imdb Rating")
                 createdAt: DateTime! @alias(property: "_createdAt")
             }
         `;
@@ -74,21 +74,21 @@ describe("Cypher Aggregations Many with Alias directive", () => {
 
         expect(formatCypher(result.cypher)).toMatchInlineSnapshot(`
             "MATCH (this:\`Movie\`)
-            RETURN { id: { shortest: min(this.\`_id\`), longest: max(this.\`_id\`) }, title: { shortest:
-                                        reduce(aggVar = collect(this.\`_title\`)[0], current IN collect(this.\`_title\`) |
+            RETURN { id: { shortest: min(this._id), longest: max(this._id) }, title: { shortest:
+                                        reduce(aggVar = collect(this._title)[0], current IN collect(this._title) |
                                             CASE
                                             WHEN size(current) < size(aggVar) THEN current
                                             ELSE aggVar
                                             END
                                         )
                                     , longest:
-                                        reduce(aggVar = collect(this.\`_title\`)[0], current IN collect(this.\`_title\`) |
+                                        reduce(aggVar = collect(this._title)[0], current IN collect(this._title) |
                                             CASE
                                             WHEN size(current) > size(aggVar) THEN current
                                             ELSE aggVar
                                             END
                                         )
-                                     }, imdbRating: { min: min(this.\`_imdbRating\`), max: max(this.\`_imdbRating\`), average: avg(this.\`_imdbRating\`) }, createdAt: { min: apoc.date.convertFormat(toString(min(this.\`_createdAt\`)), \\"iso_zoned_date_time\\", \\"iso_offset_date_time\\"), max: apoc.date.convertFormat(toString(max(this.\`_createdAt\`)), \\"iso_zoned_date_time\\", \\"iso_offset_date_time\\") } }"
+                                     }, imdbRating: { min: min(this.\`_imdb Rating\`), max: max(this.\`_imdb Rating\`), average: avg(this.\`_imdb Rating\`) }, createdAt: { min: apoc.date.convertFormat(toString(min(this._createdAt)), \\"iso_zoned_date_time\\", \\"iso_offset_date_time\\"), max: apoc.date.convertFormat(toString(max(this._createdAt)), \\"iso_zoned_date_time\\", \\"iso_offset_date_time\\") } }"
         `);
 
         expect(formatParams(result.params)).toMatchInlineSnapshot(`"{}"`);
diff --git a/packages/graphql/tests/tck/connections/connect-or-create/connect-or-create-alias.test.ts b/packages/graphql/tests/tck/connections/connect-or-create/connect-or-create-alias.test.ts
@@ -0,0 +1,145 @@
+/*
+ * Copyright (c) "Neo4j"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import type { DocumentNode } from "graphql";
+import gql from "graphql-tag";
+import { Neo4jGraphQL } from "../../../../src";
+import { createJwtRequest } from "../../../utils/create-jwt-request";
+import { formatCypher, formatParams, translateQuery } from "../../utils/tck-test-utils";
+
+describe("Connect or create with @alias", () => {
+    let typeDefs: DocumentNode;
+    let neoSchema: Neo4jGraphQL;
+
+    beforeAll(() => {
+        typeDefs = gql`
+            type BibliographicReference @node(labels: ["BibliographicReference", "Resource"]) {
+                iri: ID! @unique @alias(property: "_uri")
+                prefLabel: [String]
+                isInPublication: [Concept!]! @relationship(type: "isInPublication", direction: OUT)
+            }
+
+            type Concept @node(labels: ["Concept", "Resource"]) {
+                iri: ID! @unique @alias(property: "$_uri")
+                prefLabel: [String]!
+            }
+        `;
+
+        neoSchema = new Neo4jGraphQL({
+            typeDefs,
+        });
+    });
+
+    test("where with multiple filters and params", async () => {
+        const query = gql`
+            mutation {
+                updateBibliographicReferences(
+                    where: { iri: "urn:myiri2" }
+                    update: {
+                        prefLabel: "Updated Label:My BRS with Resource"
+                        isInPublication: [
+                            {
+                                connectOrCreate: {
+                                    where: { node: { iri: "new-g" } }
+                                    onCreate: { node: { iri: "new-g", prefLabel: "pub" } }
+                                }
+                            }
+                            {
+                                connectOrCreate: {
+                                    where: { node: { iri: "new-f" } }
+                                    onCreate: { node: { iri: "new-f", prefLabel: "pub" } }
+                                }
+                            }
+                        ]
+                    }
+                ) {
+                    bibliographicReferences {
+                        iri
+                        prefLabel
+                        isInPublication(where: { iri_IN: ["new-f", "new-e"] }) {
+                            iri
+                            prefLabel
+                        }
+                    }
+                }
+            }
+        `;
+
+        const req = createJwtRequest("secret", {});
+        const result = await translateQuery(neoSchema, query, {
+            req,
+        });
+
+        expect(formatCypher(result.cypher)).toMatchInlineSnapshot(`
+            "MATCH (this:\`BibliographicReference\`:\`Resource\`)
+            WHERE this._uri = $param0
+            SET this.prefLabel = $this_update_prefLabel
+            WITH this
+            CALL {
+                WITH this
+                MERGE (this_isInPublication0_connectOrCreate0:\`Concept\`:\`Resource\` { \`$_uri\`: $this_isInPublication0_connectOrCreate_param0 })
+                ON CREATE SET
+                    this_isInPublication0_connectOrCreate0.\`$_uri\` = $this_isInPublication0_connectOrCreate_param1,
+                    this_isInPublication0_connectOrCreate0.prefLabel = $this_isInPublication0_connectOrCreate_param2
+                MERGE (this)-[this_isInPublication0_connectOrCreate_this0:\`isInPublication\`]->(this_isInPublication0_connectOrCreate0)
+            }
+            WITH this
+            CALL {
+                WITH this
+                MERGE (this_isInPublication1_connectOrCreate0:\`Concept\`:\`Resource\` { \`$_uri\`: $this_isInPublication1_connectOrCreate_param0 })
+                ON CREATE SET
+                    this_isInPublication1_connectOrCreate0.\`$_uri\` = $this_isInPublication1_connectOrCreate_param1,
+                    this_isInPublication1_connectOrCreate0.prefLabel = $this_isInPublication1_connectOrCreate_param2
+                MERGE (this)-[this_isInPublication1_connectOrCreate_this0:\`isInPublication\`]->(this_isInPublication1_connectOrCreate0)
+            }
+            WITH *
+            CALL {
+                WITH this
+                MATCH (this)-[update_this0:\`isInPublication\`]->(update_this1:\`Concept\`:\`Resource\`)
+                WHERE update_this1.\`$_uri\` IN $update_param0
+                WITH update_this1 { iri: update_this1.\`$_uri\`, .prefLabel } AS update_this1
+                RETURN collect(update_this1) AS update_var2
+            }
+            RETURN collect(DISTINCT this { iri: this._uri, .prefLabel, isInPublication: update_var2 }) AS data"
+        `);
+        expect(formatParams(result.params)).toMatchInlineSnapshot(`
+            "{
+                \\"update_param0\\": [
+                    \\"new-f\\",
+                    \\"new-e\\"
+                ],
+                \\"param0\\": \\"urn:myiri2\\",
+                \\"this_update_prefLabel\\": [
+                    \\"Updated Label:My BRS with Resource\\"
+                ],
+                \\"this_isInPublication0_connectOrCreate_param0\\": \\"new-g\\",
+                \\"this_isInPublication0_connectOrCreate_param1\\": \\"new-g\\",
+                \\"this_isInPublication0_connectOrCreate_param2\\": [
+                    \\"pub\\"
+                ],
+                \\"this_isInPublication1_connectOrCreate_param0\\": \\"new-f\\",
+                \\"this_isInPublication1_connectOrCreate_param1\\": \\"new-f\\",
+                \\"this_isInPublication1_connectOrCreate_param2\\": [
+                    \\"pub\\"
+                ],
+                \\"resolvedCallbacks\\": {}
+            }"
+        `);
+    });
+});
diff --git a/packages/graphql/tests/tck/issues/1131.test.ts b/packages/graphql/tests/tck/issues/1131.test.ts
@@ -93,18 +93,18 @@ describe("https://github.com/neo4j/graphql/issues/1131", () => {
             WITH this
             CALL {
                 WITH this
-                MERGE (this_isInPublication0_connectOrCreate0:\`Concept\`:\`Resource\` { \`uri\`: $this_isInPublication0_connectOrCreate_param0 })
+                MERGE (this_isInPublication0_connectOrCreate0:\`Concept\`:\`Resource\` { uri: $this_isInPublication0_connectOrCreate_param0 })
                 ON CREATE SET
-                    this_isInPublication0_connectOrCreate0.\`uri\` = $this_isInPublication0_connectOrCreate_param1,
+                    this_isInPublication0_connectOrCreate0.uri = $this_isInPublication0_connectOrCreate_param1,
                     this_isInPublication0_connectOrCreate0.prefLabel = $this_isInPublication0_connectOrCreate_param2
                 MERGE (this)-[this_isInPublication0_connectOrCreate_this0:\`isInPublication\`]->(this_isInPublication0_connectOrCreate0)
             }
             WITH this
             CALL {
                 WITH this
-                MERGE (this_isInPublication1_connectOrCreate0:\`Concept\`:\`Resource\` { \`uri\`: $this_isInPublication1_connectOrCreate_param0 })
+                MERGE (this_isInPublication1_connectOrCreate0:\`Concept\`:\`Resource\` { uri: $this_isInPublication1_connectOrCreate_param0 })
                 ON CREATE SET
-                    this_isInPublication1_connectOrCreate0.\`uri\` = $this_isInPublication1_connectOrCreate_param1,
+                    this_isInPublication1_connectOrCreate0.uri = $this_isInPublication1_connectOrCreate_param1,
                     this_isInPublication1_connectOrCreate0.prefLabel = $this_isInPublication1_connectOrCreate_param2
                 MERGE (this)-[this_isInPublication1_connectOrCreate_this0:\`isInPublication\`]->(this_isInPublication1_connectOrCreate0)
             }

Original file line number	Diff line number	Diff line change
`@@ -19,13 +19,14 @@`
`19`	`19`
`20`	`20`	`import type { Expr } from "../types";`
`21`	`21`	`import type { CypherEnvironment } from "../Environment";`
	`22`	`+import { escapeProperty } from "./escape";`
`22`	`23`
`23`	`24`	`export function serializeMap(env: CypherEnvironment, map: Map<string, Expr>, omitCurlyBraces = false): string {`
`24`	`25`	`const serializedFields: string[] = [];`
`25`	`26`
`26`	`27`	`for (const [key, value] of map.entries()) {`
`27`	`28`	`if (value) {`
`28`		- const fieldStr = `${key}: ${value.getCypher(env)}`;
	`29`	+ const fieldStr = `${escapeProperty(key)}: ${value.getCypher(env)}`;
`29`	`30`	`serializedFields.push(fieldStr);`
`30`	`31`	`}`
`31`	`32`	`}`
Original file line number	Diff line number	Diff line change
`@@ -19,4 +19,4 @@`
`19`	`19`
`20`	`20`	`// Note: This file exists for exported utils to the user`
`21`	`21`
`22`		`-export { escapeLabel } from "./escape";`
	`22`	`+export { escapeLabel, escapeType, escapeProperty } from "./escape";`
Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,7 @@ function getAliasMeta(directive: DirectiveNode): AliasMeta \| undefined {`
`34`	`34`	`const property = (stmtArg.value as StringValueNode).value;`
`35`	`35`
`36`	`36`	`return {`
`37`		`- property: Cypher.utils.escapeLabel(property),`
	`37`	`+ property: Cypher.utils.escapeProperty(property),`
`38`	`38`	`propertyUnescaped: property,`
`39`	`39`	`};`
`40`	`40`	`}`