search

spring-projects / spring-modulith

Modular applications with Spring Boot
https://spring.io/projects/spring-modulith
Apache License 2.0
789 stars 134 forks source link

Modulith together with Gradle 'org.graalvm.buildtools.native' plugin causes strange problems #401

Closed dragetd closed 10 months ago

dragetd commented 10 months ago

I have encountered a strange bug that made the modules.verify() always fail is there is a Spring component in an internal package, even if it is not used by another module, even for very simple Spring Boot applications.

My main development language is Kotlin with Gradle + Kotlin DSL. But I believe this also happens in Java. The issue seems to be with Gradle… maybe with the org.graalvm.buildtools.native plugin for gradle.

When using this plugin AND an application contextLoads() test, the modules.verify() test tails. Either removing the contextLoads() test or removing the native Plugin makes the test pass.

I struggle to understand how this happens, as I thought the plugin would only do thing when doing native builds. I tried the same setup with Maven + Java and could not reproduce this bug.

Here is a minimal example project where you can test this behavior: https://github.com/dragetd/temp_modulith_graal_bug/tree/main

odrotbohm commented 10 months ago

This seems to be caused by the compilation of the AOT generated code not seeing dependencies of scope runtime. spring-modulith-starter-core pulls in spring-modulith-core in runtime scope and some code generated in the AOT phase refers to types of that JAR. I'll have to investigate with our Gradle experts to find out how that can be fixed.

wilkinsona commented 10 months ago

Running ./gradlew build against 23f9ed6 results in the AOT-generated main code failing to compile:

> Task :compileAotJava FAILED
/Users/awilkinson/dev/temp/temp_modulith_graal_bug/build/generated/aotSources/org/springframework/modulith/core/config/ApplicationModuleInitializerRuntimeVerification__BeanDefinitions.java:14: error: cannot find symbol
    RootBeanDefinition beanDefinition = new RootBeanDefinition(ApplicationModuleInitializerRuntimeVerification.class);
                                                               ^
  symbol:   class ApplicationModuleInitializerRuntimeVerification
  location: class ApplicationModuleInitializerRuntimeVerification__BeanDefinitions
/Users/awilkinson/dev/temp/temp_modulith_graal_bug/build/generated/aotSources/org/springframework/modulith/core/config/ApplicationModuleInitializerRuntimeVerification__BeanDefinitions.java:15: error: cannot find symbol
    beanDefinition.setInstanceSupplier(ApplicationModuleInitializerRuntimeVerification::new);
                                       ^
  symbol:   class ApplicationModuleInitializerRuntimeVerification
  location: class ApplicationModuleInitializerRuntimeVerification__BeanDefinitions
2 errors

Looking at the aotCompileClasspath, we can see that, as Ollie described above, spring-modulith-core is missing:

aotCompileClasspath - Compile classpath for null/aot.
+--- org.jetbrains.kotlin:kotlin-stdlib:1.9.20
|    \--- org.jetbrains:annotations:13.0
\--- org.springframework.modulith:spring-modulith-starter-core -> 1.1.0
     +--- org.springframework.modulith:spring-modulith-api:1.1.0
     +--- org.springframework.modulith:spring-modulith-moments:1.1.0
     |    \--- org.jmolecules:jmolecules-events:1.9.0
     \--- org.springframework.boot:spring-boot-starter:3.2.0
          +--- org.springframework.boot:spring-boot:3.2.0
          |    +--- org.springframework:spring-core:6.1.1
          |    |    \--- org.springframework:spring-jcl:6.1.1
          |    \--- org.springframework:spring-context:6.1.1
          |         +--- org.springframework:spring-aop:6.1.1
          |         |    +--- org.springframework:spring-beans:6.1.1
          |         |    |    \--- org.springframework:spring-core:6.1.1 (*)
          |         |    \--- org.springframework:spring-core:6.1.1 (*)
          |         +--- org.springframework:spring-beans:6.1.1 (*)
          |         +--- org.springframework:spring-core:6.1.1 (*)
          |         +--- org.springframework:spring-expression:6.1.1
          |         |    \--- org.springframework:spring-core:6.1.1 (*)
          |         \--- io.micrometer:micrometer-observation:1.12.0
          |              \--- io.micrometer:micrometer-commons:1.12.0
          +--- org.springframework.boot:spring-boot-autoconfigure:3.2.0
          |    \--- org.springframework.boot:spring-boot:3.2.0 (*)
          +--- org.springframework.boot:spring-boot-starter-logging:3.2.0
          |    +--- ch.qos.logback:logback-classic:1.4.11
          |    |    +--- ch.qos.logback:logback-core:1.4.11
          |    |    \--- org.slf4j:slf4j-api:2.0.7 -> 2.0.9
          |    +--- org.apache.logging.log4j:log4j-to-slf4j:2.21.1
          |    |    +--- org.apache.logging.log4j:log4j-api:2.21.1
          |    |    \--- org.slf4j:slf4j-api:1.7.36 -> 2.0.9
          |    \--- org.slf4j:jul-to-slf4j:2.0.9
          |         \--- org.slf4j:slf4j-api:2.0.9
          +--- jakarta.annotation:jakarta.annotation-api:2.1.1
          +--- org.springframework:spring-core:6.1.1 (*)
          \--- org.yaml:snakeyaml:2.2

Converting the sample to Java fixes this compilation problem and we see a test failure instead:

org.springframework.modulith.core.Violations: - Module 'root:com.example.modulithgraalbug' depends on non-exposed type com.example.modulithgraalbug.somemodule.internal.SomeThing__TestContext001_BeanDefinitions within module 'somemodule'!
Method <com.example.modulithgraalbug.ApplicationTests__TestContext001_BeanFactoryRegistrations.registerBeanDefinitions(org.springframework.beans.factory.support.DefaultListableBeanFactory)> calls method <com.example.modulithgraalbug.somemodule.internal.SomeThing__TestContext001_BeanDefinitions.getSomeThingBeanDefinition()> in (ApplicationTests__TestContext001_BeanFactoryRegistrations.java:56)
    at app//org.springframework.modulith.core.Violations.and(Violations.java:128)
    at java.base@17.0.9/java.util.stream.ReduceOps$1ReducingSink.accept(ReduceOps.java:80)
    at java.base@17.0.9/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
    at java.base@17.0.9/java.util.HashMap$ValueSpliterator.forEachRemaining(HashMap.java:1779)
    at java.base@17.0.9/java.util.stream.Streams$ConcatSpliterator.forEachRemaining(Streams.java:735)
    at java.base@17.0.9/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
    at java.base@17.0.9/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
    at java.base@17.0.9/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
    at java.base@17.0.9/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.base@17.0.9/java.util.stream.ReferencePipeline.reduce(ReferencePipeline.java:657)
    at app//org.springframework.modulith.core.ApplicationModules.detectViolations(ApplicationModules.java:400)
    at app//org.springframework.modulith.core.ApplicationModules.verify(ApplicationModules.java:365)
    at app//com.example.modulithgraalbug.ModularityTests.verifiesModularStructure(ModularityTests.java:12)
    at java.base@17.0.9/java.lang.reflect.Method.invoke(Method.java:568)
    at java.base@17.0.9/java.util.ArrayList.forEach(ArrayList.java:1511)
    at java.base@17.0.9/java.util.ArrayList.forEach(ArrayList.java:1511)

The compilation failure has been fixed because spring-modulith-core is now on the AOT compilation classpath:

aotCompileClasspath - Compile classpath for source set 'aot'.
\--- org.springframework.modulith:spring-modulith-starter-core -> 1.1.0
     +--- org.springframework.modulith:spring-modulith-api:1.1.0
     +--- org.springframework.modulith:spring-modulith-core:1.1.0
     |    +--- org.springframework.modulith:spring-modulith-api:1.1.0
     |    +--- com.tngtech.archunit:archunit:1.1.0
     |    |    \--- org.slf4j:slf4j-api:2.0.7 -> 2.0.9
     |    \--- org.springframework:spring-core:6.1.1
     |         \--- org.springframework:spring-jcl:6.1.1
     +--- org.springframework.modulith:spring-modulith-moments:1.1.0
     |    \--- org.jmolecules:jmolecules-events:1.9.0
     \--- org.springframework.boot:spring-boot-starter:3.2.0
          +--- org.springframework.boot:spring-boot:3.2.0
          |    +--- org.springframework:spring-core:6.1.1 (*)
          |    \--- org.springframework:spring-context:6.1.1
          |         +--- org.springframework:spring-aop:6.1.1
          |         |    +--- org.springframework:spring-beans:6.1.1
          |         |    |    \--- org.springframework:spring-core:6.1.1 (*)
          |         |    \--- org.springframework:spring-core:6.1.1 (*)
          |         +--- org.springframework:spring-beans:6.1.1 (*)
          |         +--- org.springframework:spring-core:6.1.1 (*)
          |         +--- org.springframework:spring-expression:6.1.1
          |         |    \--- org.springframework:spring-core:6.1.1 (*)
          |         \--- io.micrometer:micrometer-observation:1.12.0
          |              \--- io.micrometer:micrometer-commons:1.12.0
          +--- org.springframework.boot:spring-boot-autoconfigure:3.2.0
          |    \--- org.springframework.boot:spring-boot:3.2.0 (*)
          +--- org.springframework.boot:spring-boot-starter-logging:3.2.0
          |    +--- ch.qos.logback:logback-classic:1.4.11
          |    |    +--- ch.qos.logback:logback-core:1.4.11
          |    |    \--- org.slf4j:slf4j-api:2.0.7 -> 2.0.9
          |    +--- org.apache.logging.log4j:log4j-to-slf4j:2.21.1
          |    |    +--- org.apache.logging.log4j:log4j-api:2.21.1
          |    |    \--- org.slf4j:slf4j-api:1.7.36 -> 2.0.9
          |    \--- org.slf4j:jul-to-slf4j:2.0.9
          |         \--- org.slf4j:slf4j-api:2.0.9
          +--- jakarta.annotation:jakarta.annotation-api:2.1.1
          +--- org.springframework:spring-core:6.1.1 (*)
          \--- org.yaml:snakeyaml:2.2

If the Kotlin plugins are then applied again, but the code is left as Java, the compilation problem returns because org.springframework.modulith:spring-modulith-core is missing once again. I have pushed the commits that I used for this experimentation to my fork of the original sample.

I think the compilation problem is a bug in Kotlin's Gradle plugins that will need to be fixed by JetBrains.

odrotbohm commented 10 months ago

I filed a ticket with the Kotlin team.

odrotbohm commented 10 months ago

Closing this as there's nothing we can do about this here.

wilkinsona commented 10 months ago

Is there something to be done for the test failure that occurs with Java? The AOT processing seems to "corrupt" the state of the module such that the constraints are violated:

org.springframework.modulith.core.Violations: - Module 'root:com.example.modulithgraalbug' depends on non-exposed type com.example.modulithgraalbug.somemodule.internal.SomeThing__TestContext001_BeanDefinitions within module 'somemodule'!
Method <com.example.modulithgraalbug.ApplicationTests__TestContext001_BeanFactoryRegistrations.registerBeanDefinitions(org.springframework.beans.factory.support.DefaultListableBeanFactory)> calls method <com.example.modulithgraalbug.somemodule.internal.SomeThing__TestContext001_BeanDefinitions.getSomeThingBeanDefinition()> in (ApplicationTests__TestContext001_BeanFactoryRegistrations.java:56)
    at app//org.springframework.modulith.core.Violations.and(Violations.java:128)
    at java.base@17.0.9/java.util.stream.ReduceOps$1ReducingSink.accept(ReduceOps.java:80)
    at java.base@17.0.9/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
    at java.base@17.0.9/java.util.HashMap$ValueSpliterator.forEachRemaining(HashMap.java:1779)
    at java.base@17.0.9/java.util.stream.Streams$ConcatSpliterator.forEachRemaining(Streams.java:735)
    at java.base@17.0.9/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
    at java.base@17.0.9/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
    at java.base@17.0.9/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
    at java.base@17.0.9/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.base@17.0.9/java.util.stream.ReferencePipeline.reduce(ReferencePipeline.java:657)
    at app//org.springframework.modulith.core.ApplicationModules.detectViolations(ApplicationModules.java:400)
    at app//org.springframework.modulith.core.ApplicationModules.verify(ApplicationModules.java:365)
    at app//com.example.modulithgraalbug.ModularityTests.verifiesModularStructure(ModularityTests.java:12)
    at java.base@17.0.9/java.lang.reflect.Method.invoke(Method.java:568)
    at java.base@17.0.9/java.util.ArrayList.forEach(ArrayList.java:1511)
    at java.base@17.0.9/java.util.ArrayList.forEach(ArrayList.java:1511)
odrotbohm commented 10 months ago

In fact, there is. I didn't see that originally, as I assumed it'd be an intended violation. I think the culprit here is that the generated …__BeanFactoryRegistrations is located in the project root and Spring Modulith 1.1 tightened the verification to disallow code residing there to reach into the actual application modules' internals.

With manually written code this usually works well or at least can be worked around by configuration code in the application root package referring to an interface, gathering all beans of that type and the implementations hidden in the modules then implementing that interface.

I wonder if it makes sense to exclude the AOT generated files from the analysis. It looks like they all feature a double underscore in their classnames. Until we find a better means of identifying them, it might be a good enough way of detecting them? Do you think we could annotate these classes with @javax.annotation.processing.Generated?

A quick spike adding the ignore of types containing a __ makes the sample project build fine again (post the tweaks necessary to get the compiler work, of course).

wilkinsona commented 10 months ago

I wonder if it makes sense to exclude the AOT generated files from the analysis

Yes, I think it does.

It looks like they all feature a double underscore in their classnames. Until we find a better means of identifying them, it might be a good enough way of detecting them?

Sounds reasonable to me given what's currently available.

Do you think we could annotate these classes with @javax.annotation.processing.Generated?

WDYT, @snicoll? There's also jakarta.annotation.Generated which, judging by the package name alone, is perhaps less tied to annotation processing but may not always be on the classpath.

snicoll commented 10 months ago

Alright, with this being the second case for us to do this, I've triaged the existing issue and put a higher priority on it, see https://github.com/spring-projects/spring-framework/issues/30824

At face value, I am not a huge fan of using any of those annotations but we can discuss the pros and cons of a custom annotation of ours in the issue. Thanks!