Some coworkers were recently working on an object model for a simple security system. After some discussion with them, we came up with this basic model:

A permission is defined as an activity that can be assigned to a user, or group, and can be allowed or disallowed. From a Domain Driven Design perspective, we're stating that the Permission is the aggregate root. The User object itself, while involved in this aggregate, is divorced from this aggregate's relational model - you can load and work with a User object without having to load or worry about the Permission hierarchy. The addition of the "UserGroup" is solely for the many-to-many relationship between User and Group mappings with NHibernate and is not actually part of the object model's code.

Once we had this model in place, we wanted to have a simple query that allowed us to load a given permission object by activity name, for a user - whether the user was assigned directly or via a group. At a high level, this model and query should be fairly simple to work with. It turned out to be a massive learning curve in NHibernate, though.

After much trial, error, and Google searching, we ended up with this NHibernate query code:

ICriterion userIdMatches = Restrictions.Eq("Id", userId);

ICriterion activityNameMatches = Restrictions.Eq("Name", action);

ICriterion userIdAliasMatches = Restrictions.Eq("u.Id", userId);

DetachedCriteria groupPermissionCriteria = DetachedCriteria.For<Permission>()

    .SetProjection(Projections.Property("Group"))

    .CreateCriteria("Group").CreateCriteria("Users").Add(userIdMatches);

ICriterion groupSubquery = Subqueries.PropertyIn("Group", groupPermissionCriteria);

DetachedCriteria permissionCriteria = DetachedCriteria.For<Permission>()

    .CreateAlias("User", "u", JoinType.LeftOuterJoin)

    .Add(Restrictions.Or(userIdAliasMatches, groupSubquery));

permissionCriteria.CreateCriteria("Activity").Add(activityNameMatches);

ICriteria executableCriteria = permissionCriteria.GetExecutableCriteria(Session);

result = executableCriteria.List<Permission>();

return result;

There were a lot of lessons learned and a lot of parts that eventually got put together. Here's a quick run-down of what we ended up with and why.

• The ICriterion's at the top of this code block are there to provide a little better readability in the real query code below.
• The groupPermissionCriteria is set up to find a permission object where the specified userId belongs to a Group that belongs to the Permission - i.e. find permissions where the user is assigned via a group. The learning curve from this perspective was the SetProjection call. Though we are not entirely sure what a projection is at this point, we did find out that it was necessary for us to set this projection so that the detached criteria could be used as a sub-query.
• The groupSubQuery is the conversion of the groupPermissionCriteria into an ICriterion so that we can do a logical Or with it in the primary query construction.
• The permissionCriteria object sets up the core criteria logic and ties together the group permission assignment with the user permission assignment.
• CreateAlias is used so that we can shorten the criteria for loading by the User assignment directly. The JoinType on the alias needs to be Left Outer Join so that we will return a proper Permission object even when there is no direct user assignment.
• After creating the alias, we can add an "Or" criterion to the query and specify that we want to match based on the user's direct assignment or a group's assignment.
• The last line of the criteria simply adds the Activity criteria to load by the activity name.

The resulting SQL will load the permission by Activity name AND (User assignment OR group assignment where the user is part of the group).

SELECT 

                this_.PERMISSIONSID as PERMISSI1_0_3_, 

                this_.IS_ALLOWED as IS2_0_3_, 

                this_.ACTIVITYID as ACTIVITYID0_3_, 

                this_.USERID as USERID0_3_, 

                this_.GROUPID as GROUPID0_3_, 

                activity1_.ACTIVITYID as ACTIVITYID4_0_, 

                activity1_.ACTIVITY_NAME as ACTIVITY2_4_0_, 

                activity1_.DESCRIPTION as DESCRIPT3_4_0_, 

                activity1_.INACTIVE_DATE as INACTIVE4_4_0_, 

                u2_.USERID as USERID3_1_, 

                u2_.USER_NAME as USER2_3_1_, 

                u2_.INACTIVE_DATE as INACTIVE3_3_1_, 

                group6_.GROUPID as GROUPID1_2_, 

                group6_.GROUP_NAME as GROUP2_1_2_, 

                group6_.INACTIVE_DATE as INACTIVE3_1_2_ 

FROM PERMISSIONS this_ 

                inner join ACTIVITY activity1_ 

                                on this_.ACTIVITYID=activity1_.ACTIVITYID 

                inner join USERS u2_ 

                                on this_.USERID=u2_.USERID 

                left outer join GROUPS group6_ 

                                on this_.GROUPID=group6_.GROUPID 

WHERE 

                activity1_.ACTIVITY_NAME = :p1

                and 

                (

                                u2_.USERID = :p2 

                                or 

                                this_.GROUPID = 

                                (

                                                SELECT 

                                                                this_0_.GROUPID as y0_ 

                                                FROM PERMISSIONS this_0_ 

                                                                inner join GROUPS group1_ 

                                                                                on this_0_.GROUPID=group1_.GROUPID 

                                                                inner join USERS_GROUPS users5_ 

                                                                                on group1_.GROUPID=users5_.GROUPID 

                                                                inner join USERS user2_ 

                                                                                on users5_.USERID=user2_.USERID 

                                                WHERE 

                                                                user2_.USERID = :p3

                                )

                )

Using a sub-query to load based on the group is not the most optimal way of loading the permission for the group assignment. However, since all of the joins in the main query and the sub-query are done on primary and foreign keys in the tables, performance should not be an issue. The only real performance concern for this query is the activity name in the where statement. A simple unique constraint and index on the activity name, though, will solve that problem.

In the last month or two, I have been hand coding a lot of mock and stub objects and it has become a nightmare to manage. My primary reason for doing this by hand was that Rhino Mocks 3.4 and older did not fit with the BDD style unit tests that I was writing. Yes, I made it work in a few places, but it was ugly and annoying.

Fortunately, Ayende has cleaned it all up with the new syntax and made it very easy to assert that individual expectations were met, with v3.5. I finally got around to trying out the new syntax today, and I immediately fell in love with it. For example:

[Concern("User Administration")]

public class When_accessing_the_system_as_a_non_administrator : ContextSpecification

{

    private IMainView view;

    private IMainView GetView()

    {

        IMainView mockView = MockRepository.GenerateMock<IMainView>();

        mockView.Expect(v => v.EnableUserManagement()).Repeat.Never();

        mockView.Expect(v => v.DisableUserManagement()).Repeat.Once();

        return mockView;

    }

    protected override void Context()

    {

        User administrator = new User();

        administrator.IsAdministrator = false;

        CurrentUser.User = administrator;

        view = GetView();

        new MainPresenter(view);

    }

    [Observation]

    public void Should_not_display_the_User_Management_option()

    {

        view.AssertWasNotCalled(v => v.EnableUserManagement());

    }

    [Observation]

    public void Should_hide_the_user_management_option()

    {

        view.AssertWasCalled(v => v.DisableUserManagement());

    }

}

In the "GetView" method, I am setting up two very distinct expectations on my mock object.

1. Never Call the EnableUserManagement method.
2. Call the DisableUserManagement method once.

With the new Rhino Mocks syntax, I can easily verify that each one of these expectations was called via my "should" observations.

The "AssertWasNotCalled" extension method verifies that an expectation of Repeat.Never was setup and that the method was not called.

mockView.AssertWasNotCalled(v => v.EnableUserManagement());

And the "AssertWasCalled" extension method verifies that the expectation was to call the method, and that the method actually was called.

mockView.AssertWasCalled(v => v.DisableUserManagement());

I like it. The new syntax has really simplified my Context Specification testing.

Earlier this week, I was doing a training session with my current team, building a sample Org Chart application. One of the user stories we were working with, talked about assigning managers to department and had the following acceptance criteria:

• When assigning a manager to a department, ensure that they are an employee of that department, and remove them from their old department

The basic code that we came up with worked well and was encapsulated into a service object in the domain.

public class AssignmentService

{

    private IDepartmentRepository _repository;

    public AssignmentService(IDepartmentRepository repository)

    {

        _repository = repository;

    }

    public void AssignManagerToDepartment(Employee manager, Department assignedDepartment)

    {

        Department previouslyAssignedDepartment = _repository.GetDepartmentForEmployee(employee);

        if (previouslyAssignedDepartment != assignedDepartment)

        {

            previouslyAssignedDepartment.RemoveEmployee(manager);

            _repository.Save(previouslyAssignedDepartment);

        }

        assignedDepartment.AssignEmployee(manager);

        assignedDepartment.AssignManager(manager);

        _repository.Save(assignedDepartment);

    }

}

Problems

At first glance, this code seems simple enough, but we quickly ran into a problem when we started coding for another story and acceptance criteria.

• When creating a department, allow a manager to be assigned

Off-hand, this seems like a very simple situation. We'll just call the AssignmentService object after we create the Department. However, there's a large potential problem with this - the AssignmentService handles the entity persistence for both of the departments that were changed, when the manager is assigned. With that in mind, the developer working on the new requirement would have to ensure that the department is created with all of it's required fields and information before calling this service object. If the developer allows the manager to be assigned before the department's info has been completely filled in, we may accidentally allow an invalid state for a department object when calling the AssignmentService. Since the AssignmentService tries to save the department being assigned, we may also have created an semantic coupling between the department creation process and the AssignmentService - the developer needs to know that the AssignmentService will save the department. If they don't know this, they may try to save the same entities more than once, may try to save an invalid state, etc, etc, etc.

Unfortunately, we didn't have time to fix the problem during the training session - we had to stop the training almost immediately after we discovered this issue. I've got a few ideas running around in my head, on how to fix this, but I'm not 100% sure that I like any of them.

Possible Solutions

First and foremost, though, it's becoming rather apparent to me that we want to avoid entity persistence calls from within the domain service objects. This essentially means that the AssignmentService object would have to return the assigned department, and the 'previously assigned department' objects, so that the coordinating presenter could save them both. That seems a little odd, if you ask me.

One of my coworkers suggested that we allow the service to save the previously assigned department, since that is an self-contained operation in the method (the method loads the department, modifies it, and saves it) and then return the assigned department so that the presenter can save it. This option seems a bit better off-hand, but I'm still worried about the possible side effects of the AssignmentService saving the previously assigned department.

Questions

What are your thoughts on this situation... How would you solve the dilemmas that I'm pointing out? Is there a better design for the model that would eliminate this problem and make the department persistence more obvious, without having to 'know' when it is persisted?

Is the idea of not allowing entity persistence from a domain service object valid? Should we go with allowing the self-contained operations to persist the entity in that operation?

Or ???

Visual Studio 2008 installs .NET 3.5 SDK v6.0A, which is what NAnt 0.86 expects to run against – however, the .NET 3.5 SDK that is available for download, via the "Windows 2008 and .NET 3.5 SDK", is v6.1. 

It’s a small difference, but that difference will cause NAnt 0.86 to not be able to target .NET 3.5 if you are using the downloadable .NET 3.5 SDK. When you try to run NAnt 0.86 and have it target .NET 3.5 with the downloaded .NET SDK, you will get an “Object reference” error from NAnt and the build will fail immediately.

Page Brooks has the details of how to fix NAnt to work against the v6.1 SDK. It is an easy fix, but it requires that every machine running NAnt have the v6.1 SDK installed (and if you want your devs to be able to run NAnt from their local machine, you’ll need all of the developers to install the updated version of the SDK).

End Result:

If you want to automate your VS2008 project builds with NAnt, you should probably have every developer on your team to install the new version of the SDK and have NAnt target that version.

During a TDD/BDD training session with my team, today, we wrote the following code:

public void SaveRequested()

{

    Employee employee = Department.CreateEmployee(firstName, lastName, email);

    if (employee == null)

        repository.Save(Department);

}

I ignored this when we first wrote it, as it was "technically correct", but later returned to this code as a point of refactoring for greater insight and expressiveness. When we came back to it, though, I started off by asking a series of questions:

• Why would that method return a null?
• Why would it not return a null?
• Why is it important to not call repository.Save if the employee is null?

The code is essentially a mystery to anyone that hasn't memorized the details of the Department.CreateEmployee method. Worse yet, I can make a lot of assumptions about this code - but every one of them is going to be wrong until I can prove that they are right by reading the code in Department.CreateEmployee.

So how do we refactor this code for better insight and more expressiveness? How do we change this code so that the assumptions a reader creates in their own mind are more likely to be correct - or better yet, are irrelevant because you don't need to even care about the details of the Department.CreateEmployee method? First and foremost, you need to understand what this code is supposed to be expressing - what it's supposed to be doing and why. If you don't know, you need to ask the person that wrote it. If they don't know (or aren't around), you need to find someone that can help you determine the intended behavior. Ultimately, you may need to dig into the Department.CreateEmployee method to figure it out for yourself.

In this situation, we were trying to express some business rules from a set of acceptance criteria on a user story:

• When creating an employee, the employee must belong to a department
• When creating an employee, require a first name, last name, and email address

This code is a factory method on the Department object that is used to ensure these rules are satisfied. In this case, if CreateEmployee returns a null, one of these rules was violated and we should not attempt to save the Department (with the associated employee, or lack there-of). If CreateEmployee returned an employee, we satisfied the rules and we can save the Department and it's associated employees.

The code that we have in place doesn't express any of that intent, though. What we really want is code that tells us that the employee creation failed - and probably why it failed. Fortunately, the changes can be extraordinarily simple.

public void SaveRequested()

{

    EmploymentResult employmentResult = Department.Employ(firstName, lastName, email);

    if (employmentResult.Success)

    {

        repository.Save(Department);

    }

    else

    {

        view.DisplayError(employmentResult.FailureReason);

    }

}

UPDATE: Thanks to Martin Linkhorst for the comments, and making this code even more expressive (changing from Department.CreateEmployee to Department.Employ)

By creating an AddEmployeeResult object and supplying a few additional bits of information in it, our code became much more expressive of it's intent. We can now read the SaveRequested method and have a much better chance of knowing what it is really doing - try to create an employee and if it successfully created, then save it. Otherwise, display the failure reason.

What assumptions does your code create in the minds of the readers? Are these assumptions correct, incorrect, or unintelligible? And finally, what can you do to make your code assumption-free and/or assumption-irrelevant?

Over the weekend, I spent some time refactoring some business rules in my current project, to use the ISpecification<t> framework that I previously created.

One of my coworkers had asked about logical groupings - making sure that we can handle complex logic such as, "(this and this) or this". Since my project actually made use of logical groupings I wanted to share with the world and show how easy it is.

In this example, I have a Date Range that is being selected – a From date and a To date. The rules are as follows for validating the range:

• If a From date and a To date are provided, the From date must be equal to, or before, the To date
• Or, you can specify a From date without To date
• Or, you can specify a To date without From date

The following is the actual code that I’m using to define these business rules, via the ISpecification<t> framework. 

PredicateSpec<ExtractSearchCriteria> fromDateProvided = new PredicateSpec<ExtractSearchCriteria>(crit => crit.FromDate > DateTime.MinValue);

PredicateSpec<ExtractSearchCriteria> toDateProvided = new PredicateSpec<ExtractSearchCriteria>(crit => crit.ToDate > DateTime.MinValue);

PredicateSpec<ExtractSearchCriteria> fromDateBeforeToDate = new PredicateSpec<ExtractSearchCriteria>(crit => crit.FromDate <= crit.ToDate);  

ISpecification<ExtractSearchCriteria> validDateRangeSpec =

    (

        (fromDateProvided.And(toDateProvided))

        .And(fromDateBeforeToDate)

    )

    .Or(

        fromDateProvided.Not(toDateProvided)

    )

    .Or(

        toDateProvided.Not(fromDateProvided)

    );

Here, I am defining the core of the business rules – to check if the From date was provided, to check if the To date was provided, and to check if the From date is before or equal to the To date. From these core specifications, I can build the aggregate spec that handles all of the stated rules for validating the date range.

There are three logical groupings using parenthesis to represent each of the business rules that can comprise a valid date range. The first logical grouping checks to see if both a From and To date are provided, and checks if the From date is before or equal to the To date, if both are provided.

(

    (fromDateProvided.And(toDateProvided))

    .And(fromDateBeforeToDate)

) 

The second group is specified with an Or, and checks to see if we provided a From date but not a To date.

.Or(

    fromDateProvided.Not(toDateProvided)

)

And the third group also uses an Or, and checks to see if we provided a To date but not a From date.

.Or(

    toDateProvided.Not(fromDateProvided)

)

The end result is that we have one of three possible ways for this date range to be valid – all logically grouped and relatively easy to read (compared to a bunch of if-then statements, at least). Even better – we re-used two out of three simple ISpecification<t> objects to create all of the rules for this validation check. This helps to create a validation that is not only easier to read, but easier to maintain and enhance in the future.

My Previous Post talked about a sample Specification pattern implementation. Today, during a discussion with some team members, I had the revelation of how fluent interfaces are really built and how they operate under Closure of Operations. So I decided to expand my previous sample code with a more operators on the spec class, and I came up with a very powerful fluent interface for creating specifications.

Here is the example I coded into my spike, to illustrate a fluent interface. Note that I changed nothing in my implementation of the ISpecification<t> or base Specification<t> from yesterday - other than to add the "Or" and "Not" specifications. The fluent interface was already there and available!

//show the real power of Closure Of Operations, in creating a fluent interface!

ISpecification<Foo> chainedSpec = equalSpec

    .And(passingSpec.And(trueSpec))

    .Or(passingNotSpec)

    .Not(failingSpec.Or(falseSpec))