RedirectToAction and Reference Types

[Brian]

Hello,

I've got a design decision question for you regarding
RedirectToAction. I have several controller methods that accept model
objects as parameters. There is a filter that runs to populate these
parameters via query string/route lookup based on a slug value. In the
MVCContrib implementation of RedirectToAction, there is a method
called RemoveReferenceTypesFromRouteValues which strips these values
from the generated RedirectToActionResult.

Why was this determination made? The comment on the method indicates
that you can override this behavior by providing an overridden
implementation of ToString but the method's logic doesn't actually do
anything like this:

/// If you really wanted to have a reference type in the route value
dictionary,
/// you should override ToString() in the object and have it return
something
/// meaningful that could be added to the route value dictionary. If
you do that,
/// this method will see the route value as a string and will not
strip it out.

I've resorted to my own forked version of RedirectToAction that
doesn't call this method. I'd be open to providing a patch with an
alternative means of providing both behaviors but I'm curious of the
logic here.

Brian

[Jeremy Skinner]

Hi Brian,

I do not recall the decision behind this - the RedirectToAction<T> extensions were provided as a patch quite a long time ago and I haven't really looked at them since.

You're right about the ability to override ToString - normally, this would be handled by the RedirectToRouteResult internally but RemoveReferenceTypesFromRouteValues removes them from the dictionary before it can get this far, so it never happens. In my opinion, reference types should not be removed from the RouteValueDictionary.

RedirectToAction<T> also has a dependency on TempData which is not something that I'm happy with. I think the idea behind this is that calling RedirectToAction acts like a method call so that all the parameters that you pass to the expression are serialized to tempdata and then at the other end of the redirect they are deserialized back into action parameters. This way, you don't have to worry about how complex types are represented in the URL.

However, I do not think this is a particularly helpful abstraction - it is inconsistent with the MVC framework's built-in overloads for RedirectToAction/RedirectToRoute and is not an accurate representation of what happens during an HTTP redirect.

I propose that we introduce a new class - Redirect<T> which inherits from RedirectToRouteResult and encapsulates the expression parsing logic. This should not have a dependency on TempData, nor should it discard reference types. For backwards compatibility, I think the PassParameterDuringRedirect filter could be modified to encapsulate all of the tempdata-related tasks so anyone relying on this behaviour can still use it.

Any other thoughts would be welcome

Thanks

Jeremy

2009/10/12 Brian <br...@accessity.net>

[Brian]

Jeremy,

Thanks for responding. I actually hadn't seen the
PassParametersDuringRedirect filter. If I'm reading things correctly
it looks like the original intent was to apply this attribute along
with the RedirectToAction call in order to get the behavior of passing
parameters via the expressions. This feels sloppy to me. If the
attribute is missing redirects will fail to pass parameters
introducing a non-obvious bug. The filter could be applied in a base
controller but I don't love that approach either.

The Redirect<T> class does seem much more flexible and could
potentially introduce some fluent methods to still provide the
PassParameters functionality if people really wanted to call via that
API.

[Jeremy Skinner]

I've finally got round to implementing this.

I've added a RedirectToRouteResult<T> class that encapsulates the expression parsing logic. I've also moved all of the code that depends on TempData into the PassParametersDuringRedirect filter, so normal uses of RedirectToAction<T> do not store anything in TempData.

Jeremy

2009/10/12 Brian <br...@accessity.net>

[Eric Hexter]

Is this a branch or trunk feature?

[Jeremy Skinner]

I added it to master and also merged it into the mvc2 branch.

Jeremy

2009/10/31 Eric Hexter <eric....@gmail.com>

[Eric Hexter]

I would like to officially nominate Jeremy for MVC.. Most Valuable Committer 2009 .  sorry there is not cash prize to go along with this great distinction....

Eric Hexter

Principal Consultant | Headspring Systems | www.HeadspringSystems.com
email | ehe...@HeadspringSystems.com
blog | http://Hex.LosTechies.com
info | http://www.linkedin.com/in/erichexter

[Jeremy Skinner]

haha, I shall treasure this award forever and add it to my email signature ;)

2009/10/31 Eric Hexter <eric....@gmail.com>