ASP.NET MVC: What part of controller action logic should we/should not move to attributes?

Let's assume that we have a controller with some action which could look like:

    [HttpPost, Authorize]
    public ActionResult Update(Guid itemId)
    {
        var item  = repository.GetById(itemId);
        if (item == null)
            return NotFoundResult();

        //Do something with item

        return View();
    }

We already have applied an Authorize attribute to be sure that only some specific users can perform our action.

We could also move the following block

        var item  = repository.GetById(itemId);
        if (item == null)
            return NotFoundResult();

to another attribute and apply it to our action.

There are another actions where we could extract some specific pieces of logic to attribute and apply them to our actions again and again.

Here comes my question: when should we do it and when we should not? Is it actually a good thing to move such kind of logic to action method attributes?

I've came to this question when I was reviewing unit tests for some project. I was actually looking for code documentation and it was strange for me not seeing what should controller action do if, for开发者_开发知识库 example, item was not found. I've found this part in attributes unit tests, but is this actually attribute's responsibility?

I can understand Authorize attribute, it is actually another layer, but what about the logic that I've described above? Is it controller's action responsibility to work with it or ..?

Thanks in advance for any comments :)

If you want to run logic like this for a group of actions, you can override the OnActionExecuting method to perform the check.

Creating an attribute would be difficult, as you would also need to create a base controller and write some reflection code to inspect the action method to see if the attribute exists, then act on it.

protected override void OnActionExecuting(ActionExecutingContext ctx) 
{
    if(!ctx.ActionDescriptor.ActionName.StartsWith("Create"))
    {
        var item  = repository.GetById(itemId);
        if (item == null)
            ctx.Result = NotFoundResult();
    }
}

Attributes are used for various things in ASP.net MVC (for example filtering or exception handling). I would however not move logic like this to attributes as this is basic logic for the action you're executing. Moving this code to attributes will only make it harder to find what the action is actually doing.