Saturday, 11 April 2015

An Updated Template For Enterprise Applications

My template that forms the architectural basis for many of my projects is continuously improved as my knowledge and experience increases. The latest version contains many of the patterns and practices that I have been using over the last year or so.

The code for this post can be found here on Github.

Overview

In fact, this is not so much of a template but more of a sample application. It is based on a camper van rental company (anyone one who knows me will know that this is something I briefly dabbled with before concluding my future actually did lie with software development after all). There is a front end for hiring, and a back end for logging pick ups, drop offs and maintenance.

Architecture

The Domain Model still forms the basis of what I do, and while I am constantly on the lookout for alternatives when circumstances dictate, I still find that most of the time it is the default solution.

One change is that I have abandoned the Application layer (sometimes called the Service layer). I could no longer justify having this layer in code. The purpose of this layer was simply to load entities from repositories, call the business logic contained in the entities and then save them. The problem is that this just became another layer for business logic to seep into from the domain. This functionality now sits in the controller of the UI. Some may argue that this violates the Single Responsibility Principle, but pragmatism wins over ideology here. It only adds a couple of lines of code to the controller and ultimately leads to more maintainable code, which is more important than religiously adhering to design principles.

Design

Here is a use case diagram of the functionality included:

Security

I have used ASP.NET Identity, but at arms length. Authentication, including storage of the UserProfile, password encryption and cookie management is handled by ASP.NET Identity. However details about the user are stored within the domain model, and store a reference key to the ASP.NET Identity UserProfile. This is because the UserProfile object must inherit IdentityUser, therefore meaning the Domain would have to have a reference to Microsoft.AspNet.Identity.EntityFramework. My Domain references look like this:

Image demonstrating how domain only has references to System and System.Core

and I want it to stay that way. What's worse, this dependency propagates through the entire project. Most of the assemblies end up needing a reference to it - not great. Also I like to keep my role/permission management within the Domain, as sometimes permissions are intertwined with business logic, as explained here.

Also, I have been working my way through the OWASP Top Ten, but this is far from complete. I will be blogging about these issues in future posts.

Accessibility

Another area that my work has been focusing on recently is the much forgotten area of accessibility, and in particular, the tailoring of websites for screen readers. There is barely a website on the internet that does not fall down in some way on this, partly down to lack of priority, but mainly due to lack of awareness. Also the technology is inconsistent and sometimes unreliable, and when you consider that instead of having to support x number of browsers, you are now supporting x number of browsers multiplied by y number of screen readers, you can begin to see the challenge. However, we have a moral duty to accommodate non-sighted users, and sooner or later, we will have a legal one (the physical domain have accessibility laws in the form of building regulations, so you can be sure that the digital domain will do soon as well).

The Web Content Accessibility Guidelines (WCAG) 2.0 are the accepted accessibility standards, and where possible, I have adhered to these. Of particular note is the accessible form validation summary, which will feature its own blog post in the future.

Thursday, 19 February 2015

Why You Should Have One ViewModel Per View

A classic rule to ahere to in MVC is to have a one-to-one relationship between Views and ViewModels. Sometimes it is tempting to share ViewModels between Views in the name of efficiency, but the long term costs will outweigh any short term gains. The following is an explanation of one such situation.

As always, the code in this example has been recreated to illustrate the issue under discussion. I never post any of my client's code. This is a simplified version of the actual situation.

The Problem

Recently I had to modify an ASP.Net MVC Razor View that listed a Customers. The View resembled this:

@using Lucidity.SampleSystem.Enumerations
@model IEnumerable<Lucidity.SampleSystem.ViewModels.CustomerSummaryViewModel>

@{
    ViewBag.Title = "Customers";
}

<h2>Customers</h2>

<table>
    <tr>
        <th>
            Customer
        </th>
        <th>
            Registered On
        </th>
        <th>
            Contract Value
        </th>
        <th></th>
    </tr>

@foreach (var customer in Model) {

    string customerDescription = null;

    if (customer.Type == CustomerType.Individual)
    {
        customerDescription = 
            customer.Title + " " + 
            customer.GivenName + " " + 
            customer.FamilyName;
    }
    else if(customer.Type == CustomerType.Corporate)
    {
        customerDescription = customer.CompanyName;
    }
    
    <tr>
        <td>
            @customerDescription
        </td>
        <td>
            @Html.DisplayFor(modelItem => customer.RegisteredOn)
        </td>
        <td>
            @Html.DisplayFor(modelItem => customer.ContractValue)
        </td>
        <td>
            @Html.ActionLink("Edit", "Edit", 
                new { id = customer.Id }, 
                new { aria_label = "Edit " + customerDescription }) |
            @Html.ActionLink("Details", "Details", 
                new { id = customer.Id }, 
                new { aria_label = "Details for " + customerDescription }) |
            @Html.ActionLink("Delete", "Delete", 
                new { id = customer.Id }, 
                new { aria_label = "Delete " + customerDescription })
        </td>
    </tr>
}

</table>

SyntaxHighlighter is doing some funny things here - this is best viewed in plain text.

This offending code lies between lines 26 and 38. The issue is the presence of logic in the View. This is bad practice for numerous reasons:

  • This is not how ASP.Net MVC is intended to be used, and so causes confusion for future maintainers of the code.
  • The View is difficult to test. Yes, there are options, but it is far simpler for this logic to be in a ViewModelFactory, and for that to be unit tested. And in the case of testing, if it's simpler, it's more likely to be done.
  • There are already various places for logic to be spread around, rightly or wrongly (Domain, ViewModelFactory, Controller). We don't need another one.

Why was This Decision Made?

So why would a developer have done this? It does not appear to be the work of an incompetent or complacent developer.

The rest of the code base would suggest that the team had a good grasp of ASP.Net MVC, and would have known this was not standard practice.

One thing that caught my eye was the @model for the View:

@model IEnumerable<Lucidity.SampleSystem.ViewModels.CustomerSummaryViewModel>

It doesn't look like this ViewModel has been designed specifically for this View. In fact, it is used on a PartialView called _CustomerSummary that is displayed at the top of many of the pages.

Standard practice would be for this sort of logic calculation to be performed in a ViewModelFactory, and set a field in the ViewModel. What is odd is that a field called 'Description' does actually exist in the ViewModel:

namespace Lucidity.SampleSystem.ViewModels
{
    public class CustomerSummaryViewModel
    {
        public Guid Id { get; set; }
        public string Title { get; set; }
        public string GivenName { get; set; }
        public string FamilyName { get; set; }
        public string CompanyName { get; set; }
        public CustomerType Type { get; set; }
        public DateTime RegisteredOn { get; set; }
        public decimal ContractValue { get; set; }
        public string Description { get; set; }
    }
}

However, this field was already being set in a ViewModelFactory for the _CustomerSummary View, but it's just a simple concatenation - no logic to display CompanyName for corporate customers.

The Background Cause

When the requirement came for a list of customers, a design decision was made to reuse the CustomerSummaryViewModel for the new View. This would have seemed like a good idea at the time: The fields to be displayed in the Customer list were just a subset of those displayed in the CustomerSummary. The Description field was always the same for both. So the new View was bound to the existing ViewModel, and the new controller used the Existing ViewModelFactory to generate the ViewModel from the list of Customer entities. A search through the source control history confirms this - the requirements were once aligned.

The problem with this approach comes when the a change is requested for either of the Views that are bound to that ViewModel. In this case, a new requirement was raised for the Company name to be displayed for corporates. Where does this new logic go? The ViewModelFactory doesn't know if it is creating a CustomerSummaryViewModel for _CustomerSummary or the Index. It would have been possible, perhaps, to pass in a flag to the ViewModelFactory, telling it what it was creating it for, and it could then make a decision accordingly. But it is likely time constraints caused the developer to take this approach.

We are all capable of hacky behaviour like this, if pushed down a dead end and when the pressure's on. The trick is not to push oursleves or our fellow develeopers into such dead ends. Keeping to the rule of one View per ViewModel is one such way of avoiding such dead ends.

Sources

http://lostechies.com/jimmybogard/2009/06/30/how-we-do-mvc-view-models/ http://lostechies.com/jimmybogard/2013/07/17/how-we-do-mvc-4-years-later http://stackoverflow.com/questions/9413034/viewmodel-per-view-or-per-model

Monday, 9 December 2013

Shortcomings of Entity Framework

The first time I used Entity Framework, I was blown away by how simple it was to use, and how quickly I could get up and running. Until recently, most of my work has revolved around NHibernate, which in comparison is far more complex. For some reason, it did not occur to me that this simplicity would bring with it some inflexibility.

One issue that has bitten me recently is the lack of an equivalent to IUserType. NHibernate understands how to map a simple type from a database field to a property in an entity, but what if it is a more complex type stored in an XML column, or what if the data is coming from a web service? The way to achieve this in NHibernate is explained very well in this post.

This is a useful feature (that should be used sparingly) that I just assumed would be available in Entity Framework. Strictly speaking it isn't, although there is a work around.

Take the following objects:
public class Parent
{
    public virtual Guid Id {get;set;}
    public virtual string Description {get;set;}
    public virtual Child Child {get;set;}
}

public class Child
{
    public virtual Guid Id {get;set;}
    public virtual string Description {get;set;}
    // And other stuff...
}
I want to persist the Parent in a row in a SQL Server table, and persist the Child in a an XML column in that row:
CREATE TABLE [Parent](
[Id] [uniqueidentifier] NOT NULL PRIMARY_KEY,
[Description] [nvarchar](50) NULL,
[Child] [xml] NULL
This would have been possible in NHibernate with IUserType, but with Entity Framework we have to do things differently.

Firstly, unfortunately, this impacts on our domain model - never a good thing for an O/RM to inflict. It will have to look like this:
public class Parent
{
    public virtual Guid Id {get;set;}
    public virtual string Description {get;set;}
    public virtual string ChildXml {get;set;}
    public virtual Child Child {get;set;}
}

public class Child
{
    public virtual Guid Id {get;set;}
    public virtual string Description {get;set;}
    // And other stuff...
}
And I have renamed the column in the database:
CREATE TABLE [Parent](
[Id] [uniqueidentifier] NOT NULL PRIMARY_KEY,
[Description] [nvarchar](50) NULL,
[ChildXml] [xml] NULL
)
Now, in out class that overrides DbContext, we need to intercept the creation and saving of this entity. Intercepting the creation is done by handling the ObjectMaterialized event of the ObjectContext, and in here we construct our child entity from the xml:
public class Context : DbContext
{
    public Context()
    {
         //...
         ObjectContext.ObjectMaterialized += new ObjectMaterializedEventhandler(ObjectContext_ObjectMaterialized);
    }

    //...

    public ObjectContext ObjectContext
    {
        get { return ((IObjectContextAdapter)this).ObjectContext; }
    }

    public void ObjectContext_ObjectMaterialized(object sender, ObjectMaterializedEventArgs e)
    {
        var parent = e.Entity as Parent;

        if (parent != null)
            parent.Child = XmlObjectSerializer.Deserialize(applicationForm.XmlData);
    }

    //...
}
And for saving, we need to override the SaveChanges() method of DbContext, as descibed by Chris McKenzie in this post.
public class Context : DbContext
{
    private void InterceptBefore(ObjectStateEntry item)
    {
        var parent = item.Entity as Parent;

        if (parent!= null)
            parent.XmlData = XmlObjectSerializer.Serialize(applicationForm.Child);
    }

    public override int SaveChanges()
    {
        const EntityState entitiesToTrack = EntityState.Added | EntityState.Modified | EntityState.Deleted;

        var elementsToSave =
            this.ObjectContext
                .ObjectStateManager
                .GetObjectStateEntries(entitiesToTrack)
                .ToList();

        elementsToSave.ForEach(InterceptBefore);
        var result = base.SaveChanges();
        return result;
    }
}
Now if I want to display a list of Parent entities with just their description, this could all become very inefficient. What is needed is some way of lazy loading the child. This could mean the child is a separate entity mapped to a table with its own XML field, but what about in this scenario?
public class Parent
{
    public virtual Guid Id {get;set;}
    public virtual string Description {get;set;}
    public virtual string ChildrenXml {get;set;}
    public virtual ICollection Children {get;set;}
}

public class Child
{
    public virtual Guid Id {get;set;}
    public virtual string Description {get;set;}
    // And other stuff...
}
And I have renamed the column in the database:
CREATE TABLE [Parent](
[Id] [uniqueidentifier] NOT NULL PRIMARY_KEY,
[Description] [nvarchar](50) NULL,
[ChildrenXml] [xml] NULL
)
This rules out the previous option. So surely it's just a simple case of setting the ChildrenXml property to be lazy loaded? Again (and I think more justifiably) I just assumed this would be possible in Entity Framework. I was somewhat surprised to learn that Entity Framework doesn't support lazy loading of properties.

again, the solution is to change our Domain to handle this:
public class Parent
{
    public virtual Guid Id {get;set;}
    public virtual string Description {get;set;}
    public virtual Guid DetailsId {get;set;}
    public virtual ParentDetails Details {get;set;}
}

public class ParentDetails
{
    public virtual Guid Id {get;set;}
    public virtual string ChildrenXml {get;set;}
    public virtual ICollection Children {get;set;}
}

public class Child
{
    public virtual Guid Id {get;set;}
    public virtual string Description {get;set;}
    // And other stuff...
}
And I have renamed the column in the database:
CREATE TABLE [Parent](
[Id] [uniqueidentifier] NOT NULL PRIMARY_KEY,
[Description] [nvarchar](50) NULL,
[DetailsId] [uniqueidentifier] NOT NULL
)

CREATE TABLE [Parent](
[Id] [uniqueidentifier] NOT NULL PRIMARY_KEY,
[ChildrenXml] [xml] NULL
)
This seems to be the officially sanctioned way of doing things. If you have a large field - a BLOB, a VARBINARY, a VARCHAR(MAX), it has to go in a 'Details' table. Surely an O/RM on version 6 should have this functionality?

Tuesday, 26 November 2013

Entities Shouldn't Have Getters

This post follows on from Entities Shouldn't Have Setters.

The principle of Tell, Don't Ask states that one of the consequences of an object having getters is that there is often the temptation for another object to read the values from the getters, make a decision based on those values, and then update the object using the setters (as covered in the last post). A more Object-Oriented design would be to simply 'tell' the object what you want to do, and allow it to make the decision based on its internal state.

However, as acknowledged by Martin Fowler, it is very often the case that we need to display the state of an entity to a UI. However, the use of getters in this case can be avoided by displaying a persistent view model instead of the entity. The persistent view model is what is displayed to the user, and it is updated whenever the entity changes. This is the basic principle behind CQRS.

Imagine our Post object from our blog example in the last post. If we were to have an Edit use case for this, the traditional series of events might be:

  • Call Edit on the Post entity with the new content of the post.
  • Save the Post entity.
  • Load the new Post entity from the database.
  • Display the various properties of the Post entity by accessing its getters.

With CQRS, it might look more like this:

  • Call Edit on the Post entity with the new content of the post.
  • The Post entity raises a PostEdited domain event containing the new values.
  • The application layer handles the domain event, saves the entity and publishes an event on NServiceBus containing the new values.
  • The read model subscribes to this event, and updates the persistent read model accordingly.
  • When the UI is refreshed, it displays the properties of the persistent view model by accessing its getters.

This way we have avoided using the getters of the entity.

For more information on publishing domain events, read This post by Udi Dahan.

So I should immediately stop using getters and setters on my entities?


No, this is somewhat ideological. The first issue I have come across is that it is often desirable to use getters and setters when testing. Getters are useful for the asserts, and setters are useful because for many tests, we do not need to set up the whole entity. Maybe that is breaking some testing ideology, but in the real world, sometimes we need to be able to quickly set up entities without using their use cases.

And of course, CQRS is not always appropriate for every application.

So what was the point of these posts? The point is that you should understand that it can be done, and why and when you may choose to do it. This way you can apply these rules where it makes sense.

So it is unlikely I will be writing entities without getters or setters for now, but there are some rules to take from this:

  • If you are calling getters or setters of an entity from another entity in your domain, you are probably doing something wrong.
  • If you are calling getters or setters of an entity in your application layer you are probably doing something wrong (unless you are mapping to a DTO).
  • If you are calling setters of an entity in your UI you are almost certainly doing something wrong.

Sunday, 27 October 2013

Entities Shouldn't Have Setters

Setters on entities are redundant and a code smell. Take the following example of a blog post:
public class Post : Entity<Guid>
{
    private DateTime _postedOn;
    private Blog _blog;
    private string _text;

    public virtual DateTime PostedOn
    {
        get { return _postedOn; }
        set { _postedOn = value; }
    }

    public virtual Blog Blog 
    {
        get { return _blog; }
        set { _blog = value; }
    }

    public virtual string Text
    {
        get { return _text; }
        set { _text = value; }
    }
}
The problem here is that is that it leaves open the posibility that the entity will be modified from another entity, or even from another layer. Take this example in the application layer:
public class PostService : IPostService
{ 
    public void Compose(ComposePostRequest request)
    {
        var post = new Post();
        post.PostedOn = DateTime.Now;
        post.Blog = _blogRepository.GetById(request.BlogId);
        post.Text = request.Text;
        _postRepository.Save(post);
    }
}
In this example, the Post entity is being constructed in the application layer. The main problem with this is that inevitably business logic seeps into the application layer. What if we had to set an expiry date for the post based on a window set in the blog? Where does this logic go? The result may look like this:
public class PostService : IPostService
{ 
    public void Compose(ComposePostRequest request)
    {
        var blog = _blogRepository.GetById(request.BlogId);
        var post = new Post();
        post.PostedOn = DateTime.Now;
        post.Blog = blog;
        post.Text = request.Text;
        post.ExpiryDate = post.PostedOn.AddDays(blog.PostExpiryWindow);
        _postRepository.Save(post);
    }
}
As more logic comes in, more will be added to the application layer. The logic of the domain cannot be tested in isolation. The only way to test this logic is to mock out the PostRepository and capture the Post that is being saved. A far better way is to do this:
public class Post : Entity<Guid>
{
    private DateTime _postedOn;
    private Blog _blog;
    private string _text;
    private DateTime _expiryDate

    public virtual DateTime PostedOn
    {
        get { return _postedOn; }
    }

    public virtual Blog Blog 
    {
        get { return _blog; }
    }

    public virtual string Text
    {
        get { return _text; }
    }

    public virtual DateTime ExpiryDate
    {
        get { return _expiryDate; }
    }

    public static Post Compose(Blog blog, string text)
    {
        var post = new Post();
        post._postedOn = DateTime.Now;
        post._blog = blog;
        post._text = text;
        post._expiryDate = _postedOn.AddDays(blog.PostExpiryWindow);
        return post;
    }
}

public class PostService : IPostService
{ 
    public void Compose(ComposePostRequest request)
    {
        var blog = _blogRepository.GetById(request.BlogId);
        var post = Post.Compose(blog, request.Text;
        _postRepository.Save(post);
    }
}
Now new business logic can be added to the Compose method in Post, and the logic of Post can be tested in isolation. If you think about it, this all makes perfect sense: Compose is a use case of Post, and why would you want to change the state of an entity out side of a use case?

Persistence


Again, my persistence example is with NHibernate. NHibernate allows us to specify the way it accesses properties, using the 'access' attribute of property:
<?xml version="1.0" encoding="utf-8" ?>
<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2"
                   namespace="Lucid.Domain.Entities"
                   assembly="Lucid.Domain">
  <class name="Post" table="`Post`">
    <id name="Id" column="Id" type="guid">
      <generator class="assigned"/>
    </id>
    <property name="_postedOn" column="`PostedOn`" access="field" />
    <many-to-one name="_blog" class="Bus" column="`BusId`" cascade="save-update" access="field" />
    <property name="_text" column="`Text`" access="field" />
    <property name="_expiryDate" column="`ExpiryDate`" access="field" />
  </class>
</hibernate-mapping>
The blog and post example may not have been the best example here, as Blog would be an ideal candidate for an aggregate route. In this case, the Blog entity would have a ComposePost method. The Blog would be loaded, the Post would be added and the Blog would be saved. However, aggregate routes are for another post, and for now this serves as a good example.

Monday, 1 July 2013

Mapping the Decorator Pattern in NHibernate

The Decorator Pattern is an useful way of avoiding multiple inheritance class-explosion madness, but in domain modelled enterprise applications, it's not much use unless you can persist it. Neither I, nor anyone on Stack Overflow could figure out a way to do it, until now.

My working code can be found on Github, and a brief overview is described here.

Sorry to use a contrived example, but I could hardly use a production example, and din't have the time to think up anything else, so pizzas it is. At least it's not coffee.

Here is a typical implementation of the pattern:

public interface IPizza
{
    Guid? Id { get; set; }
    int Size { get; set; }
    Quantity Cheese { get; set; }
    Quantity Tomato { get; set; }
    decimal Cost { get; }
    Order Order { get; set; }
}

public class Pizza : Entity, IPizza
{
    public virtual int Size { get; set; }
    public virtual Quantity Cheese { get; set; }
    public virtual Quantity Tomato { get; set; }
    public virtual Order Order { get; set; }

    public static IPizza Create(int size, Quantity cheese, Quantity tomato)
    {
        // Create code...
    }

    public virtual decimal Cost
    {
        // Calculate cost...
    }
}

public class ToppingDecorator : Entity, IPizza
{
    public virtual IPizza BasePizza { get; set; }
    public virtual Order Order { get; set; }

    public ToppingDecorator(IPizza basePizza)
    {
        Id = Guid.NewGuid();
        BasePizza = basePizza;
    }

    public virtual int Size
    {
        get { return BasePizza.Size; }
        set { BasePizza.Size = value; }
    }

    public virtual Quantity Cheese
    {
        get { return BasePizza.Cheese; }
        set { BasePizza.Cheese = value; }
    }

    public virtual Quantity Tomato
    {
        get { return BasePizza.Tomato; }
        set { BasePizza.Tomato = value; }
    }

    public virtual decimal Cost
    {
        get { return BasePizza.Cost; }
    }
}

public class PepperoniDecorator : ToppingDecorator
{
    public virtual bool ExtraSpicy { get; set; }

    public PepperoniDecorator(IPizza basePizza, bool extraSpicy)
        : base(basePizza)
    {
        ExtraSpicy = extraSpicy;
    }

    public override decimal Cost
    {
        get
        {
            // Add to cost...
        }
    }
}

public class OliveDecorator : ToppingDecorator
{
    public virtual OliveColour Colour { get; set; }

    public OliveDecorator(IPizza basePizza, OliveColour colour) : base(basePizza)
    {
        Colour = colour;
    }

    public override decimal Cost
    {
        get
        {
            // Add to cost...
        }
    }
}

public class Order : Entity
{
    public virtual string CustomerName { get; set; }
    public virtual string DeliveryAddress { get; set; }
    public virtual IList Items { get; set; } 

    //Create/Add methods etc...
}
When it came to the database, it was always pretty clear that there would be a Pizza table which would contain all the properties specified in the interface, and then there would be tables for each decorator which contained the particular fields they added, and also a foreign key to either a Pizza or another decorator:
USE [Master]

IF EXISTS (SELECT * FROM sys.databases WHERE NAME = 'Decorator')
BEGIN
 EXEC msdb.dbo.sp_delete_database_backuphistory database_name = N'Decorator';
 ALTER DATABASE [Decorator] SET SINGLE_USER WITH ROLLBACK IMMEDIATE;
 DROP DATABASE [Decorator];
END
GO

CREATE DATABASE [Decorator]
GO

USE [Decorator]
GO

IF NOT EXISTS(SELECT * FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = 'Pizza')
BEGIN
 CREATE TABLE [dbo].[Pizza](
  [Id] uniqueidentifier NOT NULL PRIMARY KEY,
  [Size] int NULL,
  [Cheese] int NULL,
  [Tomato] int NULL,
  [OrderId] uniqueidentifier NULL
 ); 
END
GO

IF NOT EXISTS(SELECT * FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = 'PepperoniDecorator')
BEGIN
 CREATE TABLE [dbo].[PepperoniDecorator](
  [Id] uniqueidentifier NOT NULL PRIMARY KEY,
  [BasePizzaId] uniqueidentifier NULL,
  [ExtraSpicy] bit NULL,
  [OrderId] uniqueidentifier NULL
 );
END
GO

IF NOT EXISTS(SELECT * FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = 'OliveDecorator')
BEGIN
 CREATE TABLE [dbo].[OliveDecorator](
  [Id] uniqueidentifier NOT NULL PRIMARY KEY,
  [BasePizzaId] uniqueidentifier NULL,
  [Colour] int NULL,
  [OrderId] uniqueidentifier NULL
 );
END

IF NOT EXISTS(SELECT * FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = 'Order')
BEGIN
 CREATE TABLE [dbo].[Order](
  [Id] uniqueidentifier NOT NULL PRIMARY KEY,
  [CustomerName] nvarchar(100) NULL,
  [DeliveryAddress] nvarchar(200) NULL
 );
END
GO
The trick bit was mapping between them. After several failed attempts at using table per class heirachy and table per subclass I came to the conclusion that it wasn't the way to go.

I experimented with table per concrete class using implicit polymorphism but found the limitations of that to be a major issue. Eventually the solution was found using table per concrete class using union-subclass.

Here is how the mappings look:
<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2"
                   namespace="Decorator.Domain.Entities"
                   assembly="Decorator.Domain">
  <class name="IPizza" abstract="true">
    <id name="Id" column="Id" type="guid">
      <generator class="assigned"/>
    </id>
    <many-to-one name="Order" class="Order" column="`OrderId`" cascade="save-update" />
    
    <union-subclass name="Pizza" table ="`Pizza`" >
      <property name="Size" column="`Size`" />
      <property name="Cheese" />
      <property name="Tomato" />
    </union-subclass>

    <union-subclass name="PepperoniDecorator" table ="`PepperoniDecorator`" >
      <many-to-one name="BasePizza" class="IPizza" column="`BasePizzaId`" cascade="all" />
      <property name="ExtraSpicy" column="`ExtraSpicy`" />
    </union-subclass>

    <union-subclass name="OliveDecorator" table ="`OliveDecorator`" >
      <many-to-one name="BasePizza" class="IPizza" column="`BasePizzaId`" cascade="all" />
      <property name="Colour" column="`Colour`" />
    </union-subclass>
  </class>
</hibernate-mapping>

<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2"
                   namespace="Decorator.Domain.Entities"
                   assembly="Decorator.Domain">
  <class name="Order" table="`Order`">
    <id name="Id" column="Id" type="guid">
      <generator class="assigned"/>
    </id>
    <property name="CustomerName" />
    <property name="DeliveryAddress" />

    <bag name="Items" inverse="true" cascade="save-update">
      <key column="`OrderId`"></key>
      <one-to-many class="IPizza" />
    </bag>
  </class>
</hibernate-mapping>
I have included the Order entity for a good reason here: If you create a Pizza, decorate it with pepperoni, then decorate it with olives and save it, when you get all pizzas, it will actually return 3 pizzas! NHibernate has no way of knowing which pizza is the top level one. This could be avoided by having an IsTopLevel flag, but as pizzas will always be created in the context of an order, it makes sense to only have the orderId on the top level. A similar solution will apply to most scenarios.

Thursday, 27 June 2013

Learning the Hard Way: NHibernate Collections

Here's on that's bitten out team recently: the issue of managing collections of child entities on an parent entity. A number of our records were going missing from the database. How could this be? We don't really delete anything, we just 'soft delete' - setting a flag to mark something as deleted. Take the following entities:
public class Employee
{
    public virtual Guid Id { get; set; }
    public virtual string Name { get; set; }
    public virtual bool Deleted { get; set; }

    public static Employee Create(string name)
    {
        return new Employee
                   {
                       Id = Guid.NewGuid(),
                       Name = name,
                       Deleted = false
                   };
    }
}

public class Team 
{
    public virtual Guid Id { get; set; }
    public virtual string Name { get; set; }
    public virtual IList TeamEmployees { get; set; }
    public virtual bool Deleted { get; set; }

    public static Team Create(string name)
    {
        return new Team
                    {
                        Id = Guid.NewGuid(),
                        Name = name,
                        TeamEmployees = new List(),
                        Deleted = false
                    };
    }

    public virtual void UpdateEmployees(IList employees)
    {
        foreach(var teamEmployee in TeamEmployees.Where(x => !employees.Contains(x.Employee)).Reverse())
        {
            TeamEmployees.Remove(teamEmployee);
        }

        foreach(var employee in employees.Where(x => !TeamEmployees.Select(y => y.Employee).Contains(x)))
        {
            TeamEmployees.Add(TeamEmployee.Create(employee, this));
        }
    }
}

public class TeamEmployee
{
    public virtual Guid Id { get; set; }
    public virtual Employee Employee { get; set; }
    public virtual Team Team { get; set; }
    public virtual bool Deleted { get; set; }

    public static TeamEmployee Create(Employee employee, Team team)
    {
        return new TeamEmployee
                    {
                        Id = Guid.NewGuid(),
                        Employee = employee,
                        Team = team,
                        Deleted = false
                    };
    }
}
The problem here is when you load a team, update the employees and save it - you can be deleting records without realising it. The mapping on Team for TeamEmployees was set to 'all-delete-orphans', so when the association between a Team and an Employee was removed, any record it ever existed was also lost. Even if the cascade had just been 'all', the foreign key to Team would have been nullified and the history would have been lost.

There are a few ways to limit these problems, such as revoking delete access for the database login, and setting all cascades to 'save-update', but it also pays to be cleverer about how collections are handled.

Instead of removing the TeamEmployee record, it is flagged as deleted:
public class Team 
{
    public virtual Guid Id { get; set; }
    public virtual string Name { get; set; }
    public virtual IList TeamEmployees { get; set; }
    public virtual bool Deleted { get; set; }

    public static Team Create(string name)
    {
        return new Team
                    {
                        Id = Guid.NewGuid(),
                        Name = name,
                        TeamEmployees = new List(),
                        Deleted = false
                    };
    }

    public virtual void UpdateEmployees(IList employees)
    {
        foreach(var teamEmployee in TeamEmployees.Where(x => !employees.Contains(x.Employee)).Reverse())
        {
            TeamEmployees.Remove(teamEmployee);
        }

        foreach(var employee in employees.Where(x => !TeamEmployees.Select(y => y.Employee).Contains(x)))
        {
            TeamEmployees.Add(TeamEmployee.Create(employee, this));
        }
    }
}
And the Mapping file has a where clause added to it, so that it only loads the undleted records:
<?xml version="1.0" encoding="utf-8" ?>
<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2"
                   namespace="Collections.Domain"
                   assembly="Collections.Domain">
  <class name="Team" table="`Team`">
    <id name="Id" column="Id" type="Guid">
      <generator class="assigned"/>
    </id>
    <property name="Name" column="`Name`" />
 <property name="Deleted" />
 <bag name="TeamEmployees" cascade="save-update" where="Deleted = 0" >
      <key column="TeamId"/>
   <one-to-many class="TeamEmployees" />
 </bag>
  </class>
</hibernate-mapping>
Care must be taken when accessing the collection in the same session after deleting a record, as it will be present but marked as deleted. A linq clause '.Where(x => !x.Deleted) should be used.

Another problem would be if the Team and Employee relationship was mapped as many-to-many. There may be a way to soft delete the relationships, but I am not currently aware of it.

Really, this whole scenario is another argument in favour of breaking down all many-to-many relationships with an extra entity. There are others, such as having somewhere to store information about the relationship. Many times I have found it necessary to break down a many-to-many, but never to go the other way. Therefore I am favouring breaking down these relationships as a default.

Fortunately due to our rightfully paranoid auditing and event logging, all customer records were retrived and the data was returned to its expected state.