codeconsole commented on code in PR #15964: URL: https://github.com/apache/grails-core/pull/15964#discussion_r3562569205
########## grails-gsp/grails-sitemesh3/src/main/java/org/grails/plugins/sitemesh3/Sitemesh3EnvironmentPostProcessor.java: ########## @@ -0,0 +1,92 @@ +/* + * 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.sitemesh3; + +import java.util.LinkedHashMap; +import java.util.Map; + +import org.springframework.boot.SpringApplication; +import org.springframework.boot.env.EnvironmentPostProcessor; +import org.springframework.core.Ordered; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.MapPropertySource; + +import org.grails.web.util.WebUtils; + +/** + * Contributes the Grails defaults for the SiteMesh 3 configuration keys — + * layout selection via the {@code layout} meta tag, the + * {@code /layouts/} decorator prefix and, when configured, the application's + * default layout — before the application context refreshes. + * + * <p>Because these defaults are in the {@link ConfigurableEnvironment} from the + * start, both the SiteMesh starter's {@code @Value} placeholders and the Grails + * configuration (which is built from the environment) observe them without any + * post-hoc reassignment. This replaces the property-source manipulation the + * plugin previously performed in {@code doWithSpring()}.</p> + * + * <p>Each default is contributed only when the application has not set the key + * itself, and the source is appended with lowest precedence, so application + * configuration always wins.</p> + */ +public class Sitemesh3EnvironmentPostProcessor implements EnvironmentPostProcessor, Ordered { Review Comment: Deliberate: this module keeps its Spring infrastructure in Java (`CaptureAwareContentProcessor`, `Sitemesh3LayoutFinder`, `GrailsSiteMeshViewResolver` are all Java), these classes have no Groovy idiom to benefit from, and Java puts them under checkstyle. As a bonus it avoids the `grails.util.Environment` / Spring `Environment` name clash that forces fully-qualified references in Groovy. ########## grails-gsp/grails-sitemesh3/src/main/groovy/org/grails/plugins/sitemesh3/Sitemesh3GrailsPlugin.groovy: ########## @@ -49,39 +66,10 @@ class Sitemesh3GrailsPlugin extends Plugin { Sitemesh3LayoutTagLib, ] - static PropertySource getDefaultPropertySource(ConfigurableEnvironment configurableEnvironment, String defaultLayout) { - Map props = [ - 'sitemesh.decorator.metaTag': 'layout', - 'sitemesh.decorator.attribute': WebUtils.LAYOUT_ATTRIBUTE, - 'sitemesh.decorator.prefix': '/layouts/', - ] - if (defaultLayout) { - props['sitemesh.decorator.default'] = defaultLayout - } - props.clone().each { - if (configurableEnvironment.getProperty(it.key)) { - props.remove(it.key) - } - } - new MapPropertySource('defaultSitemesh3Properties', props) - } - - Closure doWithSpring() { - { -> - ConfigurableEnvironment configurableEnvironment = grailsApplication.mainContext.environment as ConfigurableEnvironment - def propertySources = configurableEnvironment.getPropertySources() - // The SiteMesh 3 specific key wins; fall back to the SiteMesh 2 - // plugin's grails.views.layout.default so existing apps keep - // their configured default layout when switching. - String defaultLayout = grailsApplication.getConfig().getProperty('grails.sitemesh.default.layout') ?: - grailsApplication.getConfig().getProperty('grails.views.layout.default') - propertySources.addFirst(getDefaultPropertySource(configurableEnvironment, defaultLayout)) - (grailsApplication as DefaultGrailsApplication).config = new PropertySourcesConfig(propertySources) - - // Unwraps the SiteMesh view for "render template:" partials so - // they are never decorated with a layout (the SiteMesh 2 plugin - // does the same with its GrailsLayoutRenderViewMutator). - grailsRenderViewMutator(Sitemesh3RenderViewMutator) + @Override + BeanRegistrar beanRegistrar() { + return { BeanRegistry registry, Environment environment -> + registry.registerBean('grailsRenderViewMutator', Sitemesh3RenderViewMutator) Review Comment: Agreed — the coexistence framing was wrong, and as you noted the stand-down promise was already broken by registrar ordering. Fixed in ba2c2ec2: all SiteMesh 2 accommodation logic is removed (including the `ApplicationListener` exclusions in both post-processors, which only existed to detect the SM2 resolver), the javadoc now states the modules are mutually exclusive, and `Sitemesh3EnvironmentPostProcessor` fails startup with a clear message when grails-layout is detected on the classpath. The `grails.views.layout.default` fallback stays — honoring existing config is what makes the replacement drop-in. ########## grails-gsp/grails-sitemesh3/src/main/java/org/grails/plugins/sitemesh3/Sitemesh3ViewResolverDefinitionPostProcessor.java: ########## @@ -0,0 +1,142 @@ +/* + * 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.sitemesh3; + +import org.sitemesh.webmvc.SiteMeshViewResolver; +import org.sitemesh.webmvc.SiteMeshViewResolverPostProcessor; + +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.annotation.AnnotatedBeanDefinition; +import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.beans.factory.config.ConstructorArgumentValues; +import org.springframework.beans.factory.config.RuntimeBeanReference; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; +import org.springframework.beans.factory.support.GenericBeanDefinition; +import org.springframework.context.ApplicationListener; +import org.springframework.core.type.MethodMetadata; +import org.springframework.util.ClassUtils; + +import org.grails.plugins.web.GroovyPagesPostProcessor; + +/** + * Grails-flavoured {@link SiteMeshViewResolverPostProcessor} — the upstream + * bean-definition wrap mode ({@code sitemesh.viewResolver.wrapMode=bean-definition}) + * expressed with Grails semantics. It rewrites the {@code jspViewResolver} bean + * definition into a {@link GrailsSiteMeshViewResolver} definition, so that every + * instantiation of the bean — however early — yields the decorating resolver. + * + * <p>Wrapping at the bean-definition level (rather than post-processing the bean + * instance) closes an initialization-order race: {@code jspViewResolver} is + * registered lazy, so it is instantiated by whichever component first asks for + * it. If that consumer is initialized before the SiteMesh + * {@code BeanPostProcessor} takes effect — Spring Boot's + * {@code ContentNegotiatingViewResolver} collecting every {@code ViewResolver} + * while it initializes is one such consumer — it captures the raw, undecorating + * resolver and keeps rendering through it, silently disabling layouts. This + * mirrors the approach the SiteMesh 2 module takes with its + * {@code GrailsLayoutViewResolverPostProcessor}.</p> + * + * <p>It deliberately diverges from the upstream implementation on one point: + * upstream re-registers the unwrapped resolver as a separate named bean + * ({@code innerBeanName}) that the wrapper references, which leaves the raw + * resolver discoverable by {@code getBeansOfType(ViewResolver)} sweeps — the + * exact exposure this class exists to close. The original definition is instead + * embedded as an anonymous inner-bean definition of the wrapper, making the + * undecorated resolver structurally unreachable.</p> + * + * <p>Runs after {@link GroovyPagesPostProcessor} (which contributes the default + * GSP resolver definition when no plugin has registered one) so the definition + * being wrapped is final, whether it came from grails-gsp, the scaffolding + * plugin, or the application.</p> + */ +public class Sitemesh3ViewResolverDefinitionPostProcessor extends SiteMeshViewResolverPostProcessor { + + /** + * After {@link GroovyPagesPostProcessor#ORDER} so the default GSP resolver + * definition exists, and after the SiteMesh 2 module's post-processor + * (ORDER - 1) so legacy layout wrapping, when present, wins and is detected. + */ + public static final int ORDER = GroovyPagesPostProcessor.ORDER + 10; + + public Sitemesh3ViewResolverDefinitionPostProcessor() { + setTargetViewResolverBeanName(GrailsSiteMeshViewResolverBeanPostProcessor.TARGET_VIEW_RESOLVER_BEAN_NAME); + setSiteMeshViewResolverClass(GrailsSiteMeshViewResolver.class); + setOrder(ORDER); + } + + @Override + public void postProcessBeanDefinitionRegistry(BeanDefinitionRegistry registry) throws BeansException { + if (!registry.containsBeanDefinition(getTargetViewResolverBeanName()) || + !registry.containsBeanDefinition(getContentProcessorBeanName()) || + !registry.containsBeanDefinition(getDecoratorSelectorBeanName())) { + // Decoration is not possible in this context (no GSP view resolver, or a + // context without the SiteMesh beans, e.g. the lightweight unit-test + // contexts built by grails-testing-support) — leave the definition alone. + return; + } + BeanDefinition existing = registry.getBeanDefinition(getTargetViewResolverBeanName()); + if (isAlreadyDecorating(existing)) { + return; + } + registry.removeBeanDefinition(getTargetViewResolverBeanName()); + + GenericBeanDefinition wrapper = new GenericBeanDefinition(); + wrapper.setBeanClass(getSiteMeshViewResolverClass()); + wrapper.setLazyInit(existing.isLazyInit()); + wrapper.setPrimary(true); + ConstructorArgumentValues arguments = wrapper.getConstructorArgumentValues(); + arguments.addIndexedArgumentValue(0, existing); + arguments.addIndexedArgumentValue(1, new RuntimeBeanReference(getContentProcessorBeanName())); + arguments.addIndexedArgumentValue(2, new RuntimeBeanReference(getDecoratorSelectorBeanName())); + arguments.addIndexedArgumentValue(3, new RuntimeBeanReference(getServletContextBeanName())); + if (getDispatchMode() != null) { + wrapper.getPropertyValues().add("dispatchMode", getDispatchMode()); + } + wrapper.getPropertyValues().add("includeErrorPages", isIncludeErrorPages()); + registry.registerBeanDefinition(getTargetViewResolverBeanName(), wrapper); + } + + /** + * Skips definitions that already decorate: a {@link SiteMeshViewResolver} + * (this module's wrapper, or a custom one), or the legacy grails-layout + * module's {@code GrailsLayoutViewResolver} — an {@link ApplicationListener} + * that performs SiteMesh 2 decoration itself, matching the instance-level + * exclusion {@link GrailsSiteMeshViewResolverBeanPostProcessor} applies. + */ + private boolean isAlreadyDecorating(BeanDefinition definition) { + String className = definition.getBeanClassName(); + if (className == null && definition instanceof AnnotatedBeanDefinition annotated) { + MethodMetadata factoryMethod = annotated.getFactoryMethodMetadata(); + if (factoryMethod != null) { + className = factoryMethod.getReturnTypeName(); + } + } + if (className == null) { + return false; + } + try { + Class<?> beanClass = ClassUtils.forName(className, getClass().getClassLoader()); Review Comment: Good catch — fixed in ba2c2ec2 exactly as suggested: the check resolves against the bean factory's bean class loader when the registry is a `ConfigurableBeanFactory`, falling back to `ClassUtils.getDefaultClassLoader()`, so app-defined `SiteMeshViewResolver` subclasses in a child/restart class loader are recognized instead of double-wrapped. -- 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]
