Skip to content
This repository has been archived by the owner on Jul 1, 2023. It is now read-only.

Question: Why is Application Layer depending on Persistence Layer? #18

Closed
petar-m opened this issue Oct 25, 2018 · 20 comments
Closed

Question: Why is Application Layer depending on Persistence Layer? #18

petar-m opened this issue Oct 25, 2018 · 20 comments
Assignees
Labels

Comments

@petar-m
Copy link

petar-m commented Oct 25, 2018

Hey,
great work on this Clean Architecture example.

There is one contradiction that bothers me -
the Application Layer ReadMe states: It is dependent on the domain layer, but has no dependencies on any other layer or project.
Looking at the Northwind.Application project it has a dependency on Northwind.Persistence.

Imo this violates Clean Architecture principles because it introduces coupling with the persistence details.
Persistence should be just another plug-in into the Application Layer - just like the Northwind.Infrastructure is.

Some ways for fixing this I can think of are:

  • make this a compromise for the sake of simplicity;
  • commands and query classes related to persistence can become abstractions exposed by Application Layer to be implemented in Persistence Layer;
  • fall back to Repository and UnitOfWork abstractions.

Will appreciate your opinion on the matter.

@Miggleness
Copy link

Miggleness commented Oct 26, 2018

I saw Jason's video on YouTube yesterday and had the same exact question. Sorry, @jasongt . This is our only avenue to ask this question for now. This GH repository may just be a work in progress and the end result is we'll see the expected dependency graph adhering to Clean Architecture as in the talk.

One way is to create an interface for our DbContext so we can reverse the dependency between Application and Persistence. For example, the EF Reverse Engineer POCO partners an interface DbContext in the generated code. Alternatively, you can always right-click -> Extract Interface.

///we'll need something like this in our core project
public interface IDbContext : IDisposable
    {
        int SaveChanges();
        int SaveChanges(bool acceptAllChangesOnSuccess);
        Task<int> SaveChangesAsync(bool acceptAllChangesOnSuccess, CancellationToken cancellationToken);
        Task<int> SaveChangesAsync(CancellationToken cancellationToken);
    }

However, this approach does leave some stomach cramp since our application still ends up having a dependency on EF Core. Like there are other methods of implementing the interface so this makes me go, "Who are we kidding?". Might as well define your DbContext inside the application layer.

Thoughts?

@petar-m
Copy link
Author

petar-m commented Oct 26, 2018

I would rather not deal with DbContext in Application Layer at all.
(Considering it an I/O tool, just like FileSystem or WebRequest - an implementation detail)

e.g. GetCustomerDetailQueryHandler:

// in Application use only abstraction
public IGetCustomerDetailQueryHandler : IRequestHandler<GetCustomerDetailQuery, CustomerDetailModel>
{
}

and then wire up whatever handler implementation I like

// in Persistence
public class GetCustomerDetailQueryHandler : IGetCustomerDetailQueryHandler
{
    public async Task<CustomerDetailModel> Handle(GetCustomerDetailQuery request, CancellationToken cancellationToken)
    {
        // use EF, Dapper, FileSystem, whatever is suitable
    }
}

thus inverting the dependency.

If I want to be really "Clean" then I will not use Domain Entities as EF Entities.
Domain Entities will be known only to Application and exposed to Persistence layer as DTOs (or only interfaces).
The Persistence will have it's own EF Entities best suited for storage and will transform from/to DTOs.

I guess the decision point here is whether I will be ok to couple my Application with EF.
Thankfully there is an exact answer for this: "It Depends". 😉

@jasontaylordev
Copy link
Owner

Hi @petar-m and @Miggleness. Thank you for your feedback and insights. I will update the Application layer readme to correctly reflect the current architecture & design.

Viewing the original blog post on clean architecture, the following two points are relevant:

(1) Independent of Frameworks
(4) Independent of Database

In the case of Entity Framework Core, you could say that we violate Clean Architecture because we are not independent of frameworks. If we want to be in the position to switch out EF Core for another data access technology in the future, we won't be in the best position to do so.

However, we are in fact independent of database, in that we could change Oracle for SQL Server or something else, our logic is not bound to the database.

So why did I choose this approach? For the sake of simplicity. I am all for implementing a repository / unit of work or some other abstraction if there is value in doing so. For example, if I wanted to restrict access to my entities, I could create repositories for the aggregate root only. I don't want to implement an abstraction just because I might need to change my data access technology in the future. I'll deal with that pain should it ever occur.

@Miggleness - I agree with you regarding interfacing DbContext, it doesn't add much value. DbContext is already an abstraction.

To fix this issue and be "100%" clean, I would implement a repository combined with the specification pattern. This combination works well and would help to avoid some of the pain usually associated with repositories.

What do you think? Let's discuss further. Maybe it's time for an improvement.

@Miggleness
Copy link

Miggleness commented Oct 29, 2018

Thanks for responding, @jasongt

I agree that we shouldn’t design for “what if” scenarios. They dont happen. Deal with the pain when we do get there.

In my experience the best way to a greenfield project is to use a template that is well understood, code to meet requirements in the quickest way possible and continuously refactor in the process. There is a balance between practicality and correctness, and it’s different for each solution.

Re: specifications. I somewhat prefer a more traditional approach of using a method to achieve something against your persistence layer. Im talking from a perspective of having a dev team of mixed skills where intention revealing interfaces in code has strong cognitive strength.

I really enjoyed your talk, and yours wasnt the first talk on clean architecture ive seen. I’m basing one of the systems im working on on your talk and get better clarity from it, I can better weigh the tradeoffs of possible implementations for clean architecture

@petar-m
Copy link
Author

petar-m commented Oct 29, 2018

I absolutely agree with the simplicity point. With this approach it is very easy to get up and running fast and also be totally decoupled form the UI.

Also a very valid point - this may be everything you'll ever need. It depends greatly on the problem being solved. My preference is to have more options down the road.

I personally also tend to move away from repositories. Generic repository with specifications is good but only to some extent - read-only data, caching, this weird query we have, batch processing - trying to fit all these behind a repository interface is a burden.

As a suggestion - what if you use Mediatr within Application Layer for manipulating data?
Have a request and response classes defined in Application and implement handlers in Persistence Layer.
This way Application will take care of business logic and Persistence will handle the DB.

@tedchirvasiu
Copy link

tedchirvasiu commented Oct 29, 2018

Well, just to be the devil's advocate, aren't we already dealing with pains ahead of time and writing for "what if" scenarios by striving for a "clean architecture"? Simply decoupling the presentation from the rest of the application may be unnecessary for many applications as they'll either never implement more than one presentation layer or they'll die (or be rewritten) before the next best front end framework comes around. And you could deal with that particular pain when need be too, right?

Sure, perhaps front end frameworks may lose coolness faster than ORMs at the moment, but still I think it doesn't feel quite right to close an eye on the Persistence Layer and Entity Framework making the rest of your system dependent on them when they should ideally not be.

Now to be real, repositories and more abstractions give you back the purity in the Core but I have to agree with you that it's too much of a burden to deal with them and you lose a lot of what EF has to give. Personally I do tolerate this impure dependency between Application and Persistence in my projects too.

I think the fundamental problem is that persisting data sucks and has sucked for decades by design because we had to map relational data to objects and ORMs were never perfect in completely hiding the seam between the two paradigms. Until we will have big, fast and cheap enough SSD's to treat storage like we treat RAM (as in Uncle Bob's vision), we probably will be forced to chose between impurity and the burden of abstractions upon abstractions.

@jasontaylordev
Copy link
Owner

jasontaylordev commented Nov 2, 2018

As a suggestion - what if you use Mediatr within Application Layer for manipulating data?
Have a request and response classes defined in Application and implement handlers in Persistence Layer.
This way Application will take care of business logic and Persistence will handle the DB.

This will work, as long as we remember that all dependencies must point inwards. That would mean that our request handlers inside persistence could only take a dependency on application, and could not take a dependency in infrastructure (as shown in the video).

Well, just to be the devil's advocate, aren't we already dealing with pains ahead of time and writing for "what if" scenarios by striving for a "clean architecture"

This is a really good point and something that I hadn't really considered until you mentioned it. If we can work out how long the app will be around, it is easy to make the right choice here.

Based on this conversation I think I'll continue working with DbContext for now. There are some other improvements that I would like to make including:

  • Removing the domain from the queries (and avoid projecting from domain to view model)
  • Using query types or otherwise to optimise simplicity and performance of queries
  • Injecting or configuring a DbContext with change tracking disabled, one DbContext for reads another for writes

Let me know what you think. Any other suggestions?

@danielhunex
Copy link

I agree with @petar-m. The principle of Clean Architecture or Onion Architecture is to create dependency from the outer layer towards in the inner layer. But here the Application layer ( which is part of the core) depends on 1. on the persistence layer ( which should be essentially infrastructure layer) and 2. on a particular implementation ( say I want to use NHiberate, this breaks the core). In addition to that, @jasongt pointed out the Core shouldn't be dependent on frameworks or external libraries but here it depends on the MediatR library. I think the code needs refactoring in order to be an example for Clean /Onion Architecture

@gliljas
Copy link

gliljas commented Nov 3, 2018

It's great as it is. Complicating stuff with persistence ignorance would take away from the usefulness and approachability of this example. Switching to NHibernate would require minor changes and a full recompile. Worth the effort. Other data persistence technologies often have different behaviors when it comes to ACID, queryability etc, so it's usually impractical to make them "pluggable".

@jasontaylordev
Copy link
Owner

I agree that if we remove the dependency on EF Core and MediatR, that this would be an even better example of Clean Architecture. However, when it comes to moving away from the example project and building the next real-world project we will have many choices to make. We might choose to sacrifice flexibility for simplicity by using EF Core and MedaitR directly, or we might not. The project currently reflects an outcome of those choices.

It would be hard for me to incorporate all different options into a single project and presentation. Next time I run the presentation, I'll make those choices clear so that people understand the trade-offs. Thanks again for all of the feedback!

@AdrianoAE
Copy link
Contributor

Hello, before anything thank you very mutch for doing this sample. Before the random youtube suggestion of one of your presentations talkin abou that, I was finishing reading the Microsoft Microservice e-book about eShopOnContainers. But for someone like me that is just starting and don't have the previous knowledge of enterprise stuff (more advanced stuff) the eShopOnContainers was being very overwhelming to start learning (again at least for someone who is completly new to all the tech being used.

Your sample + the presentation is very clear and simple. So glad I found that and hope you keep updating this (waiting for the improvements you posted above 😄).

I know this probably isn't the best place to say this but I'm really thankfull for that. And if any tip from where to start (I started learning Entity Framework Core (Pluralsigh courses from Julie Lerman) and I'm starting to learn ASP.Net Core) would be very helpfull.

Once again, thank you very mutch.
Wish you the best of luck.

@Hoopie
Copy link

Hoopie commented Nov 4, 2018

Been reviewing the code over the last few days, observed the Application had a dependency on persistence. Just found this thread, what a good read!

  • I guess to add also.. Looking back at the conversations about re' "it is truly clean" - well technically it may miss the principal of clean architecture for the reasons you all have been debating. But one thing i can say is - everything has it's place and that place is obvious! This code sample is a dream for any senor responsible for a number of juniors in a team. Any pull request with code out of place would "stand out" that's for sure. Clean promotes clean i suppose.

  • if only my companies source was half as clean. 😜

@jasongt appreciate your efforts on this. I enjoyed reviewing this sample and everyone's feedback. Many Thankx

@lloydjatkinson
Copy link

lloydjatkinson commented Nov 10, 2018

Say we wasn't using a DB at all and instead some NoSQL such as Azure Table Storage, and therefore no DbContext. How would that fit into this design?

I was slightly surprised by Application depending on a DbContext provided by Persistence. Currently every Command/Query handler has this DbContext injected. Wouldn't this mean that that Application is now not data storage agnostic, as it assumes there will always be a DB, and would need to be refactored to support something like Azure Table Storage?

How would you go about decoupling Application from the specific data storage being used? I initially thought that Persistence was responsible for all data access concerns and that Application would depend on interfaces defined in Persistence with their implementations being defined in Infrastructure or Persistence (not sure which), and then all wired up in IoC later.

I guess in order to allow Application to support either a DB or NoSQL a "repository" should be made - not the kind to allow you to swap DB (that's what DbContext is for) but one to allow you to have a totally different data storage mechanism? If that's the way forward, where should the repository interfaces and implementations be? Is there a better approach to allowing for decoupled data storage here?

e.g.

public class GetCustomerDetailQueryHandler : IRequestHandler<GetCustomerDetailQuery, CustomerDetailModel>
    {
        private readonly ICustomerPersistence _persistence;

        public GetCustomerDetailQueryHandler(ICustomerPersistence persistence)
        {
            _persistence = persistence;
        }

        public async Task<CustomerDetailModel> Handle(GetCustomerDetailQuery request, CancellationToken cancellationToken)
        {
            var customer = await _persistence.GetCustomerDetailAsync(...);
            ...
            return customer;
        }
    }

    // Persistence interface (perhaps should be declared in Infrastructure instead?):
    // ICustomerPersistence

    // Persistence implementation (perhaps should be declared in Infrastructure instead?):
    // CustomerPersistenceDbContext, CustomerPersistenceTableStorage, CustomerPersistenceFlatFile etc.

Do the things I'm asking make sense? Let me know if not 😄

Thanks for making this repo to accompany your video @jasongt

@petar-m
Copy link
Author

petar-m commented Nov 11, 2018

There will always be trade-offs when implementing an architecture in the real world.
For me it was important to understand these trade-offs for the presented solution and the reasoning behind the decisions - which was made clear in this thread.

I got the answer to the original question, so I guess it is up you @jasongt - you can close this issue because it is not an issue in the context of your approach or leave it open - it makes a good read.
Thanks everyone.

@Miggleness
Copy link

@lloydjatkinson. As @petar-m mentioned, it depends on your requirements of your architecture. You cant build every application with the assumption that you’ll jump from relational to one of the various nosql variants and back.

I have a bias towards your repositories pattern in most implementations because of them being “intention revealing”

@lloydjatkinson
Copy link

It was more a question of why Application is aware of what the data access is, when I was under the impression that Persistence should abstract that concern away and deal with it itself. Would you say Persistence would be a good place to do so?

@jasontaylordev
Copy link
Owner

Hello, before anything thank you very mutch for doing this sample. Before the random youtube suggestion of one of your presentations talkin abou that, I was finishing reading the Microsoft Microservice e-book about eShopOnContainers. But for someone like me that is just starting and don't have the previous knowledge of enterprise stuff (more advanced stuff) the eShopOnContainers was being very overwhelming to start learning (again at least for someone who is completly new to all the tech being used.

Your sample + the presentation is very clear and simple. So glad I found that and hope you keep updating this (waiting for the improvements you posted above 😄).

I know this probably isn't the best place to say this but I'm really thankfull for that. And if any tip from where to start (I started learning Entity Framework Core (Pluralsigh courses from Julie Lerman) and I'm starting to learn ASP.Net Core) would be very helpfull.

Once again, thank you very mutch.
Wish you the best of luck.

Thanks for your kind feedback. I think Pluralsight is a great place to get started. I like this course https://app.pluralsight.com/library/courses/aspdotnet-core-fundamentals/ by Scott Allen and this course https://app.pluralsight.com/library/courses/aspnetcore-mvc-efcore-bootstrap-angular-web/ by Shawn Wildermuth.

@jasontaylordev
Copy link
Owner

It was more a question of why Application is aware of what the data access is, when I was under the impression that Persistence should abstract that concern away and deal with it itself. Would you say Persistence would be a good place to do so?

@lloydjatkinson thanks for your feedback. If implementing a data access abstraction the interfaces would be contained within the Application layer and the implementations contained within Persistence. We shouldn't put the interfaces in Infrastructure / Persistence, since this would force Application to depend on Infrastructure / Persistence, rather than the other way around. Cheers!

@jasontaylordev
Copy link
Owner

I will close this issue off now, thanks to everyone for contributing to this discussion!

@dumchikov
Copy link

@jasongt Wouldn't be better to create a separate IoC project on the infrastructure level to resolve dependencies and it would allow us to have just Application and IoC refferenced. Thus, all implementations from would be hidden for Presentation level. Does it make sense?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

10 participants