ModelAndView is replaced when a RedirectView is used [SPR-14045]

[spring-projects-issues]

Christopher Smith opened SPR-14045 and commented

I have a HandlerInterceptor that is Doing Things for me based on the contents of the model I populate in a controller. I have a controller that returns a ModelAndView with a RedirectView, filling data into the model to be processed by the interceptor before the redirect fires.

However, the ModelAndView object is replaced by Spring between the controller return and the interceptor invocation (the objects have different IDs when inspected). The view is copied, but the model is discarded.

This appears to be a result of interactions between the handler's ignoreDefaultModelOnRedirect and the ModelAndViewContainer; even though a method that returns a ModelAndView is not using the "default" model, the returned model is discarded.

If a controller method returns an explicit model in a ModelAndView, then that model should be retained even when ignoreDefaultModelOnRedirect is set.

---

Affects: 3.2.16, 4.2.5

Issue Links:

• @SessionAttributes not populated when going directly to POST and redirecting while ignoreDefaultModelOnRedirect=true [SPR-12542] #17146 @SessionAttributes not populated when going directly to POST and redirecting while ignoreDefaultModelOnRedirect=true

Referenced from: commits 9e3bb1e, 5828648, d7062f6

Backported to: 3.2.17

[spring-projects-issues]

Rossen Stoyanchev commented

I get your point about returning an explicit model but the explicitness becomes a bit diluted when you consider the original very long standing issue where default model attributes were in some cases unintentionally appended to the redirect URL, coupled with the fact that ModelAndView itself is a very long standing return value type even preceding annotated methods.

It's not very clear then from a ModelAndView return value whether its content was prepared specifically for the redirect, or simply created in the beginning of the method with attributes added regardless of a redirect or not. This is exactly the purpose of the ignoreDefaultModelOnRedirect flag to indicate unequivocally the default model should be ignored on redirect.

The ignoreDefaultModelOnRedirect flag is off by default even with the MVC config. If you do keep it off, then you your use case should work and you can still have RedirectAttributes passed into the controller method if you want to use that for a redirect instead. In other words a more case-by-case basis.

[spring-projects-issues]

Christopher Smith commented

I am using Boot, and it's being turned on by default at some point. At least a major component of my surprise is that "default model" seems to be ambiguous; I expected that to be essentially @ModelAttribute and/or things present in the passed-in Model. I didn't expect "default" to include "I created a brand-new one and returned it". Based on what I saw of the codebase, there's no inherent reason why a returned ModelAndView couldn't be treated distinctly from what I understood as the "default".

If there's a major backwards-compatibility issue because of existing code that expects values added to the explicit pre-merged ModelAndView to be suppressed, then I think this is an area where substantially amplified documentation would be useful, and I can help with that.

[spring-projects-issues]

Rossen Stoyanchev commented

Upon taking a closer a look there appears to be a very long standing bug in ModelAndViewContainer's getModel() method. When neither using the default model, nor a redirect model had been created yet, it returns a new model without saving it. I will investigate further.

[spring-projects-issues]

Rossen Stoyanchev commented

Okay it turns out the code in ModelAndViewMethodReturnValueHandler behaves as you expected. I was confused by it at first but setRedirectModelScenario simply flags that we are in a redirect. After that we copy the attributes from the returned ModelAndView and they end up in the redirect model. At any rate this should all be transparent for you aside from the fact that the model you see in the interceptor is not the same reference.

The real issue turned out to be a 5 year old bug and addressing resolves this issue. Thanks!

[spring-projects-issues]

Christopher Smith commented

I don't have any need for the model to be the same object (although the test seems to suggest that it actually is), just for it to have the contents for the interceptor to inspect. I provided the notes about the object identity just because the replacement seemed to be relevant to the data evaporation I was seeing. I'll swap out for 4.2.6 or 4.3RC1, whichever's out first.

Thanks for double-checking on the root cause for this. I expect not very many clients try to explicitly pass a model to a redirect.

[spring-projects-issues]

Christopher Smith commented

@Jürgen: I'm tagging Priority based on the standard JIRA description of having a significant loss of functionality and no simple workaround. If the Spring team uses Priority in more of a "big picture" mode, I'll keep that in mind going forward, and I think there's a place in JIRA to change the explanatory text.

[spring-projects-issues]

Juergen Hoeller commented

Good point: We don't define our actual rules for priority handling anywhere. They are effectively pretty simple: For 'bug' issues, 'major' is mostly functional regressions and other incompatibilities; 'minor' is for stuff which should arguably work but never really did... that is, functional limitations which are hard to argue as an 'improvement', so we do consider them as a 'bug' and backport them, whereas we usually do not backport improvement tickets at such a rather late point.

I'm also using the priorities of the bug issues for a specific release target as an indicator for the urgency of that release. 'minor' basically implies nice-to-have, even for bugs, whereas 'major' suggests that the fix should be released in a rather timely fashion.

So from my perspective, the most important categorization is 'bug' vs 'improvement', with the former implying a backport. 'major' vs 'minor' is always arguable; I'm mostly interpreting it in terms of urgency. I hope that clarifies it a bit...