codeconsole commented on code in PR #15966: URL: https://github.com/apache/grails-core/pull/15966#discussion_r3562587964
########## grails-scaffolding/src/main/groovy/grails/plugin/scaffolding/ScaffoldingViewResolverDefinitionPostProcessor.java: ########## @@ -0,0 +1,89 @@ +/* + * 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.scaffolding; + +import org.springframework.beans.BeansException; +import org.springframework.beans.MutablePropertyValues; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; +import org.springframework.beans.factory.support.BeanDefinitionRegistryPostProcessor; +import org.springframework.beans.factory.support.GenericBeanDefinition; +import org.springframework.context.EnvironmentAware; +import org.springframework.core.Ordered; +import org.springframework.core.env.Environment; + +import grails.util.Metadata; +import org.grails.plugins.web.GroovyPagesPostProcessor; + +/** + * Registers the {@code jspViewResolver} bean definition as a + * {@link ScaffoldingViewResolver} unless a definition already exists, replacing + * the registration the plugin previously performed through the + * {@code doWithSpring()} bean DSL. Registering a definition (with the same + * {@code abstractViewResolver} parent the GSP plugin's default uses) rather + * than building the resolver directly keeps the view-resolver configuration in + * one place and preserves the established post-processor pipeline. + */ +public class ScaffoldingViewResolverDefinitionPostProcessor implements BeanDefinitionRegistryPostProcessor, EnvironmentAware, Ordered { + + /** + * Runs before the SiteMesh 2 module's {@code GrailsLayoutViewResolverPostProcessor} + * ({@code GroovyPagesPostProcessor.ORDER - 1}), which embeds the definition + * registered here as its inner view resolver, and before + * {@link GroovyPagesPostProcessor} itself, which contributes the plain GSP + * resolver only when no definition exists by then. + */ + public static final int ORDER = GroovyPagesPostProcessor.ORDER - 2; + + private static final String JSP_VIEW_RESOLVER_BEAN_NAME = "jspViewResolver"; + + private Environment environment; + + @Override + public void postProcessBeanDefinitionRegistry(BeanDefinitionRegistry registry) throws BeansException { + if (registry.containsBeanDefinition(JSP_VIEW_RESOLVER_BEAN_NAME)) { + // an application- or plugin-supplied view resolver wins Review Comment: Confirmed intentional — stand-down-if-present is the contract, consistent with `GroovyPagesPostProcessor`. You're right that my "matching the old override semantics" claim was only true for application precedence, not plugin-vs-plugin: the old `doWithSpring()` registration overwrote definitions from earlier-loading plugins, and this doesn't. Fixed the inline comment in be023f8e42 to state "any existing definition wins" and to note explicitly that this is stricter than the DSL registration was. ########## grails-scaffolding/src/test/groovy/grails/plugin/scaffolding/ScaffoldingViewResolverDefinitionPostProcessorSpec.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.plugin.scaffolding + +import org.springframework.beans.factory.config.BeanDefinition +import org.springframework.beans.factory.support.DefaultListableBeanFactory +import org.springframework.beans.factory.support.GenericBeanDefinition +import org.springframework.core.env.MapPropertySource +import org.springframework.core.env.StandardEnvironment +import org.springframework.web.servlet.view.InternalResourceViewResolver + +import spock.lang.Specification + +class ScaffoldingViewResolverDefinitionPostProcessorSpec extends Specification { + + ScaffoldingViewResolverDefinitionPostProcessor postProcessor = new ScaffoldingViewResolverDefinitionPostProcessor() + + void "registers the scaffolding view resolver definition with the GSP parent template"() { + given: + DefaultListableBeanFactory registry = new DefaultListableBeanFactory() + postProcessor.environment = new StandardEnvironment() + + when: + postProcessor.postProcessBeanDefinitionRegistry(registry) + BeanDefinition definition = registry.getBeanDefinition('jspViewResolver') + + then: + definition.beanClassName == ScaffoldingViewResolver.name + definition.parentName == 'abstractViewResolver' + definition.lazyInit + definition.propertyValues.contains('enableReload') + definition.propertyValues.getPropertyValue('enableNamespaceViewDefaults').value == false + } + + void "the enableNamespaceViewDefaults property is read from the environment"() { + given: + DefaultListableBeanFactory registry = new DefaultListableBeanFactory() + StandardEnvironment environment = new StandardEnvironment() + environment.propertySources.addFirst( + new MapPropertySource('test', ['grails.scaffolding.enableNamespaceViewDefaults': 'true'])) + postProcessor.environment = environment + + when: + postProcessor.postProcessBeanDefinitionRegistry(registry) + + then: + registry.getBeanDefinition('jspViewResolver') + .propertyValues.getPropertyValue('enableNamespaceViewDefaults').value == true + } + + void "an existing jspViewResolver definition wins"() { + given: + DefaultListableBeanFactory registry = new DefaultListableBeanFactory() + registry.registerBeanDefinition('jspViewResolver', + new GenericBeanDefinition(beanClass: InternalResourceViewResolver)) + postProcessor.environment = new StandardEnvironment() + + when: + postProcessor.postProcessBeanDefinitionRegistry(registry) + + then: + registry.getBeanDefinition('jspViewResolver').beanClassName == InternalResourceViewResolver.name + } + + void "runs before the SiteMesh 2 layout post-processor and the GSP default post-processor"() { + expect: "GrailsLayoutViewResolverPostProcessor runs at -1 and GroovyPagesPostProcessor at 0" + postProcessor.order < -1 Review Comment: Done in be023f8e42 — the test now asserts against both real constants (`new GrailsLayoutViewResolverPostProcessor().order` and `GroovyPagesPostProcessor.ORDER`), with `grails-layout` added as a `testImplementation` dependency. That dep felt justified rather than merely tolerable: the -2 slot exists precisely because grails-layout's post-processor embeds this definition at -1, so the contract test naming that collaborator directly is the point. -- 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]
