Skip to content

Commit

Permalink
feat: improve top-level Query and Mutation type handling (#72)
Browse files Browse the repository at this point in the history
* unit test

* refactor

* pass test

* add mutation tests

* fix integration test

* refactor

* reorganize

* separate field definition functions

* pull out boolean arg

* pull out other boolean arg

* refactor

* refactor

* bun version

* set up docs workspace

* docs
  • Loading branch information
danadajian authored May 28, 2024
1 parent ff1811a commit 884884b
Show file tree
Hide file tree
Showing 10 changed files with 219 additions and 31 deletions.
13 changes: 1 addition & 12 deletions codegen.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { GraphQLKotlinCodegenConfig } from "./src/plugin";
import { CodegenConfig } from "@graphql-codegen/cli";

export default {
Expand All @@ -9,17 +8,7 @@ export default {
},
generates: {
"test/integration/Types.kt": {
plugins: [
{
"dist/plugin.cjs": {
resolverInterfaces: [
{
typeName: "Query",
},
],
} satisfies GraphQLKotlinCodegenConfig,
},
],
plugins: ["dist/plugin.cjs"],
},
},
} satisfies CodegenConfig;
129 changes: 129 additions & 0 deletions docs/docs/inheritance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
---
sidebar_position: 6
---

# Inheritance

When dealing with GraphQL schema containing [field arguments](https://graphql.com/learn/arguments/),
generating Kotlin code can be a bit tricky. This is because the generated code cannot in itself be the implementation
in a resolver.

Here's an example:

```graphql
type Query {
resolveMyType: MyType!
}

type MyType {
resolveMe(input: String!): String!
}
```

Generated Kotlin:

```kotlin
package com.types.generated

open class Query {
open fun resolveMyType(): MyType = throw NotImplementedError("Query.resolveMyType must be implemented.")
}

open class MyType {
open fun resolveMe(input: String): String = throw NotImplementedError("MyType.resolveMe must be implemented.")
}
```

Source code:

```kotlin
import com.expediagroup.graphql.server.operations.Query
import com.types.generated.MyType as MyTypeInterface
import com.types.generated.Query as QueryInterface

class MyQuery : Query, QueryInterface() {
override fun resolveMyType(): MyTypeInterface = MyType()
}

class MyType : MyTypeInterface() {
override fun resolveMe(input: String): String = "Hello world!"
}
```

As you can see, the generated code is not part of the implementation. Rather, it becomes an interface to inherit from in your implementation.
This enforces a type contract between the schema and your resolver code.

Note that GraphQL Kotlin will use the implementation classes to generate the schema, not the generated interfaces.
This means that all `@GraphQLDescription` and `@Deprecated` annotations have to be added to implementation classes
in order to be propagated to the resulting schema.

## Top Level Types

When dealing with top-level types, i.e. `Query` and `Mutation`, you can inherit from the corresponding generated class
to enforce the type contract. This is fine as long as all of your resolvers are contained in the same Query or Mutation class.

```graphql
type Query {
foo: String!
bar: String!
}
```

```kotlin
import com.expediagroup.graphql.server.operations.Query
import com.types.generated.Query as QueryInterface

class MyQuery : Query, QueryInterface() {
override fun foo(): String = "Hello"
override fun bar(): String = "World"
}
```

However, you might want to separate the implementation into multiple classes like so:

```kotlin
import com.expediagroup.graphql.server.operations.Query

class FooQuery : Query {
override fun foo(): String = "Hello"
}

class BarQuery : Query {
override fun bar(): String = "World"
}
```

If you try to inherit from the generated `Query` class, you will get an error during schema generation.

```kotlin
import com.expediagroup.graphql.server.operations.Query
import com.types.generated.Query as QueryInterface

class FooQuery : Query, QueryInterface() {
override fun foo(): String = "Hello"
}

class BarQuery : Query, QueryInterface() {
override fun bar(): String = "World"
}
```

This is because the generated `Query` class contains both `foo` and `bar` fields, which creates a conflict when inherited by multiple implementation classes.

Instead, you should inherit from the field-level generated `Query` classes like so:

```kotlin
import com.expediagroup.graphql.server.operations.Query
import com.types.generated.FooQuery as FooQueryInterface
import com.types.generated.BarQuery as BarQueryInterface

class FooQuery : Query, FooQueryInterface() {
override fun foo(): String = "Hello"
}

class BarQuery : Query, BarQueryInterface() {
override fun bar(): String = "World"
}
```

This way, schema generation can complete without conflict, and you can separate your implementation into multiple classes!
33 changes: 15 additions & 18 deletions docs/docs/recommended-usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ type Query {
}

type MyType {
field1: String!
field2: String
foo: String!
bar: String
}
```

Expand All @@ -38,8 +38,8 @@ open class Query {
}

data class MyType(
val field1: String,
val field2: String? = null
val foo: String,
val bar: String? = null
)
```

Expand All @@ -53,16 +53,15 @@ import com.types.generated.Query as QueryInterface
class MyQuery : Query, QueryInterface() {
override fun resolveMyType(input: String): MyType =
MyType(
field1 = myExpensiveCall1(),
field2 = myExpensiveCall2()
foo = myExpensiveCall1(),
bar = myExpensiveCall2()
)
}

```

The resulting source code is extremely unperformant. The `MyType` class is a data class, which means
that the `field1` and `field2` properties are both initialized when the `MyType` object is created, and
`myExpensiveCall1()` and `myExpensiveCall2()` will both be called in sequence! Even if I only query for `field1`, not
that the `foo` and `bar` properties are both initialized when the `MyType` object is created, and
`myExpensiveCall1()` and `myExpensiveCall2()` will both be called in sequence! Even if I only query for `foo`, not
only will `myExpensiveCall2()` still run, but it will also wait until `myExpensiveCall1()` is totally finished.

### Instead, use the `resolverInterfaces` config!
Expand Down Expand Up @@ -91,30 +90,28 @@ open class Query {
}

open class MyType {
open fun field1(): String = throw NotImplementedError("MyType.field1 must be implemented.")
open fun field2(): String? = throw NotImplementedError("MyType.field2 must be implemented.")
open fun foo(): String = throw NotImplementedError("MyType.foo must be implemented.")
open fun bar(): String? = throw NotImplementedError("MyType.bar must be implemented.")
}
```

Source code:

```kotlin
import com.types.generated.MyType as MyTypeInterface
import com.expediagroup.graphql.generator.annotations.GraphQLIgnore

class MyQuery : Query, QueryInterface() {
override fun resolveMyType(input: String): MyType = MyType()
}

@GraphQLIgnore
class MyType : MyTypeInterface() {
override fun field1(): String = myExpensiveCall1()
override fun field2(): String? = myExpensiveCall2()
override fun foo(): String = myExpensiveCall1()
override fun bar(): String? = myExpensiveCall2()
}
```

This code is much more performant! The `MyType` class is no longer a data class, so the `field1` and `field2` properties
can now be resolved independently of each other. If I query for only `field1`, only `myExpensiveCall1()` will be called, and
if I query for only `field2`, only `myExpensiveCall2()` will be called.
This code is much more performant! The `MyType` class is no longer a data class, so the `foo` and `bar` properties
can now be resolved independently of each other. If I query for only `foo`, only `myExpensiveCall1()` will be called, and
if I query for only `bar`, only `myExpensiveCall2()` will be called.

Check out the [related GraphQL Kotlin docs](https://opensource.expediagroup.com/graphql-kotlin/docs/schema-generator/execution/fetching-data/) for more information on this topic.
1 change: 1 addition & 0 deletions src/config/build-config-with-defaults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export function buildConfigWithDefaults(
includeDependentTypes: true,
unionGeneration: "MARKER_INTERFACE",
extraImports: ["com.expediagroup.graphql.generator.annotations.*"],
resolverInterfaces: [{ typeName: "Query" }, { typeName: "Mutation" }],
} as const satisfies GraphQLKotlinCodegenConfig;

return merge(defaultConfig, config) as GraphQLKotlinCodegenConfig &
Expand Down
2 changes: 1 addition & 1 deletion src/config/find-type-in-resolver-interfaces-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export function findTypeInResolverInterfacesConfig(
node: ObjectTypeDefinitionNode | InterfaceTypeDefinitionNode,
config: CodegenConfigWithDefaults,
) {
return config.resolverInterfaces?.findLast(
return config.resolverInterfaces.findLast(
(resolverInterface) => resolverInterface.typeName === node.name.value,
);
}
21 changes: 21 additions & 0 deletions src/definitions/object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,23 @@ export function buildObjectTypeDefinition(
? node.fields
: fieldsWithArguments;

const isTopLevelType =
node.name.value === "Query" || node.name.value === "Mutation";
if (isTopLevelType) {
const individualQueryClasses = node.fields?.map((fieldNode) => {
const className = `${titleCase(fieldNode.name.value)}${node.name.value}`;
return `${annotations}${outputRestrictionAnnotation}open class ${className}${interfaceInheritance} {
${getClassMembers({ node, fieldNodes: [fieldNode], schema, config })}
}`;
});
const consolidatedQueryClass = `${annotations}${outputRestrictionAnnotation}open class ${name}${interfaceInheritance} {
${getClassMembers({ node, fieldNodes, schema, config })}
}`;
return [consolidatedQueryClass, ...(individualQueryClasses ?? [])].join(
"\n\n",
);
}

const shouldGenerateFunctions = shouldGenerateFunctionsInClass(
node,
typeInResolverInterfacesConfig,
Expand Down Expand Up @@ -139,3 +156,7 @@ export function shouldGenerateFunctionsInClass(
node.fields?.some((fieldNode) => fieldNode.arguments?.length),
);
}

function titleCase(str: string) {
return str.charAt(0).toUpperCase() + str.slice(1);
}
2 changes: 2 additions & 0 deletions test/integration/expected.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ directive @specifiedBy(
) on SCALAR

type Query {
getStuff: String!
getStuffWithInput(input: String!): String!
testQuery1: SomeType!
testQuery2: SomeHybridType!
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { GraphQLKotlinCodegenConfig } from "../../../src/plugin";

export default {
resolverInterfaces: [{ typeName: "Mutation", classMethods: "SUSPEND" }],
} satisfies GraphQLKotlinCodegenConfig;
35 changes: 35 additions & 0 deletions test/unit/should_handle_top_level_types_properly/expected.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package com.kotlin.generated

import com.expediagroup.graphql.generator.annotations.*

@GraphQLValidObjectLocations(locations = [GraphQLValidObjectLocations.Locations.OBJECT])
open class Query {
open fun getStuff(dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String = throw NotImplementedError("Query.getStuff must be implemented.")
open fun getStuffWithInput(input: String, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String = throw NotImplementedError("Query.getStuffWithInput must be implemented.")
}

@GraphQLValidObjectLocations(locations = [GraphQLValidObjectLocations.Locations.OBJECT])
open class GetStuffQuery {
open fun getStuff(dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String = throw NotImplementedError("Query.getStuff must be implemented.")
}

@GraphQLValidObjectLocations(locations = [GraphQLValidObjectLocations.Locations.OBJECT])
open class GetStuffWithInputQuery {
open fun getStuffWithInput(input: String, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String = throw NotImplementedError("Query.getStuffWithInput must be implemented.")
}

@GraphQLValidObjectLocations(locations = [GraphQLValidObjectLocations.Locations.OBJECT])
open class Mutation {
open suspend fun mutateStuff(dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String = throw NotImplementedError("Mutation.mutateStuff must be implemented.")
open suspend fun mutateStuffWithInput(input: String, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String = throw NotImplementedError("Mutation.mutateStuffWithInput must be implemented.")
}

@GraphQLValidObjectLocations(locations = [GraphQLValidObjectLocations.Locations.OBJECT])
open class MutateStuffMutation {
open suspend fun mutateStuff(dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String = throw NotImplementedError("Mutation.mutateStuff must be implemented.")
}

@GraphQLValidObjectLocations(locations = [GraphQLValidObjectLocations.Locations.OBJECT])
open class MutateStuffWithInputMutation {
open suspend fun mutateStuffWithInput(input: String, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String = throw NotImplementedError("Mutation.mutateStuffWithInput must be implemented.")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
type Query {
getStuff: String!
getStuffWithInput(input: String!): String!
}

type Mutation {
mutateStuff: String!
mutateStuffWithInput(input: String!): String!
}

0 comments on commit 884884b

Please sign in to comment.