Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ private class InMemorySchemaSource(

private data class DependencySchemaSource(
val file: File,
val entryIndex: Int,
val entryName: String,
val schema: String,
)

Expand Down Expand Up @@ -274,18 +274,20 @@ class CodeGen private constructor(
}
}

jarFile.entries().asSequence().forEachIndexed { index, entry ->
jarFile.entries().asSequence().forEach { entry ->
if (!entry.isDirectory && entry.name.hasGraphQLSchemaExtension()) {
logger.info("Generating schema from {}: {}", file.name, entry.name)
val schema = jarFile.getInputStream(entry).bufferedReader().use { it.readText() }
dependencySchemaSources += DependencySchemaSource(file, index, schema)
dependencySchemaSources += DependencySchemaSource(file, entry.name, schema)
}
}
}
}

// Jars are ordered by path, as in 8.7.0. Entries within a jar are ordered by name, not by position in the
// archive: Gradle's classpath fingerprint ignores entry order, so the zip order must not affect the output.
dependencySchemaSources
.sortedWith(compareBy<DependencySchemaSource> { it.file }.thenBy { it.entryIndex })
.sortedWith(compareBy<DependencySchemaSource> { it.file }.thenBy { it.entryName })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Change makes sense for Gradle caching but with ordering by name it may break existing usages, i.e.

# a-base.graphqls
type Query { foo: Foo }
type Foo { id: String }

# z-legacy.graphqls
extend type Foo { legacy: String }

# b-current.graphqls
extend type Foo { current: String }

where if legacy came before current previously, a constructor taking positional args may switch the positions since b-current.graphqls is written first. Since this is intentional maybe worth a docs update or finding consumers that would break

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. I’ve restored the 8.7.0 behavior: codegen reads schemas in ZIP entry order, so existing field and positional constructor order stays the same. Gradle now includes that schema entry order in generateJava’s cache key, which prevents a stale cache hit when a jar is repacked in a different order. The task remains cacheable and can reuse output when the order matches. Checking the key requires scanning entry names, but a cache hit still skips code generation.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I missed that force-push failed and accidentally merged the PR without the fix - correcting it all in a follow-up #980

.mapTo(schemaSources) { InMemorySchemaSource(it.schema, "codegen") }
return schemaSources
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,146 @@ package com.netflix.graphql.dgs.codegen
import com.squareup.kotlinpoet.TypeSpec
import org.assertj.core.api.Assertions
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.io.TempDir
import java.io.File
import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.Paths
import java.util.jar.JarOutputStream
import java.util.zip.ZipEntry

class SchemaMergingTest {
private fun fooFields(config: CodeGenConfig): List<String> =
CodeGen(config)
.generate()
.javaDataTypes
.single { it.typeSpec().name() == "Foo" }
.typeSpec()
.fieldSpecs()
.map { it.name() }

private fun createJar(
file: File,
entries: List<Pair<String, String>>,
): File {
file.parentFile.mkdirs()
JarOutputStream(file.outputStream()).use { jar ->
entries.forEach { (name, content) ->
jar.putNextEntry(ZipEntry(name))
jar.write(content.toByteArray())
jar.closeEntry()
}
}
return file
}

@Test
fun `schema files are ordered by absolute path across roots`(
@TempDir tempDir: Path,
) {
val baseRoot = Files.createDirectories(tempDir.resolve("base")).toFile()
val zRoot = Files.createDirectories(tempDir.resolve("z-root")).toFile()
val aRoot = Files.createDirectories(tempDir.resolve("a-root")).toFile()
File(baseRoot, "base.graphqls").writeText("type Query { foo: Foo } type Foo { base: String }")
File(zRoot, "same.graphqls").writeText("extend type Foo { fromZ: String }")
File(aRoot, "same.graphqls").writeText("extend type Foo { fromA: String }")

// Every permutation of the configured roots yields the 8.7.0 order of the extensions: a-root, then z-root.
listOf(baseRoot, zRoot, aRoot).let { roots ->
listOf(roots, roots.reversed(), roots.drop(1) + roots.first()).forEach {
Assertions
.assertThat(fooFields(CodeGenConfig(schemaFiles = it.toSet(), packageName = "com.example")))
.containsExactly("base", "fromA", "fromZ")
}
}
}

@Test
fun `dependency jar schemas are ordered by entry name instead of zip order`(
@TempDir tempDir: Path,
) {
val schemaJar =
createJar(
tempDir.resolve("schemas.jar").toFile(),
listOf(
"c-extension.graphqls" to "extend type Foo { third: String }",
"a-base.graphqls" to "type Query { foo: Foo } type Foo { first: String }",
"b-extension.graphqls" to "extend type Foo { second: String }",
),
)
val reorderedJar =
createJar(
tempDir.resolve("reordered/schemas.jar").toFile(),
listOf(
"a-base.graphqls" to "type Query { foo: Foo } type Foo { first: String }",
"b-extension.graphqls" to "extend type Foo { second: String }",
"c-extension.graphqls" to "extend type Foo { third: String }",
),
)

listOf(schemaJar, reorderedJar).forEach {
Assertions
.assertThat(fooFields(CodeGenConfig(schemaJarFilesFromDependencies = listOf(it), packageName = "com.example")))
.containsExactly("first", "second", "third")
}
}

@Test
fun `dependency jars are ordered by path, not by input order`(
@TempDir tempDir: Path,
) {
val aJar =
createJar(
tempDir.resolve("a/lib.jar").toFile(),
listOf("schema.graphqls" to "type Query { foo: Foo } type Foo { base: String } extend type Foo { fromA: String }"),
)
val zJar = createJar(tempDir.resolve("z/lib.jar").toFile(), listOf("schema.graphqls" to "extend type Foo { fromZ: String }"))

listOf(listOf(aJar, zJar), listOf(zJar, aJar)).forEach {
Assertions
.assertThat(fooFields(CodeGenConfig(schemaJarFilesFromDependencies = it, packageName = "com.example")))
.containsExactly("base", "fromA", "fromZ")
}
}

@Test
fun `dependency type mapping precedence keeps the input jar order`(
@TempDir tempDir: Path,
) {
val schemaJar =
createJar(
tempDir.resolve("schema.jar").toFile(),
listOf(
"schema.graphqls" to "scalar BigDecimal type Query { foo: Foo } type Foo { value: BigDecimal }",
"META-INF/dgs.codegen.typemappings" to "BigDecimal=java.math.BigDecimal",
),
)
val mappingJar =
createJar(
tempDir.resolve("mapping.jar").toFile(),
listOf("META-INF/dgs.codegen.typemappings" to "BigDecimal=java.math.BigInteger"),
)

val result =
CodeGen(
CodeGenConfig(
schemaJarFilesFromDependencies = listOf(schemaJar, mappingJar),
packageName = "com.example",
),
).generate()

Assertions
.assertThat(
result.javaDataTypes
.single { it.typeSpec().name() == "Foo" }
.typeSpec()
.fieldSpecs()
.single()
.type()
.toString(),
).isEqualTo("java.math.BigDecimal")
}

@Test
fun mergeSchemasJava() {
val schemaDir = Paths.get("src/test/resources/schemas").toAbsolutePath().toFile()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,28 @@ open class GenerateJavaTask
val schemaFiles: ConfigurableFileCollection =
objectFactory.fileCollection().from(providerFactory.provider { schemaPaths })

/**
* The schema roots in the order `CodeGen` reads them. `CodeGen` sorts the files under all roots by absolute
* path, so the roots come out in path order, with `/` appended to a directory because its files continue
* with that separator. A root is taken for a file by its name, so that the order does not probe the file
* system, which would make the configuration cache entry depend on the roots' existence. `schemaFiles` fingerprints each root separately and with relative paths, so it cannot
* see this order. Two layouts with the same files but a different absolute sort order (swapped roots, a
* workspace moved to a path that sorts differently) would share a cache key and produce different output.
* `@Classpath` keeps the order and ignores the paths. Nested roots are not modeled: their files are read
* twice, interleaved by path.
*
* Only the root paths are listed, never the files under them. The files are fingerprinted when the task
* executes. A list of files would be evaluated when the configuration cache entry is stored, before an
* earlier task has produced the schema, and frozen from then on.
*/
@get:Classpath
val orderedSchemaRoots: ConfigurableFileCollection =
objectFactory.fileCollection().from(
providerFactory.provider { schemaFiles.files.sortedBy { if (it.isSchemaFileName()) it.path else it.path + "/" } },
)

private fun File.isSchemaFileName(): Boolean = name.endsWith(".graphql") || name.endsWith(".graphqls")

init {
outputs.doNotCacheIf("Generated sources contain timestamps") {
addGeneratedAnnotation &&
Expand Down Expand Up @@ -221,6 +243,15 @@ open class GenerateJavaTask
project.configurations.findByName("dgsCodegen"),
)

/**
* `CodeGen` orders dependency jars by absolute path, which `dgsCodegenClasspath` (classpath order, no paths)
* cannot see. Same reasoning as [orderedSchemaRoots]. Type mapping precedence follows classpath order and is
* already covered by `dgsCodegenClasspath`.
*/
@get:Classpath
val sortedDgsCodegenJars: ConfigurableFileCollection =
objectFactory.fileCollection().from(providerFactory.provider { dgsCodegenClasspath.files.sorted() })

@Input
val jacksonVersionOverride: ListProperty<String> =
objectFactory.listProperty(String::class.java).convention(emptyList())
Expand Down
Loading
Loading