ruthst00 commented on PR #16492:
URL: https://github.com/apache/grails-core/pull/16492#issuecomment-5973309607
@jamesfredley, thanks very much for your approval and feedback. All great
suggestions so I updated the tests:
**Change in `UrlMappingUtils.java`** (`forwardRequestForUrlMappingInfo`
method): Added
`webRequest.removeAttribute(GrailsApplicationAttributes.TEMPLATE_MODEL, 0)`
immediately after the existing `MODEL_AND_VIEW` removal. This prevents template
metadata from a failed action from leaking through to the error action when a
request is forwarded (e.g., during error handling via URL mappings).
**New test in `UrlMappingUtilsSpec.groovy`**: Added `"test
forwardRequestForUrlMappingInfo clears TEMPLATE_MODEL before forwarding"` which
uses a capturing `MockHttpServletRequest` to verify that `TEMPLATE_MODEL` is
`null` at the point the `RequestDispatcher.forward()` call is made, even when
it was set on the web request before the forward.
Implemented all 6 non-blocking recommendations::
**r4174533293 & r4174533295** — `TEMPLATE_MODEL does not leak into the
interceptor of a forwarded action` (both `views-functional-tests` and
`hibernate7` copies):
- Reset `modelInterceptor.latestModel` and `modelByAction` before the
request so a prior test cannot satisfy the assertion
- Changed from `given/expect` to `given/when/then` structure
- Replaced `modelInterceptor.latestModel == null` with
`modelInterceptor.modelByAction.containsKey('forwardTarget')` (proves the
interceptor actually ran) + `modelByAction['forwardTarget'] == null`
- Added assertion on `response.body.text == 'ok'` to verify the forwarded
action's own output
**r4174533294 & r4174533298** — `TEMPLATE_MODEL does not leak into the
interceptor of an included action` (both copies):
- Reset `modelByAction` before the request
- Changed from `given/expect` to `given/when/then` structure
- Added `modelInterceptor.modelByAction.containsKey('includeTarget')`
assertion (a missing key also compares equal to null, so this proves the
interceptor ran)
- Kept `modelByAction['includeTarget'] == null` as a separate `and:` block
**r4174533301** — `UrlMappingUtilsSpec`: Added `"test
includeForUrlMappingInfo restores outer TEMPLATE_MODEL even when the include
throws"` — a dispatcher that sets a distinct inner model then throws, with an
identity comparison (`.is()`) against the saved outer map to lock in the
`finally` restore path.
**r4174533306** — `RequestForwarderSpec`: Added `"test request forward
clears TEMPLATE_MODEL even when the dispatcher throws"` — a dispatcher that
sets a new `TEMPLATE_MODEL` then throws a `RuntimeException`, asserting the
`finally` block still clears it.
All 11 unit tests across `grails-web-url-mappings` and `grails-controllers`
pass (BUILD SUCCESSFUL).
--
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]
