Kotlin classes generated from apollo3 gradle plugin have cyclic dependencies

[rakshithrao98]

Summary Kotlin classes generated from apollo3 gradle plugin have cyclic dependencies.

Version 3.0.0

Description The kotlin classes generated have cyclic dependencies. We use the apollo3 kotlin GQL client to talk to another service. The GQL operation that is executed is:

query appSegmentWriteKeyInfo($appKey: String!){
    appByKey(appKey: $appKey){
        appId
        appKey
        partnerId
        segmentWriteKey
    }
}

The package structure that is generated is in the following manner:

Here, there is cyclic dependency introduced between AppSegmentWriteKeyInfoQuery class which is at the package root and the classes in adapter subpackage,

This is the packageName set for apollo plugin in build.gradle:

The sources are being generated from the gradle task: generateApolloSources

The cyclic dependencies are identified using ArchUnit. https://www.archunit.org/userguide/html/000_Index.html

The test we used to detect this is,

class ArchUnitTest {
    private val rootPackage = "io.atlassian.micros"
    private val javaClasses = ClassFileImporter()
        .withImportOption(ImportOption.Predefined.DO_NOT_INCLUDE_ARCHIVES)
        .withImportOption(ImportOption.Predefined.DO_NOT_INCLUDE_JARS)
        .withImportOption(ImportOption.Predefined.DO_NOT_INCLUDE_TESTS)
        .importPackages(rootPackage)

    @Test
    fun `no circular packages`() {
        val softly = SoftAssertions()
        softly.assertThat(javaClasses)
            .describedAs("At least one class should be found under ROOT_PACKAGE")
            .isNotEmpty
        softly.assertAll()
        slices().matching("$rootPackage.(**)")
            .should().beFreeOfCycles()
            .check(javaClasses)
    }
}

[martinbonnin]

Hi @rakshithrao98 👋

Indeed, the operation references the adapter, which itself references the operation Data. It hasn't been an issue so far though. Can you elaborate more what issues that causes besides the failing test?

[martinbonnin]

Hi @rakshithrao98 ! Any update here?

[rakshithrao98]

Hi @martinbonnin , sorry missed the earlier reply. No, there hasn't been an issue because of this and the only issue is that it introduces a cyclic dependency as reported by archunit library.

[martinbonnin]

Thanks for the feedback! Can you somehow relax the test so that it passes? If there's no other consequence, it sounds like the test is a bit too strict?

[rakshithrao98]

Yeah, that is true. We'll relax it for now. But it would be great if this packaging behaviour can be patched, considering it is not the right/best way to structure concerned classes in a cyclic way.

[martinbonnin]

Closing as won't fix for now. I'm not even sure there's a cycle.

• TestQuery depends on TestQuery_ResponseAdapter
• TestQuery_ResponseAdapter depends on TestQuery.Data

So it has more to do with namespacing and the way we split code across different files rather than actual classes cycles.

If there are other reasons to avoid this, please leave a message and I'll reopen.

[rakshithrao98]

Yeah I understand your argument. But the way these generated classes are packaged do introduce cycles if you look closely in the images I've put. This is not recommended, and is a sign of code smell.

Please leave the issue open, but as a low priority in case you are not able to take it right away

[martinbonnin]

Sounds good, I'll keep this open 👍 .

I'm not really sure how to change it though. We could put everything in one big file but that'll most likely make things harder to read. Do you have a favorite way you'd like things to be structured?