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.
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 ???
I'm currently working on a search screen for a shipment tracking system. At the bottom of this screen, there are 4 checkboxes that will determine whether or not we are supposed to display Imports, Exports, Air shipments, or Ocean shipments - in whatever combination the user wants: Down in the depths of the search process, I am creating a "finder" object, as Ayende talked about quite a while back. In this object, I have to account for the checkbox values here - whether or not the user wants to display whatever types of shipment (import or export) and/or the method of shipment (air or ocean). What gets really interesting is that when a box is un-checked, I should not show that particular type or method. A quick analysis of these 4 checkboxes will come up with the following variants that must be accounted for when building the query. - Show Imports Only
- Ocean and Air
- Ocean only
- Air Only
- Show Exports Only
- Ocean and Air
- Ocean only
- Air only
- Show Imports and Export
- Ocean and Air
- Ocean only
- Air only
- Show no imports or exports (returns no results)
- Show no ocean or air (returns no results)
All totaled up, that's 11 different query variants that have to be accounted for. The end result of my query build methods is the following: private void AddShipmentMethodCriteria(DetachedCriteria criteria) { ICriterion air = Restrictions.Eq("ShipmentMethod", "Air"); ICriterion ocean = Restrictions.Eq("ShipmentMethod", "Ocean"); if (searchCriteria.ViewAirShipments && searchCriteria.ViewOceanShipments) criteria.Add(Restrictions.Or(air, ocean)); else { if (searchCriteria.ViewAirShipments) criteria.Add(air); else criteria.Add(Restrictions.Not(air)); if (searchCriteria.ViewOceanShipments) criteria.Add(ocean); else criteria.Add(Restrictions.Not(ocean)); } } private void AddShipmentTypeCriteria(DetachedCriteria criteria) { ICriterion import = Restrictions.Eq("ShipmentType", "Import"); ICriterion export = Restrictions.Eq("ShipmentType", "Export"); if (searchCriteria.ViewImports && searchCriteria.ViewExports) criteria.Add(Restrictions.Or(import, export)); else { if (searchCriteria.ViewImports) criteria.Add(import); else criteria.Add(Restrictions.Not(import)); if (searchCriteria.ViewExports) criteria.Add(export); else criteria.Add(Restrictions.Not(export)); } }
By having the first If statement check for both 'Import' and 'Export' being requested, we can properly create our query to show both of them via the Restrictions.Or() criteria. Additionally, we have to account for either or both of them not being checked and explicitly call them out to say that we do not want to show whichever one is not selected. The same is true for the 'Air' and 'Ocean' shipment methods.
The end result is that the user can select or un-select whichever shipment methods and shipment types they want, resulting in the correct data being displayed.
...
On a side note, there's probably some abstraction that I could create where I pass in the ICriterions and the boolean flags to help reduce code redundancy but that's not the point of this post. I'm really trying to illustrate the complex logic used in creating the correct NHibernate ICriteria/DetachedCriteria, and the analysis that you need to undertake for what looks like the most simple of situations. After all, how hard could it be to create a query based on 4 check boxes? ... more difficult than you might imagine, at first. Take the time to analyze even simple scenarios like this.
A coworker asked this question about our automated integration test suite, recently: "with the RowTest feature of NUnit, does the test data being used need to be hard coded into my code or can it be a variable that gets defined based on some rule?" Here is my response, outlining the need to know the state of the database (know every value of every field, in every table) before and after the test suite is run: The short answer is that the tests will have data hard coded into them. This may be a change from how you have looked at integration testing, previously – where the data is changed on each run, so as not to duplicate the data in the system. Rather than changing the data that is used during each test, we need to ensure that the data is exactly what we expect, before and after each test. If the data is not what we expect before the test is run, the test cannot produce the data that we expect to have after it is run. If the data is not what we expect after a test is run, the test fails. At a very high level, automated tests against the database require three things: - A known, state of ALL data in the database
- A known, suite of unit tests that manipulate said data in predictable ways
- An expected, verifiable suite of data returned by the software and manipulated in the database
The data is not necessarily "perfect" in that it has no flaws in it – rather, every last character in every last field of every last table is known, and all of the ramifications of that data is known with the explicit purpose of that data being there to support the automated tests. The automated tests expect this "perfect" data to produce the desired results and verify that the system works as expected. Any data that is manipulated in the system is expected to be in a predictable state, so the tests can verify that the system manipulated the data correctly. When it comes to implementation of this, there are some fairly strict requirements for it to work: - Before the first test is run from the test suite, the data must be exactly what the tests expect
- After the tests have run, the data must be exactly what the tests expect
- Repeat for each run of the test suite
What this really means is that we cannot add or modify data between two test runs, without changing the tests that work with the data. In order for the tests to run multiple times throughout the day and/or on-demand, we have to ensure that the data being manipulated is what we expect it to be. Therefore, the first step of any run of the test suite is to revert the database to the known state, through sql scripts or code that are automatically run before the tests are run.
Myself and 10 other developers in my company went through a day of BDD / TDD training, with Scott Bellware, yesterday. It was a lot of fun, very challenging at times, and covered a lot of topics including an overview of Agile and Behavior Driven Development, all the way down to writing Specification Tests, doing Test Driven Development and refactoring the model to improve readability, maintainability, flexibility, etc. I took notes via index cards (love that cool-aid) and wanted to share. I don't expect these notes to make sense to everyone. Hopefully it will spark some dialog in someone's mind and cause them to dig further. First off - the quote of the day. "Can I be honest with you and say that I've been wanting to touch your keyboard, all day?" Now for my notes.  User Stories [Role], [Goal], [Motiviation] - As a [role], I want to [goal], so that [motivation]
- Example: As a nurse, I want to record a patients vital signs, so that I can determine their medication and care needs
- Motivation is critical - it determines how the development team understands and implements the story. It determines the user experience, how things are integrated, how the software is designed, etc.
Acceptance Criteria - Acceptance Criteria is used to drive code, not the story, directly
- may change at any point, up to implementation
- is used to drive code design, test design, implementations, etc.
- should be spoken in domain language
- may include non-functional, technical details such as database tables, infrastructure, performance, etc
- All acceptance criteria must be met and tested / verified before a story is considered done
Specification Tests - Test Fixture per Class is an anti-pattern (on a personal note, this problem bothered me for months before I discovered BDD)
- Context Specification or Behavior Specification testing
- When [verb] then [verb]
- "When [verb]" is the context
- "Then [verb]" is an observation of the behavior
- Based on Acceptance Criteria, but not "code-gen'd" from acceptance criteria
Story Estimation - Agile Poker: uses generalized Fibonacci sequence as order of complexity
- "?", 0, 1/2, 1, 2, 3, 5, 8, 13, 20, 40, 100, infinite
- everyone throws their estimate at same time
- if estimates have significant outliers, discussion occurs to understand why, get more detail, etc. and re-throw may happen
Entity Data vs. Aggregate Data - Entities should never contain aggregate data
- Aggregate data is for reporting and other aggregate needs
- If you need aggregate data to process something, write an SQL query, stored proc, etc. - don't use an ORM like NHibernate
- We don't want a "Customer" entity to need 10,000 "Order" entities, to aggregate data for processing; write a query to aggregate instead
- We don't want to persist data that can be calculated / aggregated, generally (performance issues may override this)
Domain Services - Can have dependencies on external systems
- are part of domain logic, therefore are in domain model / assembly
- are "Doers" of process that don't fit into entity and entity logic, directly
- coordination of entity logic
- can include calls to data access, logging, etc.
Continuous Integration - Not just continuous compilation of code
- Full end to end integration of all code, components, databases, services, etc
- Full suite of integration testing including database testing
- Do not allow commits if build is currently broken
- do not allow defects to live - fix immediately, to fix build
- "Defect" is broken software, "Bug" is functional but wrong
Daily Scrum - 3 Questions everyone answers:
- What did I do yesterday?
- What am I doing today?
- What issues am I having?
- Each person should answer quickly - 1 or 2 minutes, max
- further discussion happens outside of the Scrum meeting
Productivity of Dev Team - RAD and other non-review, non-iterative based management causes problems and loss of productivity
- we need constant review of the design to ensure good design
- shorten the feedback loop and get constant review of the design, to always improve the design, via pair programming, work cells, retrospectives, etc.
- good design will cause productivity gains in the development team beyond the capabilities of any tools
Whiteboard Diagramming vs. Details Specs - White board diagramming and human interaction is always better than detailed documents and specs in UML
- Human interaction leads to knowledge crunching and learning, not just reading a repeating
- Take pictures, don't re-draw in UML; don't waste time with it
- Video the entire conversation is even better, so others can learn from the knowledge crunching that occurs; capture the human interaction, body language, etc.
In my previous post, I talked about my base repository class that I use, to abstract the NHibernate details away from the actual repository. Now that I have the Do and DoTransaction methods to further the abstraction, I thought it would be good to share my whole abstraction. using System; using NHibernate; using NHibernate.Cfg; namespace RepositoryBase { public abstract class Repository : IDisposable { #region Vars private static readonly Configuration _config; private static readonly ISessionFactory _factory; #endregion #region Constructor / Destructor static BaseDAL() { _config = new Configuration(); _config.Configure(typeof(BaseDAL).Assembly, "RepositoryBase.hibernate.cfg.xml"); _factory = _config.BuildSessionFactory(); } ~BaseDAL() { Dispose(); } #endregion #region Properties public ISession Session { get; private set; } public ITransaction Transaction { get; private set; } #endregion #region Methods protected void Do(Action unitOfWork) { try { OpenSession(); unitOfWork(); } finally { CloseSession(); } } protected void DoTransaction(Action unitOfWork) { try { |