Skip to content

Commit

Permalink
Merge pull request #1653 from Netflix/BFG-489-missing-dfe-fields
Browse files Browse the repository at this point in the history 
Update creation of new data fetching environment so that fields are not copied individually for entity fetching errors.
  • Loading branch information
@kailyak
kailyak authored Oct 4, 2023
2 parents 25e0de7 + 2ac9a00 commit ed5ef68
Show file tree
Hide file tree
Showing 3 changed files with 226 additions and 8 deletions.
3 changes: 3 additions & 0 deletions graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/DgsDataFetchingEnvironment.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ import graphql.schema.DataFetchingEnvironment
import org.dataloader.DataLoader

class DgsDataFetchingEnvironment(private val dfe: DataFetchingEnvironment) : DataFetchingEnvironment by dfe {
fun getDfe(): DataFetchingEnvironment {
return this.dfe
}

fun getDgsContext(): DgsContext {
return DgsContext.from(this)
Expand Down
28 changes: 20 additions & 8 deletions ...ql-dgs/src/main/kotlin/com/netflix/graphql/dgs/federation/DefaultDgsFederationResolver.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@ import com.netflix.graphql.types.errors.TypedGraphQLError
import graphql.execution.DataFetcherExceptionHandler
import graphql.execution.DataFetcherExceptionHandlerParameters
import graphql.execution.DataFetcherResult
import graphql.execution.ExecutionStepInfo
import graphql.execution.ResultPath
import graphql.schema.DataFetcher
import graphql.schema.DataFetchingEnvironment
import graphql.schema.DataFetchingEnvironmentImpl
import graphql.schema.TypeResolver
import org.dataloader.Try
import org.slf4j.Logger
Expand Down Expand Up @@ -88,6 +90,7 @@ open class DefaultDgsFederationResolver() :
if (!fetcher.second.parameterTypes.any { it.isAssignableFrom(Map::class.java) }) {
throw InvalidDgsEntityFetcher("@DgsEntityFetcher ${fetcher.first::class.java.name}.${fetcher.second.name} is invalid. A DgsEntityFetcher must accept an argument of type Map<String, Object>")
}

val result =
if (fetcher.second.parameterTypes.any { it.isAssignableFrom(DgsDataFetchingEnvironment::class.java) }) {
fetcher.second.invoke(fetcher.first, values, DgsDataFetchingEnvironment(env))
Expand Down Expand Up @@ -122,21 +125,23 @@ open class DefaultDgsFederationResolver() :
)
.errors(
trySequence
.filter { tryResult -> tryResult.isFailure }
.map { tryResult -> tryResult.throwable }
.flatMap { e ->
.mapIndexed { index, tryResult -> Pair(index, tryResult) }
.filter { iter -> iter.second.isFailure }
.map { iter -> Pair(iter.first, iter.second.throwable) }
.flatMap { iter: Pair<Int, Throwable> ->
// extract exception from known wrapper types
val exception = when {
e is InvocationTargetException && e.targetException != null -> e.targetException
e is CompletionException && e.cause != null -> e.cause!!
else -> e
iter.second is InvocationTargetException && (iter.second as InvocationTargetException).targetException != null -> (iter.second as InvocationTargetException).targetException
iter.second is CompletionException && iter.second.cause != null -> iter.second.cause!!
else -> iter.second
}
// handle the exception (using the custom handler if present)
if (dgsExceptionHandler.isPresent) {
val dfeWithErrorPath = createDataFetchingEnvironmentWithPath(env, iter.first)
val res = dgsExceptionHandler.get().handleException(
DataFetcherExceptionHandlerParameters
.newExceptionParameters()
.dataFetchingEnvironment(env)
.dataFetchingEnvironment(dfeWithErrorPath)
.exception(exception)
.build()
)
Expand All @@ -145,7 +150,7 @@ open class DefaultDgsFederationResolver() :
sequenceOf(
TypedGraphQLError.newInternalErrorBuilder()
.message("%s: %s", exception::class.java.name, exception.message)
.path(ResultPath.parse("/_entities"))
.path(ResultPath.parse("/_entities,${iter.first}"))
.build()
)
}
Expand All @@ -156,6 +161,13 @@ open class DefaultDgsFederationResolver() :
}
}

open fun createDataFetchingEnvironmentWithPath(env: DataFetchingEnvironment, pathIndex: Int): DgsDataFetchingEnvironment {
val pathWithIndex = env.executionStepInfo.path.segment("$pathIndex")
val executionStepInfoWithPath = ExecutionStepInfo.newExecutionStepInfo(env.executionStepInfo).path(pathWithIndex).build()
val dfe = if (env is DgsDataFetchingEnvironment) env.getDfe() else env
return DgsDataFetchingEnvironment(DataFetchingEnvironmentImpl.newDataFetchingEnvironment(dfe).executionStepInfo(executionStepInfoWithPath).build())
}

open fun typeMapping(): Map<Class<*>, String> {
return emptyMap()
}
Expand Down
203 changes: 203 additions & 0 deletions graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/DefaultDgsFederationResolverTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,24 @@ package com.netflix.graphql.dgs

import com.apollographql.federation.graphqljava._Entity
import com.netflix.graphql.dgs.exceptions.DefaultDataFetcherExceptionHandler
import com.netflix.graphql.dgs.exceptions.DgsEntityNotFoundException
import com.netflix.graphql.dgs.exceptions.DgsInvalidInputArgumentException
import com.netflix.graphql.dgs.federation.DefaultDgsFederationResolver
import com.netflix.graphql.dgs.internal.DgsSchemaProvider
import com.netflix.graphql.dgs.internal.EntityFetcherRegistry
import com.netflix.graphql.dgs.internal.method.MethodDataFetcherFactory
import com.netflix.graphql.types.errors.ErrorDetail
import com.netflix.graphql.types.errors.TypedGraphQLError
import graphql.GraphQLError
import graphql.execution.DataFetcherExceptionHandler
import graphql.execution.DataFetcherExceptionHandlerParameters
import graphql.execution.DataFetcherExceptionHandlerResult
import graphql.execution.DataFetcherResult
import graphql.execution.ExecutionStepInfo
import graphql.execution.MergedField
import graphql.execution.ResultPath
import graphql.execution.TypeResolutionParameters
import graphql.language.Field
import graphql.schema.DataFetchingEnvironment
import graphql.schema.DataFetchingEnvironmentImpl
import graphql.schema.GraphQLList
Expand Down Expand Up @@ -299,6 +305,30 @@ class DefaultDgsFederationResolverTest {
assertThat(result.get().data).hasSize(1).first().isInstanceOf(Movie::class.java)
assertThat(result.get().data.first() as Movie).extracting { it.movieId }.isEqualTo("1")
}

private fun testEntityFetcherWithoutExceptionHandler(movieEntityFetcher: Any) {
every { applicationContextMock.getBeansWithAnnotation(DgsScalar::class.java) } returns emptyMap()
every { applicationContextMock.getBeansWithAnnotation(DgsDirective::class.java) } returns emptyMap()
every { applicationContextMock.getBeansWithAnnotation(DgsComponent::class.java) } returns mapOf("MovieEntityFetcher" to movieEntityFetcher)

dgsSchemaProvider.schema("""type Query {}""")

val arguments = mapOf<String, Any>(
_Entity.argumentName to listOf(mapOf("__typename" to "Movie", "movieId" to "1"), mapOf("__typename" to "Movie", "movieId" to "2"))
)
val dataFetchingEnvironment = constructDFE(arguments)

val result =
(
DefaultDgsFederationResolver(entityFetcherRegistry, Optional.of(dgsExceptionHandler))
.entitiesFetcher()
.get(dataFetchingEnvironment) as CompletableFuture<DataFetcherResult<List<*>>>
)

assertThat(result).isNotNull
assertThat(result.get().data).hasSize(1).first().isInstanceOf(Movie::class.java)
assertThat(result.get().data.first() as Movie).extracting { it.movieId }.isEqualTo("1")
}
}

@Nested
Expand Down Expand Up @@ -458,6 +488,176 @@ class DefaultDgsFederationResolverTest {
.satisfies(Consumer { assertThat(it).contains("IllegalArgumentException") })
}

@Test
fun `Entity Fetcher throws DgsEntityNotFoundException contains path in error`() {
val movieEntityFetcher = object {
@DgsEntityFetcher(name = "Movie")
fun movieEntityFetcher(values: Map<String, Any>, dfe: DgsDataFetchingEnvironment?): Movie {
if (dfe == null) {
throw RuntimeException()
}
if (values["movieId"] == "2") {
throw DgsEntityNotFoundException("No entity found for movieId 2")
}
return Movie(values["movieId"].toString())
}
}
every { applicationContextMock.getBeansWithAnnotation(DgsScalar::class.java) } returns emptyMap()
every { applicationContextMock.getBeansWithAnnotation(DgsDirective::class.java) } returns emptyMap()
every { applicationContextMock.getBeansWithAnnotation(DgsComponent::class.java) } returns mapOf("MovieEntityFetcher" to movieEntityFetcher)

dgsSchemaProvider.schema("""type Query {}""")

val arguments = mapOf<String, Any>(
_Entity.argumentName to listOf(mapOf("__typename" to "Movie", "movieId" to "1"), mapOf("__typename" to "Movie", "movieId" to "2"))
)
val dataFetchingEnvironment = constructDFE(arguments)

val result =
DefaultDgsFederationResolver(entityFetcherRegistry, Optional.of(dgsExceptionHandler))
.entitiesFetcher().get(dataFetchingEnvironment) as CompletableFuture<DataFetcherResult<List<*>>>

assertThat(result).isNotNull
assertThat(result.get().data).hasSize(2)
assertThat(result.get().errors).hasSize(1).first().extracting { it.path }
.satisfies(Consumer { assertThat(it.toString()).contains("_entities, 1") })
}

@Test
fun `Entity Fetcher throws DgsEntityNotFoundException for different types with custom handling`() {
// Define a mock movie entity fetcher that throws an EntityNotFoundException for movieId 1
val movieEntityFetcher = object {
@DgsEntityFetcher(name = "Movie")
fun movieEntityFetcher(values: Map<String, Any>, dfe: DgsDataFetchingEnvironment?): Movie {
if (values["movieId"] == "1") {
throw DgsEntityNotFoundException("No entity found for movieId 1")
}
return Movie(values["movieId"].toString(), "Some Movie Title")
}
}

// Define a mock show entity fetcher that throws an EntityNotFoundException for showId 2
val showEntityFetcher = object {
@DgsEntityFetcher(name = "Show")
fun showEntityFetcher(values: Map<String, Any>, dfe: DgsDataFetchingEnvironment?): Show {
if (values["showId"] == "2") {
throw DgsEntityNotFoundException("No entity found for showId 2")
}
return Show(values["showId"].toString(), "Some Show Title")
}
}

// Mock the ApplicationContext to return the mock entity fetchers
every { applicationContextMock.getBeansWithAnnotation(DgsScalar::class.java) } returns emptyMap()
every { applicationContextMock.getBeansWithAnnotation(DgsDirective::class.java) } returns emptyMap()
every { applicationContextMock.getBeansWithAnnotation(DgsComponent::class.java) } returns mapOf(
"MovieEntityFetcher" to movieEntityFetcher,
"ShowEntityFetcher" to showEntityFetcher
)

// Initialize the schema with a minimal query type
dgsSchemaProvider.schema("""type Query {}""")

// Construct arguments with _entities argument
val arguments = mapOf<String, Any>(
_Entity.argumentName to listOf(mapOf("__typename" to "Movie", "movieId" to "1"), mapOf("__typename" to "Show", "showId" to "2"))
)

val dataFetchingEnvironment = constructDFE(arguments)

// Create a custom exception handler which uses fields in DFE available when doing custom handling.
val customExceptionHandler = object : DataFetcherExceptionHandler {
override fun handleException(handlerParameters: DataFetcherExceptionHandlerParameters?): CompletableFuture<DataFetcherExceptionHandlerResult> {
return if (handlerParameters?.exception is DgsEntityNotFoundException) {
// Check DFE field
val fieldName = handlerParameters.dataFetchingEnvironment.field.name

val exception = handlerParameters.exception
val graphqlError: GraphQLError =
TypedGraphQLError
.newBuilder()
.errorDetail(ErrorDetail.Common.ENHANCE_YOUR_CALM)
.message("$fieldName Error: " + exception.message)
.path(handlerParameters.path)
.build()
CompletableFuture.completedFuture(
DataFetcherExceptionHandlerResult.newResult()
.error(graphqlError)
.build()
)
} else {
dgsExceptionHandler.handleException(handlerParameters)
}
}
}

// Invoke the entitiesFetcher to get the result
val result = DefaultDgsFederationResolver(entityFetcherRegistry, Optional.of(customExceptionHandler))
.entitiesFetcher().get(dataFetchingEnvironment) as CompletableFuture<DataFetcherResult<List<*>>>

// Assertions to check the result and errors
assertThat(result).isNotNull
assertThat(result.get().data).hasSize(2)
assertThat(result.get().errors).hasSize(2).satisfiesExactly(
{ error -> assertThat(error.path.toString().contains("_entities, 0")) },
{ error -> assertThat(error.path.toString().contains("_entities, 1")) }
)
assertThat(result.get().errors).hasSize(2).satisfiesExactly(
{ error -> assertThat(error.message.contains("No entity found for movieId 1")) },
{ error -> assertThat(error.message.contains("No entity found for movieId 2")) }
)
assertThat(result.get().errors[0].message).contains(dataFetchingEnvironment.getDfe().field.name)
}

@Test
fun `DgsEntityNotFoundException contains path indexes when multiple entities of same type not found in query`() {
val movieEntityId1 = "111111"
val movieEntityId2 = "222222"
val movieEntityId3 = "333333"

val movieEntityFetcher = object {
@DgsEntityFetcher(name = "Movie")
fun movieEntityFetcher(values: Map<String, Any>, dfe: DgsDataFetchingEnvironment?): Movie {
if (dfe == null) {
throw RuntimeException()
}
if (values["movieId"] == movieEntityId1) {
throw DgsEntityNotFoundException("No entity found for movieId $movieEntityId1")
}
if (values["movieId"] == movieEntityId2) {
throw DgsEntityNotFoundException("No entity found for movieId $movieEntityId2")
}
return Movie(values["movieId"].toString())
}
}

every { applicationContextMock.getBeansWithAnnotation(DgsScalar::class.java) } returns emptyMap()
every { applicationContextMock.getBeansWithAnnotation(DgsDirective::class.java) } returns emptyMap()
every { applicationContextMock.getBeansWithAnnotation(DgsComponent::class.java) } returns mapOf("MovieEntityFetcher" to movieEntityFetcher)

dgsSchemaProvider.schema("""type Query {}""")

val arguments = mapOf<String, Any>(
_Entity.argumentName to listOf(
mapOf("__typename" to "Movie", "movieId" to movieEntityId1),
mapOf("__typename" to "Movie", "movieId" to movieEntityId2),
mapOf("__typename" to "Movie", "movieId" to movieEntityId3)
)
)
val dataFetchingEnvironment = constructDFE(arguments)

val result =
DefaultDgsFederationResolver(entityFetcherRegistry, Optional.empty())
.entitiesFetcher().get(dataFetchingEnvironment) as CompletableFuture<DataFetcherResult<List<*>>>

assertThat(result).isNotNull
assertThat(result.get().data).hasSize(3).last().isNotNull.hasFieldOrPropertyWithValue("movieId", movieEntityId3)
assertThat(result.get().errors).hasSize(2).satisfiesExactly(
{ error -> assertThat(error.path.contains("_entities, 0")) },
{ error -> assertThat(error.path.contains("_entities, 1")) }
)
}

@Test
fun `Invoking an Entity Fetcher missing an argument`() {
val arguments = mapOf<String, Any>(_Entity.argumentName to listOf(mapOf("__typename" to "Movie")))
Expand Down Expand Up @@ -503,9 +703,12 @@ class DefaultDgsFederationResolverTest {
.newDataFetchingEnvironment()
.arguments(arguments)
.executionStepInfo(executionStepInfo)
.mergedField(MergedField.newMergedField(Field("Movie")).build())
.build()
)
}

data class Movie(val movieId: String = "", val title: String = "")

data class Show(val showId: String = "", val title: String = "")
}

0 comments on commit ed5ef68

Please sign in to comment.