Filterless Sitemesh 3#15585
Open
codeconsole wants to merge 27 commits intoapache:7.2.xfrom
Open
Conversation
Introduce Sitemesh3CapturedPage, a SiteMesh 3 Content implementation backed by StreamCharBuffers that the GSP compile-time capture taglib populates directly. Adds Sitemesh3LayoutTagLib under the grailsLayout namespace to mirror the existing GrailsLayoutPreprocessor rewrites and skip SiteMesh's HTML parser.
CaptureAwareContentProcessor skips SiteMesh 3's HTML parse when a Sitemesh3CapturedPage is already populated on the request (the normal path for GSP-rendered responses), and delegates to a TagBasedContent- Processor with the SM2 bundle when it is not.
GrailsViewDispatchContext extends WebAppContext and overrides dispatch() to resolve a decorator path through a Spring ViewResolver and call View.render() directly. This avoids RequestDispatcher.include() so layout GSPs run through the same Grails view pipeline as the main view. GrailsContentBufferingResponse wraps the servlet response with SiteMesh 3's HttpServletResponseBuffer, forces HTML buffering, and exposes a Content that short-circuits parsing when the GSP capture taglib has already populated a Sitemesh3CapturedPage on the request.
Sitemesh3LayoutView buffers an inner Spring View, obtains a SiteMesh 3 Content, and drives decoration through a GrailsViewDispatchContext. INCLUDE dispatches are passed straight through; other dispatch types go through decorator selection. Sitemesh3LayoutViewResolver wraps each resolved view in a layout view, bypassing layout-path names and redirect views. Sitemesh3LayoutViewResolverPostProcessor renames the primary GSP view resolver bean to gspViewResolverInner and registers the wrapping layout resolver under the original name, mirroring grails-layout's approach for integrating at the Spring MVC level.
Sitemesh3LayoutFinder ports GroovyPageLayoutFinder's resolution order to the SiteMesh 3 DecoratorSelector interface: request attribute, meta layout, controller static property, action-specific layout, controller-specific layout, then configured default. Caches results outside of the DEVELOPMENT environment.
Sitemesh3GrailsPlugin now registers the view-resolver beans (sitemesh3ContentProcessor, sitemesh3DecoratorSelector, sitemesh3LayoutViewResolverPostProcessor) and replaces the upstream SiteMeshAutoConfiguration 'sitemesh' filter with a disabled no-op FilterRegistrationBean. SiteMeshAutoConfiguration's bean is @ConditionalOnMissingBean(name = "sitemesh") so defining this bean disables the upstream filter integration entirely. Drop the grails.gsp.sitemesh=false and grails.gsp.view.layoutViewResolver =false overrides so GSP compile-time capture stays enabled and our view resolver can take over. Remove the sitemesh.filter.order property since there is no longer a filter. RenderSitemeshTagLib is rewired via @Autowired against the ContentProcessor and DecoratorSelector beans rather than reaching into the filter registration bean. GspAutoConfiguration drops the GrailsLayoutHandlerMapping bean and the AutoConfigureBefore reference to SiteMeshAutoConfiguration. GrailsLayoutHandlerMapping is deleted.
Sitemesh3LayoutFinderSpec covers all six layout resolution paths: request attribute, meta layout, controller static property, action convention, controller convention, and configured default (including the no-match-no-default empty result). CaptureAwareContentProcessorSpec verifies the capture short-circuit and fallback-processor behavior. GrailsViewDispatchContextSpec verifies dispatch resolves views via the Spring ViewResolver and calls View.render() rather than forwarding. Pulls in byte-buddy and objenesis so Spock can mock the concrete GrailsConventionGroovyPageLocator.
…ion, multi-decorator resolution Three defects uncovered by end-to-end validation against grails-test-examples-gsp-sitemesh3: 1. CaptureAwareContentProcessor short-circuited during the decoration second pass, returning the inner's captured page instead of parsing the layout's output. This left <sitemesh:write property="..."/> placeholders unexpanded in the final response. Now checks context.getContentToMerge() — non-null means we're in the decorate phase and must run the fallback parser so SiteMeshWriteRule fires. 2. Added DecoratorTagRuleBundle to the fallback TagBasedContentProcessor. Without it <sitemesh:write> has no tag rule registered and passes through as literal text. 3. GrailsViewDispatchContext.dispatch now clears and restores the captured-page request attribute around the layout render, so the layout's own preprocessed <head>/<body>/<title> captures don't clobber the inner page's buffers that <sitemesh:write> reads from. 4. Sitemesh3LayoutFinder now splits comma-separated layout names from the request attribute or meta tag, enabling decorator chaining (<meta name="layout" content="a, b"/>). Updated CaptureAwareContentProcessorSpec to stub getContentToMerge() explicitly (Spock's smart defaults return a non-null Content). Added a spec for the decoration-phase fallback path. All 13 integration tests in grails-test-examples-gsp-sitemesh3 pass, 15 unit tests pass.
Two changes remove the parse pass that ran on every decorated response: 1. RenderSitemeshTagLib.layoutBody / layoutHead / layoutTitle inline-expand at tag-render time by pulling the property from WebAppContext.CONTENT_KEY (the Content being merged, same mechanism <g:pageProperty> already uses) and writing directly to out, instead of emitting <sitemesh:write property="..."/> placeholders that would need SiteMeshWriteRule to expand them in a second HTML parse. 2. GrailsViewDispatchContext.dispatch pushes a fresh Sitemesh3CapturedPage on the request before the layout render (replacing the previous "clear the attribute" logic). The layout's own capture taglibs now populate this page, giving us head/body/title extraction for chained decoration without re-parsing. CaptureAwareContentProcessor uses the layout's captured page directly during decoration: attaches the raw rendered output via a new setRenderedContent() on Sitemesh3CapturedPage, then returns the page as the decorated Content. Falls back to the parser only if no capture happened (layouts without <head>/<body> skeleton). Sitemesh3CapturedPage.getData() returns a RawDataChunk when renderedContent is attached. Needed because InMemoryContent's default data chunk writes the property tree via writeValueTo, not the underlying data value — so setValue() on the delegate's chunk doesn't produce the expected output. RawDataChunk.writeValueTo appends the raw string verbatim. Net: grails-sitemesh3 now matches grails-layout's decoration cost — zero HTML parses on the hot path for GSP responses. The fallback parser still runs for non-GSP responses that don't populate capture taglibs. Tests: 16 unit + 13 integration, all pass.
<g:applyLayout> in RenderSitemeshTagLib was instantiating a plain upstream WebAppContext, whose default dispatch does RequestDispatcher.forward(). That re-enters the servlet pipeline on every apply. In isolation this still worked (a handler resolved /layouts/<name>). But nested applyLayout — applyLayout inside applyLayout inside an outer Sitemesh3LayoutView render — tore down the outer request scope during the inner forward, and the outer render then hit 'Cannot set request attribute - request is not active anymore!' when GSP's WebOutputContextLookup tried to rebind. Swap the context for our GrailsViewDispatchContext so dispatch goes through Spring's ViewResolver + View.render(). No forward, same request scope throughout, nesting now works. Requires autowiring ViewResolver into RenderSitemeshTagLib. The primary bean is our Sitemesh3LayoutViewResolver, which correctly passes through /layouts/** view names without re-wrapping them in Sitemesh3LayoutView (avoiding double decoration). Removes the @PendingFeature annotation from EndToEndSpec's 'multiple levels of layouts' — it passes now. Tests: 16 unit + 14 integration, all pass.
Closes the remaining allocation delta vs grails-layout on the decoration hot path. Both changes leverage the fact that sitemesh 3's 3.2.x branch already accepts CharSequence for ContentProperty.setValue and that StreamCharBuffer implements CharSequence. Sitemesh3CapturedPage.materializeProperties stops calling .toString() on bodyBuffer, titleBuffer, pageBuffer, and each content-tag buffer. setValue(CharSequence) now gets a direct reference, and sitemesh 3's InMemoryContentChunk iterates via Appendable.append / CharSequenceBuffer fast path. One String allocation per capture is eliminated, roughly response-body-size bytes per decorated request. extractHead() still materializes as String because the <title> strip is regex-driven — CharSequence regex support in the JDK copies anyway, so there's no win there. RawDataChunk (the getData() shortcut used during decoration) now stores CharSequence and takes a fast path through Writer.write(char[], int, int) when the value is a CharBuffer with a backing array (the common case since BaseSiteMeshContext hands us a CharBuffer wrapping a CharArrayWriter). Falls back to Appendable.append for other shapes. Also drops the dead delegate.getData().setValue(renderedContent) call — getData() returns RawDataChunk directly so the delegate path is never read. Tests: 16 unit + 14 integration, all pass.
…t, specs
Addresses high-priority issues from code review.
1. RenderSitemeshTagLib.applyLayout: null-guard on context.decorate()
return value. The sitemesh 3 contract allows decorate() to return
null (no decoration possible). The loop was overwriting content
unconditionally, then calling content.getData() — NPE when the
decorator selector resolved a layout name but the dispatch couldn't
produce content. Also swap setAttribute(key, null) for removeAttribute
on the LAYOUT_ATTRIBUTE restore path, matching the pattern used in
Sitemesh3LayoutView.render().
2. Sitemesh3LayoutViewResolver.isLayoutPath: tighten the pass-through
prefix match. The previous startsWith("/layouts") also matched
sibling paths like /layoutsManagement/index, silently disabling
decoration for any controller whose view name shared the prefix.
Now matches only the folder itself ("/layouts") or paths under it
("/layouts/..."). Added Sitemesh3LayoutViewResolverSpec covering
the four code paths (null / redirect / already-wrapped / wrapped)
and both halves of the prefix-match decision.
3. Sitemesh3LayoutViewResolverPostProcessor: drop unused
MutablePropertyValues import (would fail codeStyle check).
Advisory items from the review (extractHead String allocation,
unbounded Sitemesh3LayoutFinder caches, missing specs for
Sitemesh3CapturedPage.materializeProperties and Sitemesh3LayoutView.render
internals) are noted but not addressed in this commit — not blocking
for merge.
Tests: 24 unit + 14 integration, all pass.
Upstream sitemesh3 adds a SiteMeshViewResolverBeanPostProcessor variant that wraps the target ViewResolver bean after it is instantiated (instead of rewriting the bean definition pre-instantiation). Grails needs the live-bean variant because gspViewResolver is not registered at the time BeanDefinitionRegistryPostProcessors fire. Bump the starter version to pull in the new type, and add an explicit spring-webmvc-sitemesh dependency entry so the module's types are visible on the Grails plugin compile classpath (for the GrailsSiteMeshViewResolver subclass).
Replace the locally-duplicated SiteMeshView / SiteMeshViewResolver / SiteMeshViewResolverPostProcessor / WebAppContext-subclass / HttpServletResponseBuffer-subclass with slim Grails-specific subclasses of the upstream spring-webmvc-sitemesh types: - GrailsSiteMeshViewContext extends SiteMeshViewContext — pushes a fresh Sitemesh3CapturedPage on the request per decorator dispatch so each layout level captures into its own page (without popping, so chained decoration can re-read the just-filled page). - GrailsSiteMeshView extends SiteMeshView — overrides preRender / postRender to push/pop a Sitemesh3CapturedPage around the main view render, and overrides createContext to return the Grails context. - GrailsSiteMeshViewResolver extends SiteMeshViewResolver — overrides createSiteMeshView to return GrailsSiteMeshView. - GrailsSiteMeshViewResolverBeanPostProcessor extends the upstream SiteMeshViewResolverBeanPostProcessor, preconfigured to target "jspViewResolver" (the bean name Grails' GroovyPagesPostProcessor and GspAutoConfiguration register the GSP view resolver under) and to wrap with GrailsSiteMeshViewResolver. Wiring: - Sitemesh3EnvironmentPostProcessor seeds sitemesh.integration=view-resolver and sitemesh.viewResolver.wrapMode=bean-instance (both required to make the upstream auto-config pick the new BPP variant instead of the BDRP variant — the BDRP variant cannot find Grails' view resolver because its bean definition is registered after BDRPs fire). - Sitemesh3AutoConfiguration registers GrailsSiteMeshViewResolverBeanPostProcessor with @AutoConfigureBefore the upstream auto-config so its @ConditionalOnMissingBean guard suppresses upstream's default. - Sitemesh3GrailsPlugin renames sitemesh3ContentProcessor / sitemesh3DecoratorSelector to contentProcessor / decoratorSelector so upstream's @ConditionalOnMissingBean(name = "contentProcessor" / "decoratorSelector") guards pick up our implementations; drops the now-unused Sitemesh3LayoutViewResolverPostProcessor registration. - RenderSitemeshTagLib consumes the renamed beans and injects gspViewResolver with @lazy + @qualifier("jspViewResolver") to break the RenderSitemeshTagLib -> ViewResolver -> groovyPagesTemplateEngine -> gspTagLibraryLookup -> RenderSitemeshTagLib cycle.
Replaces the deleted GrailsViewDispatchContextSpec / Sitemesh3LayoutViewResolverSpec (those test subjects moved upstream) with specs for the new Grails subclasses: - GrailsSiteMeshViewContextSpec — covers the dispatch push semantics, the no-pop-after-dispatch contract (required for chained decoration), and view-resolver delegation. - GrailsSiteMeshViewResolverSpec — covers wrapping decisions: wrap for normal paths, pass-through for layout paths, pass-through for RedirectView, null-safe on missing inner view. - GrailsSiteMeshViewResolverBeanPostProcessorSpec — covers the preconfigured target bean name / wrapper class and wrap-vs-passthrough. - Sitemesh3EnvironmentPostProcessorSpec — covers default property seeding and the "respect existing user values" contract.
…inear scan, EPP forward-compat, applyLayout cleanup Addresses findings from post-refactor code review. 1. Sitemesh3CapturedPage.propertiesMaterialized is now volatile. Captured pages can escape to async-dispatch threads in Grails 7 (async controller returns, Callable actions), and the JMM gives no happens-before guarantee on a plain boolean across threads. Two threads could race to materialize the property tree and corrupt the delegate. Volatile is cheap insurance. 2. extractHead() replaced its regex-on-toString path with a linear CharSequence scan. StreamCharBuffer implements CharSequence, so the scan operates on the buffer directly — no intermediate String allocation when no <title> is present, and a single StringBuilder when there is. Return type widened from String to CharSequence so ContentProperty.setValue(CharSequence) can hold the sequence without materialization. Saves ~head-size bytes per decorated request with a title (the common case). 3. Added META-INF/spring/org.springframework.boot.env.EnvironmentPostProcessor.imports alongside the existing spring.factories. Spring Boot 3.x prefers the imports-file mechanism for EnvironmentPostProcessor registration; spring.factories is still read for back-compat but may be dropped in a future release. Listing in both keeps us forward-compatible. 4. RenderSitemeshTagLib.applyLayout now does the initial contentProcessor.build() and the LAYOUT_ATTRIBUTE assignment inside the try block. The previous layout had the attribute assignment outside the try — robust in current code but fragile against future reordering. Moving it inside the try means the finally's save/restore correctly covers any exception path including during content building. 5. RenderSitemeshTagLib.content taglib streams directly to out instead of building a StringBuilder and emitting it as one write. Matches the streaming style used everywhere else in the taglib and eliminates a per-invocation String allocation. Tests: 26 unit + 13 integration, all pass.
Upstream now exposes protected getContentProcessor/getServletContext/ getViewResolver on SiteMeshView (sitemesh3 9a99354). Use those getters in createContext() instead of mirroring the collaborators as local fields. Eliminates three redundant fields and the constructor assignments that populated them. Behavior unchanged.
Extends the existing sonatype snapshots repo entry (previously scoped to cloud.wondrify.*) with an org.sitemesh.* regex match so the 3.2.3-SNAPSHOT artifacts published there resolve without requiring GRAILS_INCLUDE_MAVEN_LOCAL=1 / a local publishToMavenLocal step. Applied to both pluginManagement and dependencyResolutionManagement repository blocks in GrailsRepoSettingsPlugin so every subproject picks up the configuration consistently.
Contributor
|
Let's do this against 8.0.x, 7.1.x is shipped and we have to get 8.0.x shipping ASAP to beat the Spring Boot 3.5.x June 30th EOL. |
Contributor
Author
@jamesfredley this won't work in 8.0.x This is an incremental pathway to 8.0.x and it isn't enabled by default. |
Contributor
Author
@jamesfredley could also do a 7.2.x branch, merge and ship 7.2 after 8.0 |
Contributor
|
@jamesfredley given that sitemesh3 has known problems, I view this as a bug fix. @codeconsole does this fix the async & forwarding workflows? (there are ignored tests in the associated test-examples project). I'm ok doing a 7.2 and having this change, especially because this is the non-default path. |
jdaugherty
reviewed
Apr 21, 2026
|
Your suggestion to update '7.1.0' to an arbitrary large value (like '99.0.0') instead of changing the expected result to true makes sense if the test needs a clear false case for unsupported versions. This keeps the test's intent while using a more obvious version. grails-datamapping-support/src/test/groovy/org/grails/datastore/mapping/core/grailsversion/GrailsVersionSpec.groovy "99.0.0" | false |
Contributor
|
I've created a 7.2.x branch after discussion with the team. Can you please set the base of this PR to 7.2 & update your branch? |
jdaugherty
previously requested changes
Apr 23, 2026
Contributor
jdaugherty
left a comment
jdaugherty
left a comment
There was a problem hiding this comment.
It's great that the async, etc seems solved now. I think it's very likely we can switch to sitemesh3 for Grails 8 as a defualt with these changes. In addition to my other comments, we should enhance the grails-docs for this change:
- add upgrading72x.adoc to for 7.2.x instructions and document the sitemesh changes
- update any other grails docs specific to sitemesh3
Contributor
|
@codeconsole is this ready to re-review? |
codeconsole
requested review from
jamesfredley,
jdaugherty,
matrei and
sbglasius
Contributor
Author
@jdaugherty sure, unless you want me to bound the cache. I updated the doc, just didn't bound the cache yet. |
jdaugherty
reviewed
Apr 28, 2026
Contributor
jdaugherty
left a comment
jdaugherty
left a comment
There was a problem hiding this comment.
I took another pass, I think other than the license & sitemesh 2 clarifications, this looks good.
…viewresolver # Conflicts: # dependencies.gradle
✅ All tests passed ✅🏷️ Commit: 35d1d4d 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
3 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.
Sitemesh 3 Filterless implementation