MVC2: Fat model, skinny controller

Ik schrijf regelmatig blogs met als uitgangspunt het oplossen van een
broncode probleemgeval. Maar dit keer wil ik vanuit een bestaande oplossing een verbetering aandragen.

Scott Hanselman riep het al tijdens zijn presentatie op de DevDays.
Daar heeft hij een presentatie gegeven met 32 manieren zodat je blog ‘sucks less’.

Op 16 staat de Code Garage Sale. En daarom gooi ik hier mijn verbeterde Model
validatie in de aanbieding.

Iedereen die vanuit Visual Studio 2010 een nieuwe (gevulde) MCV2
applicatie laat genereren krijgt daarbij al een aardig Account model mee. En
hierin zitten heel wat goodies want MVC2 is gebaseerd op het patroon Fat model, skinny controller (dus niet dit http://en.wikipedia.org/wiki/The_Fat_Controller).

Fat and Skinny

Dit betekent zoveel als dat alle mogelijke businesslogica uit de
controllers gehouden wordt. Dus de controlers worden alleen gebruikt voor het
doorgeven van modeldata of viewdata aan views en het weer doorgeven van de
wijzigingen uit de views aan het model. Zelfs het valideren van wijzigingen in
het model valt eigenlijk buiten de verantwoording van de controller. De
controller roept gewoon de validatie aan…

Dit aanroepen van de validatie ziet er voor de controller dan zo
uit:

[HttpPost]public ActionResult Register(RegisterModel model)
{
  if (ModelState.IsValid)
  {
    ...
  }
}

Ahum, waar blijft die validatie logica dan wel? In een eerdere blog heb
ik al eens gerefereerd naar een repository http://oud.adi.atosoriginblog.nl/2010/02/22/apsnet-mvc2-Met-eenvoudig-testbare-database-laag . Maar aan de basis van MVC2 staat het model en daar kan een heleboel validatie logica landen.

Kijk maar eens naar de RegisterModel class. Wat direct opvalt zijn alle extra attributen, waaronder die voor de validaties:

[PropertiesMustMatch("NewPassword", "ConfirmPassword",
ErrorMessage =
"The new password and confirmation password do not match.")]
public class RegisterModel
{
  ...

  [Required]
  [ValidatePasswordLength]
  [DataType(DataType.Password)]
  [DisplayName("Password")]
  public string Password { get; set; }

  [Required]
  [DataType(DataType.Password)]
  [DisplayName("Confirm password")]
  public string ConfirmPassword { get; set; }
}

Dit zijn meestal validaties op een specifiek veld, zoals het verplicht
zijn of dat de vulling aan een bepaalde vulling moet voldoen. Dit betekent wel
dat nu op alle controllers dezelfde validaties afgedwongen worden en dat overal
dezelfde foutmelding aan de gebruikers getoond wordt. Dit is cool. Dit is
handig. Dit is wat ik altijd al nodig had.

Maar het meest in het oog springende was voor mij het attribuut op de
class zelf. Hier worden namelijk twee velden onderling vergeleken. Hier wordt
het gebruikt om een correct wachtwoord af te dwingen. Maar dit is ook handig om af te dwingen dat bv. een startdatum voor een einddatum ligt.

Ok. Dit is dus de oplossing. Maar hoe zit het met de verbetering die
ik aan het begin van deze blog heb beloofd?

Mijn bezwaar voor de geboden oplossing is dat deze heel specifiek
maar één oplossing biedt. Zo’n attribuut kan met een kleine wijziging zowel voor
het vergelijken van gelijke als ongelijke velden gebruikt kan worden. Ik heb het
attribuut iets aangepast en kom dus met een ‘verbeterde’ versie. Deze wordt dan
zo gebruikt voor het vergelijken van velden die gelijk moeten zijn:

[CompareTwoStringFieldsForEquality("NewPassword",
  "ConfirmPassword", true, ErrorMessage =
  "The new password and confirmation password do not match.")]

En voor velden die ongelijk moeten zijn, zal de declaratie zo
zijn:

[CompareTwoStringFieldsForEquality("EmailOfManagerOne","EmailOfManagerTwo",
  false, ErrorMessage =
  "The two email accounts of the managers must be different." )]

Om dit te bereiken heb ik een extra boolean als eigenschap opgenomen die
beschrijft of de validatie op gelijkheid of ongelijkheid moet plaatsvinden. Ook
is er nog een correctie op de tekst van de foutmelding nodig. Maar de tekst kan
ook zoals hierboven vervangen worden met een meer beschrijvende tekst.

[AttributeUsage(AttributeTargets.Class,
                 AllowMultiple = true, Inherited = true)]
public sealed class CompareTwoStringFieldsForEqualityAttribute :
                                              ValidationAttribute
{
  private const string _defaultErrorMessage = "'{0}' is {2} '{1}' ";
  private readonly object _typeId = new object();
  public CompareTwoStringFieldsForEqualityAttribute(string firstProperty,
     string secondProperty, bool testForEquality) : base(_defaultErrorMessage)
  {
    SecondProperty = secondProperty;
    FirstProperty = firstProperty;
    TestForEquality = testForEquality;
  }
  public bool TestForEquality { get; private set; }
  public string FirstProperty { get; private set; }
  public string SecondProperty { get; private set; }
  public override object TypeId
  {
    get
    {
      return _typeId;
    }
  }
  private string _TextEqual = "the same as";
  public string TextEqual
  {
    get
    {
      return _TextEqual;
    }
    set
    {
      _TextEqual = value;
    }
  }
  private string _TextDifferent = "different from";
  public string TextDifferent
  {
    get
    {
      return _TextDifferent;
    }
    set
    {
      _TextDifferent = value;
    }
  }
  public override string FormatErrorMessage(string name)
  {
    string text = string.Empty;
    if (TestForEquality)
    {
      text = TextDifferent; // twist
    }
    else
    {
      text = TextEqual; // twist
    }
    return String.Format(CultureInfo.CurrentUICulture,
        ErrorMessageString, FirstProperty, SecondProperty, text);
  }
  public override bool IsValid(object value)
  {
    // ignoreCase
    PropertyDescriptorCollection properties =
      TypeDescriptor.GetProperties(value);
    object secondValue = properties.Find(SecondProperty,
      true).GetValue(value);
    object firstValue = properties.Find(FirstProperty,
      true).GetValue(value);
    // test only if both fields are filled
    if (secondValue == null || firstValue == null)
    {
      return true;
    }
    string secondString = (string)secondValue;
    string firstString = (string)firstValue;
    if (TestForEquality)
    {
      return firstString == secondString;
    }
    else
    {
      return firstString != secondString;
    }
  }
}

Dit ziet er prima uit. Maar zal het ook werken? Om dit te bewijzen heb ik
ook een aantal unittests toegevoegd:

class CompareUnitTestModel{
  public string FieldOne { get; set; }
  public string FieldTwo { get; set; }
}
[TestClass]
public class CompareTwoStringFieldsForEqualityAttributeTest
{
  [TestMethod]
  public void Compare_Equallity_Succeeds()
  {
    // Arrange
    CompareUnitTestModel modelToTest =
        new CompareUnitTestModel()
        { FieldOne = "a", FieldTwo = "a" };
    CompareTwoStringFieldsForEqualityAttribute
      compareTwoStringFieldsForEquality =
        new CompareTwoStringFieldsForEqualityAttribute(
          "FieldOne", "FieldTwo", true);
    // Act
    bool expected = true;
    bool actual = compareTwoStringFieldsForEquality.IsValid(modelToTest);
    // Assert
    Assert.AreEqual(expected, actual);
  }
  [TestMethod]
  public void Compare_Is_Different_Succeeds()
  {
    // Arrange
    CompareUnitTestModel modelToTest =
      new CompareUnitTestModel() {
        FieldOne = "a", FieldTwo = "b" };
    CompareTwoStringFieldsForEqualityAttribute
      compareTwoStringFieldsForEquality =
        new CompareTwoStringFieldsForEqualityAttribute(
          "FieldOne", "FieldTwo", false);
    // Act
    bool expected = true;
    bool actual = compareTwoStringFieldsForEquality.
                                        IsValid(modelToTest);
    // Assert
    Assert.AreEqual(expected, actual);
  }
  [TestMethod]
  public void Compare_Is_Different_Fails_Standard_Message()
  {
    // Arrange
    CompareUnitTestModel modelToTest =
       new CompareUnitTestModel() {
           FieldOne = "a", FieldTwo = "a" };
    CompareTwoStringFieldsForEqualityAttribute
      compareTwoStringFieldsForEquality =
        new CompareTwoStringFieldsForEqualityAttribute(
          "FieldOne", "FieldTwo", false);
    // Act
    bool expected = false;
    bool actual = compareTwoStringFieldsForEquality.IsValid(modelToTest);
    // Assert
    Assert.AreEqual(expected, actual);
    string errorText =
      compareTwoStringFieldsForEquality.FormatErrorMessage("");
    Assert.AreEqual("'FieldOne' is the same as 'FieldTwo' ", errorText);
  }
}

De drie testen maken hier gebruik van een hulp class CompareUnitTestModel
welke een model class moeten voorstellen.

Ik wil je graag wijzen op de unittests welke bij een MVC2 applicatie meegeleverd kunnen worden. Hier staan een heleboel goodies in, ook over andere unittesten met attributen. Eigenlijk zou altijd alle code begeleid moeten gaan met unittests. Naast het bewijzen dat het werkt, levert het ook een aardige inkijkje over hoe de code gebruikt moet worden. Helaas is dit laatste voor Attributes iets minder van
toepassing.

Ik heb mijn uitgebreidere validatie in de uitverkoop gedaan. En wellicht leidt dit tot inspiratie voor het schrijven van andere validaties zodat jouw model slimmer en ‘fatter’ wordt en alle controllers die kennis kunnen delen. Want zelf validaties schrijven is gewoon heel eenvoudig. En je kunt ze eerst zelf rustig unittesten zonder steeds visueel de schermen te controleren.