Skip to content

Treat array of non-null scalars as scalars #358

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.

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

Merged
merged 2 commits into from
Jan 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,12 @@
<version>1.0.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>javax.servlet</groupId>
<artifactId>javax.servlet-api</artifactId>
<version>3.1.0</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,12 @@ internal class MethodFieldResolver(field: FieldDefinition, search: FieldResolver
}

private fun isScalarType(environment: DataFetchingEnvironment, type: Type<*>, genericParameterType: JavaType): Boolean =
when {
type is ListType ->
List::class.java.isAssignableFrom(this.genericType.getRawClass(genericParameterType))
when (type) {
is ListType -> List::class.java.isAssignableFrom(this.genericType.getRawClass(genericParameterType))
&& isScalarType(environment, type.type, this.genericType.unwrapGenericType(genericParameterType))
type is TypeName ->
environment.graphQLSchema?.getType(type.name)?.let { isScalar(it) } ?: false
type is NonNullType && type.type is TypeName ->
environment.graphQLSchema?.getType((type.type as TypeName).name)?.let { isScalar(unwrapNonNull(it)) } ?: false
else ->
false
is TypeName -> environment.graphQLSchema?.getType(type.name)?.let { isScalar(it) } ?: false
is NonNullType -> isScalarType(environment, type.type, genericParameterType)
else -> false
}

override fun scanForMatches(): List<TypeClassMatcher.PotentialMatch> {
Expand Down
13 changes: 13 additions & 0 deletions src/test/groovy/com/coxautodev/graphql/tools/EndToEndSpec.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,19 @@ class EndToEndSpec extends Specification {
data.itemByUUID
}

def "generated schema should handle non nullable scalar types"() {
when:
def fileParts = [new MockPart("test.doc", "Hello"), new MockPart("test.doc", "World")]
def args = ["fileParts": fileParts]
def data = Utils.assertNoGraphQlErrors( gql, args) {
'''
mutation ($fileParts: [Upload!]!) { echoFiles(fileParts: $fileParts)}
'''
}
then:
(data["echoFiles"] as ArrayList<String>).join(",") == "Hello,World"
}

def "generated schema should handle any java.util.Map (using HashMap) types as property maps"() {
when:
def data = Utils.assertNoGraphQlErrors(gql) {
Expand Down
58 changes: 53 additions & 5 deletions src/test/kotlin/com/coxautodev/graphql/tools/EndToEndSpecHelper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,21 @@ import graphql.execution.DataFetcherResult
import graphql.execution.batched.Batched
import graphql.language.ObjectValue
import graphql.language.StringValue
import graphql.schema.Coercing
import graphql.schema.DataFetchingEnvironment
import graphql.schema.GraphQLScalarType
import graphql.schema.*
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.channels.ReceiveChannel
import kotlinx.coroutines.coroutineScope
import org.reactivestreams.Publisher
import java.io.InputStream
import java.util.*
import java.util.concurrent.CompletableFuture
import javax.servlet.http.Part

fun createSchema() = SchemaParser.newParser()
.schemaString(schemaDefinition)
.resolvers(Query(), Mutation(), Subscription(), ItemResolver(), UnusedRootResolver(), UnusedResolver())
.scalars(customScalarUUID, customScalarMap, customScalarId)
.resolvers(Query(), Mutation(), Subscription(), ItemResolver(), UnusedRootResolver(), UnusedResolver(), EchoFilesResolver())
.scalars(customScalarUUID, customScalarMap, customScalarId, uploadScalar)
.dictionary("OtherItem", OtherItemWithWrongName::class)
.dictionary("ThirdItem", ThirdItem::class)
.dictionary("ComplexMapItem", ComplexMapItem::class)
Expand All @@ -31,6 +31,7 @@ val schemaDefinition = """
## Private comment!
scalar UUID
scalar customScalarMap
scalar Upload
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe some testing code was left behind in this PR.

Copy link
Contributor Author

@adisesha adisesha Jan 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line is related to issue. The scalar is used in 'echoFiles' mutation which is part of the test scenario.

Copy link
Contributor Author

@adisesha adisesha Jan 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if there is anything needed.


type Query {
# Check if items list is empty
Expand Down Expand Up @@ -108,6 +109,7 @@ input ComplexInputTypeTwo {

type Mutation {
addItem(newItem: NewItemInput!): Item!
echoFiles(fileParts: [Upload!]!): [String!]!
}

type Subscription {
Expand Down Expand Up @@ -353,6 +355,10 @@ class ItemResolver : GraphQLResolver<Item> {
}
}

class EchoFilesResolver: GraphQLMutationResolver{
fun echoFiles(fileParts: List<Part>): List<String> = fileParts.map { String(it.inputStream.readBytes()) }
}

interface ItemInterface {
val name: String
val type: Type
Expand All @@ -373,6 +379,18 @@ data class ComplexNullable(val first: String, val second: String, val third: Str
data class ComplexInputType(val first: String, val second: List<List<ComplexInputTypeTwo>?>?)
data class ComplexInputTypeTwo(val first: String)
data class ItemWithGenericProperties(val keys: List<String>)
class MockPart(private val name: String, private val content: String):Part{
override fun getSubmittedFileName(): String = name
override fun getName(): String = name
override fun write(fileName: String?) = throw IllegalArgumentException("Not supported")
override fun getHeader(name: String): String? = null
override fun getSize(): Long = content.toByteArray().size.toLong()
override fun getContentType(): String? =null
override fun getHeaders(name: String?): Collection<String> = listOf()
override fun getHeaderNames(): Collection<String> = listOf()
override fun getInputStream(): InputStream = content.byteInputStream()
override fun delete() = throw IllegalArgumentException("Not supported")
}

val customScalarId = GraphQLScalarType.newScalar()
.name("ID")
Expand Down Expand Up @@ -427,3 +445,33 @@ val customScalarMap = GraphQLScalarType.newScalar()
override fun parseLiteral(input: Any?): Map<String, Any> = (input as ObjectValue).objectFields.associateBy { it.name }.mapValues { (it.value.value as StringValue).value }
})
.build()

//Definition from https://github.com/graphql-java-kickstart/graphql-java-servlet/blob/master/src/main/java/graphql/servlet/core/ApolloScalars.java
val uploadScalar: GraphQLScalarType = GraphQLScalarType.newScalar()
.name("Upload")
.description("A file part in a multipart request")
.coercing(object : Coercing<Part?, Void?> {
override fun serialize(dataFetcherResult: Any): Void? {
throw CoercingSerializeException("Upload is an input-only type")
}

override fun parseValue(input: Any?): Part? {
return when (input) {
is Part -> {
input
}
null -> {
null
}
else -> {
throw CoercingParseValueException("Expected type ${Part::class.java.name} but was ${input.javaClass.name}")
}
}
}

override fun parseLiteral(input: Any): Part? {
throw CoercingParseLiteralException(
"Must use variables to specify Upload values")
}
}).build()