jdaugherty commented on code in PR #16001: URL: https://github.com/apache/grails-core/pull/16001#discussion_r3610590322
########## grails-core/src/test/groovy/grails/aot/GrailsAotSmokeSpec.groovy: ########## @@ -0,0 +1,144 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package grails.aot + +import groovy.lang.GroovyClassLoader + +import org.springframework.aot.generate.ClassNameGenerator +import org.springframework.aot.generate.DefaultGenerationContext +import org.springframework.aot.generate.GeneratedFiles +import org.springframework.aot.generate.InMemoryGeneratedFiles +import org.springframework.context.aot.ApplicationContextAotGenerator +import org.springframework.context.support.GenericApplicationContext +import org.springframework.javapoet.ClassName + +import grails.core.DefaultGrailsApplication +import grails.core.GrailsApplication +import grails.plugins.DefaultGrailsPluginManager +import grails.plugins.GrailsPluginManager +import org.apache.grails.core.plugins.DefaultPluginDiscovery +import org.apache.grails.core.plugins.PluginDiscovery +import org.grails.spring.DefaultRuntimeSpringConfiguration +import org.spockframework.runtime.SpockAssertionError +import spock.lang.PendingFeature +import spock.lang.Specification + +class GrailsAotSmokeSpec extends Specification { + + void 'Spring AOT generates an initializer for a minimal Grails context'() { + given: 'a non-refreshed context with the minimal Grails application bean' + def context = new GenericApplicationContext() + context.registerBean(DefaultGrailsApplication) + + when: 'Spring processes the context through its public AOT API' + ClassName initializer = new ApplicationContextAotGenerator().processAheadOfTime(context, generationContext()) + + then: 'an initializer entry point is generated' + initializer != null + initializer.simpleName().contains('ApplicationContextInitializer') + + cleanup: + context.close() + } + + @PendingFeature(exceptions = [SpockAssertionError], reason = 'Blocker: Grails discovers artefact classes at runtime, and no AOT contribution currently records that runtime artefact registry as generated source.') + void 'Spring AOT records a dynamically discovered Grails artefact'() { + given: 'an artefact discovered from a runtime Groovy class loader' + def classLoader = new GroovyClassLoader() + Class<?> dynamicArtefact = classLoader.parseClass(''' + package grails.aot.dynamic + class AotDynamicController { } + ''') + def application = new DefaultGrailsApplication([dynamicArtefact] as Class<?>[], classLoader) + def context = new GenericApplicationContext() + context.beanFactory.registerSingleton(GrailsApplication.APPLICATION_ID, application) Review Comment: Similar wiring concern: the `GrailsApplication` goes in via `registerSingleton`, and Spring AOT only processes *bean definitions* — manually registered singletons are skipped entirely. So the generated source can't reference the artefact registry through any implementation, and the probe conflates two gaps: (1) singleton registration being invisible to AOT, and (2) no Grails AOT contribution for the artefact registry. Registering it as a bean definition (like the first test's `registerBean`) would isolate gap (2), which is the one the `reason` text claims to document. Also worth noting somewhere: a class parsed by a runtime `GroovyClassLoader` has no build-time bytecode, so "generated source contains the class name" is about the most a future AOT contribution could do (registry metadata), and this string match on joined sources is how the probe would flip — fine, but a comment saying so would help whoever implements it. ########## grails-core/src/test/groovy/grails/aot/GrailsAotSmokeSpec.groovy: ########## @@ -0,0 +1,144 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package grails.aot + +import groovy.lang.GroovyClassLoader + +import org.springframework.aot.generate.ClassNameGenerator +import org.springframework.aot.generate.DefaultGenerationContext +import org.springframework.aot.generate.GeneratedFiles +import org.springframework.aot.generate.InMemoryGeneratedFiles +import org.springframework.context.aot.ApplicationContextAotGenerator +import org.springframework.context.support.GenericApplicationContext +import org.springframework.javapoet.ClassName + +import grails.core.DefaultGrailsApplication +import grails.core.GrailsApplication +import grails.plugins.DefaultGrailsPluginManager +import grails.plugins.GrailsPluginManager +import org.apache.grails.core.plugins.DefaultPluginDiscovery +import org.apache.grails.core.plugins.PluginDiscovery +import org.grails.spring.DefaultRuntimeSpringConfiguration +import org.spockframework.runtime.SpockAssertionError +import spock.lang.PendingFeature +import spock.lang.Specification + +class GrailsAotSmokeSpec extends Specification { + + void 'Spring AOT generates an initializer for a minimal Grails context'() { + given: 'a non-refreshed context with the minimal Grails application bean' + def context = new GenericApplicationContext() + context.registerBean(DefaultGrailsApplication) + + when: 'Spring processes the context through its public AOT API' + ClassName initializer = new ApplicationContextAotGenerator().processAheadOfTime(context, generationContext()) + + then: 'an initializer entry point is generated' + initializer != null + initializer.simpleName().contains('ApplicationContextInitializer') + + cleanup: + context.close() + } + + @PendingFeature(exceptions = [SpockAssertionError], reason = 'Blocker: Grails discovers artefact classes at runtime, and no AOT contribution currently records that runtime artefact registry as generated source.') Review Comment: Small correction to the scoping claim in the PR description: `@PendingFeature` treats *any* `AssertionError` as pending regardless of the `exceptions` list (the list only extends handling to non-assertion throwables). So `exceptions = [SpockAssertionError]` is effectively redundant — its real effect is that non-assertion exceptions (the `IllegalStateException` guards, or `processAheadOfTime` starting to throw) hard-fail, which is the behavior you want. But an unrelated `AssertionError` from Spring internals would still be silently swallowed as pending — the "unrelated regressions still surface" guarantee only holds for exceptions, not assertion errors. ########## grails-core/src/test/groovy/grails/aot/GrailsAotSmokeSpec.groovy: ########## @@ -0,0 +1,144 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package grails.aot + +import groovy.lang.GroovyClassLoader + +import org.springframework.aot.generate.ClassNameGenerator +import org.springframework.aot.generate.DefaultGenerationContext +import org.springframework.aot.generate.GeneratedFiles +import org.springframework.aot.generate.InMemoryGeneratedFiles +import org.springframework.context.aot.ApplicationContextAotGenerator +import org.springframework.context.support.GenericApplicationContext +import org.springframework.javapoet.ClassName + +import grails.core.DefaultGrailsApplication +import grails.core.GrailsApplication +import grails.plugins.DefaultGrailsPluginManager +import grails.plugins.GrailsPluginManager +import org.apache.grails.core.plugins.DefaultPluginDiscovery +import org.apache.grails.core.plugins.PluginDiscovery +import org.grails.spring.DefaultRuntimeSpringConfiguration +import org.spockframework.runtime.SpockAssertionError +import spock.lang.PendingFeature +import spock.lang.Specification + +class GrailsAotSmokeSpec extends Specification { + + void 'Spring AOT generates an initializer for a minimal Grails context'() { + given: 'a non-refreshed context with the minimal Grails application bean' + def context = new GenericApplicationContext() + context.registerBean(DefaultGrailsApplication) + + when: 'Spring processes the context through its public AOT API' + ClassName initializer = new ApplicationContextAotGenerator().processAheadOfTime(context, generationContext()) + + then: 'an initializer entry point is generated' + initializer != null + initializer.simpleName().contains('ApplicationContextInitializer') + + cleanup: + context.close() + } + + @PendingFeature(exceptions = [SpockAssertionError], reason = 'Blocker: Grails discovers artefact classes at runtime, and no AOT contribution currently records that runtime artefact registry as generated source.') + void 'Spring AOT records a dynamically discovered Grails artefact'() { + given: 'an artefact discovered from a runtime Groovy class loader' + def classLoader = new GroovyClassLoader() + Class<?> dynamicArtefact = classLoader.parseClass(''' + package grails.aot.dynamic + class AotDynamicController { } + ''') + def application = new DefaultGrailsApplication([dynamicArtefact] as Class<?>[], classLoader) + def context = new GenericApplicationContext() + context.beanFactory.registerSingleton(GrailsApplication.APPLICATION_ID, application) + def generationContext = generationContext() + + when: 'Grails initializes its runtime artefact registry before Spring processes the context through its public AOT API' + application.initialise() + if (!application.allArtefacts.contains(dynamicArtefact)) { + throw new IllegalStateException('Grails artefact registry did not contain the dynamically loaded controller before AOT processing') + } + new ApplicationContextAotGenerator().processAheadOfTime(context, generationContext) + generationContext.writeGeneratedContent() + + then: 'the generated source preserves the runtime-discovered artefact type' + generatedSource(generationContext).contains(dynamicArtefact.name) + + cleanup: + context.close() + classLoader.close() + } + + @PendingFeature(exceptions = [SpockAssertionError], reason = 'Blocker: plugin doWithSpring closures are evaluated from runtime Groovy classes, with no AOT contribution that converts their bean definitions into build-time generated source.') + void 'Spring AOT records a dynamically loaded plugin doWithSpring bean'() { + given: 'a plugin class loaded at runtime with a Groovy bean-definition closure' + def classLoader = new GroovyClassLoader() + Class<?> dynamicPlugin = classLoader.parseClass(''' + class AotDynamicGrailsPlugin { + def version = '1.0' + def doWithSpring = { + dynamicPluginBean(Object) + } + } + ''') + def application = new DefaultGrailsApplication([] as Class<?>[], classLoader) + def context = new GenericApplicationContext() + def runtimeContext = new GenericApplicationContext() + application.mainContext = context + def discovery = new DefaultPluginDiscovery([dynamicPlugin] as Class<?>[]) + discovery.loadPluginsFromClasspath = false + discovery.init(context.environment) + def pluginManager = new DefaultGrailsPluginManager(application, discovery) + pluginManager.loadPlugins() + context.beanFactory.registerSingleton(PluginDiscovery.BEAN_NAME, discovery) + context.beanFactory.registerSingleton(GrailsPluginManager.BEAN_NAME, pluginManager) + def generationContext = generationContext() + + when: 'the runtime plugin configuration phase registers its DSL bean before Spring processes the context through its public AOT API' + def springConfiguration = new DefaultRuntimeSpringConfiguration() + pluginManager.doRuntimeConfiguration(springConfiguration) + springConfiguration.registerBeansWithContext(runtimeContext) Review Comment: This probe can never flip to passing, even with full Grails AOT support: `doRuntimeConfiguration` registers `dynamicPluginBean` into `runtimeContext`, but `processAheadOfTime` runs against `context`, which never sees the bean. No AOT contribution could preserve a bean definition that was registered into a *different* context. Conversely, if the plugin's bean definitions were registered into the processed `context` before `processAheadOfTime` (i.e. `springConfiguration.registerBeansWithContext(context)`), Spring AOT would likely convert them to generated source *today* — bean definitions are exactly what it processes. That would show the real gap is lifecycle (Grails runs `doWithSpring` during refresh, after the AOT snapshot), not Spring's inability to convert plugin DSL beans. Suggest registering into `context` and letting the probe characterize the actual behavior — if it passes, the pending reason needs rewording anyway. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
