First of all
A relevant answer of mine I think you should read.
Your question is based on your interpretation on the fundamentals of repositories with EF, and that interpretation is not complete (that's not to say mine is provably complete either). This leads to the issues you're uncovering.
However, it requires you to change your interpretation, rather than try to solve the issue the way you're trying to now. I strongly urge you to reconsider your architectural approach, because a more abstracted variant will make this problem a lot easier to tackle.
Answering the actual question
Never forget the responsibilities of each layer.
- The DAL stores and retrieves data for you.
- The BLL performs operations and adheres (and forces other to adhere) to the required business rules, which may or may not involve speaking to the data store.
- The Service layer (as per your naming) is the public interface which gives users access to the BLL operations.
Problem: Depending on the implementation of the Data Access layer, the Domain layer behaves wrong: depending on whether you Include the Orderlines of the Order in GetOrderById, the CanBeExtended behaves differently.
TL;DR The DAL is a dependency of the BLL. The BLL cannot question the DAL's responses as the BLL's actions are based on the responses received from the DAL.
The core issue here is that you are making the BLL responsible for making a judgment call (can this order be extended?), but you are not giving the BLL the autonomy to verify the information it needs to make that judgment call.
The way you currently have it, the BLL's validator can be fooled by passed a bogus list of orderlines into it. The validation is supposed to work as a line of defence which prevents storage of data that violates business rules, and as you've pointed out yourself this line of defence can be easily circumvented. The simple truth is that your line of defence is ineffective.
If the BLL is expected to be the judge on whether the order can be extended or not, the BLL must be able to check the database to count how many lines currently exist. Without it, the BLL cannot make this judgment call.
In other words, you expect something along the lines of:
public bool CanOrderBeExtended(Order order)
{
var existingOrderLineCount = unitOfWork
.OrderLineRepository
.GetByOrderId(order.Id)
.Count();
return existingOrderLineCount < 10;
}
And there you have your answer. The BLL will not behave differently based on the data an external consumer passes to it. The BLL will protect the datastore by verifying the datastore itself.
Note that if the Order object is fetched by the BLL itself and the DAL method that was used to fetch the order already guarantees that all existing order lines are fetched as well (e.g. GetOrderWithAllOfItsOrderLines); then the validation does not need to access the database again, because the DAL itself has already guaranteed that the current list of orderlines is correct.
Something like this would also be correct:
public OrderDto GetOrderById(int orderId)
{
var order = unitOfWork
.OrderRepository
.GetOrderWithAllOfItsOrderLines(orderId);
var result = new OrderDto()
{
Order = order
CanBeExtended = order.OrderLines.Count() < 10;
};
return result;
}
The BLL relies on the DAL to provide the actual contents of the datastore.
But what if the datastore doesn't return all orderlines?
If the datastore is not performing its job (e.g. it only returns one orderline even though there are 15 order lines in the database for this order), then the BLL cannot be expected to protect against that. That is an issue you have to detect (via testing) and solve in the DAL.
The DAL is a dependency of the BLL. The BLL depends on the DAL to be correct. If the DAL is not correct, the BLL cannot be expected to know better than what its own dependencies tell it.
Your proposed solutions
- Lazy loading is considered, but it's not our favorite. It's a technical detail of EF6, and it might turn out to be problematic in a late stage, and highly dependent on the actual setup: db server access speed,...
If you lazy load from the BLL, that means you are leaking IQueryables. DO NOT DO THIS. Leaking IQueryables outside of the DAL means that you've compromised the separation of your layers.
The issue you're highlighting (a second DB call) further compounds the problem but it is not the main decision point as to why you shouldn't be doing this. There are valid solutions which do use a second db call, which might not be what you need in this particular instance but are a better solution than lazy loading nonetheless.
- Making multiple and specific methods on the repository is considered as well, but it doesn't appear to scale well
It scales as well as you can reasonably expect it to.
In this context, you stating it does "not scaling well" seems to imply that you are expecting a generic solution to load any related entity for any entity you're currently dealing with. That's not a reasonable expectation as it compromises the DAL's internal responsibility.
The DAL decides what data gets fetched from the database. The BLL does not decide that. At best, the BLL is allowed to communicate to the DAL using methods which the DAL expliticly exposes.
Simply put, the DAL decides to allow consumer to ask it to retrieve the order lines (= the DAL exposes a public method), and then consumers (the BLL in this case) are able to use this public method to do the thing the DAL has allowed them to do (fetching the order lines).
Allowing the BLL to dynamically alter the fetched data without having an explicitly defined DAL method implies that you are either leaking your IQueryables (as mentioned before, do not do this), or your DAL has some specific publci method which you've created for this purpose (which avoids IQueryable leakage).
The latter inherently means that you've custom built a DAL method that the BLL calls, which according to you "does not scale well". Therefore, by elimination, the only way in which you could have a method which "scales well" is by leaking the IQueryable, which you should not do.
In short, your expectation of how scalable something needs to be is leading you to compromise the separation of your layers. If you want to uphold the separation of your layers, the DAL needs to explicitly expose custom methods (i.e. written by the DAL developer, not just passed on from the EF library) which allow for the retrieval of related entities.
But to decide which method is needed, we'd have to review the implementation of CanBeExtended in the Domain layer.
This is a vary confusing point your making.
I think you're putting the cart before the horse. The DAL doesn't include the order lines just because the BLL wants to validate the data. The DAL includes the order lines because it has either privately decided or is explicitly asked to include them.
Why the DAL was asked to include them (by the BLL) is irrelevant as far as the DAL cares. Yes, the BLL may have asked the DAL for the order lines specifically to validate some business logic, but the DAL doesn't need to know that. All it needs to know is that it was asked to fetch the order lines from the datastore, and therefore it returns the order lines from the datastore.
That is the only responsibility of the DAL. It does not care about your business logic at all, because the DAL lives one layer deeper than the business logic.
OrderLineson BBL objectOrder, which EF could have filled in beforehand (when asked with anIncludestatement)? It seems this OO idea isn't valid in this context? – jan Oct 11 '19 at 08:56public bool CanOrderBeExtended(Order order)(BBL for the record?). On what (kind of) object is this an instance method? We would write it like this:public bool Order.CanBeExtended()Does this differ fundamentally? – jan Oct 11 '19 at 08:58Domain.Order order = orderRepository.GetOrderById(orderid);, 2. we then query that BLL/domain object:var result = order.CanBeExtended();We firstly retrieve using the DAL, and next use the BLL/domain object. Our BLL itself isn't a consumer of DAL, it relies on an external party ("Service layer" in our case) to ask the DAL the right questions. – jan Oct 11 '19 at 10:54Service -> BLL <- DAL, because this configuration keeps our BLL dependency-free (so it can be tested) as well minimizes the overall dependency graph. Any other configuration either adds a dependency to our BLL and/or increases total coupling. Second we want, at runtime, a flow of control that looks likeService -> BLL -> DAL -> BLL -> Service... – user3347715 Oct 13 '19 at 20:41BLL <- DAL != BLL -> DAL) We introduce IoC to solve this problem. That is, we resolveBLL -> DALat runtime using an interface. In concrete terms, it means that ourRepositoryinterfaces are part of theBLLand the implementations are part of theDAL. In this way we can achieve clean architecture. @jan. Your architecture is sound. The problem you are facing is that you have not discovered an appropriate model. – user3347715 Oct 13 '19 at 20:41Serviceis referring. For example a "service" is a controller method. Said another way, the ASP.NET framework is the code that surrounds (frames) the BLL. I'm afraid I don't see how this relates to my point. I'm aware of the flow of control you outlined (I read the thread) and it is correct. The point I am making is that BLL should not depend on the DAL. It makes testing a nightmare, and it's avoidable using IoC. – user3347715 Oct 13 '19 at 21:40Repositoryinterfaces into the BLL OP can achieve the correct dependency graph. Your examples, both above and in the thread with OP, show the BLL directly interacting with the DAL. This is what I'm attempting to put finer point. OP actually has a sound architecture if they move theRepositoryinterfaces (though this is still largely academic). Asking them to inject a dependency into the domain model is unnecessary. – user3347715 Oct 13 '19 at 21:50unitOfWork.OrderRepositoryis (it's anIOrderRepositorythough that was never explicitly elaborated on since it's not the focus of the answer). Putting the repository implementation in the BLL does not work because that means the DAL must therefore be exposing the db context class (to be injected in a BLL repository) which inherently means exposing EF's baseDbContextclass which in turn means you'd be leaking EF into the BLL. – Flater Oct 13 '19 at 21:57Repositoryinterfaces are in the BLL not implementations. For what reason should a method in theOrderentity need to issue a query to anyRepository?. Again, there are three concrete reasons to avoid this: performance cannot easily be optimized if hydration of the domain does not occur at the top level, we are adding a dependency to our domain model, and it cannot be easily tested (now we need some sort of mockIRepositoryin order to test a singleint). I have posted an answer outlining my position. – user3347715 Oct 13 '19 at 22:15