Convert SiteMesh 3 taglibs to method handlers and use SiteMesh 3.3.0-M1#15746
Open
codeconsole wants to merge 8 commits into
Open
Convert SiteMesh 3 taglibs to method handlers and use SiteMesh 3.3.0-M1#15746codeconsole wants to merge 8 commits into
codeconsole wants to merge 8 commits into
Conversation
Convert the seven closure-based tags in RenderSitemeshTagLib (applyLayout, pageProperty, ifPageProperty, layoutTitle, layoutHead, layoutBody, content) to method handlers so GSP dispatch uses the faster reflective method-invocation path added in apache#15465 instead of cloning a closure on every invocation. This also clears the closure-based-tag deprecation warning the apache#15465 compiler check emits for this class. The conversion is behavior-preserving: GSP dispatch already passes TagOutput.EMPTY_BODY_CLOSURE (never null) as the body to two-arg tags, so the existing body checks resolve identically whether the tag is a closure field or a method. Sitemesh3LayoutTagLib is intentionally left as closures, matching its SiteMesh 2 twin GrailsLayoutTagLib: those grailsLayout-namespace, @CompileStatic capture tags driven by GrailsLayoutPreprocessor were deliberately kept as closures in apache#15465. Follow-up to apache#15713. Addresses review feedback on RenderSitemeshTagLib.
Bump the SiteMesh 3 dependencies (spring-boot-starter-sitemesh and spring-webmvc-sitemesh, plus the transitive sitemesh core) from 3.3.0-SNAPSHOT to the released 3.3.0-M1 milestone, now available on Maven Central. SiteMesh 2 (opensymphony:sitemesh 2.6.0, used by grails-layout) is unaffected.
codeconsole
changed the title
… handlers Convert the seven closure-based capture tags in Sitemesh3LayoutTagLib (captureHead, captureBody, captureTitle, wrapTitleTag, captureMeta, captureContent, parameter) to method handlers, completing the SiteMesh 3 plugin's move off closure-based tags so both shipped taglibs use the faster method-invocation path and the apache#15465 closure-tag deprecation warning is cleared for the whole plugin. These grailsLayout-namespace capture tags are emitted by GrailsLayoutPreprocessor and dispatched through GroovyPage.invokeTag, which already routes to method handlers (invokeTagLibMethod -> TagMethodInvoker) and passes a non-null EMPTY_BODY_CLOSURE body, so the conversion is behavior-preserving even though the class is @CompileStatic. Verified by the gsp-sitemesh3 EndToEndSpec integration suite (14 tests, all passing) against SiteMesh 3.3.0-M1.
codeconsole
changed the title
Address review feedback on apache#15710: the sitemesh snapshot-repository version filter does not need to match -SNAPSHOT explicitly, because the repository is already constrained to snapshots (snapshotsOnly: true in CreateAppCommand, VersionType.SNAPSHOT in GradleRepository). Simplify the version regex from '.*-SNAPSHOT' to '.*' in both the forge generator and the shell-cli create-app command, matching how the version type is set.
codeconsole
requested review from
jamesfredley,
jdaugherty,
matrei and
sbglasius
jdaugherty
reviewed
Jun 18, 2026
| List.of( | ||
| new VersionRegexRepoFilter( | ||
| "org[.]sitemesh.*", ".*", ".*-SNAPSHOT" | ||
| "org[.]sitemesh.*", ".*", ".*" |
Contributor
There was a problem hiding this comment.
Is the M1 and actual release? I don't think this code should change since the below filter will still filter to only snapshots
Contributor
Author
There was a problem hiding this comment.
@jdaugherty per @matrei it's redundant and not needed. #15710
|
|
||
| GrailsGradleRepository sitemeshSnapshots = new GrailsGradleRepository(url: 'https://central.sonatype.com/repository/maven-snapshots', snapshotsOnly: true) | ||
| sitemeshSnapshots.includeOnly('org[.]sitemesh.*', '.*', '.*-SNAPSHOT') | ||
| sitemeshSnapshots.includeOnly('org[.]sitemesh.*', '.*', '.*') |
Contributor
There was a problem hiding this comment.
Same
comment, I don't think this needs to change
Contributor
Author
There was a problem hiding this comment.
@jdaugherty per @matrei it's redundant and not needed. #15710
Spring's BeanPostProcessorChecker warns at startup that Sitemesh3AutoConfiguration's siteMeshViewResolverBeanPostProcessor bean is created through a non-static factory method. Because the bean is a BeanPostProcessor, Spring must instantiate it before the regular post-processors are registered, which forces the enclosing @configuration class to be created prematurely and makes it ineligible for full post-processing such as auto-proxying. Declaring the factory method static removes the early instantiation and the warning. The bean is still registered and behaves identically, so existing coverage in GrailsSiteMeshViewResolverBeanPostProcessorSpec continues to apply.
…config to stop bean override The SiteMesh 3 plugin registered the contentProcessor and decoratorSelector beans in doWithSpring, relying on upstream SiteMeshViewResolverAutoConfiguration's @ConditionalOnMissingBean(name=...) guards to suppress its defaults. But Grails applies doWithSpring beans after Spring Boot evaluates auto-configuration conditions, so upstream registered its beans first and the plugin's registrations overrode them, logging at startup: Overriding bean definition for bean 'decoratorSelector' ... Overriding bean definition for bean 'contentProcessor' ... Move both registrations into Sitemesh3AutoConfiguration, which is ordered @AutoConfigureBefore the upstream config, so the Grails beans are present when upstream's guards are evaluated and the defaults back off cleanly instead of being overridden after the fact. View decoration only applies when Spring MVC is resolving views, so the two beans are gated on @ConditionalOnBean(DispatcherServlet) (with @AutoConfigureAfter the dispatcher-servlet auto-config). This keeps them out of the lightweight grails-testing-support unit-test contexts, which have no dispatcher servlet, preserving the previous behaviour where unit tests are not decorated. Adds Sitemesh3AutoConfigurationSpec covering the new factory methods.
The decoratorSelector @bean added in the previous commit tripped checkstyleMain: a blank line split the single grails/org.grails import group (ImportOrder), and the || operators in the reload-enabled expression sat at the start of the line instead of the end (OperatorWrap, option=eol). Whitespace only; no behavior change.
✅ All tests passed ✅🏷️ Commit: 36d9d39 Learn more about TestLens at testlens.app. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #15713, addressing review feedback on
RenderSitemeshTagLib:Converts both taglibs shipped by the SiteMesh 3 plugin from closure-based tags to the method handlers introduced in #15465, so GSP dispatch uses the faster reflective method-invocation path instead of cloning a closure on every tag invocation. This also clears the closure-based-tag deprecation warning the #15465 compiler check emits for these classes.
RenderSitemeshTagLib(sitemeshnamespace) —applyLayout,pageProperty,ifPageProperty,layoutTitle,layoutHead,layoutBody,content.Sitemesh3LayoutTagLib(grailsLayoutnamespace,@CompileStatic) — the compile-time capture tagscaptureHead,captureBody,captureTitle,wrapTitleTag,captureMeta,captureContent,parameter. These are emitted byGrailsLayoutPreprocessorand dispatched throughGroovyPage.invokeTag, which already routes to method handlers (invokeTagLibMethod→TagMethodInvoker).After this change, no taglib used by the SiteMesh 3 plugin remains closure-based.
Behavior preservation
Only the tag declarations change; the tag bodies are untouched. Both the closure and method dispatch paths (
TagOutput.captureTagOutputandGroovyPage.invokeTag) share the same output-stack setup and already passTagOutput.EMPTY_BODY_CLOSURE(nevernull) as the body to two-argument tags, so the existingbody/body instanceof Closurechecks resolve identically whether each tag is a closure field or a method.SiteMesh 3.3.0-M1
Also bumps the SiteMesh 3 dependencies (
spring-boot-starter-sitemesh,spring-webmvc-sitemesh, and the transitiveorg.sitemesh:sitemeshcore) from3.3.0-SNAPSHOTto the released 3.3.0-M1 milestone, now on Maven Central. SiteMesh 2 (opensymphony:sitemesh2.6.0, used bygrails-layout) is unaffected.Tests
RenderSitemeshTagLibSpecandSitemesh3LayoutTagLibSpec, which verify via the frameworkTagMethodInvokerAPI that all fourteen tags are discovered as invokable method handlers and no longer exist as closure fields.grails-test-examples/gsp-sitemesh3(EndToEndSpec) continues to exercise the full capture/decoration pipeline at runtime.Validation
./gradlew :grails-sitemesh3:test— green (incl. both new specs)./gradlew :grails-sitemesh3:checkstyleMain :grails-sitemesh3:codenarcMain— green./gradlew :grails-test-examples-gsp-sitemesh3:integrationTest— green (EndToEndSpec14/14,GrailsLayoutSpec7/7) against SiteMesh 3.3.0-M1Snapshot-repo regex cleanup (addresses #15710 review)
Addresses two review comments on #15710: the generated-app sitemesh snapshot repository's version filter does not need to match
-SNAPSHOTexplicitly, because the repository is already constrained to snapshots —snapshotsOnly: trueinCreateAppCommandandVersionType.SNAPSHOTinGradleRepository. The version regex is simplified from.*-SNAPSHOTto.*in both places. (The pre-existing apache/groovy snapshot filters are left untouched.)