Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate RedirectAttributes with ModelMap [SPR-9418] #14054

Closed
spring-projects-issues opened this issue May 16, 2012 · 2 comments
Closed

Integrate RedirectAttributes with ModelMap [SPR-9418] #14054

spring-projects-issues opened this issue May 16, 2012 · 2 comments
Assignees
Labels
status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

marc schipperheyn opened SPR-9418 and commented

I have been playing around with RedirectAttributes and although I like the FlashAttribute function, I don't really like the way it has been implemented.
For me, FlashAttributes come into play in the GET/POST/Redirect/displayStatus chain. What I don't like is that FlashAttributes aren't a transparent part of the ModelMap but instead separated through RedirectAttributes. I would prefer to say

model.addFlashAttribute(...) in stead of having the specify RedirectAttributes separately redirectAttributes.addFlashAttribute(...).

Also, when I add a FlashAttribute to a Model/RedirectAttributes and I don't redirect, I expect the FlashAttribute to be treated as a normal model attribute. and because they are separate, it isn't.

In the flow of a Controller, whether one redirects often depends on the context. And because RedirectAttributes are separate from ModelAttributes (when they are conceptually the same), I find myself having to duplicate certain actions, which is exactly what I want to avoid.

if(!member.isActive()){
   redirectAttributes.addFlashAttribute("status","Denied",RequestUtils.getLocale(request)));
   model.addAttribute("status","Denied",RequestUtils.getLocale(request)));
}

if(group.isHiddenToNonMembers()){
   return "empty";
}else{
   return "redirect:" + URLUtils.getGroupUrl(group, request);
}

What I would expect is to have a single action against the ModelMap
model.addFlashAttribute(...)
if the controller redirects, it is treated as a flash attribute
if the controller doesn't redirect, it treated as a model attribute


No further details from SPR-9418

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented May 17, 2012

Rossen Stoyanchev commented

Thanks for your comments. I think there is a typo in the example code. Both addAttribute and addFlashAttribute have 2 arguments, not 3.

That aside, for rendering we need a Model with various simple and complex objects, while for redirecting we append simple String values to the URL as query parameters or pass more complex objects as flash attributes. RedirectView has relied on the same model used for rendering for a very long time but there have been many requests that indicate the challenges around that approach. See for example #5995 and its related issues.

RedirectAttributes is not just a way to specify flash attributes. Think of it as a separate model instance to use in case of a redirect. While the model for rendering might contain an attribute called Account, RedirectAttributes doesn't need the whole account but only the account id to append to the URL. In fact if you add Account to RedirectAttributes via addAttribute, it will be formatted as a String immediately assuming there is a registered Converter or PropertyEditor. Then it is automatically appended to the URL. As for flash attributes, it makes sense to keep them in RedirectAttributes.

It would be good to see the example above corrected and possibly with more context but if you're adding the same attribute to the Model and to RedirectAttributes is it possibly it should be managed via @SessionAttributes?

@spring-projects-issues
Copy link
Collaborator Author

marc schipperheyn commented

Thanks for that comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants