ASP.NET MVC 3 MusicStore Tutorial

[dasyad]

Been working my way through the MVC MusicStore tutorial and I'm really enjoying it to be honest.

I've been playing with routing and managed to get URLs like "/Store/Rock" working as opposed to "/Store/Browse?Genre=Rock", via this:

public static void RegisterRoutes(RouteCollection routes)
{
 routes.IgnoreRoute("{resource}.axd/{*pathInfo}");

 routes.MapRoute(
  "Genre",
  "Store/Browse/{genre}",
  new { controller = "Store", action = "Browse" }
 );

 routes.MapRoute(
  "Default", // Route name
  "{controller}/{action}/{id}", // URL with parameters
  new { controller = "Home", action = "Index", id = UrlParameter.Optional }
  // Parameter defaults
  );

}

Now if I put an invalid genre into the URL it will crash, because the StoreController always assumes that a genre is valid. So I was wondering what is the accepted method of validating GET parameters in MVC?

I was thinking it might be one of the following:

1. Check if genre GET is valid, if not redirect to the Index action and display the /Store/Index View, else carry on.
2. Check if genre GET is valid, if not redirect to an "error" View, else carry on.
3. Check if genre GET is valid, if not display an error within the /Store/Browse View we are attempting to display.
4. Something else.

 

[GravyMonster]

You should be looking to validate the parameters within your action method. First checking the basics like !string.isnullorempty, then attempting to find the related record in your database. If none are found, then redirect to a 404. I'm guessing its that last step you're missing.

The alternative is to add a route constraint, however I think that'd be overkill in this case.

 

[Xenoxide]

I was thinking it might be one of the following:

1. Check if genre GET is valid, if not redirect to the Index action and display the /Store/Index View, else carry on.
2. Check if genre GET is valid, if not redirect to an "error" View, else carry on.
3. Check if genre GET is valid, if not display an error within the /Store/Browse View we are attempting to display.
4. Something else.

Out of those options I would say #3.

Typically if something doesn't exist (ie. the genre the user requested), the browser should be told it doesn't exist. Therefore it should send back a 404 error code on the page that was requested.

If you were to redirect back to the index page, or to display an error on another page, you would need to send a 302 redirection, which suggests to the browser the content does exist, it's just somewhere else.

 

[dasyad]

Okay so really I need to redirect to a 404 to tell the browser (or anything else!) that the page does not exist.

For now I'll just make it redirect to the Index Action so that it displays a valid list of selectable genres, I want to go to bed so redirecting to an error can be tomorrows lesson

Anyway I went from this:

//
// GET: /Store/

public ActionResult Index()
{
	var genres = storeDB.Genres.ToList();
	return View(genres);
}

//
// GET: /Store/Browse?genre=Disco
public ActionResult Browse(string genre)
 {
    // Retrieve Genre and its Associated Albums from database
    var genreModel = storeDB.Genres.Include("Albums")
        .Single(g => g.Name == genre);
 
    return View(genreModel);
 }

To this:

//
// GET: /Store/

public ActionResult Index()
{
	var genres = storeDB.Genres.ToList();
	return View(genres);
}

//
// GET: /Store/Browse?genre=Disco
public ActionResult Browse(string genre)
{
        //Check if null or empty, saves a trip to the database
	if (string.IsNullOrWhiteSpace(genre))
	{
		return RedirectToAction("Index");
	}
	else
	{
		// Retrieve Genre and its Associated Albums from database
		var genreModel = storeDB.Genres.Include("Albums")
		.SingleOrDefault(g => g.Name == genre);

                //Genre supplied was not found in the database
		if (genreModel == null)
			return RedirectToAction("Index");

		return View(genreModel);
	}
}