codeconsole commented on code in PR #15934: URL: https://github.com/apache/grails-core/pull/15934#discussion_r3546843127
########## grails-core/src/main/groovy/grails/boot/config/ApplicationClassScanner.groovy: ########## @@ -0,0 +1,85 @@ +/* + * 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.boot.config + +import groovy.transform.CompileStatic + +import grails.boot.config.tools.ClassPathScanner +import org.grails.compiler.injection.AbstractGrailsArtefactTransformer + +/** + * Discovers the classes that constitute a Grails application by scanning the classpath relative + * to an application class. This is the single implementation of the default scanning performed by + * {@link GrailsAutoConfiguration#classes()}, also used by + * {@link GrailsEarlyPluginRegistrationPostProcessor} to perform artefact discovery before Spring + * Boot auto-configuration is processed. + * + * @since 8.0 + */ +@CompileStatic +final class ApplicationClassScanner { Review Comment: Since `BeanRegistrar` is a functional interface, only a coerced closure is needed — no separate class and no real boilerplate. I've reworked the docs to lead with that form (see the reply on `beanRegistrar()` in `Plugin` above): ```groovy BeanRegistrar beanRegistrar() { { BeanRegistry registry, Environment env -> registry.registerBean('myService', MyServiceImpl) } as BeanRegistrar } ``` Returning a single `BeanRegistrar` is not a limitation — a registrar can register any number of beans, and returning one mirrors `doWithSpring` returning one closure. Kept the return type because it's Spring's native abstraction and a reusable first-class object; happy to switch to passing the `BeanRegistry` in if you'd prefer. ########## grails-core/src/main/groovy/grails/boot/config/ApplicationClassScanner.groovy: ########## @@ -0,0 +1,85 @@ +/* + * 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.boot.config + +import groovy.transform.CompileStatic + +import grails.boot.config.tools.ClassPathScanner +import org.grails.compiler.injection.AbstractGrailsArtefactTransformer + +/** + * Discovers the classes that constitute a Grails application by scanning the classpath relative + * to an application class. This is the single implementation of the default scanning performed by + * {@link GrailsAutoConfiguration#classes()}, also used by + * {@link GrailsEarlyPluginRegistrationPostProcessor} to perform artefact discovery before Spring + * Boot auto-configuration is processed. + * + * @since 8.0 + */ +@CompileStatic +final class ApplicationClassScanner { Review Comment: Done — renamed to `ApplicationArtefactScanner` (it scans the application's *artefact* classes; the single Application/main class is found by `FindMainClass`). Happy to use a different name if you'd prefer one. ########## grails-core/src/main/groovy/grails/boot/config/GrailsApplicationPostProcessor.groovy: ########## @@ -194,8 +240,11 @@ class GrailsApplicationPostProcessor implements BeanDefinitionRegistryPostProces def application = grailsApplication Holders.setGrailsApplication(application) - // first register plugin beans - pluginManager.doRuntimeConfiguration(springConfig) + if (!earlyPluginRegistrationRan) { + // first register plugin beans; when the early phase ran they were + // already drained into the registry ahead of auto-configuration + pluginManager.doRuntimeConfiguration(springConfig) Review Comment: Good catch — fixed. The `GrailsApplicationPostProcessor` fallback path (contexts not booted through `GrailsApp`, e.g. unit-test slices) now applies each enabled plugin's `beanRegistrar()` too, with the same scope/environment filtering the early phase uses, after the DSL flush so registrars win DSL name conflicts. A plugin's `beanRegistrar()` now behaves identically on both paths. ########## grails-core/src/main/groovy/grails/boot/config/GrailsEarlyPluginRegistrationPostProcessor.java: ########## @@ -0,0 +1,278 @@ +/* + * 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.boot.config; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.BeanRegistrar; +import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; +import org.springframework.beans.factory.support.BeanDefinitionRegistryPostProcessor; +import org.springframework.beans.factory.support.BeanRegistryAdapter; +import org.springframework.context.ApplicationListener; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.event.ContextRefreshedEvent; +import org.springframework.core.convert.support.ConfigurableConversionService; +import org.springframework.core.env.AbstractEnvironment; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.io.Resource; +import org.springframework.util.ClassUtils; + +import grails.core.DefaultGrailsApplication; +import grails.core.GrailsApplication; +import grails.core.GrailsApplicationClass; +import grails.plugins.DefaultGrailsPluginManager; +import grails.plugins.GrailsPlugin; +import grails.plugins.GrailsPluginManager; +import grails.util.Environment; +import grails.util.Holders; +import org.apache.grails.core.plugins.PluginDiscovery; +import org.grails.config.NavigableMap; +import org.grails.config.PropertySourcesConfig; +import org.grails.spring.DefaultRuntimeSpringConfiguration; +import org.grails.spring.RuntimeSpringConfiguration; + +/** + * Runs the plugin bean-registration phase of the Grails lifecycle <em>before</em> Spring Boot's + * auto-configuration is processed, so that beans contributed by plugins via {@code doWithSpring} + * are already present in the registry when Boot evaluates its {@code @ConditionalOnMissingBean} + * guards — auto-configured defaults then back off in favour of the plugin beans, without any + * override or removal afterwards. + * + * <p>It is added to the context programmatically (see {@link GrailsPluginLifecycleInitializer}), so its + * {@code postProcessBeanDefinitionRegistry} runs ahead of Boot's {@code ConfigurationClassPostProcessor} + * (which expands the {@code @AutoConfiguration} imports). Manually-added + * {@code BeanDefinitionRegistryPostProcessor}s always run before registry-discovered ones; Spring does + * not sort manually-added post-processors by {@code getOrder()}, so this class deliberately does not + * implement {@code PriorityOrdered}. + * + * <p>This phase builds the one true {@link GrailsApplication} and {@link GrailsPluginManager}: plugins + * are discovered via the promoted {@link PluginDiscovery} singleton and instantiated exactly once. + * Artefact discovery also happens here, mirroring + * {@code GrailsApplicationPostProcessor.performGrailsInitializationSequence()}, because core plugins + * (controllers, services, interceptors) iterate {@code grailsApplication} artefacts inside their + * {@code doWithSpring} closures. Application classes are resolved from the source classes stashed by + * {@link grails.boot.GrailsApp} (see {@link #APPLICATION_SOURCE_CLASSES_BEAN_NAME}) and scanned with the + * same logic {@link GrailsAutoConfiguration#classes()} uses; when the application was not started + * through {@code GrailsApp} the phase proceeds without application classes. + * + * <p>Once complete, the {@code grailsApplication} and {@code pluginManager} singletons are promoted to + * the bean factory together with the {@link #EARLY_REGISTRATION_COMPLETE_BEAN_NAME} marker, so + * {@link GrailsApplicationPostProcessor} reuses them instead of rebuilding and skips the already-drained + * plugin runtime configuration. + * + * @since 8.0 + */ +public class GrailsEarlyPluginRegistrationPostProcessor + implements BeanDefinitionRegistryPostProcessor, ApplicationListener<ContextRefreshedEvent> { + + /** + * Name of the {@code Class[]} singleton under which {@link grails.boot.GrailsApp} stashes the + * application source classes so this phase can perform early artefact discovery. + */ + public static final String APPLICATION_SOURCE_CLASSES_BEAN_NAME = "grailsApplicationSourceClasses"; + + /** + * Name of the marker singleton registered once this phase has completed, checked by + * {@link GrailsApplicationPostProcessor} to reuse the promoted singletons and skip the + * already-performed lifecycle steps. Always checked on the local bean factory only. + */ + public static final String EARLY_REGISTRATION_COMPLETE_BEAN_NAME = "grailsEarlyPluginRegistrationComplete"; + + private static final Logger LOG = LoggerFactory.getLogger(GrailsEarlyPluginRegistrationPostProcessor.class); + + private final ConfigurableApplicationContext applicationContext; + + public GrailsEarlyPluginRegistrationPostProcessor(ConfigurableApplicationContext applicationContext) { + this.applicationContext = applicationContext; + } + + @Override + public void postProcessBeanDefinitionRegistry(BeanDefinitionRegistry registry) throws BeansException { + // Check the LOCAL singleton only — a parent context's discovery must not cause us to re-run + // the early phase in a child context (containsBean/getBean would delegate to the parent). + Object discovery = applicationContext.getBeanFactory().getSingleton(PluginDiscovery.BEAN_NAME); + if (!(discovery instanceof PluginDiscovery pluginDiscovery)) { + // No plugin discovery promoted to this context (e.g. unit-test slice) — nothing to do. + return; + } + + Environment.setInitializing(true); Review Comment: Fixed. The early phase wraps its body in try/catch and resets `Environment.setInitializing(false)` before rethrowing, so a failed context no longer poisons later contexts in the same JVM (test forks). The success path still resets via the refresh listener. (The pre-existing `GrailsApplicationPostProcessor` has the same shape — worth the same treatment as a follow-up.) -- 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]
