Preventing presentation layer from directly accessing repositories?

Question

I'm trying to design my application in line with the "onion architecture" principles, and have something like the following:

HospitalController (depends on IHospitalService)
\
  --- HospitalService : IHospitalService (depends on IMedicalRepository)
      \
        --- SqlMedicalRepository : IMedicalRepository

My IMedicalRepository allows hospital data to be stored, but I have some business logic rules restricting how the data can be stored (for instance, either field A or field B must be non-null, and field C's format must match a regex to ensure that it is in a standardized format). These rules are enforced in IHospitalService.

The problem with onion architecture and transitive references is that HospitalController must reference the project containing IMedicalRepository as well as the one containing IHospitalService. So it's going to be possible for the HospitalController to directly request IMedicalRepository and store hospital data, bypassing these rules. Is this just an inherent thing in onion architecture where you have to just remember not to do this? Ideally, there would be a way of designing things where the presentation layer simply can't request IMedicalRepository directly as there's no design-time reference to it, thereby forcing it to go through the business logic of IHospitalService.

Answer 1

The problem with onion architecture and transitive references is that HospitalController must reference the project containing IMedicalRepository as well as the one containing IHospitalService.

That is not correct, it's not necessary for the web layer to directly access the data layer. I'll discuss the specific implementation later, but we first need to address whether we should be enforcing this to begin with.

The rabbit hole of compliance

The issue here is a distinction on whether good practice should be expected to be followed, or rigorously enforced.
By that I mean that we're all in agreement that the controller shouldn't be referencing the repository as a matter of good practice, but not everyone specifically implements it so that it is actually impossible to do so, instead relying on the implicit expectation of developers doing the right thing and following good practice.

Part of the reason for not spending time on enforcing it is because you can never truly enforce it, and the more you try to approach a complete enforcement, the more overhead it requires (at an exponential rate).

By that I mean that when push comes to shove, a third project (business) cannot prevent one project (web) from depending on another (data). If the web project decides to reference the data project, it does not need consent from other projects to do so.

In order to block that, you would need rigorous access control between these layers, i.e. the web project developer shouldn't even be able to access the data layer's source code (or nuget package).
That bring a boatload of overhead with it. Other than the effort required to manage it all, it creates a lot of hurdles for developers to get over, especially when they're expected to develop features from top to bottom (database to controller, in this case).

If you have separate teams for the web project, the business project and the data project, then this becomes more feasible, as hardly any developer needs access to all layers at once. But this entails a lot of cooperation between separate teams, which is an overhead cost in time, effort, and risk of bugs/miscommunication. It also slows down development, as a change to the data layer needs time to permeate through to the business layer and then the web layer.

Most projects tend to not do this because of the overhead cost. Instead, developers have access to all the projects, usually in a single solution file, and are expected to uphold proper layer good practice. But the only thing holding them back from sullying this good practice is a stringent code review process.

---

Increasing compliance

Having established that compliance is a spectrum that leads to a rabbit hole, you clearly seem to favor increasing compliance, even if not down the entire rabbit hole. So can we enforce more compliance than just blindly trusting our developers and their code reviewers?

The problem with onion architecture and transitive references is that HospitalController must reference the project containing IMedicalRepository as well as the one containing IHospitalService.

What you state here isn't fully correct.

Your interpretation relies on the assumption that your top-level application (web, in this) case has direct access to all of the dependencies that need to be registered in the service provider. But that is avoidable, by delegating the registration responsibility to each layer.

It's easier to show this in code.

The simplest way of doing registration is by hardcoding it into the top-level application:

// in Web project

services.AddScoped<IFooRepository, FooRepository>(); // data dependency
services.AddScoped<IFooService, FooService>();       // domain dependency

But most projects tend to already segregate this registration to the project itself:

// in Data project

public static class ServiceCollectionExtensions
{
    public static IServiceCollection RegisterDataDependencies(this IServiceCollection services)
    {
        services.AddScoped<IFooRepository, FooRepository>();

        return services;
    }
}

// in Business project

public static class ServiceCollectionExtensions
{
    public static IServiceCollection RegisterBusinessDependencies(this IServiceCollection services)
    {
        services.AddScoped<IFooService, FooService>();

        return services;
    }
}

// in Web project

services
    .RegisterDataDependencies()
    .RegisterBusinessDependencies();

In the current example, the web project still depends on both the business and data project. You specifically want to break the reference between the web and data project, to prevent direct access to the data project's dependencies.

This can be done, but then you have to shift the responsibility of registering the data dependencies (i.e. calling the RegisterDataDependencies() method) to a project that does have a reference to the data project. Luckily, the business project still has a reference!

All you have to do to accomplish this is to have the business project's dependency registration logic call the data project's dependency registration logic:

// in Data project

public static class ServiceCollectionExtensions
{
    public static IServiceCollection RegisterDataDependencies(this IServiceCollection services)
    {
        services.AddScoped<IFooRepository, FooRepository>();

        return services;
    }
}

// in Business project

public static class ServiceCollectionExtensions
{
    public static IServiceCollection RegisterBusinessDependencies(this IServiceCollection services)
    {            
        services.RegisterDataDependencies();

        services.AddScoped<IFooService, FooService>();

        return services;
    }
}

// in Web project

services.RegisterBusinessDependencies();

And this provides you with some reasonable protection against wantonly referencing the repositories directly. You can now remove the reference between the web and data projects. And as long as that reference does not exist, it's impossible for the web project to actually use the data project's repositories.

Note that I've hidden some additional complexities, such as if the Web project has more than one dependency which in turn depends on the same data project. Those can be worked around, but you need to address some key concerns such as what to do in case one of those projects depends on a different version of the same data project, if that is a possibility in your situation. It usually isn't, but I can't exclude that with certainty.

This is not perfect protection. Developers are likely still able to re-add this project reference.

This is where we get back to the rabbit hole. You could try to devise some clever enforcement making it impossible for developers to do this, but the benefits from doing so rarely outweighs the effort required to do so.

Instead, it's a lot easier to catch these kinds of errors during a code review process. While this does rely on the ability of the code reviewer to catch this, you should already be doing code reviews and those reviewers should already be aware of good practice rules like these.

Answer 2

The big problem here is the IHospitalService contains business logic.

The rules governing how data should be created and manipulated are called Class Invariants. These invariants should be enforced in classes that specialize in business rules, and business rules only.

A repository is a service in its own right — it is a domain service. The IMedicalRepository is a domain service for medical data that provides a layer of abstraction between SQL Server and the domain model. Repositories should abstract away data access. Repositories should definitely not implement business logic.

The IHospitalService object is a layer of abstraction between the controller and use case. It should not implement the kind of logic you describe, where one of two fields should have a value, or field C should match a certain pattern. Instead, it should coordinate the interaction between controller, domain model and IMedicalRepository. Services are the proper place to put logic that spans multiple layers.

The thing you are missing in between the controller, service class and repository is your domain model. The domain model should be enforcing that one of field A or B should be not-null. The domain model should enforce that field C follows a particular pattern (which might be a candidate for a value object).

Now it doesn't matter whether you use the IMedicalRepository or IHospitalService because those common constraints on data do not exist in the service layer. Instead, they exist in the domain model itself, and are thereby portable.

Pushing domain logic into a "service layer" results in an anemic domain model, and gives rise to problems like you describe.

Answer 3

I know this might not be the answer you are looking for, but as soon as you can access an interface/class/whatever outside of a project (IHospitalService -> IMedicalRepository) every other project can access this too. Finer grained access-control is only possible within on project. So no one forbids to reference IMedicalRepository from the HospitalController-Project.

The only possibility would be to put IHospitalService and IMedicalRepository in the same project and set the visibility of IMedicalRepository to internal, but I guess that kills your layered architecture idea.