Poznámky z code reviews MVC aplikací

ASP.NET MVC má jednu velkou nevýhodu - leccos tam lze udělat hned několika různými způsoby a navíc jsou pravidla psána poměrně obecně a nejsou systémem vynucována. A tak se často sklouzne k tomu, že controller třídy jsou přeplácány metodami a akční metody pak kódem. Přičemž obecné pravidlo je, že by controller měl být co nejjednodušší (to jest přehledný).  Mně se líbí porovnání s dirigentem - ten také jen ukazuje, co má který nástroj kdy hrát, ale sám nehraje na žádný. Podobně i metody v controlleru nemají obsahovat žádný kód navíc než je nezbytně nutné pro řízení provedení požadované operace.Ne vždy se to povede a tak se na pár příkladech pokusím ukázat, jak to napravit - a začnu validacemi.


Ideálně má controller jen vzít vstupní model, zkontrolovat, zda byl v pořádku zkonstruován (ModelState), někam jej posunout ke zpracování a pak vrátit volajícími výsledek těchto operací (View, Json, Content, Redirect apod.). Sám by ale neměl žádné zpracování či validaci provádět. Ne vždy se to ale povede.

Validace v controlleru

V controlleru by validace vstupních dat probíhat neměla. Občas ale programátor řeší neobvyklý problém a tak se validační kód objeví v metodě controlleru. To je i případ následující metody pro upload obrázků - nahrávaný obrázek měl mít maximálně 300kB, maximální rozměry 90*582 px a být typu jpg, gif a nebo jpeg. O tom, jak získat informace o obrázku jsem už na tomto blogu psal, takže to není nutné popisovat znovu (ImageFileDecoder použitý v následujících ukázkách kódu implementuje kód z uvedeného článku).

Tady je kód, jak to tedy na začátku vypadalo v metodě controlleru:

[HttpPost]
public ActionResult UploadOwnerLogo(IEnumerable<HttpPostedFileBase> image, int ownerId)
{
    if (image.IsNullOrEmpty())
        return Content("notImage");

    var imageToBeSaved = image.First();

    var extension = Path.GetExtension(imageToBeSaved.FileName).Substring(1);

    if (!ALLOWED_IMAGE_EXTENSIONS.Any(x => x.Equals(extension, System.StringComparison.OrdinalIgnoreCase)))
        return Content("notImage");

    if (imageToBeSaved.ContentLength > 300 * 1024)
        return Content("tooBigLength");

    var fileName = string.Format("ownerLogo_{0}.jpg", ownerId);

    using (imageToBeSaved.InputStream)
    {
        var imageInfo = ImageInfoDecoder.GetInfo(imageToBeSaved.InputStream);

        if (!ALLOWED_IMAGE_EXTENSIONS.Any(x => x.Equals(imageInfo.Extension, System.StringComparison.OrdinalIgnoreCase)))
            return Content("notImage");

        if (imageInfo.Height > 90 || imageInfo.Width > 582)
            return Content("tooBigPixels");

        // delete old image if present
        this.DeleteOwnerLogo(ownerId);

        var path = Server.MapPath(SiteSetting.OwnerLogoServerPath);

        imageToBeSaved.SaveAs(Path.Combine(path, fileName));
    }

    return Json(new { url = this.GetOwnerLogoUri(fileName)}, "text/plain");
}

Jak to udělat lépe?

První změnou je přesunutí některých operací mimo controller, tedy zejména SaveOwnerLogo a GetOwnerLogoUri - ale to je vcelku jednoduché, lze to řešit singleton třídou OwnerImageProvider.

Druhou změnou je přesunutí validace mimo controller. ASP.NET MVC validuje celý objekt a tak bude nutné založit si novou třídu UploadOwnerImageModel:

public class UploadOwnerImageModel
{
    [IsImageFile(30 * 1024, new[] { "jpg", "jpeg", "gif" }, 90, 582)]
    public IEnumerable<HttpPostedFileBase> Images { get; set; }

    [Required]
    public int OwnerID { get; set; }
}

(poznámka- šlo vy validovat i jednotlivé vstupní parametry metody controlleru, jak na to je popsáno například tady: http://stackoverflow.com/questions/3665655/prevent-mvc-action-method-from-executing-if-a-parameter-is-null - zkráceně je nutné pomocí filtru provést validaci před spuštením metody(akce)).

Na validaci hodnot nabízí ASP.NET MVC validační atributy - ale zatím nevím o žádném, který by validoval uploadovaný souboru. Je nutné si tedy napsat vlastní validátor - což v tomto případě jen znamená, že se  validační kód přesunul z controlleru do třídy IsImageFileAttribute, potomka třídy ValidationAttribute:

public class IsImageFileAttribute : ValidationAttribute
{
    private int maximumSize = 0;
    private int maxHeight = 0;
    private int maxWidth = 0;
    private string[] allowedTypes = null;

    public IsImageFileAttribute(int maximumSize, string[] allowedTypes, int maxHeight, int maxWidth)
    {
        this.maximumSize = maximumSize;
        this.maxHeight = maxHeight;
        this.maxWidth = maxWidth;
        this.allowedTypes = allowedTypes;
    }

    protected override ValidationResult IsValid(object value, ValidationContext validationContext)
    {
        var postedImages = value as IEnumerable<HttpPostedFileBase>;

        if(postedImages.IsNullOrEmpty())
            return new ValidationResult("notImage");

        var postedImage = postedImages.First();

        using (var stream = postedImage.InputStream)
        {
            var imageInfo = ImageInfoDecoder.GetInfo(stream);

            if(stream.Length > this.maximumSize)
            return new ValidationResult("tooBigLength");

            if (!this.allowedTypes.Any(x => x.Equals(imageInfo.Extension, System.StringComparison.OrdinalIgnoreCase)))
                return new ValidationResult("notImage");                    

            if (imageInfo.Height > this.maxHeight || imageInfo.Width > this.maxWidth)
                return new ValidationResult("tooBigPixels");
        }

        return ValidationResult.Success;
    }
}

A metoda v controlleru pak vypadá takto:

[HttpPost]
public ActionResult UploadOwnerLogo(UploadOwnerImageModel ownerImage)
{
    if (ModelState.IsValid)
    {
        var provider = OwnerImageProvider.Instance;
        provider.SaveOwnerLogo(ownerImage.Images.First(), ownerImage.OwnerID);
        return Json(new { url = provider.GetOwnerLogoUri(ownerImage.OwnerID) }, "text/plain");
    }

    var errorMessage = this.ModelState.Values.SelectMany(s => s.Errors).First().ErrorMessage;
    return Content(errorMessage);
}