Architektur (3 Schichten Model und Dependency Injection)

Servus,

ich habe vor einigen Jahren als Quereinsteiger in der Softwareentwicklung angefangen. Dem entsprechend sehe viele meiner älteren Projekte aus, viel gewuchert und kaum sortiert. Nun bin ich beim Refactoring meines ersten Projekts (WebAPI) und mir haben sich (bis jetzt) zwei Fragen ergeben.

Wie setze ich das 3 Schichten Model und Dependency Incection richtig um?

Meine Controller sehen beispielsweise so aus:

    public class PersonController : Controller
    {
        private readonly PersonService personService;
        public PersonController(IPersonService _personService)
        {
            personService = _personService;                       
        }
        
        public IActionResult UpdatePerson(Person person)
        {
            IActionResult result = personService.UpdatePerson(person);
            return DeletedResult(result);
        }

        public IActionResult DeletePerson(int id)
        {
            bool result = personService.DeletePerson(id);
            return result;
        }
    }

Jeder Controller bekommt per DI einen entsprechenden Service in den Konstruktor. Die Services bekommen ein Repository und de Repository bekommen Credentials, aller per DI. So weit, so klar.

Meine ServiceImplementierung sieht beispielsweise so aus:

    public class PersonService : IPersonService
    {
        private readonly IPersonRepository personRepository;
        public PersonService(IPersonRepository _personRepository)
        {
            personRepository = _personRepository;
        }

        public bool DeletePerson(int id)
        {
            int result = personRepository.DeletePerson(id);
            return (result == 1);
        }
    }

Nun möchte ich aber beispielsweise in meiner PersonService.Update(Person) unterschiedliche Handler/Klassen nutzen, um die Person weiter zu bearbeiten:

        public IActionResult UpdatePerson(Person person)
        {
            IActionResult result;
            MaleHandler maleHandler = new MaleHandler();
            FemaleHandler femaleHandler = new FemaleHandler();
            if (person.Gender == Gender.Male)
            {
                result = maleHandler.Update(person);

            }
            else
            {
                result = femaleHandler.Update(person);
            }
            return result;
        }

Wenn ich nun in meinen Handlern (FemaleHandler bzw MaleHandler)  auf die Datenbankschicht zugreifen möchte, müsste ich ja hier im Kontruktor das bereits erstellte personRepository übergeben:

MaleHandler maleHandler = new MaleHandler(personRepository);

Das wirkt mir aber irgendwie "nicht richtig", das Repository weiter durch zu reichen, "bis man es braucht".

Dieses funktioniert leider nicht:

    public class MaleHandler
    {
        private readonly IPersonRepository personRepository;
        public MaleHandler()
        {
            personRepository = PersonRepository.Instance;
        }

        public void Update(Person person)
        {
            ...
        }
    }

Da ich im PersonRepository-Konstruktor die Credentials injiziere.

Gibt es da einen anderen Ansatz?

Wie sollte der generelle Aufbau der Solution sein?

Ich sehe immer wieder, dass die verschiedenen Schichten in verschiedenen Projekten innerhalb einer Solution abgebildet sind. Was genau bringt dieses Vorgehen? Aktuell bilde ich die verschiedenen Schichten per Ordner ab und sehe in dem Mehraufwand der einzelnen Projekte keinen Mehrwert.

Danke schonmal!

Kriz

Erster Grundregel: Deine ASP.NET Controller / Actions sind Deine Schnittstelle nach Außen.
Du solltest also eigene Modelle nur für diese Außenwelt bereitstellen: API Modelle. Deine Entitäten oder "Business Models" haben in dieser Welt nichts zu suchen.

Code wie IActionResult result = personService.UpdatePerson(person); zeigt, dass die Logik falsch aufgebaut ist.
Die Logik darf die UI Technologie (hier ASP.NET) nicht kennen. Tut sie aber, weil sie hier IActionResult zurück gibt. Das wiederspricht also der Grundregel, dass Schichten von unten nach oben bekannt sein dürfen, aber nicht umgekehrt. Sonst kann man sich das gesamte mehrschichtige Modell sparen.

Zweiter wichtiger Punkt: jeder Request ist isoliert. Du solltest nur die Instanzen zwischen Requests (=Threads) teilen, die dafür konzipiert sind.
Deine Repositories sind das nicht, und daher Deine Logikbausteine ebenfalls nicht. Das muss also alles als Transient registriert werden - statischen Instance Sharing wie PersonRepository.Instanceist ein No-Go. Technisch gesehen funktioniert das auch gar nicht, weil bei einer korrekten Repository Implementierung Du den Context injiziert bekommst und dieser nicht Thread-Safe ist. Du musst also mit individuellen Instanzen arbeiten.

Generell:

Lass sowas wie "PersonService". Das ist das, was innerhalb von kürzester Zeit in riesen, unflexiblen Code-Klassen endet.
Arbeite mit Use Cases wie eben den Handlern, die Du unten angedeutet hast. Du kannst dafür fertige Frameworks (Wie MediatR) nehmen, die Dir paar Sachen zusätzlich geben, oder alles selbst machen und simpel halten.
Handler werden nach Use Cases aufgebaut und haben dann wirklich nur die Dinge injiziert, die Du für diesen Use Case brauchst; bei großen Service-Klassen hast das Problem, dass Du oft Zeugs injizierst, das Du eben nur in ein paar Methoden brauchst, aber nicht in allen ⇒ Aufbau von unnötigen Overhead, sowohl Runtime wie auch beim Testen.

Das wirkt mir aber irgendwie "nicht richtig", das Repository weiter durch zu reichen, "bis man es braucht".

Doch, das funktioniert genau so. Nur reicht man es nicht durch, sondern das macht alles das DI Framework automatisch. Du erstellst es halt von Hand, was nicht Sinn der Sache ist. Aber der Flow ist:

• Action nutzt Handler
• Handler nutzt Repository

Damit ein Handler erzeugt werden kann, braucht man das Repository. So funktioniert DI.

Es kommt dann eben auf den Use Case an, ob das Repository nur etwas zurück gibt oder eben der Handler mehr aufgaben hat.

Was in Deinem Code nicht passt ist, dass Deine UpdatePerson Action Logik beinhaltet.
Sie prüft welcher Handler ausgeführt wird: das ist ebenfalls Logik.
Es ist absolut valide (und notwendig) dass Handler andere Handler aufrufen.

Der Handler würde hier nach Use Case aber eher UpdateMalePersoHandler heissen: ein Handler hat immer nur eine einzige Aufgabe.

Da ich im PersonRepository-Konstruktor die Credentials injiziere.

Das ist falsch, und siehst Du auch in jedem Code Sample zu Datenbanken in den Docs. Verbindungen sollen/müssen von mehreren Repository-Instanzen geteilt werden. Daher bekommt es den Context injiziert, und der Context wurde vom DI (mit Credentials) initialisiert.
Hast Du Credentials im Repository, stimmt bereits die ganze Context-Pipeline nicht - dann kann Dein gesamter DAL nicht funktionieren.

Beispiel aus dem Forum hier:

Wir haben einen ForumPostAddController - der Use Case ist nur: Forum Post Add.

Dieser bekommt zwei Dinge injiziert:

 public ForumPostAddController(
     IEventDispatcher eventDispatcher,
     IIdentityContextAccessor identityContext,

Wir erstellen diese Dependencies nicht von Hand: das macht alles das DI Framework.

In der Action wird dann ein Command ausgeführt, der von einem Handler verarbeitet wird:

ForumPostAddCommand cmd = new(forum post values)
_engine.Send(cmd);

Der ForumPostAddCommandHandler wird nun von der MediatR Handler Pipeline aufgerufen und bekommt alle Abhängigkeiten injiziert

    public ForumPostAddCommandHandler(
       IForumPostRepository forumPostRepository,
       IForumPostAttachmentsRepository forumPostAttachmentsRepository,
       IForumUserNotificationsRepository forumUserNotificationsRepository,
       IEventDispatcher engine)

Im Handler selbst also die Handler-Pipeline bekannt, sodass man weitere Queries/Commands/Notifications aufrufen kann.

Der Pattern nennt sich CQS/CQRS bzw. einfach nur Mediator-Pattern.

Alle Dependencies müssen aber injiziert werden, sonst funktioniert das gesamte System nicht und Deine Unit Tests sind die Hölle.
Deswegen gibts DI Frameworks, die einem das abnehmen.

Zitat von Abt

Erster Grundregel: Deine ASP.NET Controller / Actions sind Deine Schnittstelle nach Außen.
Du solltest also eigene Modelle nur für diese Außenwelt bereitstellen: API Modelle. Deine Entitäten oder "Business Models" haben in dieser Welt nichts zu suchen.

Mit API Modelle meinst Du Models, die nur als Rückgabe für die Actions dienen? Also beispielsweise:

public class UpdateResult
{
    public bool Success { get; set; }
    public DateTime ExecutionTime { get; set; }
}

auf die Delete-Action? Klar, ein entsprechender ResponseCode tuts auch, aber nur verständnisshalber...

Code wie IActionResult result = personService.UpdatePerson(person); zeigt, dass die Logik falsch aufgebaut ist.

Der obige Code ist "zusammen gezimmerter Beispielcode", und kommt so in meinem Projekt nicht vor. Ja, da häte ich etwas genauer sein können. Methoden wie **personService.UpdatePerson(person)** geben dann eher einen bool, oder integer zurück. Das würde dann doch eher wieder ins Schema passen, oder?

Zweiter wichtiger Punkt: jeder Request ist isoliert. Du solltest nur die Instanzen zwischen Requests (=Threads) teilen, die dafür konzipiert sind.
Deine Repositories sind das nicht, und daher Deine Logikbausteine ebenfalls nicht. Das muss also alles als Transient registriert werden - statischen Instance Sharing wie PersonRepository.Instanceist ein No-Go.

In einem anderen Thread(finde ihn gerade nicht) wurde mir nahe gelegt Datenbanken als Singleton anzulegen. Was ist hier denn in dieser Situation anders, dass man es nicht als Singleton machen sollte?

Lass sowas wie "PersonService". Das ist das, was innerhalb von kürzester Zeit in riesen, unflexiblen Code-Klassen endet.
Arbeite mit Use Cases wie eben den Handlern, die Du unten angedeutet hast. Du kannst dafür fertige Frameworks (Wie MediatR) nehmen, die Dir paar Sachen zusätzlich geben, oder alles selbst machen und simpel halten.

Mein Gedankengang war, dass ich so wenig Logik-Code wie möglich in meinen Controllern habe. Also erstelle ich einen PersonService, an den werden die Requests weiter gegeben und dieser ruft dann die entsprechenden Handler auf. Order macht man da (je nach Komplexität) pro Action einen eigenen Handler der dann die Request übergeben bekommt?

Doch, das funktioniert genau so. Nur reicht man es nicht durch, sondern das macht alles das DI Framework automatisch. Du erstellst es halt von Hand, was nicht Sinn der Sache ist. Aber der Flow ist:

• Action nutzt Handler
• Handler nutzt Repository

Mit durchreichen meine ich: Wenn ich im Handler das Repository nutzen möchte, dann injiziere ich es dort im Konstruktor. Wenn ich nun den Handler im Controller nutzen möchte, würde ich im Controller das Repository injizieren und es an den Handler weiter geben. Oder würde der Handler bereits im Controller injiziert werden?

Was in Deinem Code nicht passt ist, dass Deine UpdatePerson Action Logik beinhaltet.
Sie prüft welcher Handler ausgeführt wird: das ist ebenfalls Logik.

Ne, das passiert in diesem Beispiel im PersonService, nicht im Controller.

Es heisst Dependency Injection, weil Du Abhängigkeiten extern mitgibst. Machst Du das nicht, ist es kein Dependency Injection.
Du untergräbst die gesamte Idee von DI mit Deinem Singleton-Konstrukt.

Eine Instanz sollte/darf niemals die Kontrolle haben, die Abhängigkeiten erstellt werden.
Das ist einzig und alleine Aufgabe der DI Konfiguration.

In einem anderen Thread(finde ihn gerade nicht) wurde mir nahe gelegt Datenbanken als Singleton anzulegen. Was ist hier denn in dieser Situation anders, dass man es nicht als Singleton machen sollte?

Quelle?

Das ist absoluter Käse und funktioniert technisch auch nicht. 
Wenn Du einen ADO.NET DB Context verwendest, so ist dieser nicht Thread Safe. Du kannst also keinen Singleton für Dein Repository verwenden.

Nur ganz gewisse Datenbankenverbindungen sind Thread-Safe (zB MongoDB). Relationale Datenbanken generell nicht. Machst das bei relationalen DBs fliegt Dir das ganze Ding um die Ohren, sobald zwei Thread zugreifen. Du musst bei Webanwendungen, anders als auf dem Desktop, immer in Multi-Threading denken.
Es funktioniert technisch also 0. Halt Dich an die MS Docs. Die sind hier korrekt.

Repositories als Singleton macht aber auf Desktops auch i.d.R. keinen Sinn. Eine Verbindung kann immer unterbrechen, und muss dann neu erzeugt werden. Und allein wegen des Testens, etc niemals über eine Property.
Wenn man in einem DI System etwas als Singleton registriert, dann über die Registrierung. Nicht über Properties.

Bitte mach in Deinem eigenen Interesse das Grundlagen Tutorial durch, sonst wirds mit dem generellen Verständnis von DI schwer.

Mein Gedankengang war, dass ich so wenig Logik-Code wie möglich in meinen Controllern habe. Also erstelle ich einen PersonService, an den werden die Requests weiter gegeben und dieser ruft dann die entsprechenden Handler auf. Order macht man da (je nach Komplexität) pro Action einen eigenen Handler der dann die Request übergeben bekommt?

Der Gedankengang ist prinzipiell okay, nur verwendet man in modernen Architekturen keine monolithischen Service Klassen wie "PersonService" mehr. Nur noch Handler. Selbst die moderne ASP.NET Minimal API basiert ausschließlich auf Handlern und verzichtet auf monolitische Controller.

Oder würde der Handler bereits im Controller injiziert werden?

Natürlich im Controller. Du musst den Flow einhalten. Aber Du machst das nicht selbst, das macht das DI Framework für Dich.
Instanzen selbst erzeugen = schlecht. Schlecht weil schlecht Testbar, keine Modularisierung etc. Mach nen DI Tutorial durch und Du verstehst es.

Der obige Code ist "zusammen gezimmerter Beispielcode", und kommt so in meinem Projekt nicht vor.

Dann zeig Dein echten Code, denn niemand halt Lust zu "zusammen gezimmertem Beispielcode" Feedback zu geben, wenn es zu stark verfälscht ist. 😃

Zitat von Abt

Es ist absolut valide (und notwendig) dass Handler andere Handler aufrufen.

Hierzu hatte ich vor einer Weile eine gegenteilige Aussage gelesen - Quelle finde ich nicht mehr.

Verstehe ich das richtig, dass ein Handler auch andere Handler aufrufen darf, also eine theoretisch beliebig lange Kette von Handlern entstehen kann?
Mit MediatR würde also ein RequestHandler eine IMediator-Dependency erhalten und andere Requests absenden.

Das ist ein Detail, was mir auch noch unklar ist:
Was ist, wenn mehrere Handler teilweise die gleiche oder ähnliche Logik haben?
Es wäre einfach, das alles in einen weiteren Handler auszulagern und den aufzurufen, aber ist das der beste Weg?

Zitat von Palladin007

Verstehe ich das richtig, dass ein Handler auch andere Handler aufrufen darf, also eine theoretisch beliebig lange Kette von Handlern entstehen kann?

Das ist prinzipiell völlig valide, sowohl aus Sicht des Mediator-Patterns selbst wie auch aus Sicht von MediatR.
Jimmy schreibt in einem Issue, dass man das aufgrund der Übersichtlichkeit zwar vermeiden soll und es ein paar Dinge wie Refaktorierung schwerer macht (das ist wahr). Das gilt aber generell für den Mediator-Pattern: man bekommt Einfachheit, Flexibilität, Effizient und Co nicht umsonst, sondern leider zulasten der Übersichtlichkeit.
Es ist aber eine konzeptionelle Sache, was Du inhaltlich tust - kein generelles "Nein".

Anders sieht es bei MediatR Notifications aus: diese sind in MediatR "besonders" implementiert und sollten keine Requests (Queries/Commands) absenden. Grund ist, dass hier kein isoliertes DI existiert.
Daher haben wir hier im Forum eine eigene Notification Pipeline in MediatR implementiert, mit der das wiederum technisch möglich ist.

In ASP.NET basiert das System im Endeffekt auf Handlern, die aus anderen Handlern aufrufen wurden.
Das ist halt einfach eine Stuktursache. Man muss sich trotzdem an die Grundregeln einer Architektur halten.

Mit MediatR würde also ein RequestHandler eine IMediator-Dependency erhalten und andere Requests absenden.
Was ist, wenn mehrere Handler teilweise die gleiche oder ähnliche Logik haben?
Es wäre einfach, das alles in einen weiteren Handler auszulagern und den aufzurufen, aber ist das der beste Weg?

Duplikate sollten immer vermieden werden. Auslagern ist der richtige Weg.
Ob Du dafür eine extra Handle-Klasse anlegst oder einen Mediator-Handler macht technisch gesehen wenig Unterschied.
Ich verwende beide Varianten, je nachdem.

Alles über MediatR zu machen hat den technischen Vorteil, dass man darauf Behaviors anwenden kann (zB Logging, Time Tracing etc..).
Daher ist das mein bevorzugter Weg. Siehe mein Beispielprojekt: https://github.com/BenjaminAbt/AspNetCoreEnterprisePlatform

Edit: vielleicht ist es einfacher in der Formulierung:

• Alle Gemeinsamkeiten, die IO-Operationen beinhalten, sind auf alle Fälle in einem gemeinsamen MediatR Handler
• Alle anderen Gemeinsamkeiten sind überwiegend in simplen Handler-Klassen.