Code Review: SharpArchitecture.MultiTenant
It has been suggested that I’ll look at a more modern implementation of SharpArchitecture, and I was directed toward the MultiTenant project.
The first thing to notice is the number of projects. It is actually hard to keep the number of projects down, as I well know, but this has several strange choices.
I am not really sure what is the point in separating the controllers into a separate assembly, or why we have a separate project for the ApplicationServices.
I am not the only one thinking so, I think:
Then there is the Core project:
Personally, I wouldn’t create a project for just two files, but I can live with that. I don’t like attributes like DomainSignature. It is hard for me to really say what, except that I think that they encourage a way of thinking that puts the model in the Center of All Things. I am usually much more interested in what something is doing than how it is shaped.
The data project is mostly concerned with setting up NHibernate via Fluent NHibernate.
Next up is the Framework project. And there we run into the following marker interfaces. I really don’t like marker interfaces, and having those here doesn’t seem to be adding anything important to the application.
It seems that there is a lot going on simply to try to get a fallback to a non tenant situation, but the problem here is that it is usually much better to be explicit about those sort of things. You have the CentralSession, and you have the TenantSession, and you are working with each in a different manner. It makes the infrastructure easier to manage and usually result in code that is clearer to follow.
So far, it has all been pretty much uninteresting, I would strongly encourage merging the solution into just two projects, the web & the tests projects, but that is about it.
Now we move into the fancy bits, the controllers project. And there we find the following piece of code:
public class TenantListQuery : NHibernateQuery, ITenantListQuery
public IPagination<TenantViewModel> GetPagedList(int pageIndex, int pageSize)
var query = Session.QueryOver<Tenant>()
.OrderBy(customer => customer.Name).Asc;
var countQuery = query.ToRowCountQuery();
var totalCount = countQuery.FutureValue<int>();
var firstResult = (pageIndex - 1) * pageSize;
TenantViewModel viewModel = null;
var viewModels = query.SelectList(list => list
.Select(mission => mission.Id).WithAlias(() => viewModel.Id)
.Select(mission => mission.Name).WithAlias(() => viewModel.Name)
.Select(mission => mission.Domain).WithAlias(() => viewModel.Domain)
.Select(mission => mission.ConnectionString).WithAlias(() => viewModel.ConnectionString))
return new CustomPagination<TenantViewModel>(viewModels, pageIndex, pageSize, totalCount.Value);
I quite like this code. It is explicit about what it is doing. There is a good reason to hide this sort of thing behind a class, because while it is easy to read, it is also a lot of detailed code that should be abstracted. I like the use of futures to reduce the number of queries, and that we have explicit paging here. I also like the projection directly into the view model.
What I don’t like is that I really don’t understand how the Session instance is being selected. Oh, I understand how MultiTenantSessionFactoryKeyProvider is working, and that we get the central database because we aren’t using a tenanted entity, but it still seems too much magic here I would rather a CentralSession instead.
Another thin that I liked was the code structure:
All of the code is grouped by feature in a very nice fashion.
My main peeve with the application is that this is basically it. We are talking about an application that is basically two CRUD pages, nothing more. Yes, it is a sample app to show something very specific, but I would have liked to see some more meat there to look at.
Multi tenancy is a hard problem, and this application spend quite a bit of time doing what is essentially connection string management.
(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)