Client Generation - Allow schema to be on the classpath #1136

AlexRiedler · 2021-04-28T21:46:29Z

📝 Description

Simply allow loading the schema file from your classpath, this allows you to include a maven dependency that stores your graphql SDLs as a package.

<plugin>
	<groupId>com.expediagroup</groupId>
	<artifactId>graphql-kotlin-maven-plugin</artifactId>
	<version>4.0.0-SNAPSHOT</version>
	<dependencies>
		<dependency>
			<groupId>com.expedia.schemas</groupId>
			<artifactId>schemas</artifactId>
		</dependency>
	</dependencies>
	<executions>
		<execution>
			<goals>
				<goal>generate-client</goal>
			</goals>
			<configuration>
				<packageName>com.expedia.graphql.generated.foobarservice</packageName>
				<schemaFile>expedia-schemas/foobar-service/schema.graphql</schemaFile>
				<queryFileDirectory>${project.basedir}/src/main/resources/queries</queryFileDirectory>
			</configuration>
		</execution>
	</executions>
</plugin>

I would love to understand how to test this for Gradle as well since it changed, I have tested for maven only so far!

Feedback appreciated :)

🔗 Related Issues

dariuszkuc

Thanks for raising this.

dariuszkuc · 2021-04-29T20:19:07Z

...in-client-generator/src/main/kotlin/com/expediagroup/graphql/plugin/client/generateClient.kt

@@ -32,7 +37,7 @@ fun generateClient(
    allowDeprecated: Boolean = false,
    customScalarsMap: List<GraphQLScalar> = emptyList(),
    serializer: GraphQLSerializer = GraphQLSerializer.JACKSON,
-    schema: File,
+    schema: String, // this is the file location, not the contents


lets rename this field to schemaPath: String and drop the comment

dariuszkuc · 2021-04-29T20:31:12Z

...in-client-generator/src/main/kotlin/com/expediagroup/graphql/plugin/client/generateClient.kt

+
+private fun getSchemaReader(schemaPath: String): Reader =
+    if (schemaPath.startsWith("classpath://")) {
+        val resource = object {}.javaClass.getResource(URI(schemaPath).path) ?: throw RuntimeException("specified GraphQL schema not found, $schemaPath")


i think anonymous objects like this might be throwing some detekt violations -> instead lets move the logic of reading from classpath to the GraphQLClientGenerator (i.e. change its constructor from accepting type registry to a path and initialize type registry in init block)

private val graphQLSchema: TypeDefinitionRegistry init { graphQLSchema = parseSchema(schemaPath) } private fun parseSchema(path: String): TypeDefinitionRegistry { val schemaFile = File(path) return if (schemaFile.isFile) { SchemaParser().parse(schemaFile) } else { val schemaInputStream = this.javaClass.classLoader.getResourceAsStream(path) ?: throw RuntimeException("specified schema file=$path does not exist") SchemaParser().parse(schemaInputStream) } }

we'll need some unit tests around this

dariuszkuc · 2021-04-29T20:32:06Z

...in-client-generator/src/main/kotlin/com/expediagroup/graphql/plugin/client/generateClient.kt

    val generator = GraphQLClientGenerator(graphQLSchema, config)
    return generator.generate(queries)
 }
+
+private fun getSchemaReader(schemaPath: String): Reader =
+    if (schemaPath.startsWith("classpath://")) {


classpath:// is not a valid URL protocol so we should not code against this (yes Spring supports it but it is non-standard)

dariuszkuc · 2021-04-29T20:38:42Z

...n-plugin/src/main/kotlin/com/expediagroup/graphql/plugin/maven/GenerateClientAbstractMojo.kt

-        val graphQLSchemaFile = schemaFile ?: File(project.build.directory, "schema.graphql")
-        validateGraphQLSchemaExists(graphQLSchemaFile)
+
+        val schemaPath =


you can rewrite this as

Suggested change

val schemaPath =

val schemaPath = schemaFile ?: File(project.build.directory, "schema.graphql").path

which avoids the !!

dariuszkuc · 2021-04-29T20:41:00Z

...n/src/main/kotlin/com/expediagroup/graphql/plugin/gradle/tasks/AbstractGenerateClientTask.kt

@@ -66,7 +66,7 @@ abstract class AbstractGenerateClientTask : DefaultTask() {
     */
    @InputFile
    @Optional
-    val schemaFile: RegularFileProperty = project.objects.fileProperty()
+    val schemaFile: Property<String> = project.objects.property(String::class.java)


there already is schemaFileName property for that use case, this should remain a RegularFileProperty

dariuszkuc · 2021-04-29T20:41:54Z

...n/src/main/kotlin/com/expediagroup/graphql/plugin/gradle/tasks/AbstractGenerateClientTask.kt

@@ -147,13 +147,10 @@ abstract class AbstractGenerateClientTask : DefaultTask() {
        logger.debug("generating GraphQL client")

        val graphQLSchema = when {
-            schemaFile.isPresent -> schemaFile.get().asFile
-            schemaFileName.isPresent -> File(schemaFileName.get())
+            schemaFile.isPresent -> schemaFile.get() as String


lets rename the enclosing variable graphQLSchema -> graphQLSchemaPath

dariuszkuc · 2021-04-29T20:42:40Z

...rc/main/kotlin/com/expediagroup/graphql/plugin/gradle/parameters/GenerateClientParameters.kt

@@ -37,7 +37,7 @@ interface GenerateClientParameters : WorkParameters {
    /** Type of JSON serializer that will be used to generate the data classes. */
    val serializer: Property<GraphQLSerializer>
    /** GraphQL schema file that will be used to generate client code. */
-    val schemaFile: Property<File>
+    val schemaFile: Property<String>


lets also update javadoc to reflect this is a path to a schema file

Suggested change

val schemaFile: Property<String>

val schemaPath: Property<String>

dariuszkuc

Can we also add a unit test for GraphQLClientGenerator to verify it will blow up with the exception when invalid path is specified?

dariuszkuc · 2021-04-30T02:32:25Z

...in-client-generator/src/main/kotlin/com/expediagroup/graphql/plugin/client/generateClient.kt

@@ -32,7 +31,7 @@ fun generateClient(
    allowDeprecated: Boolean = false,
    customScalarsMap: List<GraphQLScalar> = emptyList(),
    serializer: GraphQLSerializer = GraphQLSerializer.JACKSON,
-    schema: File,
+    schemaPath: String, // this is the file location, not the contents


lets remove the comment as it is unnecessary

Suggested change

schemaPath: String, // this is the file location, not the contents

schemaPath: String,

dariuszkuc · 2021-04-30T02:34:14Z

...nerator/src/test/kotlin/com/expediagroup/graphql/plugin/client/generator/GraphQLTestUtils.kt

-        SchemaParser().parse(schemaFileStream)
-    }
-}
+internal fun testSchema(): String = "testSchema.graphql"


since this is a constant string lets change it to

Suggested change

internal fun testSchema(): String = "testSchema.graphql"

internal const val TEST_SCHEMA = "testSchema.graphql"

dariuszkuc · 2021-04-30T02:35:35Z

...-gradle-plugin/src/main/kotlin/com/expediagroup/graphql/plugin/gradle/GraphQLGradlePlugin.kt

@@ -104,15 +104,15 @@ class GraphQLGradlePlugin : Plugin<Project> {
                        introspectSchemaTask.headers.convention(project.provider { extension.clientExtension.headers })
                        introspectSchemaTask.timeoutConfig.convention(project.provider { extension.clientExtension.timeoutConfig })
                        generateClientTask.dependsOn(introspectSchemaTask.path)
-                        generateClientTask.schemaFile.convention(introspectSchemaTask.outputFile)
+                        generateClientTask.schemaFile.convention(introspectSchemaTask.outputFile.get())


lets revert changes in this file as we should rely on the provider

dariuszkuc · 2021-04-30T02:39:00Z

...r/src/main/kotlin/com/expediagroup/graphql/plugin/client/generator/GraphQLClientGenerator.kt

+        return if (schemaFile.isFile) {
+            SchemaParser().parse(schemaFile)
+        } else {
+            val schemaInputStream = this.javaClass.classLoader.getResourceAsStream(path) ?: throw RuntimeException("specified schema file=$path does not exist")


lets extract this exception to explicit SchemaUnavailableException (or something like that) under com.expediagroup.graphql.plugin.client.generator.exceptions

AlexRiedler · 2021-04-30T13:07:14Z

I put the test next to the parameterized IT test, hopefully that makes sense. Thanks for being so responsive, it has been a pleasure contributing this! learned a few things as well along the way.

One thing is the user no longer needs to specify classpath:// ; not sure if we should require that on the 2nd path; what do you think?

dariuszkuc · 2021-04-30T17:33:13Z

One thing is the user no longer needs to specify classpath:// ; not sure if we should require that on the 2nd path; what do you think?

Nah, current logic of just checking if it is a file and/or resource on classpath should be fine. We could technically go fancy with support for URLs (so you could point to a http resource) but that might be an overkill.

…#1136) * Client Generation - Allow schema SDL to be on the classpath * Suggestions and Refactors * Minor touchup * More improvements to the code, remove redundant edits

Client Generation - Allow schema SDL to be on the classpath

d78bf1a

dariuszkuc requested changes Apr 29, 2021

View reviewed changes

AlexRiedler added 2 commits April 29, 2021 18:05

Suggestions and Refactors

6d3db3d

Minor touchup

bd7f624

dariuszkuc requested changes Apr 30, 2021

View reviewed changes

More improvements to the code, remove redundant edits

77ab985

dariuszkuc approved these changes Apr 30, 2021

View reviewed changes

dariuszkuc merged commit edad0b4 into ExpediaGroup:master Apr 30, 2021

dariuszkuc added changes: patch Changes require a patch version module: plugin Issue affects the plugins code labels Apr 30, 2021

foo4u mentioned this pull request Jun 21, 2021

Add local schema file support to Gradle plugin #1190

Closed

dariuszkuc mentioned this pull request Jul 8, 2021

Add local schema file support to Gradle plugin extension #1212

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Client Generation - Allow schema to be on the classpath #1136

Client Generation - Allow schema to be on the classpath #1136

Uh oh!

AlexRiedler commented Apr 28, 2021 •

edited

Loading

Uh oh!

dariuszkuc left a comment

Uh oh!

dariuszkuc Apr 29, 2021

Uh oh!

dariuszkuc Apr 29, 2021

Uh oh!

dariuszkuc Apr 29, 2021

Uh oh!

dariuszkuc Apr 29, 2021

Uh oh!

dariuszkuc Apr 29, 2021

Uh oh!

dariuszkuc Apr 29, 2021

Uh oh!

dariuszkuc Apr 29, 2021

Uh oh!

dariuszkuc left a comment

Uh oh!

dariuszkuc Apr 30, 2021

Uh oh!

dariuszkuc Apr 30, 2021

Uh oh!

dariuszkuc Apr 30, 2021

Uh oh!

dariuszkuc Apr 30, 2021

Uh oh!

AlexRiedler commented Apr 30, 2021 •

edited

Loading

Uh oh!

dariuszkuc commented Apr 30, 2021

Uh oh!

Uh oh!

	val schemaPath =
	val schemaPath = schemaFile ?: File(project.build.directory, "schema.graphql").path

	val schemaFile: Property<String>
	val schemaPath: Property<String>

	schemaPath: String, // this is the file location, not the contents
	schemaPath: String,

	internal fun testSchema(): String = "testSchema.graphql"
	internal const val TEST_SCHEMA = "testSchema.graphql"

Client Generation - Allow schema to be on the classpath #1136

Client Generation - Allow schema to be on the classpath #1136

Uh oh!

Conversation

AlexRiedler commented Apr 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Description

🔗 Related Issues

Uh oh!

dariuszkuc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dariuszkuc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexRiedler commented Apr 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dariuszkuc commented Apr 30, 2021

Uh oh!

Uh oh!

AlexRiedler commented Apr 28, 2021 •

edited

Loading

AlexRiedler commented Apr 30, 2021 •

edited

Loading