"L'utente può fare X se l'utente possiede l'oggetto Y": implementare la logica nella convalida del modello o nella logica del controller?

Question

Considera, ad esempio, la logica "Un utente può solo modificare o eliminare un commento che l'utente ha creato".

Le azioni del mio controller ripeteranno la logica per controllare se l'utente attualmente connesso può influenzare il commento.Esempio

[Authorize]
public ActionResult DeleteComment(int comment_id)
{
    var comment = CommentsRepository.getCommentById(comment_id);
    if(comment == null) 
        // Cannot find comment, return bad input
        return new HttpStatusCodeResult(400); 
    if(comment.author != User.Identity.Name)
        // User not allowed to delete this comment, return Forbidden
        return new HttpStatusCodeResult(403);
    // Error checking passed, continue with delete action
    return new HttpStatusCodeResult(200);
}

Naturalmente, posso raggruppare quella logica in un metodo in modo da non copiare / incollare quello snippet;tuttavia, togliendo quel codice dal controller e inserendolo in un ValidationAttribute, la mia azione rimane più piccola e più facile per cui scrivere test.Esempio

public class MustBeCommentAuthorAttribute : ValidationAttribute
{
    // Import attribute for Dependency Injection
    [Import]
    ICommentRepository CommentRepository { get; set; }

    protected override ValidationResult IsValid(object value, ValidationContext validationContext)
    {
        int comment_id = (int)value;
        var comment = CommentsRepository.getCommentById(comment_id);
        if(comment == null) 
            return new ValidationResult("No comment with that ID"); 
        if(comment.author != HttpContext.Current.User.Identity.Name)
            return new ValidationResult("Cannot edit this comment");
        // No errors
        return ValidationResult.Success;
     }
}

public class DeleteCommentModel
{
    [MustBeCommentAuthor]
    public int comment_id { get; set; }
}

La convalida del modello è lo strumento giusto per questo lavoro?Mi piace togliere questa preoccupazione dall'azione del controller;ma in questo caso può complicare ulteriormente le cose.Ciò è particolarmente vero se si considera che questa azione fa parte di un'API RESTful e deve restituire un codice di stato HTTP diverso a seconda degli errori di convalida in ModelState.

Esiste una "best practice" in questo caso?

Answer 1

Personalmente, penso che sia carino, ma ti stai lasciando trasportare dalle annotazioni.Penso che questo non appartenga al tuo livello di presentazione e dovrebbe essere gestito dal tuo livello di servizio.

Vorrei qualcosa sulla falsariga di:

[Authorize] 
public ActionResult DeleteComment(int comment_id) 
{ 
    try
    {
       var result = CommentsService.GetComment(comment_id, Auth.Username);

       // Show success to the user
    }
    catch(Exception e)
    {
       // Handle by displaying relevant message to the user
    }
}