Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore source locations #522

Merged
merged 3 commits into from
Mar 12, 2024
Merged

Ignore source locations #522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
811d9b5
Ignore source locations
gnawf Mar 11, 2024
e719a94
Add option to keep source locations
gnawf Mar 11, 2024
2093b44
Fix test
gnawf Mar 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions lib/src/main/java/graphql/nadel/NadelSchemas.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import graphql.nadel.schema.UnderlyingSchemaGenerator
import graphql.nadel.util.SchemaUtil
import graphql.parser.MultiSourceReader
import graphql.schema.GraphQLSchema
import graphql.schema.idl.SchemaParser
import graphql.schema.idl.TypeDefinitionRegistry
import graphql.schema.idl.WiringFactory
import java.io.Reader
Expand Down Expand Up @@ -37,6 +36,12 @@ data class NadelSchemas constructor(
internal var underlyingSchemaReaders = mutableMapOf<String, Reader>()
internal var underlyingTypeDefs = mutableMapOf<String, TypeDefinitionRegistry>()

private var captureSourceLocation = false

fun captureSourceLocation(value: Boolean): Builder = also {
captureSourceLocation = value
}

fun schemaTransformationHook(value: SchemaTransformationHook): Builder = also {
schemaTransformationHook = value
}
Expand Down Expand Up @@ -153,10 +158,14 @@ data class NadelSchemas constructor(
"serviceExecutionFactory must be set"
}

val schemaParser = SchemaParser()

// Combine readers & type defs
val readersToTypeDefs = underlyingSchemaReaders.mapValues { (_, value) -> schemaParser.parse(value) }
val readersToTypeDefs = underlyingSchemaReaders
.mapValues { (_, reader) ->
SchemaUtil.parseTypeDefinitionRegistry(
reader,
captureSourceLocation = captureSourceLocation,
)
}
val resolvedUnderlyingTypeDefs = readersToTypeDefs + underlyingTypeDefs

// Ensure we didn't have dupes i.e. we didn't merge and ignore a value
Expand All @@ -165,14 +174,20 @@ data class NadelSchemas constructor(
"There is an illegal intersection of underlying schema keys $intersection"
}

return Factory(builder = this, serviceExecutionFactory, resolvedUnderlyingTypeDefs).create()
return Factory(
builder = this,
serviceExecutionFactory = serviceExecutionFactory,
underlyingTypeDefs = resolvedUnderlyingTypeDefs,
captureSourceLocation = captureSourceLocation,
).create()
}
}

internal class Factory(
private val builder: Builder,
private val serviceExecutionFactory: ServiceExecutionFactory,
private val underlyingTypeDefs: Map<String, TypeDefinitionRegistry>,
private val captureSourceLocation: Boolean,
) {
fun create(): NadelSchemas {
val services = createServices()
Expand All @@ -187,7 +202,10 @@ data class NadelSchemas constructor(
val underlyingSchemaGenerator = UnderlyingSchemaGenerator()

return builder.overallSchemaReaders.map { (serviceName, reader) ->
val nadelDefinitions = SchemaUtil.parseDefinitions(reader)
val nadelDefinitions = SchemaUtil.parseSchemaDefinitions(
reader,
captureSourceLocation = captureSourceLocation,
)
val nadelDefinitionRegistry = NadelDefinitionRegistry.from(nadelDefinitions)

// Builder should enforce non-null entry
Expand Down
44 changes: 33 additions & 11 deletions lib/src/main/java/graphql/nadel/util/SchemaUtil.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,47 @@ package graphql.nadel.util

import graphql.language.SDLDefinition
import graphql.parser.Parser
import graphql.parser.ParserEnvironment
import graphql.parser.ParserOptions
import graphql.schema.idl.SchemaParser
import graphql.schema.idl.TypeDefinitionRegistry
import java.io.Reader

object SchemaUtil {
private val defaultParserOptions = ParserOptions.newParserOptions()
.maxTokens(Int.MAX_VALUE)
.build()

internal object SchemaUtil {
private val parser = Parser()
private val schemaParser = SchemaParser()

fun parseDefinitions(schema: String): List<AnySDLDefinition> {
fun parseSchemaDefinitions(
schema: Reader,
maxTokens: Int = Int.MAX_VALUE,
captureSourceLocation: Boolean = false,
): List<AnySDLDefinition> {
return parser
.parseDocument(schema, defaultParserOptions)
.parseDocument(
ParserEnvironment.newParserEnvironment()
.document(schema)
.parserOptions(
ParserOptions.newParserOptions()
.maxTokens(maxTokens)
.captureSourceLocation(captureSourceLocation)
.build(),
)
.build(),
)
.getDefinitionsOfType(SDLDefinition::class.java)
}

fun parseDefinitions(schema: Reader): List<AnySDLDefinition> {
return parser
.parseDocument(schema, defaultParserOptions)
.getDefinitionsOfType(SDLDefinition::class.java)
fun parseTypeDefinitionRegistry(
schema: Reader,
maxTokens: Int = Int.MAX_VALUE,
captureSourceLocation: Boolean = false,
): TypeDefinitionRegistry {
return schemaParser.parse(
schema,
ParserOptions.newParserOptions()
.maxTokens(maxTokens)
.captureSourceLocation(captureSourceLocation)
.build(),
)
}
}
119 changes: 95 additions & 24 deletions lib/src/test/kotlin/graphql/nadel/NadelSchemasTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package graphql.nadel

import graphql.language.NamedNode
import graphql.language.SourceLocation
import graphql.nadel.engine.util.getField
import graphql.nadel.engine.util.makeFieldCoordinates
import graphql.nadel.schema.ServiceSchemaProblem
Expand All @@ -10,6 +11,7 @@ import graphql.schema.idl.SchemaParser
import graphql.schema.idl.TypeDefinitionRegistry
import io.kotest.core.spec.style.DescribeSpec
import org.junit.jupiter.api.assertThrows
import kotlin.test.assertTrue

val NadelDefinitionRegistry.typeNames: Set<String>
get() = definitions
Expand Down Expand Up @@ -73,9 +75,9 @@ class NadelSchemasTest : DescribeSpec({
.build()

// then
assert(schemas.engineSchema.userTypeNames == setOf("World", "Echo", "Query", "JSON"))
assertTrue(schemas.engineSchema.userTypeNames == setOf("World", "Echo", "Query", "JSON"))
val testService = schemas.services.single()
assert(testService.underlyingSchema.userTypeNames == setOf("World", "Echo", "Query", "Food"))
assertTrue(testService.underlyingSchema.userTypeNames == setOf("World", "Echo", "Query", "Food"))
}

it("throws wrapping ServiceSchemaProblem") {
Expand Down Expand Up @@ -104,8 +106,8 @@ class NadelSchemasTest : DescribeSpec({
}

// then
assert(ex.serviceName == "test")
assert(ex.message.contains("A schema MUST have a 'query' operation defined"))
assertTrue(ex.serviceName == "test")
assertTrue(ex.message.contains("A schema MUST have a 'query' operation defined"))
}

it("works if you exclusively supply type defs") {
Expand Down Expand Up @@ -149,9 +151,9 @@ class NadelSchemasTest : DescribeSpec({
.build()

// then
assert(schemas.engineSchema.userTypeNames == setOf("World", "Echo", "Query", "JSON"))
assertTrue(schemas.engineSchema.userTypeNames == setOf("World", "Echo", "Query", "JSON"))
val testService = schemas.services.single()
assert(testService.underlyingSchema.userTypeNames == setOf("World", "Echo", "Query", "Food"))
assertTrue(testService.underlyingSchema.userTypeNames == setOf("World", "Echo", "Query", "Food"))
}

it("works if you supply both type defs and readers") {
Expand Down Expand Up @@ -211,13 +213,13 @@ class NadelSchemasTest : DescribeSpec({
.build()

// then
assert(schemas.engineSchema.userTypeNames == setOf("World", "Echo", "Query", "Issue", "JSON"))
assertTrue(schemas.engineSchema.userTypeNames == setOf("World", "Echo", "Query", "Issue", "JSON"))

val issueService = schemas.services.single { it.name == "issue" }
assert(issueService.underlyingSchema.userTypeNames == setOf("Query", "Issue"))
assertTrue(issueService.underlyingSchema.userTypeNames == setOf("Query", "Issue"))

val testService = schemas.services.single { it.name == "test" }
assert(testService.underlyingSchema.userTypeNames == setOf("Query", "Echo", "World"))
assertTrue(testService.underlyingSchema.userTypeNames == setOf("Query", "Echo", "World"))
}

it("combines the overall schemas") {
Expand Down Expand Up @@ -272,7 +274,7 @@ class NadelSchemasTest : DescribeSpec({
.build()

// then
assert(schemas.engineSchema.userTypeNames == setOf("World", "Echo", "Query", "Issue", "JSON"))
assertTrue(schemas.engineSchema.userTypeNames == setOf("World", "Echo", "Query", "Issue", "JSON"))
}

it("does not validate the schemas") {
Expand Down Expand Up @@ -321,15 +323,15 @@ class NadelSchemasTest : DescribeSpec({
.build()

// then
assert(schemas.engineSchema.userTypeNames == setOf("Query", "World", "Echo", "Issue", "JSON"))
assertTrue(schemas.engineSchema.userTypeNames == setOf("Query", "World", "Echo", "Issue", "JSON"))

val testService = schemas.services.first { it.name == "test" }
assert(testService.definitionRegistry.typeNames == setOf("Query", "Echo", "World"))
assert(testService.underlyingSchema.userTypeNames == setOf("Query"))
assertTrue(testService.definitionRegistry.typeNames == setOf("Query", "Echo", "World"))
assertTrue(testService.underlyingSchema.userTypeNames == setOf("Query"))

val issueService = schemas.services.first { it.name == "issue" }
assert(issueService.underlyingSchema.userTypeNames == setOf("Query", "Task"))
assert(issueService.definitionRegistry.typeNames == setOf("Query", "Issue"))
assertTrue(issueService.underlyingSchema.userTypeNames == setOf("Query", "Task"))
assertTrue(issueService.definitionRegistry.typeNames == setOf("Query", "Issue"))
}

it("retains line numbers for string input") {
Expand Down Expand Up @@ -378,25 +380,94 @@ class NadelSchemasTest : DescribeSpec({

// when
val schemas = NadelSchemas.newNadelSchemas()
.captureSourceLocation(true)
.overallSchemas(overallSchema)
.underlyingSchemas(underlyingSchema)
.stubServiceExecution()
.build()

// then
assert(schemas.engineSchema.typeMap["Echo"]?.definition?.sourceLocation?.line == 4)
assert(schemas.engineSchema.typeMap["Issue"]?.definition?.sourceLocation?.line == 4)
assert(schemas.engineSchema.getField(echoCoordinates)?.definition?.sourceLocation?.line == 2)
assert(schemas.engineSchema.getField(worldCoordinates)?.definition?.sourceLocation?.line == 5)
assert(schemas.engineSchema.getField(issueIdCoordinates)?.definition?.sourceLocation?.line == 6)
assertTrue(schemas.engineSchema.typeMap["Echo"]?.definition?.sourceLocation?.line == 4)
assertTrue(schemas.engineSchema.typeMap["Issue"]?.definition?.sourceLocation?.line == 4)
assertTrue(schemas.engineSchema.getField(echoCoordinates)?.definition?.sourceLocation?.line == 2)
assertTrue(schemas.engineSchema.getField(worldCoordinates)?.definition?.sourceLocation?.line == 5)
assertTrue(schemas.engineSchema.getField(issueIdCoordinates)?.definition?.sourceLocation?.line == 6)

val testService = schemas.services.single { it.name == "test" }
assert(testService.underlyingSchema.typeMap["Echo"]?.definition?.sourceLocation?.line == 5)
assert(testService.underlyingSchema.getField(echoCoordinates)?.definition?.sourceLocation?.line == 2)
assert(testService.underlyingSchema.getField(worldCoordinates)?.definition?.sourceLocation?.line == 6)
assertTrue(testService.underlyingSchema.typeMap["Echo"]?.definition?.sourceLocation?.line == 5)
assertTrue(testService.underlyingSchema.getField(echoCoordinates)?.definition?.sourceLocation?.line == 2)
assertTrue(testService.underlyingSchema.getField(worldCoordinates)?.definition?.sourceLocation?.line == 6)

val issueService = schemas.services.single { it.name == "issue" }
assert(issueService.underlyingSchema.typeMap["Task"]?.definition?.sourceLocation?.line == 4)
assertTrue(issueService.underlyingSchema.typeMap["Task"]?.definition?.sourceLocation?.line == 4)
}

it("drops source location if not asked for") {
val overallSchema = mapOf(
"test" to """
type Query {
echo: Echo
}
type Echo {
world: String
}
""".trimIndent(),
"issue" to """
type Query {
issue: Issue
}
type Issue {
# This is a comment
id: ID!
}
""".trimIndent(),
)
val underlyingSchema = mapOf(
"test" to """
type Query {
echo: Echo
}

type Echo {
world: String
}
""".trimIndent(),
"issue" to """
type Query {
issue: Task
}
type Task {
id: ID!
}
""".trimIndent(),
)

val echoCoordinates = makeFieldCoordinates("Query", "echo")
val worldCoordinates = makeFieldCoordinates("Echo", "world")
val issueIdCoordinates = makeFieldCoordinates("Issue", "id")

// when
val schemas = NadelSchemas.newNadelSchemas()
.captureSourceLocation(false)
.overallSchemas(overallSchema)
.underlyingSchemas(underlyingSchema)
.stubServiceExecution()
.build()

// then
assertTrue(schemas.engineSchema.typeMap["Echo"]?.definition?.sourceLocation == SourceLocation.EMPTY)
assertTrue(schemas.engineSchema.typeMap["Issue"]?.definition?.sourceLocation == SourceLocation.EMPTY)
assertTrue(schemas.engineSchema.getField(echoCoordinates)?.definition?.sourceLocation == SourceLocation.EMPTY)
assertTrue(schemas.engineSchema.getField(worldCoordinates)?.definition?.sourceLocation == SourceLocation.EMPTY)
assertTrue(schemas.engineSchema.getField(issueIdCoordinates)?.definition?.sourceLocation == SourceLocation.EMPTY)

val testService = schemas.services.single { it.name == "test" }
assertTrue(testService.underlyingSchema.typeMap["Echo"]?.definition?.sourceLocation == SourceLocation.EMPTY)
assertTrue(testService.underlyingSchema.getField(echoCoordinates)?.definition?.sourceLocation == SourceLocation.EMPTY)
assertTrue(testService.underlyingSchema.getField(worldCoordinates)?.definition?.sourceLocation == SourceLocation.EMPTY)

val issueService = schemas.services.single { it.name == "issue" }
assertTrue(issueService.underlyingSchema.typeMap["Task"]?.definition?.sourceLocation == SourceLocation.EMPTY)
}
}
})
Loading