Code ReviewSharpArchitecture.MultiTenant
Originally posted at 3/22/2011
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)) .TransformUsing(Transformers.AliasToBean(typeof(TenantViewModel))) .Skip(firstResult) .Take(pageSize) .Future<TenantViewModel>(); 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.
More posts in "Code Review" series:
- (26 Jun 2016) The bounded queue
- (15 Apr 2011) Who Can Help Me
- (01 Apr 2011) SharpArchitecture.MultiTenant
- (28 Nov 2007) PetShop 3.0
- (28 Nov 2007) The PetShop Application
Comments
I think having a separate assembly for controllerers can be pretty useful in certain situations: In a scenario where you have a project with two public facing surface areas, say:
www.myproject.com & mobile.myproject.com. Both sites expose the same functionality - with different UIs for different situations. They can both hook into the same controllers - which appears to me to be a nice way of dealing with the problem.
I guess this project does it because it is based on sharp architecture - and in there it is the default setup.
Hi Ayende,
I don't use sharp architecture but I do usually have a seperate assembly for my application services (I did actually take the idea from s.a.)
So I'm curious what you would normally do with services that could be loosly or otherwise described as "applicatoin services". Within the top level? Or with the domain/core project?
Cheers,
Adam.
@Adam
I like to keep my application services a layer up from my domain. for example, my use cases drive my behavior when designing my application services. I can reuse my core domain between clients (Web, Windows, etc.), however, my app services are really shaped around the consuming client. With a rich client, I can take advantage of a stateful environment, thus my CreateXXX command is much richer than say from the web, where my CreateXXX commands are much smaller and broken up thru various screens.
@Marco yeah me too, so they definitely don't belong in the domain I suppose - which leaves the client if you're not going to have a separate assembly. Always good to hear what others are doing, thanks.
I really like the way marker interfaces are used to distinguish between shared entities and tenant specific entities. With this approach, actually copied one by one from SharpArchitecture, I was able to port a full-blown enterprise project from a single-tenant architecture to a multi-tenant one with minimum effort.
In general I also don't like when there is too much magic going on, but things like multi-tenancy markers or the DomainSignature attribute are really minimizing the amount of code which needs to be written over and over again.
@Oren: How do you implement the Equals-methods for standard-entities? Do you really want to write such trivial code for each of your classes again?
Comment preview