Description
Describe the bug
Right now the library generates the cypher query based on the provided GraphQL query only fetching the attributes that were requested from neo4j. This might seem like a good idea when it comes to performance (why fetch the whole node if just a couple of attributes were requested). The problem occurs when using additional custom Data Fetchers. These Data Fetchers can be used to run custom cypher queries that would not be supported by the @cypher directive or running any computation code, accessing other databases ... but in order to do that they need context. This context should be provided by the env.getSource()
method where all the parent data should be present. In most cases you will need the id
of the parent to be able to do any additional computation. But in this case the parent contains only the data that were requested from neo4j which is wrong.
In the example below the javaData.name
relies on the title
and if this is not requested it will not be in the parent thus javaData.name
will have the value "test null"
which is wrong.
Test Case
Modify the content of the class: https://github.com/neo4j-graphql/neo4j-graphql-java/blob/master/examples/dgs-spring-boot/src/test/kotlin/org/neo4j/graphql/examples/dgsspringboot/datafetcher/AdditionalDataFetcherTest.kt to the following code:
package org.neo4j.graphql.examples.dgsspringboot.datafetcher
import com.jayway.jsonpath.TypeRef
import com.netflix.graphql.dgs.DgsQueryExecutor
import com.netflix.graphql.dgs.client.codegen.GraphQLQueryRequest
import org.assertj.core.api.Assertions
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.neo4j.driver.Driver
import org.neo4j.driver.springframework.boot.autoconfigure.Neo4jDriverProperties
import org.neo4j.graphql.examples.dgsspringboot.types.DgsConstants
import org.neo4j.graphql.examples.dgsspringboot.types.client.MoviesGraphQLQuery
import org.neo4j.graphql.examples.dgsspringboot.types.client.MoviesProjectionRoot
import org.neo4j.graphql.examples.dgsspringboot.types.types.Movie
import org.neo4j.graphql.examples.dgsspringboot.types.types.MovieOptions
import org.neo4j.graphql.examples.dgsspringboot.types.types.MovieSort
import org.neo4j.graphql.examples.dgsspringboot.types.types.SortDirection
import org.skyscreamer.jsonassert.JSONAssert
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.boot.autoconfigure.EnableAutoConfiguration
import org.springframework.boot.context.properties.ConfigurationProperties
import org.springframework.boot.test.context.SpringBootTest
import org.springframework.boot.test.context.TestConfiguration
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Primary
import org.testcontainers.containers.Neo4jContainer
import org.testcontainers.junit.jupiter.Container
import org.testcontainers.junit.jupiter.Testcontainers
import java.net.URI
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE, properties = ["database=neo4j"])
@EnableAutoConfiguration
@Testcontainers
internal class AdditionalDataFetcherTest(
@Autowired
val dgsQueryExecutor: DgsQueryExecutor,
@Autowired
val driver: Driver
) {
@BeforeEach
fun setup() {
driver.session().use {
it.run("""
CREATE (:Movie {title:'The Matrix', released:1999, tagline:'Welcome to the Real World'})
CREATE (:Movie {title:'The Matrix Reloaded', released:2003, tagline:'Free your mind'})
CREATE (:Movie {title:'The Matrix Revolutions', released:2003, tagline:'Everything that has a beginning has an end'})
""".trimIndent())
}
}
@AfterEach
fun tearDown() {
driver.session().use { it.run("MATCH (n) DETACH DELETE n") }
}
@Test
fun testHybridDataFetcher() {
val graphQLQueryRequest = GraphQLQueryRequest(
MoviesGraphQLQuery.newRequest()
.options(MovieOptions(sort = listOf(MovieSort(title = SortDirection.DESC))))
.build(),
MoviesProjectionRoot().also { movie ->
// movie.title() // do not query for title
movie.bar()
movie.javaData().also { javaData ->
javaData.name()
}
}
)
val request = graphQLQueryRequest.serialize()
// Assertions.assertThat(request).isEqualTo("query {movies(options: {sort:[{title:DESC }] }){ title bar javaData { name } } }")
Assertions.assertThat(request).isEqualTo("query {movies(options: {sort:[{title:DESC }] }){ bar javaData { name } } }")
val response = dgsQueryExecutor.executeAndGetDocumentContext(request)
//language=JSON
// JSONAssert.assertEquals("""
// {
// "data": {
// "movies": [
// {
// "title": "The Matrix Revolutions",
// "bar": "foo",
// "javaData": [
// {
// "name": "test The Matrix Revolutions"
// }
// ]
// },
// {
// "title": "The Matrix Reloaded",
// "bar": "foo",
// "javaData": [
// {
// "name": "test The Matrix Reloaded"
// }
// ]
// },
// {
// "title": "The Matrix",
// "bar": "foo",
// "javaData": [
// {
// "name": "test The Matrix"
// }
// ]
// }
// ]
// }
// }
// """.trimIndent(), response.jsonString(), true)
// since title was not queried it was not present in the parent while the additional data fetcher for javaData was executed
// therefore javaData.name will be: "test null"
//language=JSON
JSONAssert.assertEquals("""
{
"data": {
"movies": [
{
"bar": "foo",
"javaData": [
{
"name": "test The Matrix Revolutions"
}
]
},
{
"bar": "foo",
"javaData": [
{
"name": "test The Matrix Reloaded"
}
]
},
{
"bar": "foo",
"javaData": [
{
"name": "test The Matrix"
}
]
}
]
}
}
""".trimIndent(), response.jsonString(), true)
val list = response.read("data.${DgsConstants.QUERY.Movies}", object : TypeRef<List<Movie>>() {})
Assertions.assertThat(list).hasSize(3)
}
@TestConfiguration
open class Config {
@Bean
@ConfigurationProperties(prefix = "ignore")
@Primary
open fun properties(): Neo4jDriverProperties {
val properties = Neo4jDriverProperties()
properties.uri = URI.create(neo4jServer.boltUrl)
properties.authentication = Neo4jDriverProperties.Authentication()
properties.authentication.username = "neo4j"
properties.authentication.password = neo4jServer.adminPassword
return properties
}
}
companion object {
@Container
private val neo4jServer = Neo4jContainer<Nothing>("neo4j:4.4.1")
}
}
and run the test.
Additional context
I believe that it should be configurable for each GraphQL type do define what data will be always fetched from neo4j to provide sufficient context for additional custom Data Fetchers. This way the necessary data will be always fetched and the additional custom Data Fetchers will be satisfied and will be able to do their computation and the rest of the data will be fetched only if requested by the QraphQL query for better performance. The graphql-java
library should then remove data that was not requested from the result returning only that the user asked for with the benefit that the additional custom Data Fetchers were able to do their computation and return the correct result.
Note that this modification will break the Translator
approach where there is no postprocessing of the fetched data thus the result will contain also the data needed for satisfaction of the additional custom Data Fetchers and not only what the user asked for.