Skip to content

Commit

Permalink
Revert "Fix entity error path (#1605)" (#1621)
Browse files Browse the repository at this point in the history 
This reverts commit 329ca02.
  • Loading branch information
@srinivasankavitha
srinivasankavitha authored Aug 30, 2023
1 parent 6d04025 commit 99cdd73
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 203 deletions.
44 changes: 8 additions & 36 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,11 +28,9 @@ 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 @@ -90,7 +88,6 @@ 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 @@ -125,23 +122,21 @@ open class DefaultDgsFederationResolver() :
)
.errors(
trySequence
.mapIndexed { index, tryResult -> Pair(index, tryResult) }
.filter { iter -> iter.second.isFailure }
.map { iter -> Pair(iter.first, iter.second.throwable) }
.flatMap { iter: Pair<Int, Throwable> ->
.filter { tryResult -> tryResult.isFailure }
.map { tryResult -> tryResult.throwable }
.flatMap { e ->
// extract exception from known wrapper types
val exception = when {
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
e is InvocationTargetException && e.targetException != null -> e.targetException
e is CompletionException && e.cause != null -> e.cause!!
else -> e
}
// 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(dfeWithErrorPath)
.dataFetchingEnvironment(env)
.exception(exception)
.build()
)
Expand All @@ -150,7 +145,7 @@ open class DefaultDgsFederationResolver() :
sequenceOf(
TypedGraphQLError.newInternalErrorBuilder()
.message("%s: %s", exception::class.java.name, exception.message)
.path(ResultPath.parse("/_entities,${iter.first}"))
.path(ResultPath.parse("/_entities"))
.build()
)
}
Expand All @@ -161,29 +156,6 @@ 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()
return DgsDataFetchingEnvironment(
DataFetchingEnvironmentImpl
.newDataFetchingEnvironment()
.arguments(env.arguments)
.dataLoaderRegistry(env.dataLoaderRegistry)
.graphQLContext(env.graphQlContext)
.root(env.getRoot())
.graphQLSchema(env.graphQLSchema)
.fragmentsByName(env.fragmentsByName)
.dataLoaderRegistry(env.dataLoaderRegistry)
.locale(env.locale)
.document(env.document)
.operationDefinition(env.operationDefinition)
.variables(env.variables)
.executionId(env.executionId)
.executionStepInfo(executionStepInfoWithPath)
.build()
)
}

open fun typeMapping(): Map<Class<*>, String> {
return emptyMap()
}
Expand Down
167 changes: 0 additions & 167 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,7 +18,6 @@ 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
Expand Down Expand Up @@ -300,30 +299,6 @@ 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 @@ -483,146 +458,6 @@ 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`() {
// 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)

// Invoke the entitiesFetcher to get the result
val result = DefaultDgsFederationResolver(entityFetcherRegistry, Optional.of(dgsExceptionHandler))
.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)
assertThat(result.get().errors[0].path.toString()).contains("_entities, 0")
assertThat(result.get().errors[0].message).contains("No entity found for movieId 1")
assertThat(result.get().errors[1].path.toString()).contains("_entities, 1")
assertThat(result.get().errors[1].message).contains("No entity found for showId 2")
}

@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 @@ -673,6 +508,4 @@ class DefaultDgsFederationResolverTest {
}

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

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

0 comments on commit 99cdd73

Please sign in to comment.