codeconsole commented on code in PR #15994: URL: https://github.com/apache/grails-core/pull/15994#discussion_r3609152830
########## grails-cache/src/main/groovy/grails/plugin/cache/GrailsCacheAutoConfiguration.groovy: ########## @@ -0,0 +1,73 @@ +/* + * 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.plugin.cache + +import groovy.transform.CompileStatic + +import org.springframework.beans.factory.annotation.Value +import org.springframework.boot.autoconfigure.AutoConfiguration +import org.springframework.boot.autoconfigure.condition.ConditionalOnBean +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty +import org.springframework.context.annotation.Bean + +import org.grails.plugin.cache.GrailsCacheManager + +/** + * Auto-configures the cache plugin's default cache manager and key generator. Registered here + * rather than by the plugin descriptor so that a bean contributed by the application or another + * plugin — for example a cache-provider plugin's {@code grailsCacheManager} — makes the default + * back off cleanly instead of triggering a bean-definition override. + * + * <p>Gated on the {@code CachePluginConfiguration} definition contributed by the cache plugin + * descriptor's registrar (which runs before auto-configuration conditions are evaluated), so the + * auto-configuration backs off entirely when the plugin is not active — e.g. the jar is on the + * classpath but the plugin is excluded — keeping it in lockstep with the descriptor.</p> + * + * @since 8.0 + */ +@AutoConfiguration +@ConditionalOnProperty(name = 'grails.cache.enabled', matchIfMissing = true) Review Comment: Applied in 76bdd8c079. ########## grails-cache/src/main/groovy/grails/plugin/cache/GrailsCacheAutoConfiguration.groovy: ########## @@ -0,0 +1,73 @@ +/* + * 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.plugin.cache + +import groovy.transform.CompileStatic + +import org.springframework.beans.factory.annotation.Value +import org.springframework.boot.autoconfigure.AutoConfiguration +import org.springframework.boot.autoconfigure.condition.ConditionalOnBean +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty +import org.springframework.context.annotation.Bean + +import org.grails.plugin.cache.GrailsCacheManager + +/** + * Auto-configures the cache plugin's default cache manager and key generator. Registered here + * rather than by the plugin descriptor so that a bean contributed by the application or another + * plugin — for example a cache-provider plugin's {@code grailsCacheManager} — makes the default + * back off cleanly instead of triggering a bean-definition override. + * + * <p>Gated on the {@code CachePluginConfiguration} definition contributed by the cache plugin + * descriptor's registrar (which runs before auto-configuration conditions are evaluated), so the + * auto-configuration backs off entirely when the plugin is not active — e.g. the jar is on the + * classpath but the plugin is excluded — keeping it in lockstep with the descriptor.</p> + * + * @since 8.0 + */ +@AutoConfiguration +@ConditionalOnProperty(name = 'grails.cache.enabled', matchIfMissing = true) +@ConditionalOnBean(CachePluginConfiguration) +@CompileStatic +class GrailsCacheAutoConfiguration { + + @Value('${grails.cache.cacheManager:}') + String cacheManagerType + + @Bean + @ConditionalOnMissingBean(name = 'customCacheKeyGenerator') + CustomCacheKeyGenerator customCacheKeyGenerator() { + new CustomCacheKeyGenerator() + } + + @Bean + @ConditionalOnMissingBean(name = 'grailsCacheManager') + GrailsCacheManager grailsCacheManager(CachePluginConfiguration grailsCacheConfiguration) { + if (cacheManagerType == 'GrailsConcurrentLinkedMapCacheManager') { + GrailsConcurrentLinkedMapCacheManager cacheManager = new GrailsConcurrentLinkedMapCacheManager() + cacheManager.configuration = grailsCacheConfiguration + return cacheManager + } + GrailsConcurrentMapCacheManager cacheManager = new GrailsConcurrentMapCacheManager() + cacheManager.configuration = grailsCacheConfiguration + return cacheManager Review Comment: Applied in 76bdd8c079. ########## grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/ControllersAsyncGrailsPlugin.groovy: ########## @@ -29,14 +35,17 @@ import org.grails.plugins.web.async.spring.PromiseFactoryBean * @author Graeme Rocher * @since 2.0 */ +@CompileStatic class ControllersAsyncGrailsPlugin extends Plugin { def grailsVersion = '7.0.0-SNAPSHOT > *' Review Comment: Yes — the converted descriptors now require the Grails 8-only `BeanRegistrar` API, so 76bdd8c079 bumps every descriptor this PR converts to `8.0.0-SNAPSHOT > *` (including gorm-graphql's `7.1.0 > *`), matching the hibernate and undertow plugins. ########## grails-cache/src/test/groovy/grails/plugin/cache/CacheGrailsPluginSpec.groovy: ########## @@ -0,0 +1,74 @@ +/* + * 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.plugin.cache + +import org.springframework.beans.factory.BeanRegistrar +import org.springframework.beans.factory.support.BeanRegistryAdapter +import org.springframework.beans.factory.support.DefaultListableBeanFactory +import org.springframework.core.env.MapPropertySource +import org.springframework.core.env.StandardEnvironment + +import spock.lang.Specification + +class CacheGrailsPluginSpec extends Specification { + + void "beanRegistrar registers the cache infrastructure beans"() { + given: + DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory() Review Comment: Applied across all specs in this PR in 76bdd8c079. ########## grails-controllers/src/main/groovy/org/grails/plugins/web/controllers/ControllerBeanDefinitionsPostProcessor.groovy: ########## @@ -0,0 +1,94 @@ +/* + * 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 org.grails.plugins.web.controllers + +import groovy.transform.CompileStatic +import groovy.util.logging.Slf4j + +import org.springframework.beans.BeansException +import org.springframework.beans.factory.support.AbstractBeanDefinition +import org.springframework.beans.factory.support.BeanDefinitionRegistry +import org.springframework.beans.factory.support.BeanDefinitionRegistryPostProcessor +import org.springframework.beans.factory.support.GenericBeanDefinition +import org.springframework.core.Ordered +import org.springframework.core.PriorityOrdered + +import grails.core.GrailsApplication +import grails.core.GrailsClass +import grails.core.GrailsControllerClass +import org.grails.core.artefact.ControllerArtefactHandler + +/** + * Registers a bean definition for every controller artefact, replacing the registration the + * controllers plugin previously performed through the {@code doWithSpring()} bean DSL. Controller + * beans autowire by name, use the scope declared on the controller class and cannot be expressed + * through the {@link org.springframework.beans.factory.BeanRegistry} API, so the definitions are + * contributed by this post-processor instead. + * + * <p>Runs as a {@link PriorityOrdered} post-processor with highest precedence so the controller + * definitions are registered before Spring Boot's configuration-class post-processor evaluates + * auto-configuration conditions — the same visibility the {@code doWithSpring()} registration had. + * An existing definition for a controller name wins, preserving the ability of the application + * (or another plugin) to override a controller bean.</p> + * + * @since 8.0 + */ +@Slf4j +@CompileStatic +class ControllerBeanDefinitionsPostProcessor implements BeanDefinitionRegistryPostProcessor, PriorityOrdered { + + private final GrailsApplication grailsApplication + private final boolean useJsessionId + + ControllerBeanDefinitionsPostProcessor(GrailsApplication grailsApplication, boolean useJsessionId) { + this.grailsApplication = grailsApplication + this.useJsessionId = useJsessionId + } + + @Override + void postProcessBeanDefinitionRegistry(BeanDefinitionRegistry registry) throws BeansException { + for (GrailsClass controller in grailsApplication.getArtefacts(ControllerArtefactHandler.TYPE)) { + log.debug('Configuring controller {}', controller.fullName) + GrailsControllerClass controllerClass = (GrailsControllerClass) controller + if (!controllerClass.available || registry.containsBeanDefinition(controllerClass.fullName)) { + continue + } + Object lazyInit = controllerClass.hasProperty('lazyInit') ? controllerClass.getPropertyValue('lazyInit') : true + + GenericBeanDefinition definition = new GenericBeanDefinition() + definition.beanClass = controllerClass.clazz + definition.lazyInit = lazyInit as boolean + String beanScope = controllerClass.getScope() + definition.scope = beanScope + definition.autowireMode = AbstractBeanDefinition.AUTOWIRE_BY_NAME + if (beanScope == 'prototype') { + definition.dependencyCheck = AbstractBeanDefinition.DEPENDENCY_CHECK_NONE + } + if (useJsessionId) { + definition.propertyValues.addPropertyValue('useJessionId', useJsessionId) + } + registry.registerBeanDefinition(controllerClass.fullName, definition) + } + } Review Comment: Applied in 76bdd8c079, with one tweak: `resolveControllerLazyInit` keeps the `as boolean` coercion since the class is `@CompileStatic` and `getPropertyValue` returns `Object`. ########## grails-controllers/src/main/groovy/org/grails/plugins/web/controllers/ControllersGrailsPlugin.groovy: ########## @@ -51,51 +56,35 @@ class ControllersGrailsPlugin extends Plugin { def dependsOn = [core: version, i18n: version, urlMappings: version] @Override - Closure doWithSpring() { - { -> - def application = grailsApplication - def config = application.config - - boolean useJsessionId = config.getProperty(Settings.GRAILS_VIEWS_ENABLE_JSESSIONID, Boolean, false) + BeanRegistrar beanRegistrar() { + return { BeanRegistry registry, Environment environment -> + boolean useJsessionId = environment.getProperty(Settings.GRAILS_VIEWS_ENABLE_JSESSIONID, Boolean, false) if (!Boolean.parseBoolean(System.getProperty(Settings.SETTING_SKIP_BOOTSTRAP))) { - bootStrapClassRunner(BootStrapClassRunner) + registry.registerBean('bootStrapClassRunner', BootStrapClassRunner) } - tokenResponseActionResultTransformer(TokenResponseActionResultTransformer) - - exceptionHandler(GrailsExceptionResolver) { - exceptionMappings = ['java.lang.Exception': '/error'] - } + registry.registerBean('tokenResponseActionResultTransformer', TokenResponseActionResultTransformer) - "${CompositeViewResolver.BEAN_NAME}"(CompositeViewResolver) + registry.registerBean(CompositeViewResolver.BEAN_NAME, CompositeViewResolver) - for (controller in application.getArtefacts(ControllerArtefactHandler.TYPE)) { - log.debug('Configuring controller {}', controller.fullName) - if (controller.available) { - def lazyInit = controller.hasProperty('lazyInit') ? controller.getPropertyValue('lazyInit') : true - "${controller.fullName}"(controller.clazz) { bean -> - bean.lazyInit = lazyInit - def beanScope = controller.getScope() - bean.scope = beanScope - bean.autowire = 'byName' - if (beanScope == 'prototype') { - bean.beanDefinition.dependencyCheck = AbstractBeanDefinition.DEPENDENCY_CHECK_NONE - } - if (useJsessionId) { - useJessionId = useJsessionId - } - } + // Controller beans autowire by name and use per-controller scopes, which the + // BeanRegistry API cannot express — their definitions are contributed by a + // dedicated post-processor instead + registry.registerBean('controllerBeanDefinitionsPostProcessor', ControllerBeanDefinitionsPostProcessor) { BeanRegistry.Spec<ControllerBeanDefinitionsPostProcessor> spec -> + spec.infrastructure().supplier { BeanRegistry.SupplierContext context -> + new ControllerBeanDefinitionsPostProcessor(grailsApplication, useJsessionId) } } Review Comment: Applied in 76bdd8c079 — and swept the same style through all the converted descriptors so they stay consistent. ########## grails-converters/src/main/groovy/org/grails/plugins/converters/ConvertersGrailsPlugin.groovy: ########## @@ -48,22 +55,30 @@ class ConvertersGrailsPlugin extends Plugin { ] @Override - Closure doWithSpring() { - { -> - jsonErrorsMarshaller(JsonErrorsMarshaller) + BeanRegistrar beanRegistrar() { + return { BeanRegistry registry, Environment environment -> + registry.registerBean('jsonErrorsMarshaller', JsonErrorsMarshaller) - xmlErrorsMarshaller(XmlErrorsMarshaller) + registry.registerBean('xmlErrorsMarshaller', XmlErrorsMarshaller) - convertersConfigurationInitializer(ConvertersConfigurationInitializer) + registry.registerBean('convertersConfigurationInitializer', ConvertersConfigurationInitializer) - errorsXmlMarshallerRegisterer(ObjectMarshallerRegisterer) { - marshaller = { XmlErrorsMarshaller om -> } - converterClass = XML + registry.registerBean('errorsXmlMarshallerRegisterer', ObjectMarshallerRegisterer) { BeanRegistry.Spec<ObjectMarshallerRegisterer> spec -> + spec.supplier { BeanRegistry.SupplierContext context -> + ObjectMarshallerRegisterer registerer = new ObjectMarshallerRegisterer() + registerer.marshaller = context.bean('xmlErrorsMarshaller', XmlErrorsMarshaller) + registerer.converterClass = XML + return registerer + } } - errorsJsonMarshallerRegisterer(ObjectMarshallerRegisterer) { - marshaller = { JsonErrorsMarshaller om -> } - converterClass = JSON + registry.registerBean('errorsJsonMarshallerRegisterer', ObjectMarshallerRegisterer) { BeanRegistry.Spec<ObjectMarshallerRegisterer> spec -> + spec.supplier { BeanRegistry.SupplierContext context -> + ObjectMarshallerRegisterer registerer = new ObjectMarshallerRegisterer() + registerer.marshaller = context.bean('jsonErrorsMarshaller', JsonErrorsMarshaller) + registerer.converterClass = JSON + return registerer + } } Review Comment: Applied in 76bdd8c079. ########## grails-converters/src/test/groovy/org/grails/plugins/converters/ConvertersGrailsPluginSpec.groovy: ########## @@ -0,0 +1,64 @@ +/* + * 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 org.grails.plugins.converters + +import org.springframework.beans.factory.BeanRegistrar +import org.springframework.beans.factory.support.BeanRegistryAdapter +import org.springframework.beans.factory.support.DefaultListableBeanFactory +import org.springframework.core.env.StandardEnvironment + +import grails.converters.JSON +import grails.converters.XML +import org.grails.web.converters.configuration.ConvertersConfigurationInitializer +import org.grails.web.converters.configuration.ObjectMarshallerRegisterer +import org.grails.web.converters.marshaller.json.ValidationErrorsMarshaller as JsonErrorsMarshaller +import org.grails.web.converters.marshaller.xml.ValidationErrorsMarshaller as XmlErrorsMarshaller + +import spock.lang.Specification + +class ConvertersGrailsPluginSpec extends Specification { + + DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory() + + void setup() { + BeanRegistrar registrar = new ConvertersGrailsPlugin().beanRegistrar() + new BeanRegistryAdapter(beanFactory, new StandardEnvironment(), registrar.getClass()).register(registrar) + } + + void "beanRegistrar registers the converters beans"() { + expect: + beanFactory.getBeanDefinition('jsonErrorsMarshaller').beanClassName == JsonErrorsMarshaller.name + beanFactory.getBeanDefinition('xmlErrorsMarshaller').beanClassName == XmlErrorsMarshaller.name + beanFactory.getBeanDefinition('convertersConfigurationInitializer').beanClassName == ConvertersConfigurationInitializer.name + beanFactory.containsBeanDefinition('errorsXmlMarshallerRegisterer') + beanFactory.containsBeanDefinition('errorsJsonMarshallerRegisterer') Review Comment: Applied in 76bdd8c079. -- 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]
