Default model values are always copied into the redirect model when RedirectAttributes are used [SPR-10516]

[spring-projects-issues]

Dennis Homann opened SPR-10516 and commented

From comments Rossen's comment on #14054 and his comment on #11462, it seems that the following design is intended:

• to allow fine control over query parameters added automatically during a redirect, handlers should accept an argument of type RedirectAttributes
• RedirectAttributes define a redirect model, which is separate from the default model (part of the ModelAndView returned by the handler). Values in the redirect model are intended as query parameters and will therefore be converted to Strings, values in the default model are intended for rendering and may be "complex" i.e. not string-convertible.
• it is recommended to set RequestMappingHandlerAdapter#ignoreDefaultModelOnRedirect=true to prevent use of the default model for redirects (if no RedirectAttributes were used by the handler). This is the default.

Issues:

• Unfortunately, in 3.1.2, the default model is copied into the ModelAndViewContainer model in ModelAndViewMethodReturnValueHandler:76, and when the handler uses RedirectAttributes this will result in merging the default model into the redirect model.
  • This violate the separation of the two (issue Spring core JMS pom.xml #1) models
  • This triggers conversion of all default model values to Strings which may cause conversion exceptions, depending on the available converters (issue Improve annotation processing thread-safety #2).
• The redirect mechanism provided by the framework should play well with other parts of the framework, in particular ViewResolves and RedirectViews or custom redirecting SmartViews:
  • When a handler returns a ModelAndView with a RedirectView or a SmartView object (with isRedirect==true), it should be presented the redirect model for "rendering" (or default to the default model, if the handler does not use RedirectAttributes AND ignoreDefaultModelOnRedirect=false. Right now, it is presented the default model (with values converted to Strings) merged with the redirect model (issues Fixes https://jira.springsource.org/browse/SPR-7721 #3).
  • When a handler returns a view name, it will be resolved by a ViewResolver, which receives a model to make its decision. One could argue what's more useful here: the default model, a potentially existing redirect model, or both. In any case, if there is a default model, the view resolver must have access to the original model values added by the handler. Like the view before, it is presented the default model (with values converted to Strings) merged with the redirect model (issues SPR-7752 - EntityManager proxy now exposes provider specific interface. #4). I have a custom view resolver which is supposed to resolve a view based on a complex model value, which is not possible right now, if RedirectAttributes was used by the handler.
  • If a view resolver returns a RedirectView (or redirecting SmartView), it should again receive the redirect model.

---

Affects: 3.2.1

2 votes, 4 watchers

[spring-projects-issues]

Rossen Stoyanchev commented

One correction on the design summary. The default value for ignoreDefaultModelOnRedirect is false. In other words the described behavior should not occur with default settings.

If I understand correctly, you've ignoreDefaultModelOnRedirect=true, there is no RedirectAttributes argument, and the controller method returns a ModelAndView with a redirect view or "redirect:" prefixed view name? A ModelAndView is created and populated by the controller method so if you've signed up to always use RedirectAttribtues, then returning a ModelAndView is no different than declaring a RedirectAttribute argument, and returning a String-based "redirect:" view name.

The returned ModelAndView is not the same as the "default" model (which you can get by declaring a Model controller method argument) whose content depends on the use of @SessionAttributes and @ModelAttribute methods among others and can lead to surprises on a redirect.

In a redirect scenario "rendering" takes on a different meaning. Rather than rendering a template view, with a redirect there are only two ways to use model attributes -- embed them in the redirect URL, as URI template vars or query params, or store them as flash attributes. We determine if a view is going to result in a redirect as soon as the controller method returns. If you want to defer that decision, then perhaps ignoreDefaultModelOnRedirect should be left set to false.

In that sense RedirectAttributes is a feature of the @MVC programming model (i.e. confined to RequestMappingHandlerAdapter) much like @SessionAttributes and @ModelAttributes are. From the DispatcherServlet perpsective there is still only one model.

[spring-projects-issues]

Dennis Homann commented

The default value for ignoreDefaultModelOnRedirect is false. In other words the described behavior should not occur with default settings.

Ok, I just just copied that from here without verifying it.

I think I get the "there is still only one model" part. Maybe I should have formulated the description in terms of my concrete problem at hand:

In my application, I have a custom view resolver, which resolves view names (Strings) to views based on a complex object in the model. The model object is determined by the handler and returned in a ModelAndView. For compatibility, the custom view resolver adopts the convention to create a redirect view when the view name is prefixed with "redirect:".
This works fine, unless the handler defines a RedirectAttributes argument. Adding it to the signature is enough to break the application, even when it is not used. What happens is that the ModelAndViewContainer behaves differently and converts all model objects to Strings very early, immediately when the handler returns and before the view resolver is run. My complex objects cannot be converted to Strings and an exception is thrown.
I would expect the model objects to be converted to strings at the latest possible time, e.g. when adding them to the URL. That way, all the other MVC parts such as the view resolver are not affected.
This behavior seems to be independent of the ignoreDefaultModelOnRedirect value.

[spring-projects-issues]

Rossen Stoyanchev commented

I would expect the model objects to be converted to strings at the latest possible time, e.g. when adding them to the URL. That way, all the other MVC parts such as the view resolver are not affected.

That's the very problem RedirectAttributes was meant to address. Given a model, ViewResolvers have no way to decide which attributes to append as query params on a redirect. The traditional approach has always been to append simple attribute types only but that can lead to surprises. RedirectAttributes allows you to fill a separate model instance where everything it contains is should be used in the redirect, either as query params or as flash attributes. We could postpone converting complex attributes to strings, but ViewResolvers have no knowledge of any of this and that brings us back to the original problem that the ViewResolver doesn't know which attributes should be passed as query params. Futhermore, the ViewResolver doesn't know anything about the DataBinder and the ConversionService used to do the formatting.

In any case, it sounds like you'd like applications to be able to add a complex object unaltered in order for the custom ViewResolver to make some decisions. RedirectAttributes has a method to get the underlying map, which bypasses formatting. So you could do:

redirectAttrs.asMap().put("complexObject", new ComplexObject());

We could make this more explicit in the Javadoc.

[spring-projects-issues]

Dennis Homann commented

RedirectAttributes#asMap is only implemented in RedirectAttributesModelMap as "return this", so it does not bypass formatting.

To shorten the discussion, a in simple handler method like this one

    @RequestMapping("/redirect")
    public ModelAndView handle(HttpServletRequest request) {

      ModelAndView mav = new ModelAndView();
      mav.addObject("item", new Item());
      mav.setViewName("view");

      return mav;
    }

the ViewResolver will see the item as a complex object and can make the decision to return a RedirectView.

By simply adding a RedirectAttributes parameter to the method signature, the handler will break, because the framework attempts to convert "item" to a String. It does not make a difference, if a flash attribute is added or not.

    @RequestMapping("/redirect")
    public ModelAndView handle(HttpServletRequest request, RedirectAttributes redirectAttributes) {

      redirectAttributes.addFlashAttribute("message", "success");

      ModelAndView mav = new ModelAndView();
      mav.addObject("item", new Item());
      mav.setViewName("view");

      return mav;
    }

It would be more consistent to pass the mav to the view resolver (RedirectAttributes should not change that behavior), without converting the contents of mav first.
If view resolver returns a regular view, redirectAttributes could be ignored. If view resolver returns a RedirectView, that would be the right time to convert the mav contents and merge it with RedirectAttributes.

[spring-projects-issues]

Bulk closing outdated, unresolved issues. Please, reopen if still relevant.