views:

309

answers:

3

I am designing a new dynamic site from a static site. I have the route all sorted but I have a question on my Action method.

Below is the code but when testing and looking at the headers that Firebug reports, if I take out the Response.End it is a 302 redirect I assume because I set the 301 but then call another action which makes it a 302, but if I put in the Response.End I get a 301.

I am guessing that adding the Response.RedirectLocation is actually doing the 301 redirect so do I therefore change my return value to EmptyResult or null even though that line of code will never get executed just so the app compiles?

public ActionResult MoveOld(string id)
    {
        string pagename = String.Empty;
        switch (id)
        {
            case "2":
                pagename = WebPage.SingleOrDefault(x => x.ID == 5).URL;
                break;
        }

        Response.StatusCode = 301;
        Response.StatusDescription = "301 Moved Permanently";
        Response.RedirectLocation = pagename;
        Response.End();

        return RedirectToAction("Details", new { pageName = pagename });
    }
A: 

Could you use a custom IRouteHandler as noted in this blog post?

Patrick Steele
+1  A: 

The controller should not be responsible for setting the 301 and redirect location. This logic should be encapsulated within an ActionResult, and the controller should return an instance of that ActionResult. Keep in mind that the method Response.End() does not return (it throws an exception); lines that follow it will not execute.

Levi
+5  A: 

I echo Levi's comments. This is not the job of the controller. I have tended to use this custom ActionResult for 301's. Below is a modified version with more options.

public class PermanentRedirectResult : ActionResult
{
  public string Url { get; set; }

  public PermanentRedirectResult(string url)
  {
    this.Url = url;
  }

  public PermanentRedirectResult(RequestContext context, string actionName, string controllerName)
  {
    UrlHelper urlHelper = new UrlHelper(context);
    string url = urlHelper.Action(actionName, controllerName);

    this.Url = url;
  }

  public PermanentRedirectResult(RequestContext context, string actionName, string controllerName, object values)
  {
    UrlHelper urlHelper = new UrlHelper(context);
    string url = urlHelper.Action(actionName, controllerName, values);

    this.Url = url;
  }

  public PermanentRedirectResult(RequestContext context, string actionName, string controllerName, RouteValueDictionary values)
  {
    UrlHelper urlHelper = new UrlHelper(context);
    string url = urlHelper.Action(actionName, controllerName, values);

    this.Url = url;
  }

  public override void ExecuteResult(ControllerContext context)
  {
    if (context == null)
    {
      throw new ArgumentNullException("context");
    }
    context.HttpContext.Response.StatusCode = 301;
    context.HttpContext.Response.RedirectLocation = Url;
    context.HttpContext.Response.End();
  }
}

Usage in the action

//Just passing a url that is already known
return new PermanentRedirectResult(url);

or

//Redirect to a different controller/action
return new PermanentRedirectResult(this.ControllerContext.RequestContext, "ActionName", "ControllerName");
Dan Atkinson
Thanks but like Levi says calling Response.End throws an exception, is that valid?
Jon
Without executing your code, I guess that the error is something akin to `Cannot redirect after HTTP headers have been sent`. That's because you're sending a response, and then redirecting the user after you've sent the response. `Response.End();` should be the last thing you do.
Dan Atkinson
This is because in the code sample I have given you, the PermanentRedirectResult **IS** the ActionResult. In effect, in your code, you've got two ActionResults.
Dan Atkinson
I realise my code is calling it twice I was just questioning the Response.End in your example. Do you need to call it as I believe setting the header will do what is required. I'm just wondering whether its needed if it throws an exception
Jon
I don't think Levi is referring to my example, but yes, it's needed in order to send the buffered content. I don't know what happens if you remove it.
Dan Atkinson
All works well, thanks! However I don't actually see a 301 in Firebug at all, is this a concern? Will Search Engines recognise the 301 and index properly?
Jon
Hmm... Yes, a 301 should be in the headers. There's absolutely no reason in the code that I've given you that a 301 won't be returned.
Dan Atkinson
I think it probably is because the page loads on the new url but I'm just not seeing that a 301 redirect has taken place but it obviously is
Jon