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.

Advertentie

Een MS Unit DeploymentItem is soms een beetje schizofreen

De in Visual Studio standaard meegeleverde Unittest library heeft een
mindere naam op het gebied van unittesten. Gebruikers van andere unittest-tools zoals Nunit geven graag af op MSUnit maar ik vind dit niet terecht. Zelf ben ik na veelvuldig gebruik nog niet tegen specifieke beperkingen in de mogelijkheden aangelopen. Het feit dat het volledig in Visual Studio geïntegreerd is (..) en de standaard keuze binnen Microsoft is (..) maakt heel veel goed op de (verborgen) nadelen.

Het zal wel koudwatervrees zijn omdat het allemaal net iets anders
http://blogs.msdn.com/nnaderi/archive/2007/02/01/mstest-vs-nunit-frameworks.aspx  opgelost is. Maar soms zijn er zaken die je gewoon even moet weten voordat je het succesvol kunt toepassen.

Schizofreen

Zo wilden we voor ons project, waarbij heel veel met Word documenten
geschoven wordt, juist unittesten met specifieke testdocumenten toepassen. Dit
is namelijk mogelijk via het DeploymentItem attribuut. Maar voordat je dit goed
kunt toepassen, moet je wel op de hoogte zijn van enkele scherpe randjes.

Je begint dus met het opnemen van een document in je project.

Doc document aan project toegevoegd

Deze wil je refereren in de code met een relatief pad. En deze wil je ook
met het relatieve pad weer uitlezen.

[TestMethod][DeploymentItem("\\Submap1\\Test1.doc")]
public void TestMethod1()
{
  FileStream fileStream = new FileStream("\\Submap1\\Test1.doc",
    FileMode.Open);
  BinaryReader binaryReader = new BinaryReader(fileStream);
  long length = new FileInfo("\\Submap1\\Test1.doc").Length;
  byte[] byteArray = binaryReader.ReadBytes((int) length);
  Assert.IsNotNull(byteArray);
  Assert.AreNotEqual(0, length);
}

Dit ziet er goed uit. En het compileert. Maar bij het draaien van de
unittest komt een rood bolletje tevoorschijn.

Foutmelding, bestand niet aangetroffen

De melding liegt er niet om:

Test method TestProject1.UnitTest1.TestMethod1 threw exception:System.IO.DirectoryNotFoundException: Could not find a part of the path ‘c:\Submap1\Test1.doc’.

Het bestand ontbreekt? Maar het is toch netjes gekoppeld? Nou, nee. Om
dit op te lossen moeten er drie problemen gefixed worden:

  • 1. Van het te deployen document moet de property “Copy to Output Directory” ongelijk “Do not copy” zijn. (Dit heeft overigens niets met de “Build Action” te maken)

Property, copy to output directory: Copy Always

  • 2. Bij het DeploymentItem attribuut moet een submap opgegeven worden. Alle deployment item worden namelijk standaard slechts in de root van de output test map geplaatst. Een opgegeven submap met (hier eenzelfde naam) wordt door de unittest zeer gewaardeerd.
[TestMethod][DeploymentItem("Submap1\\Test1.doc", "Submap1")]
public void TestMethod1()
{
  FileStream fileStream = new FileStream("Submap1\\Test1.doc",
    FileMode.Open);
  BinaryReader binaryReader = new BinaryReader(fileStream);
  long length = new FileInfo("Submap1\\Test1.doc").Length;
  byte[] byteArray = binaryReader.ReadBytes((int) length);
  Assert.IsNotNull(byteArray);
  Assert.AreNotEqual(0, length);
}
  • 3. In Visual Studio 2005 zouden bovenstaande stappen al voldoende geweest zijn. Maar in Visual Studio 2010 faalt de unittest nog steeds na bovenstaande uitbreidingen. Nader onderzoek wijst de dader aan: de “Deployment” is nog niet actief.

Foutmelding, deployment is nog niet actief

Open hiervoor de Local.testsettings binnen de solution Items (via een dubbelclick). Daar moest namelijk nog even het vinkje bij Deployment aangezet worden.

Deployment aangevinkt

En als de unittest nu nog eens uitgevoerd wordt, verschijnt er een fraai groen bolletje snot.

Groen bolletje

Ok, dit was even wat uitzoekwerk. En hiermee kunnen nu hele fraaie reeksen van unittesten met bestanden uitgevoerd worden. Maar ik wil jullie ook even op een ander gedrag wijzen. Kijk eens naar de volgende situatie. Hier zijn binnen het testproject meerdere uit te rollen bestanden met eenzelfde naam aanwezig.

Een tweede document met dezelfde naam

Er zijn vervolgens twee bijna gelijke testen met ieder een verwijzing naar een ander bestand maar met dezelfde bestandsnaam.

[TestMethod][DeploymentItem("Submap1\\Test1.doc", "Submap")]
public void TestMethod1()
{
  FileStream fileStream = new FileStream("Submap\\Test1.doc",
    FileMode.Open);
  BinaryReader binaryReader = new BinaryReader(fileStream);
  long length = new FileInfo("Submap\\Test1.doc").Length;
  byte[] byteArray = binaryReader.ReadBytes((int) length);
  Assert.IsNotNull(byteArray);
  Assert.AreNotEqual(0, length);
}
[TestMethod]
[DeploymentItem("Submap2\\Test1.doc", "Submap")]
public void TestMethod2()
{
  FileStream fileStream = new FileStream("Submap\\Test1.doc",
    FileMode.Open);
  BinaryReader binaryReader = new BinaryReader(fileStream);
  long length = new FileInfo("Submap\\Test1.doc").Length;
  byte[] byteArray = binaryReader.ReadBytes((int)length);
  Assert.IsNotNull(byteArray);
  Assert.AreNotEqual(0, length);
}

Beide unittesten kunnen in afzonderlijke testruns prima werken. Maar
helaas kunnen ze niet direct achter elkaar in eenzelfde testtun uitgevoerd
worden.

Tweede test failt

De melding hierachter is de volgende:

Test method TestProject1.UnitTest1.TestMethod2 threw exception:
System.IO.IOException: The process cannot access the file ‘c:\documents and
settings\nl26742\my documents\visual studio
2010\Projects\TestProject1\TestResults\NL26742_NL462P13J 2010-04-15
21_42_58\Out\Submap1\Test1.doc’ because it is being used by another process.

Tijdens het uitvoeren van de tweede test is het in te lezen bestand dus al in gebruik? Laten we eens op de lokatie gaan kijken. Ik zie daar maar één bestand. Hé, beide unittesten gebruiken bestanden met dezelfde benaming. Het blijkt dat zij beiden moeten strijden voor hetzelfde plaatsje in de map. Helaas verliest altijd de laatste.

Bestanden met dezelfde naam verdringen elkaar

En dus blijkt de reden van het falen van de tweede unittest, de eerste unittest te zijn. In beide testen is de Close() methode van de FileStream vergeten. Deze eerste unittest houdt de lock vast op het eerste deployment bestand, zelfs nadat deze test succesvol is uitgevoerd.

Dus is een oplossing simpel:

[TestMethod][DeploymentItem("Submap1\\Test1.doc", "TestMethod1")]
public void TestMethod1()
{
  FileStream fileStream = new FileStream("TestMethod1\\Test1.doc",
    FileMode.Open);
  try
  {
    BinaryReader binaryReader = new BinaryReader(fileStream);
    long length = new FileInfo("TestMethod1\\Test1.doc").Length;
    byte[] byteArray = binaryReader.ReadBytes((int)length);
    Assert.IsNotNull(byteArray);
    Assert.AreNotEqual(0, length);
  }
  finally
  {
    fileStream.Close();
  }
}

De close is toegevoegd. Dit is inderdaad fraaie code en een hele verbetering.
Toch geef ik de voorkeur aan nog een andere verbetering.

Om te forceren dat deployment items van unittest met eenzelfde naam
elkaar nooit in de weg kunnen zitten, stel ik voor om voortaan altijd bij het
DeploymentItem attribuut de submap in te vullen op een specifieke manier:

[TestMethod][DeploymentItem("Submap1\\Test1.doc", "UnitTest1\\TestMethod1")]
public void TestMethod1()
{
  FileStream fileStream = new FileStream("UnitTest1\\TestMethod1\\Test1.doc",
    FileMode.Open);
  BinaryReader binaryReader = new BinaryReader(fileStream);
  long length = new FileInfo("UnitTest1\\TestMethod1\\Test1.doc").Length;
  byte[] byteArray = binaryReader.ReadBytes((int) length);
  Assert.IsNotNull(byteArray);
  Assert.AreNotEqual(0, length);
}
[TestMethod]
[DeploymentItem("Submap2\\Test1.doc", "UnitTest1\\TestMethod2")]
public void TestMethod2()
{
  FileStream fileStream = new FileStream("UnitTest1\\TestMethod2\\Test1.doc",
    FileMode.Open);
  BinaryReader binaryReader = new BinaryReader(fileStream);
  long length = new FileInfo("UnitTest1\\TestMethod2\\Test1.doc").Length;
  byte[] byteArray = binaryReader.ReadBytes((int)length);
  Assert.IsNotNull(byteArray);
  Assert.AreNotEqual(0, length);
}

Binnen een unittest project kunnen namelijk meerdere unittest methodes in
verschillende unittest classes voorkomen met eenzelfde method naam. Met
bovenstaande dubbele submap-verwijzing, een combinatie van unittest class naam en method naam, wordt het dubbel uitlezen voorkommen.

Twee groene bolletjes

En zo zullen de unittesten voortaan weer netjes groen kleuren.

Liever lui dan moe: combineer MVC2 met Dynamic Data Web App

Bij mijn huidige MVC2 project zijn we in de testfase aanbeland. En we zitten
nu met een dilemma.

Hoewel MVC2 vooral voor CRUD applicaties toegepast zal worden, hoeft dit nog niet te betekenen dat ook alle vier de acties: create, read, update en delete geïmplementeerd gaan worden. De daadwerkelijke functionaliteit van de applicatie bepaalt namelijk wat er met de data gebeurt.

Lui

Dit maakt het functioneel testen dus niet eenvoudiger. Een applicatie welke alleen invoegen, wijzigen en tonen ondersteunt is heel erg cru (..) voor de tester. Hoe kan de functionaliteit getest worden rond het representeren van een lege selectie of tabel? Wat zal een gridview tonen: “geen data aanwezig”? Of zal er een foutmelding getoond worden? Dit is lastig te testen zonder dat de mogelijkheid bestaat om ook eens een record te verwijderen. Hetzelfde geldt voor het wijzigen van records waarvoor alleen invoeg-functionaliteit is geboden.

Daarom heb ik even een onderzoekje gedaan of ik een Dynamic Data Web
Application (DDWA) hiervoor kom toepassen. Ik heb hier in het verleden al eens over geblogd en dit leek mij wel weer een poging waard.

Nou, om kort te gaan, het kan en het is bijzonder eenvoudig.

Open dus eerst de VS2010 solution met daarin jouw MVC2 applicatie.
Voeg dan een DDWA project toe, in dit geval eentje voor het Entity
framework.

Voeg DDWA toe

In de meeste MVC2 applicaties is de Entity Framework context binnen
die applicatie gecreëerd. Voeg dus een referentie toe van het MVC2 project aan
het DDWA project.

Onthoudt nog even wat de naam is van de context. In mijn geval was
dat “DatabaseEntities”.

In de global.asax van moet vervolgens die regel uit het commentaar
gehaald worden en voorzien worden van de context naam:


DefaultModel.RegisterContext(typeof(DatabaseEntities), new
ContextConfiguration() { ScaffoldAllTables = true });

Standaard staat ScaffoldAllTables op false. Zet deze op true. Dit
forceert dat alle tabellen standaard beschikbaar zijn voor onderhoud via deze
DDWA applicatie.

Als laatste moet wel dezelfde database aangeroepen worden. Kopieer
dus de Entity Framework connectionstring uit de MVC2 applicatie en plaats deze
in de DDWA web.config:

<connectionStrings>

<add    

name=”DatabaseEntities”

connectionString=”metadata=res://*/Models.Model1.csdl|res://*/Models.Model1.ssdl|

res://*/Models.Model1.msl;provider=System.Data.SqlClient;

provider connection string=&quot;Integrated Security=SSPI;Persist
Security Info=False;

Initial Catalog=DATABASE.MDF;Data
Source=.\sqlexpress&quot;”

providerName=”System.Data.EntityClient”/>

</connectionStrings>

Let op, de database moet via een server gedeeld worden. Het
dynamisch koppelen van een Sql Server Express database (b.v. “Data
Source=.\SQLEXPRESS;AttachDbFilename=|DataDirectory|\Database1.mdf” ) werkt niet en zal bij de DDWA tot een vage foutmelding leiden.

En dat is alles. Makkelijk.

Nu kan via de DDWA applicatie snel onderhoud gepleegd worden op de (test) database. En nu kan ook snel inzichtelijk gemaakt worden hoe de ingevoerde informatie in de database opgeslagen wordt. Handig toch?