Implement public method to get cypher from graphql query - #7215
Conversation
… to chance behaviour)
|
| const translationResult = await this.dryRun(query, args.variableValues || {}, schema); | ||
| console.log(translationResult?.cypher); |
There was a problem hiding this comment.
Is this just like a debug thing?
| variableValues?: Record<string, unknown>, | ||
| schema?: GraphQLSchema | ||
| ): Promise<{ cypher: string; params: Record<string, unknown> } | undefined> { | ||
| return this.neo4jGraphQL?.giveMeCypher(query, schema, variableValues); |
There was a problem hiding this comment.
Is there a chance of using this new methods instead of the hack for our tck tests?
| this.complexityEstimatorHelper = new ComplexityEstimatorHelper(!!this.features.complexityEstimators); | ||
| } | ||
|
|
||
| public async giveMeCypher( |
There was a problem hiding this comment.
I like this name a lot, but I guess we need to think of a different name for the public interface 😅
|
|
||
| public async giveMeCypher( | ||
| queryString: string, | ||
| executableSchema?: GraphQLSchema, |
There was a problem hiding this comment.
I'm not sure I understand why do we allow to pass a different executableSchema here? 🤔
|
|
||
| if (!this.translator) { | ||
| this.translator = new GraphQL2CypherTranslator({ | ||
| executableSchema, |
There was a problem hiding this comment.
This is being stored, so if the executableSchema is different when calling this method, it will still use the old one
There was a problem hiding this comment.
can it be different? executableSchema is being stored in the Neo4jGraphQL class
| /* | ||
| * 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. | ||
| */ |
There was a problem hiding this comment.
| /* | |
| * 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. | |
| */ | |
| /* | |
| * Copyright (c) "Neo4j" | |
| * Neo4j Sweden AB [http://neo4j.com] | |
| */ |
| variableValues?: Record<string, unknown> | ||
| ): Promise<{ cypher: string; params: Record<string, unknown> }> { | ||
| const parsed = await this.parseQuery(queryString, variableValues); | ||
| if (!parsed) return EMPTY_RESULT; |
There was a problem hiding this comment.
very small issue, but consider either creating the EMPTY_RESULT inside this method, or returning the object directly without a variable (return { query ....})
As it is now, the following behavior could happen in the user's code:
const res1 = neoGraphql.giveMeCypher(); // EMPTY_RESULT
res1.potato="foo" // This modifies the original reference
const res2 = graphql2cypher(); // { cypher, params, potato: "foo"}| const resolveTree = getNeo4jResolveTree(info, { args }); | ||
| return { info, args, fieldName, resolveTree }; | ||
| } catch (_err) { | ||
| console.error("Error during query parsing"); |
There was a problem hiding this comment.
Why not throw here. instead of returning undefined?
There was a problem hiding this comment.
bc this just logs a cypher query, do we want it to throw?
| if (mapping) return mapping; | ||
|
|
||
| if (this.userResolvedFields.has(fieldName)) { | ||
| console.log(`Skipping translation for "${fieldName}": field uses a user-provided JS resolver`); |
There was a problem hiding this comment.
I'm not sure about all this logging
| (res, [key, value]) => { | ||
| if (key === "Node") return res; | ||
| if (key === typeName) { | ||
| return { | ||
| ...res, | ||
| [key]: { | ||
| ...value, | ||
| id: { name: "id", alias: "id", args: {}, fieldsByTypeName: {} }, | ||
| }, | ||
| }; | ||
| } | ||
| return { ...res, [key]: value }; | ||
| }, |
There was a problem hiding this comment.
mmm, I'm not sure how performance critical is this bit, but this is cloning objects for every item in fieldsByTypename.
Consider instead just modifying res
2 participants
Description
This PR exposes a method to get the translation of a given GraphQL query.
Adds a
dryRunparam to the execution context to change some behaviour:Also calls the dry run method on every execute GraphQL query call of the testHelper.
Complexity
Complexity: Medium (maybe)
Issue
Closes
Checklist
The following requirements should have been met (depending on the changes in the branch):