Annotated controllers. Final redirect and @ModelAttribute question

[PUK_999]

Hello,

I have a controller to edit an entity (called an "agent"). The agent is linked to a timezone so I'm supplying the timezone strings via a @ModelAttribute so the jsp can show them in an HTML select input control. Here's my controller for editing agents:

@Controller
@RequestMapping(AgentEditController.viewName + "/{agentId}")
@SessionAttributes(AgentEditController.modelAttrName)
public class AgentEditController {

    public final static String viewName = "/ag/agents/edit";
    public final static String modelAttrName = "agent";
    @Autowired
    private MainServices mainServices;
    private Log log = LogFactory.getLog(AgentEditController.class);

    @RequestMapping(method = RequestMethod.GET)
    public String setupForm(@PathVariable int agentId, HttpServletRequest request, ModelMap model) throws Exception {
        WebAgent agent = new WebAgent(mainServices.findAgentById(agentId));
        model.addAttribute(modelAttrName, agent);
        return viewName;
    }

    @ModelAttribute("timezoneIds")
    public List<String> getTimezoneIds() {
        return StaticReferenceData.getTimezoneIDs();
    }

    @RequestMapping(method = RequestMethod.POST)
    public String processSubmit(@ModelAttribute(modelAttrName) WebAgent agent, BindingResult result, SessionStatus status) {
        new WebAgentValidator().validate(agent, result);
        if (result.hasErrors()) {
            return viewName;
        } else {
            mainServices.updateAgent(agent.createModelAgent());
            status.setComplete();
            return "redirect:/ag/agents/list";
        }
    }
}

The problem I'm having is that when the submit is invoked and there's no validation errors, the redirect doesn't work properly. Why? Because the timezoneIds are getting tacked on the end of the url to be redirected to. I happen to have the UrlRewriteFilter in debug still and here's what it's trying to do (scroll right to see the URL):

ServletContext.log():org.tuckey.web.filters.urlrewrite.UrlRewriter DEBUG: processing outbound url for /myapp/ag/agents/list?timezoneIds=ACT&timezoneIds=AET&timezoneIds=AGT&timezoneIds=ART&timezoneIds=AST&timezoneIds=Africa%2FAbidjan&timezoneIds=Africa%2FAccra&timezoneIds=Africa%2FAddis_Ababa&timezoneIds=Africa%2FAlgiers&timezoneIds=Africa%2FAsmara&timezoneIds=Africa%2FAsmera&timezoneIds=Africa%2FBamako&timezoneIds=Africa%2FBangui&timezoneIds=Africa%2FBanjul&timezoneIds=Africa%2FBissau&timezoneIds=Africa%2FBlantyre&timezoneIds=Africa%2FBrazzaville&timezoneIds=Africa%2FBujumbura&timezoneIds=Africa%2FCairo&timezoneIds=Africa%2FCasablanca&timezoneIds=Africa%2FCeuta&timezoneIds=Africa%2FConakry&timezoneIds=Africa%2FDakar&timezoneIds=Africa%2FDar_es_Salaam&timezoneIds=Africa%2FDjibouti&timezoneIds=Africa%2FDouala&timezoneIds=Africa%2FEl_Aaiun&timezoneIds=Africa%2FFreetown&timezoneIds=Africa%2FGaborone&timezoneIds=Africa%2FHarare&timezoneIds=Africa%2FJohannesburg&timezoneIds=Africa%2FKampala&timezoneIds=Africa%2FKhartoum&timezoneIds=Africa%2FKigali&timezoneIds=Africa%2FKinshasa&timezoneIds=Africa%2FLagos&timezoneIds=Africa%2FLibreville&timezoneIds=Africa%2FLome&timezoneIds=Africa%2FLuanda&timezoneIds=Africa%2FLubumbashi&timezoneIds=Africa%2FLusaka&timezoneIds=Africa%2FMalabo&timezoneIds=Africa%2FMaputo&timezoneIds=Africa%2FMaseru&timezoneIds=Africa%2FMbabane

(lots and lots more timezones removed)

Is this happening by design? I can stop it happening by getting rid of this:

    @ModelAttribute("timezoneIds")
    public List<String> getTimezoneIds() {
        return StaticReferenceData.getTimezoneIDs();
    }

and putting another addAttribute call in setupForm():

    @RequestMapping(method = RequestMethod.GET)
    public String setupForm(@PathVariable int agentId, HttpServletRequest request, ModelMap model) throws Exception {
        WebAgent agent = new WebAgent(mainServices.findAgentById(agentId));
        model.addAttribute(modelAttrName, agent);
        model.addAttribute("timezoneIds",StaticReferenceData.getTimezoneIDs());      <--- line added
        return viewName;
    }

but I have other submit methods (not shown for brevity) that add and remove bits of the agent and I'd need to add the calls in those handlers also.

My question again is is this by design and what's the preferred/safe/succinct way of doing what I want to do?

Thanks,

PUK

[PUK_999]

My question could perhaps be phrased like this also:

When using theSimpleFormController from the (now deprecated) controller hierarchy, one could override the referenceData() method to supply reference data to the model.

What's the preferred way of doing this using annotated controllers?

Thanks,

PUK

[Marten Deinum]

The preferred way is @ModelAttribute and/or a get method to setup the data needed by this controller.

The drawback of using @ModelAttribute is the fact that it gets invoked before ANY request handling method, which means the data is available already in your model. Spring puts all the data in the model on the url, when it concerns a redirect. So in general this is by design.

So the general approach is the get method as setting up the form object, you might also want to combine it with a SessionAttribute that way the data is destroyed when you called SessionContext.setComplete().

[PUK_999]

I like the idea of putting the timezoneIds as a @SessionAttribute and calling setComplete() before the redirect (even though there's a very large amount of data going into the session but that's ok for now). I tried the idea out and it doesn't work. The redirect is still appending all the timezoneIds. Here's the controller:

@Controller
@RequestMapping(AgentEditController.VIEW_NAME + "/{agentId}")
@SessionAttributes(value={AgentEditController.MODEL_ATTR_NAME_AGENT,AgentEditController.MODEL_ATTR_NAME_TIMEZONE_IDS})
public class AgentEditController {

    public final static String VIEW_NAME = "/ag/agents/edit";
    public final static String MODEL_ATTR_NAME_AGENT = "agent";
    public final static String MODEL_ATTR_NAME_TIMEZONE_IDS = "timezoneIds";
    @Autowired
    private MainServices mainServices;

    @RequestMapping(method = RequestMethod.GET)
    public String setupForm(@PathVariable int agentId, HttpServletRequest request, ModelMap model) throws Exception {
        WebAgent agent = new WebAgent(mainServices.findAgentById(agentId));
        model.addAttribute(MODEL_ATTR_NAME_AGENT, agent);
        model.addAttribute(MODEL_ATTR_NAME_TIMEZONE_IDS, StaticReferenceData.getTimezoneIDs());
        return VIEW_NAME;
    }

    @RequestMapping(method = RequestMethod.POST)
    public String processSubmit(@ModelAttribute(MODEL_ATTR_NAME_AGENT) WebAgent agent, BindingResult result, SessionStatus status, ModelMap model) {
        new WebAgentValidator().validate(agent, result);
        if (result.hasErrors()) {
            return VIEW_NAME;
        } else {
            mainServices.updateAgent(agent.createModelAgent());
            status.setComplete();
            return "redirect:/ag/agents/list";
        }
    }
}

I'm using 3.0.0.RC3. Do you have any further pointers?

The best I could come up with was placing the timezoneIds in the model on every request myself apart from if the validation succeeded. That's 4 different places.

Thanks,

PUK

[PUK_999]

Putting the following call before the SessionStatus.setComplete() works:

            model.remove(MODEL_ATTR_NAME_TIMEZONE_IDS);

But it doesn't make sense. I'm looking at the code to PetClinic in RC3 and the EditPetForm Controller has the following structure (unnecessary methods removed):

@Controller
@RequestMapping("/owners/*/pets/{petId}/edit")
@SessionAttributes("pet")
public class EditPetForm {

	@ModelAttribute("types")
	public Collection<PetType> populatePetTypes() {
		return this.clinic.getPetTypes();
	}

	@RequestMapping(method = RequestMethod.GET)
	public String setupForm(@PathVariable("petId") int petId, Model model) {
		Pet pet = this.clinic.loadPet(petId);
		model.addAttribute("pet", pet);
		return "pets/form";
	}

	@RequestMapping(method = { RequestMethod.PUT, RequestMethod.POST })
	public String processSubmit(@ModelAttribute("pet") Pet pet, BindingResult result, SessionStatus status) {
		new PetValidator().validate(pet, result);
		if (result.hasErrors()) {
			return "pets/form";
		}
		else {
			this.clinic.storePet(pet);
			status.setComplete();
			return "redirect:/owners/" + pet.getOwner().getId();
		}
	}
}

So I wonder why the redirect within processSubmit() doesn't have the model (pet and pet types) appended to the URL?

Thanks,

PUK

[PUK_999]

So I wonder why the redirect within processSubmit() doesn't have the model (pet and pet types) appended to the URL?

A debugging session revealed the answer: There's a RedirectView created to represent the "redirect:/owners/{ownerId}" redirect.

This knows about the model and has methods like isEligibleProperty and isEligibleValue to determine if the model elements can be exposed in the query string.

For PetClinic's EditPetForm, the model contains two entries:
types: a java.util.ArrayList of PetType objects.
pet: an object of type JdbcPet.

Neither of these are eligible because the elements of the array and the pet object are not String or primitive.

Digressing slightly: Even if one wanted to expose the model attributes on a redirect (a requirement which I find dubious), some/all of them might not be eligible!

I'm calling sessionStatus.setComplete() but that still exposes the model, so model.clear() is the sensible way to go. I wish the documentation would state this. Another option would be to create a RedirectView object directly with the constructor that takes the exposeModelAttributes flag.

What I'm doing seems to be the most common use case for form-based Controllers and has taken me ages to understand

Thanks,

PUK

[Marten Deinum]

Digressing slightly: Even if one wanted to expose the model attributes on a redirect (a requirement which I find dubious), some/all of them might not be eligible!

If you want to redirect to a status page which shows the final stuff you need an id to retrieve the, for instance, order from the database to render the page. You would put that in the model so that it can be reused.

return "redirect:/owners/" + pet.getOwner().getId();

This isn't a good sample imho (i'll register a JIRA issue for it) because now there are issues with the caching of the RedirectView, each redirect will now lead to a cached instance of RedirectView because the key is the URL and now it includes the id, if you put it in the model it would be simple /owners and spring would take care of the rest.

The problem, which I overlooked, is the fact that to make the redirection work the SessionStatus.setComplete is actually effective AFTER the processing of the request, so after the redirect. Which is if you look from a framework perspective correct imho.

I agree with you that this should be clearer in the documentation so I suggest you register a JIRA for that so that it can be addressed in the documentation.

[PUK_999]

I agree with you that this should be clearer in the documentation so I suggest you register a JIRA for that so that it can be addressed in the documentation.

Thanks for your response. Created SPR-6595.

PUK

[PUK_999]

return "redirect:/owners/" + pet.getOwner().getId();

This isn't a good sample imho (i'll register a JIRA issue for it) because now there are issues with the caching of the RedirectView, each redirect will now lead to a cached instance of RedirectView because the key is the URL and now it includes the id, if you put it in the model it would be simple /owners and spring would take care of the rest.

The problem, which I overlooked, is the fact that to make the redirection work the SessionStatus.setComplete is actually effective AFTER the processing of the request, so after the redirect. Which is if you look from a framework perspective correct imho.

I've been thinking about the simple redirect to the /owners and then Spring take care of the rest and wanted to raise a couple of things.

This means that just before the redirect, the controller code will need to remove all attributes from the model that it doesn't want to be part of the model for the redirected page, and add in any attributes that it does want to be exposed to the redirect. So rather than having this:

model.clear();
return "redirect:/owners/" + pet.getOwner().getId();

We'll have this:

model.clear();
model.put("petId",pet.getOwner().getId());
return "redirect:/owners";

Is that right? Not too bad but quite indirect.

That then leads to the question - how will Spring know to construct the URL? If it constructed /owners?petId=4 then that would cause a problem for controllers mapped via URI templates. Of course, this could be got around by some UrlRewriteFilter regex magic but I think it's easier to let the developer construct the URL in code.

PUK