jdaugherty commented on code in PR #16453:
URL: https://github.com/apache/grails-core/pull/16453#discussion_r4153650725
##########
grails-interceptors/src/main/groovy/grails/artefact/Interceptor.groovy:
##########
@@ -145,7 +145,11 @@ trait Interceptor implements ResponseRenderer,
ResponseRedirector, RequestForwar
@Generated
Map<String, Object> getModel() {
def modelAndView = (ModelAndView)
currentRequestAttributes().getAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW,
0)
- return modelAndView?.modelMap
+ if (modelAndView != null) {
+ return modelAndView.modelMap
+ }
+ // Fallback: when render template: ..., model: ... is used, the model
is stored in TEMPLATE_MODEL
+ return (Map<String, Object>)
currentRequestAttributes().getAttribute(GrailsApplicationAttributes.TEMPLATE_MODEL,
0)
Review Comment:
**Document template-model availability and rendering timing.** The guide
(`grails-doc/src/en/guide/theWebLayer/interceptors/definingInterceptors.adoc`,
lines 48–59) describes `after()` as running before rendering and shows
modifying `model` to affect the view. For this newly supported template case,
rendering has already happened inside the controller's `render()` call: the
model is available for inspection, but modifying it in `after()` does not
change the rendered response. Could we add a short explanation and example
covering that distinction? I confirmed the timing in a local probe using the
real controller `render()` method with a stub view. This would also satisfy the
repository's documentation requirement for user-facing changes.
##########
grails-interceptors/src/test/groovy/grails/artefact/InterceptorSpec.groovy:
##########
@@ -535,6 +537,52 @@ class InterceptorSpec extends Specification {
'/grails/foo/x' | true
}
+ void "Test getModel returns null when neither MODEL_AND_VIEW nor
TEMPLATE_MODEL is set"() {
+ given: "an interceptor with no model attributes on the request"
+ def i = new TestInterceptor()
+ GrailsWebMockUtil.bindMockWebRequest()
+
+ expect: "getModel returns null"
+ i.model == null
+ }
+
+ void "Test getModel returns modelMap from ModelAndView when MODEL_AND_VIEW
is set"() {
+ given: "an interceptor with a ModelAndView on the request"
+ def i = new TestInterceptor()
+ def webRequest = GrailsWebMockUtil.bindMockWebRequest()
+ def mav = new ModelAndView()
+ mav.addObject('foo', 'bar')
+ webRequest.setAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW,
mav, 0)
+
+ expect: "getModel returns the ModelAndView's model map"
+ i.model == [foo: 'bar']
+ }
+
+ void "Test getModel falls back to TEMPLATE_MODEL when no ModelAndView is
set"() {
+ given: "an interceptor with only TEMPLATE_MODEL on the request"
+ def i = new TestInterceptor()
+ def webRequest = GrailsWebMockUtil.bindMockWebRequest()
+ Map<String, Object> templateModel = [baz: 'qux']
+ webRequest.setAttribute(GrailsApplicationAttributes.TEMPLATE_MODEL,
templateModel, 0)
Review Comment:
**Add regression coverage through controller rendering.** This test manually
sets `TEMPLATE_MODEL` and reads it back, so it verifies the getter's branch but
does not exercise the reported failure: a controller calling `render template:
..., model: ...`, followed by an interceptor reading `model` in `after()`.
Could we add a regression test through that public flow and assert the actual
model contents? The existing `ModelInterceptorIntSpec` in
`grails-test-examples/views-functional-tests` is a natural place to extend
coverage; it currently covers `modelAndView`, `respond`, and returned maps.
--
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]
