codeconsole commented on code in PR #15934: URL: https://github.com/apache/grails-core/pull/15934#discussion_r3546846351
########## 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); + + DefaultGrailsApplication grailsApplication = new DefaultGrailsApplication(); + grailsApplication.setConfig(buildConfig()); + grailsApplication.setApplicationContext(applicationContext); + grailsApplication.setMainContext(applicationContext); + + DefaultGrailsPluginManager pluginManager = new DefaultGrailsPluginManager(grailsApplication, pluginDiscovery); + pluginManager.loadPlugins(); + pluginManager.setApplicationContext(applicationContext); + + pluginManager.doArtefactConfiguration(); + grailsApplication.initialise(); + // register plugin provided classes first, this gives the opportunity + // for application classes to override those provided by a plugin + pluginManager.registerProvidedArtefacts(grailsApplication); + registerApplicationArtefacts(grailsApplication, registry); + + RuntimeSpringConfiguration springConfig = new DefaultRuntimeSpringConfiguration(); + pluginManager.doRuntimeConfiguration(springConfig); + springConfig.registerBeansWithRegistry(registry); + applyBeanRegistrars(pluginManager, registry); + + ConfigurableListableBeanFactory beanFactory = applicationContext.getBeanFactory(); + beanFactory.registerSingleton(GrailsApplication.APPLICATION_ID, grailsApplication); + beanFactory.registerSingleton(GrailsPluginManager.BEAN_NAME, pluginManager); + beanFactory.registerSingleton(EARLY_REGISTRATION_COMPLETE_BEAN_NAME, Boolean.TRUE); + Holders.setGrailsApplication(grailsApplication); + + // GrailsApplicationPostProcessor resets the initializing flag on refresh, but it is not + // present in every context that runs this phase — reset here as well so the flag (a system + // property) does not leak once the context is up. + applicationContext.addApplicationListener(this); + } + + /** + * Applies the {@link BeanRegistrar} exposed by each enabled plugin through + * {@link grails.core.GrailsApplicationLifeCycle#beanRegistrar()}, in plugin order, using the + * same adapter Spring uses for {@code GenericApplicationContext.register(BeanRegistrar...)}. + * Runs after the {@code doWithSpring} drain so registrar beans win any name conflicts with the + * deprecated DSL. + */ + private void applyBeanRegistrars(DefaultGrailsPluginManager pluginManager, BeanDefinitionRegistry registry) { + String[] activeProfiles = applicationContext.getEnvironment().getActiveProfiles(); + for (GrailsPlugin plugin : pluginManager.getAllPlugins()) { + if (!plugin.supportsCurrentScopeAndEnvironment() || !plugin.isEnabled(activeProfiles)) { + continue; + } + BeanRegistrar registrar = plugin.getBeanRegistrar(); + if (registrar != null) { + new BeanRegistryAdapter(registry, applicationContext.getBeanFactory(), + applicationContext.getEnvironment(), registrar.getClass()).register(registrar); + } + } + } + + private void registerApplicationArtefacts(DefaultGrailsApplication grailsApplication, BeanDefinitionRegistry registry) { + Class<?>[] sources = resolveApplicationSourceClasses(registry); + if (sources.length == 0) { + LOG.debug("No application source classes available — proceeding without early application artefact discovery"); + return; + } + for (Class<?> source : sources) { + if (!GrailsApplicationClass.class.isAssignableFrom(source)) { + // non-application sources (plain configuration classes) never contribute artefacts + continue; + } + for (Object applicationClass : scanApplicationSource(source)) { + grailsApplication.addArtefact((Class<?>) applicationClass); + } + } + } + + /** + * Resolves the classes that constitute the application for the given source class using the + * same code path {@code GrailsApplicationPostProcessor} relies on: {@code classes()} invoked + * on a {@link GrailsAutoConfiguration} instance. This matters because the Grails compiler + * injects a {@code packageNames()} override into the application class listing every project + * package, so scanning only the application class's own package would miss artefacts living + * in other packages. The instance created here is used solely to compute the scan; the + * lifecycle bean the application interacts with is still created by Spring later. + */ + private Collection<Class> scanApplicationSource(Class<?> source) { + if (GrailsAutoConfiguration.class.isAssignableFrom(source)) { + try { + GrailsAutoConfiguration application = (GrailsAutoConfiguration) source.getDeclaredConstructor().newInstance(); + application.setApplicationContext(applicationContext); + return application.classes(); + } catch (Throwable e) { Review Comment: Both addressed: the scan catch is now `Exception | LinkageError` (no longer swallows `OutOfMemoryError`/`StackOverflowError`), and the upgrade notes call out that the application class is instantiated a second time for the early scan (a throwaway instance; the real bean is still created by Spring), so application-class constructors should stay side-effect free. ########## grails-core/src/main/groovy/grails/boot/GrailsApp.groovy: ########## @@ -142,6 +143,22 @@ class GrailsApp extends SpringApplication { } } + /** + * Stashes the application source classes as a well-known singleton so that + * {@code GrailsEarlyPluginRegistrationPostProcessor} can perform artefact discovery before + * Spring Boot auto-configuration is processed. Runs before the context initializers are + * applied, so the singleton is available by the time the early registration phase executes. + */ + @Override + protected void postProcessApplicationContext(ConfigurableApplicationContext applicationContext) { + super.postProcessApplicationContext(applicationContext) + Class<?>[] sourceClasses = getAllSources().findAll { it instanceof Class } as Class<?>[] Review Comment: Fixed both ends: `GrailsApp` now resolves `String` sources (`spring.main.sources`) to classes when stashing, and the early phase recovers a `GrailsApplicationClass` from the registry whenever the stash contains none — so a String-specified application class is still discovered even alongside `Class` sources. ########## 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); + + DefaultGrailsApplication grailsApplication = new DefaultGrailsApplication(); + grailsApplication.setConfig(buildConfig()); + grailsApplication.setApplicationContext(applicationContext); + grailsApplication.setMainContext(applicationContext); + + DefaultGrailsPluginManager pluginManager = new DefaultGrailsPluginManager(grailsApplication, pluginDiscovery); + pluginManager.loadPlugins(); + pluginManager.setApplicationContext(applicationContext); + + pluginManager.doArtefactConfiguration(); + grailsApplication.initialise(); + // register plugin provided classes first, this gives the opportunity + // for application classes to override those provided by a plugin + pluginManager.registerProvidedArtefacts(grailsApplication); + registerApplicationArtefacts(grailsApplication, registry); + + RuntimeSpringConfiguration springConfig = new DefaultRuntimeSpringConfiguration(); + pluginManager.doRuntimeConfiguration(springConfig); + springConfig.registerBeansWithRegistry(registry); + applyBeanRegistrars(pluginManager, registry); + + ConfigurableListableBeanFactory beanFactory = applicationContext.getBeanFactory(); + beanFactory.registerSingleton(GrailsApplication.APPLICATION_ID, grailsApplication); + beanFactory.registerSingleton(GrailsPluginManager.BEAN_NAME, pluginManager); + beanFactory.registerSingleton(EARLY_REGISTRATION_COMPLETE_BEAN_NAME, Boolean.TRUE); Review Comment: The `Class[]` source-classes stash is now `destroySingleton`-ed once consumed, so it no longer lingers as an autowire-by-type candidate. The completion marker stays because `GrailsApplicationPostProcessor` reads it later (in its constructor) to decide whether to reuse the promoted singletons. ########## grails-core/src/main/resources/META-INF/spring.factories: ########## @@ -22,4 +22,5 @@ org.springframework.boot.env.PropertySourceLoader=\ org.grails.config.yaml.YamlPropertySourceLoader org.springframework.boot.EnvironmentPostProcessor=grails.boot.config.GrailsEnvironmentPostProcessor org.springframework.boot.SpringApplicationRunListener=grails.config.external.ExternalConfigRunListener -org.springframework.boot.bootstrap.BootstrapRegistryInitializer=org.apache.grails.core.GrailsBootstrapRegistryInitializer \ No newline at end of file +org.springframework.boot.bootstrap.BootstrapRegistryInitializer=org.apache.grails.core.GrailsBootstrapRegistryInitializer Review Comment: Fixed. ########## grails-core/src/main/groovy/grails/plugins/Plugin.groovy: ########## @@ -105,10 +106,29 @@ abstract class Plugin implements GrailsApplicationLifeCycle, GrailsApplicationAw * Sub classes should override to provide implementations * * @return A closure that defines beans to be executed by Spring + * @deprecated since 8.0 in favour of {@link #beanRegistrar()}. The bean builder DSL continues + * to work, but {@link #beanRegistrar()} is the modern, Spring-native replacement. */ + @Deprecated(since = '8.0') @Override Closure doWithSpring() { null } + /** + * Sub classes should override to register beans with the Spring Framework + * {@link org.springframework.beans.factory.BeanRegistry} using a + * {@link org.springframework.beans.factory.BeanRegistrar}. This is the modern, Spring-native + * replacement for the {@link #doWithSpring()} bean builder DSL. + * + * <p>The returned registrar is applied before Spring Boot auto-configuration is processed, so + * beans registered here take precedence over Boot's {@code @ConditionalOnMissingBean} defaults.</p> + * + * @return A {@link org.springframework.beans.factory.BeanRegistrar} that registers beans, + * or {@code null} if none (the default) + * @since 8.0 + */ + @Override + BeanRegistrar beanRegistrar() { null } Review Comment: Kept `beanRegistrar()` returning `BeanRegistrar` deliberately: it's Spring's own registration abstraction (so plugin authors learn a concept that transfers to plain Spring), the returned registrar is a first-class object (unit-testable, reusable, composable with Spring's `@Import`/AOT machinery), and it mirrors `doWithSpring` returning a value. You're right that the separate-class form is boilerplate, though — and `BeanRegistrar` is a functional interface, so a coerced closure is all that's needed. I've reworked the docs to lead with that concise form: ```groovy BeanRegistrar beanRegistrar() { { BeanRegistry registry, Environment env -> registry.registerBean('myService', MyServiceImpl) } as BeanRegistrar } ``` and added a test proving it compiles and works — that's essentially the closure you were expecting. If you'd still rather the method receive the `BeanRegistry` directly (`void doWithBeanRegistry(registry, env)`), I'm happy to switch it — I just held off changing the public signature unilaterally. ########## grails-core/src/main/groovy/grails/core/DefaultGrailsApplication.java: ########## @@ -219,6 +219,18 @@ public GrailsApplicationClass getApplicationClass() { return applicationClass; } + /** + * Sets the application class. Used to adopt the application class when this instance was + * constructed before the {@link GrailsApplicationClass} was available, e.g. during early + * plugin registration ahead of Spring Boot auto-configuration. + * + * @param applicationClass The application class + * @since 8.0 + */ + public void setApplicationClass(GrailsApplicationClass applicationClass) { Review Comment: Guarded as set-once: it throws `IllegalStateException` on reassignment to a different class (idempotent for the same value). It's deliberately *not* guarded on `initialised` — the reuse path legitimately adopts the class after the early phase has already built and initialised the application without it, so a "not after initialise" guard would break that. Set-once prevents the real risk (silent reassignment). Test added. ########## grails-core/src/main/groovy/grails/plugins/Plugin.groovy: ########## @@ -105,10 +106,29 @@ abstract class Plugin implements GrailsApplicationLifeCycle, GrailsApplicationAw * Sub classes should override to provide implementations * * @return A closure that defines beans to be executed by Spring + * @deprecated since 8.0 in favour of {@link #beanRegistrar()}. The bean builder DSL continues Review Comment: Strengthened on both `GrailsApplicationLifeCycle.doWithSpring` and `Plugin.doWithSpring`: the DSL remains available but is no longer actively supported and won't receive fixes for new issues; users are strongly urged to migrate. ########## grails-core/src/main/groovy/grails/boot/config/GrailsApplicationPostProcessor.groovy: ########## @@ -122,11 +148,31 @@ class GrailsApplicationPostProcessor implements BeanDefinitionRegistryPostProces Environment.setInitializing(true) grailsApplication.applicationContext = applicationContext grailsApplication.mainContext = applicationContext - pluginManager.loadPlugins() - pluginManager.applicationContext = applicationContext + if (!earlyPluginRegistrationRan) { + pluginManager.loadPlugins() + pluginManager.applicationContext = applicationContext + } loadApplicationConfig() customizeGrailsApplication(grailsApplication) - performGrailsInitializationSequence() + if (earlyPluginRegistrationRan) { + registerRemainingApplicationClasses() + } + else { + performGrailsInitializationSequence() + } + } + + /** + * When the early plugin registration phase already performed artefact discovery, only the + * application classes it could not resolve (e.g. a customized {@code classes()} implementation) + * still need to be registered. + */ + private void registerRemainingApplicationClasses() { + for (cls in classes) { + if (!grailsApplication.isArtefact(cls)) { + grailsApplication.addArtefact(cls) + } + } } Review Comment: Applied your suggestion — builds a `Set` of registered artefact names once, then a `contains` check, so it's linear again. ########## grails-doc/src/en/guide/plugins/hookingIntoRuntimeConfiguration.adoc: ########## @@ -23,7 +23,52 @@ Grails provides a number of hooks to leverage the different parts of the system ==== Hooking into the Grails Spring configuration -First, you can hook in Grails runtime configuration overriding the `doWithSpring` method from the link:{api}grails/plugins/Plugin.html[Plugin] class and returning a closure that defines additional beans. For example the following snippet is from one of the core Grails plugins that provides link:i18n.html[i18n] support: +The recommended way to register beans from a plugin is to override the `beanRegistrar` method from the link:{api}grails/plugins/Plugin.html[Plugin] class and return a Spring Framework {springapi}org/springframework/beans/factory/BeanRegistrar.html[BeanRegistrar]: Review Comment: Added a "Plugin Beans Register Before Spring Boot Auto-Configuration" section to the what's-new guide covering the retimed lifecycle and the `beanRegistrar()` API. ########## grails-doc/src/en/guide/plugins/hookingIntoRuntimeConfiguration.adoc: ########## @@ -23,7 +23,52 @@ Grails provides a number of hooks to leverage the different parts of the system ==== Hooking into the Grails Spring configuration -First, you can hook in Grails runtime configuration overriding the `doWithSpring` method from the link:{api}grails/plugins/Plugin.html[Plugin] class and returning a closure that defines additional beans. For example the following snippet is from one of the core Grails plugins that provides link:i18n.html[i18n] support: +The recommended way to register beans from a plugin is to override the `beanRegistrar` method from the link:{api}grails/plugins/Plugin.html[Plugin] class and return a Spring Framework {springapi}org/springframework/beans/factory/BeanRegistrar.html[BeanRegistrar]: + +[source,groovy] +---- +import org.springframework.beans.factory.BeanRegistrar +import org.springframework.beans.factory.BeanRegistry +import org.springframework.core.env.Environment +import org.springframework.web.servlet.i18n.CookieLocaleResolver +import org.springframework.web.servlet.i18n.LocaleChangeInterceptor +import org.springframework.context.support.ReloadableResourceBundleMessageSource +import grails.plugins.Plugin + +class I18nGrailsPlugin extends Plugin { + + def version = "0.1" + + @Override + BeanRegistrar beanRegistrar() { + new I18nBeanRegistrar() 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. -- 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]
