Skip to content

[generator] avoid duplicate argument deserialization #1379

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 12 commits into from
Mar 8, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
update input object validation to verify it has a public primary cons…
…tructor
  • Loading branch information
Dariusz Kuc committed Mar 8, 2022
commit 721b05327fe96b07baa3ed024eb5e4f97888e2bd
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,9 @@

package com.expediagroup.graphql.generator.exceptions

class CouldNotConstructAValidKotlinObject(input: Any?) : GraphQLKotlinException("Could not convert the input $input to a valid Kotlin object")
import kotlin.reflect.KClass

/**
* Thrown when unable to locate the public primary constructor of an input class.
*/
class PrimaryConstructorNotFound(klazz: KClass<*>) : GraphQLKotlinException("Invalid input object ${klazz.simpleName} - missing public primary constructor")
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package com.expediagroup.graphql.generator.execution

import com.expediagroup.graphql.generator.exceptions.CouldNotConstructAValidKotlinObject
import com.expediagroup.graphql.generator.exceptions.PrimaryConstructorNotFound
import com.expediagroup.graphql.generator.internal.extensions.getKClass
import com.expediagroup.graphql.generator.internal.extensions.getName
import com.expediagroup.graphql.generator.internal.extensions.getTypeOfFirstArgument
Expand Down Expand Up @@ -91,7 +91,7 @@ private fun convertValue(
* At this point all custom scalars have been converted by graphql-java so the only thing left to parse is object maps into the nested Kotlin classes
*/
private fun <T : Any> mapToKotlinObject(inputMap: Map<String, *>, targetClass: KClass<T>): T {
val targetConstructor = targetClass.primaryConstructor ?: throw CouldNotConstructAValidKotlinObject(targetClass)
val targetConstructor = targetClass.primaryConstructor ?: throw PrimaryConstructorNotFound(targetClass)
val params = targetConstructor.parameters
val constructorValues: Map<KParameter, Any?> = params.associateWith { parameter ->
convertArgumentValue(parameter.getName(), parameter, inputMap)
Expand All @@ -103,6 +103,6 @@ private fun mapToEnumValue(paramType: KType, enumValue: String): Enum<*> =
paramType.getKClass().java.enumConstants.filterIsInstance(Enum::class.java).first { it.name == enumValue }

private fun <T : Any> mapToInlineValueClass(value: Any?, targetClass: KClass<T>): T {
val targetConstructor = targetClass.primaryConstructor ?: throw CouldNotConstructAValidKotlinObject(targetClass)
val targetConstructor = targetClass.primaryConstructor ?: throw PrimaryConstructorNotFound(targetClass)
return targetConstructor.call(value)
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@ import com.expediagroup.graphql.generator.internal.extensions.getValidProperties
import com.expediagroup.graphql.generator.internal.extensions.safeCast
import com.expediagroup.graphql.generator.internal.types.utils.validateGraphQLName
import com.expediagroup.graphql.generator.internal.types.utils.validateObjectLocation
import com.expediagroup.graphql.generator.internal.types.utils.validatePrimaryConstructorExists
import graphql.introspection.Introspection.DirectiveLocation
import graphql.schema.GraphQLInputObjectType
import kotlin.reflect.KClass

internal fun generateInputObject(generator: SchemaGenerator, kClass: KClass<*>): GraphQLInputObjectType {
validateObjectLocation(kClass, GraphQLValidObjectLocations.Locations.INPUT_OBJECT)
validatePrimaryConstructorExists(kClass)

val name = kClass.getSimpleName(isInputClass = true)
validateGraphQLName(name, kClass)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright 2022 Expedia, Inc
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.expediagroup.graphql.generator.internal.types.utils

import com.expediagroup.graphql.generator.exceptions.PrimaryConstructorNotFound
import com.expediagroup.graphql.generator.internal.extensions.isPublic
import kotlin.reflect.KClass
import kotlin.reflect.full.primaryConstructor

/**
* Throws an exception if this KClass does not specify valid primary constructor.
*/
internal fun validatePrimaryConstructorExists(kClass: KClass<*>) {
if (kClass.primaryConstructor?.isPublic() == false) {
throw PrimaryConstructorNotFound(kClass)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import com.expediagroup.graphql.generator.annotations.GraphQLName
import com.expediagroup.graphql.generator.annotations.GraphQLValidObjectLocations
import com.expediagroup.graphql.generator.exceptions.InvalidGraphQLNameException
import com.expediagroup.graphql.generator.exceptions.InvalidObjectLocationException
import com.expediagroup.graphql.generator.exceptions.PrimaryConstructorNotFound
import com.expediagroup.graphql.generator.test.utils.SimpleDirective
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertDoesNotThrow
Expand Down Expand Up @@ -56,6 +57,8 @@ class GenerateInputObjectTest : TypeTestHelper() {

class `Invalid$InputTypeName`

class MissingPublicConstructor private constructor(val id: Int)

@Test
fun `Test naming`() {
val result = generateInputObject(generator, InputClass::class)
Expand Down Expand Up @@ -110,9 +113,16 @@ class GenerateInputObjectTest : TypeTestHelper() {
}

@Test
fun `Generation of output object will fail if it specifies invalid name`() {
fun `Generation of input object will fail if it specifies invalid name`() {
assertFailsWith(InvalidGraphQLNameException::class) {
generateInputObject(generator, `Invalid$InputTypeName`::class)
}
}

@Test
fun `Generation of input object will fail if it does not have public constructor`() {
assertFailsWith(PrimaryConstructorNotFound::class) {
generateInputObject(generator, MissingPublicConstructor::class)
}
}
}
5 changes: 5 additions & 0 deletions website/docs/schema-generator/writing-schemas/arguments.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ input WidgetInput {

Note that only fields are exposed in the input objects. Functions will only be available on the GraphQL output types.

:::caution
All input object fields have to be exposed through a public primary constructor. This primary constructor is used to instantiate
the input objects at runtime when resolving the queries.
:::

If you know a type will only be used for input types you can call your class something like `CustomTypeInput`. The library will not
append `Input` if the class name already ends with `Input` but that means you can not use this type as output because
the schema would have two types with the same name and that would be invalid.
Expand Down
31 changes: 10 additions & 21 deletions website/docs/schema-generator/writing-schemas/lists.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,15 @@
id: lists
title: Lists
---
Both `kotlin.Array` and `kotlin.collections.List` are automatically mapped to the GraphQL `List` type (for unsupported
use cases see below). Type arguments provided to Kotlin collections are used as the type arguments in the GraphQL `List`
type. Kotlin specialized classes (e.g. `IntArray`) representing arrays of Java primitive types without boxing overhead
are also supported.
`kotlin.collections.List` is automatically mapped to the GraphQL `List` type. Type arguments provided to Kotlin collections
are used as the type arguments in the GraphQL `List` type.

```kotlin
class SimpleQuery {
fun generateList(): List<Int> {
fun generateList(): List<String> {
// some logic here that generates list
}

fun doSomethingWithIntArray(ints: IntArray): String {
// some logic here that processes array
}

fun doSomethingWithIntList(ints: List<Int>): String {
// some logic here that processes list
}
Expand All @@ -27,24 +21,19 @@ The above Kotlin class would produce the following GraphQL schema:

```graphql
type Query {
generateList: [Int!]!
doSomethingWithIntArray(ints: [Int!]!): String!
generateList: [String!]!
doSomethingWithIntList(ints: [Int!]!): String!
}
```

## Primitive Arrays

Due to the [argument deserialization issues](https://github.com/ExpediaGroup/graphql-kotlin/pull/1379), primitive arrays
are currently not supported.

## Unsupported Collection Types
## Arrays and Unsupported Collection Types

Currently, the GraphQL spec only supports `Lists`. Therefore, even though Java and Kotlin support number of other collection
types, `graphql-kotlin-schema-generator` only explicitly supports `Lists` and primitive arrays. Other collection types
such as `Sets` (see [#201](https://github.com/ExpediaGroup/graphql-kotlin/issues/201)) and arbitrary `Map` data
structures are not supported out of the box. While we do not reccomend using `Map` or `Set` in the schema,
they are supported with the use of the schema hooks.
types, `graphql-kotlin-schema-generator` only explicitly supports `Lists`. Other collection types such as `Sets` (see [#201](https://github.com/ExpediaGroup/graphql-kotlin/issues/201))
and arbitrary `Map` data structures are not supported out of the box. While we do not recommend using `Map` or `Set` in the schema,
they could be supported with the use of the schema hooks.

Due to the [argument deserialization issues](https://github.com/ExpediaGroup/graphql-kotlin/pull/1379), arrays are currently not supported

```kotlin
override fun willResolveMonad(type: KType): KType = when (type.classifier) {
Expand Down