neo4j-graphql / neo4j-graphql-js

NOTE: This project is no longer actively maintained. Please consider using the official Neo4j GraphQL Library (linked in README).
Other
609 stars 147 forks source link

Improper variable handling in cypher directive #136

Closed johnymontana closed 5 years ago

johnymontana commented 5 years ago

GraphQL schema:

type PointOfInterest {
  node_osm_id: Int!
  name: String
  lat: Float
  lon: Float
  location: Point
  poi_id: String
  shortestPathRouteToPOI(end_poi_id: String): [LatLng] @cypher(statement: """
    MATCH (b:PointOfInterest) WHERE b.poi_id = $end_poi_id
    MATCH p=shortestPath((this)-[:ROUTE*..200]-(b))
    UNWIND nodes(p) AS n
    RETURN {lon: n.location.longitude, lat: n.location.latitude} AS route
  """)
  dijkstraRouteToPOI(poi_id: String): [LatLng] @cypher(statement: """
    MATCH (b:PointOfInterest) WHERE b.poi_id = $poi_id
    CALL apoc.algo.dijkstra(this, b, 'ROUTE', 'distance') YIELD path, weight
    UNWIND nodes(path) AS n
    RETURN {lon: n.location.longitude, lat: n.location.latitude} AS route
  """)
  aStarRouteToPOI(poi_id: String): [LatLng] @cypher(statement: """
    MATCH (b:PointOfInterest) WHERE b.poi_id = $poi_id
    CALL apoc.algo.aStar(this, b, 'ROUTE', 'distance', 'lat', 'lon') YIELD path, weight
    UNWIND nodes(path) AS n
    RETURN {lon: n.location.longitude, lat: n.location.latitude} AS route
  """)
}

type LatLng {
  lat: Float
  lon: Float
}

type Query {
  POIsInRadius(lat: Float, lon: Float, radius: Float): [PointOfInterest] @cypher(statement: """
  MATCH (p:PointOfInterest) 
  WHERE distance(p.location, point({latitude: $lat, longitude:$lon})) < ( $radius * 1000)
  RETURN p
  """)
}

GraphQL query:

query routeQuery($startPOI: String, $endPOI: String) {
  PointOfInterest(poi_id: $startPOI) {
    name
    route: shortestPathRouteToPOI(end_poi_id: $endPOI) {
      lat
      lon
    }
  }
}

GraphQL variables:

{
"startPOI": "Old Ship Saloon37.7977871-122.4007139",
"endPOI": "Gestalt37.7647122-122.4233225"
}

generated cypher query:

MATCH (pointOfInterest:PointOfInterest {poi_id:$poi_id}) RETURN pointOfInterest { .name ,shortestPathRouteToPOI: [ pointOfInterest_shortestPathRouteToPOI IN apoc.cypher.runFirstColumn("MATCH (b:PointOfInterest) WHERE b.poi_id = $end_poi_id
MATCH p=shortestPath((this)-[:ROUTE*..200]-(b))
UNWIND nodes(p) AS n
RETURN {lon: n.location.longitude, lat: n.location.latitude} AS route", {this: pointOfInterest}, true) | pointOfInterest_shortestPathRouteToPOI { .lat , .lon }] } AS pointOfInterest SKIP $offset

genereated cypher parameters:

{ offset: 0,
  first: -1,
  poi_id: 'Old Ship Saloon37.7977871-122.4007139',
  '1_end_poi_id': undefined }

Expected value of 1_end_poi_id to be Gestalt37.7647122-122.4233225 and not undefined.

johnymontana commented 5 years ago

Note that when GraphQL variables are not used the value is inlined in the generated cypher query:

 {
  PointOfInterest(poi_id: "Old Ship Saloon37.7977871-122.4007139") {
    name
    route: shortestPathRouteToPOI(poi_id: "Gestalt37.7647122-122.4233225") {
      lat
      lon
    }
  }
}
MATCH (pointOfInterest:PointOfInterest {poi_id:$poi_id}) RETURN pointOfInterest { .name ,shortestPathRouteToPOI: [ pointOfInterest_shortestPathRouteToPOI IN apoc.cypher.runFirstColumn("MATCH (b:PointOfInterest) WHERE b.poi_id = $poi_id
MATCH p=shortestPath((this)-[:ROUTE*..200]-(b))
UNWIND nodes(p) AS n
RETURN {lon: n.location.longitude, lat: n.location.latitude} AS route", {this: pointOfInterest, poi_id: "Gestalt37.7647122-122.4233225"}, true) | pointOfInterest_shortestPathRouteToPOI { .lat , .lon }] } AS pointOfInterest SKIP $offset

{ offset: 0,
  first: -1,
  poi_id: 'Old Ship Saloon37.7977871-122.4007139',
  '1_poi_id': 'Gestalt37.7647122-122.4233225' }
johnymontana commented 5 years ago

After #200 the generated Cypher query and parameters are:

MATCH (`pointOfInterest`:`PointOfInterest` {poi_id:$poi_id}) RETURN `pointOfInterest` { .name ,shortestPathRouteToPOI: [ pointOfInterest_shortestPathRouteToPOI IN apoc.cypher.runFirstColumn("MATCH (b:PointOfInterest) WHERE b.poi_id = $end_poi_id
MATCH p=shortestPath((this)-[:ROUTE*..200]-(b))
UNWIND nodes(p) AS n
RETURN {lon: n.location.longitude, lat: n.location.latitude} AS route", {this: pointOfInterest, end_poi_id: "Gestalt37.7647122-122.4233225"}, true) | pointOfInterest_shortestPathRouteToPOI { .lat , .lon }] } AS `pointOfInterest` SKIP $offset
{ offset: 0,
  first: -1,
  poi_id: 'Old Ship Saloon37.7977871-122.4007139',
  '1_end_poi_id': 'Gestalt37.7647122-122.4233225' }

which fixes the issue of not passing the end_poi_id parameter value to the subquery in apoc.cypher.runFirstColumn, however the value is inlined to the subquery for end_poi_id. To be consistent with parameterization we should make use of the Cypher param 1_end_poi_id instead:

{this: pointOfInterest, end_poi_id: $1_end_poi_id},
danielmcmillan commented 5 years ago

Seems like there is still an issue when a GraphQL variable is used for a field within an object passed as a parameter.

Schema:

input BInput {
  param: String
}
type A {
  b(bInput: BInput): String
    @cypher(statement: "RETURN $bInput.param;")
}

Query:

query(var: String) {
  A {
    b(bInput: {param: $var})
  }
}

Variables: {"var": "VALUE"}

In the generated Cypher parameters, 1_bInput is { param: undefined } instead of { param: 'VALUE' }.

johnymontana commented 5 years ago

Fixed in #213