neos / Neos.EventSourcing

A library for Event Sourcing and CQRS for Flow projects.
MIT License
44 stars 30 forks source link

Defining the directory structure #9

Open bwaidelich opened 8 years ago

bwaidelich commented 8 years ago

We need to come up with a directory structure which sets the basis for the conventions used in this package.

(issue imported from https://github.com/dfeyer/Ttree.Cqrs/issues/1)

bwaidelich commented 8 years ago

Suggestion (by @robertlemke):

Your.Package/
  Application/
    Controller/
    Command/
    Service/

  Domain/
    Service/
    Aggregate/
      YourAggregate/
        Command/
        CommandHandler/
        Event/
        EventListener/
        Service/
        YourAggregate.php
        YourAggregateRepository.php

  Projection/
    MostValuableBooks/
      MostValuableBooksProjector
      MostValuableBooksReadModel
      MostValuableBooksFinder

  CommandHandler/
  EventListener/
bwaidelich commented 8 years ago

A concrete example for that:

My.CrmContext
  Application
    Controller
      CustomerController.php
    Service
      MailService.php
  Domain
     Service
        ?
     Aggregate
       Customer
         Command
           RegisterCustomer.php
         CommandHandler
           CustomerCommandHandler.php
         Customer.php
         CustomerRepository.php
         Event
           CustomerRegistered.php
         EventListener
           ?
         Service
           CustomerOnboardingService.php (?)
  Projection
    CustomerDetails
      CustomerDetails.php
      CustomerDetailsProjection.php
      CustomerDetailsFinder.php
    CustomerListItem
      CustomerListItem.php
      CustomerListItemProjection.php
      CustomerListItemFinder.php
bwaidelich commented 8 years ago

With the decision to skip the CommandBus for now (#3), I'd suggest one slight adjustment: There will usually be one CommandHandler for each Aggregate, so I'd suggest to move that one level up so that you'd have sth like:

Customer
  Command
  Customer.php
  CustomerCommandHandler.php 
  CustomerRepository.php
  Event

Besides I'm not 100% about the term "Projection" as 1st level directory name.. Any thoughts on that?

bwaidelich commented 8 years ago

FYI: There are some related questions on discourse but we decided to keep the technical discussions in the respective GH issues

sorenmalling commented 8 years ago

Will bounded context with more directories (Contexts) in the "root" of the package cause any issues here? Or is that not in the scope of this discussion?

bwaidelich commented 8 years ago

Will bounded context with more directories (Contexts) in the "root" of the package cause any issues here?

I don't think so, but I think that BCs should even reside in their own packages. At least that's what Vaughn Vernon and other experts keep telling: separate the BCs as if they were completely different services (microservice style) i.e. one team = one BC (even if you don't have so many teams I think it helps to think in these boundaries)

sorenmalling commented 8 years ago

Okay, then that discussion can go somewhere else :-)

bwaidelich commented 8 years ago

FYI: Christian just argued that we should -not- avoid using Subpackages (see https://discuss.neos.io/t/deprecating-subpackage-concept/1415). One way around this would be to establish the best practice to have only one Bounded Context per Package and consider the UI part as one (or at least have the Application part in a separate package)

kitsunet commented 8 years ago

should avoid! :)

dfeyer commented 8 years ago

I will update the README with single and multiple package recommentation

bwaidelich commented 8 years ago

As just discussed in our weekly hangout we should probably recommend something along those lines: Especially in larger applications it's recommended to only have one Bounded Context per Package and to consider UI/API part as one. If one decides to have it all in one package it's advised to use a SubPackage at least to encapsulate the Application logic.

Robert also brought up the point that Projections could be considered part of the Application

kitsunet commented 7 years ago

I don't really see why you need the Application folder. Seems superfluous to me. Why was this introduced?

bwaidelich commented 7 years ago

I don't really see why you need the Application folder

It's only needed if you want to put application & domain logic into one package in order to avoid mixups like this one: https://github.com/neos/Neos.Cqrs/tree/master/Classes/Command

kitsunet commented 7 years ago

Maybe the wording should be worked on then?

bwaidelich commented 7 years ago

Maybe the wording should be worked on then?

Sure, do you have any suggestions or can you elaborate what you don't like please?

IMO the naming in that case is just a suggestion. This is mainly about using a SubPackage when having all in one package, see https://github.com/neos/Neos.Cqrs/issues/9#issuecomment-247601359

kitsunet commented 7 years ago

As said, I think the Application folder is not needed. I see that Command (CLI) and Command (CQRS) shouldn't mix, but then maybe one of them should be reworded (probably CLI) to solve this issue.

bwaidelich commented 7 years ago

But if we rename Command that would be quite a breaking change in Flow. That's why we said: You probably want to separate your Bounded Context via Packages (and consider the Application/UI one BC) or use SubPackages if you insist on a single package.

I'm all for renaming Command (or at least remove the hard-coded part that expects CLI command controllers to be in a command folder) but IMO that's a different topic

bwaidelich commented 7 years ago

BTW: I also dislike SubPackages. Robert came up with the idea to make the "MVC-Entrypoint" configurable somehow. So you could put your App logic in some(tm) folder and point there

bwaidelich commented 7 years ago

Some new insights after our workshop with Mathias that we should consider in the suggested directory structure:

bwaidelich commented 7 years ago

Related to that: Where to put DTOs/ValueObjects?

In general I think we should encourage a structure around processes rather than nouns (aka Aggregates/Entities) so maybe sth along those lines:

My.CrmContext
  Application
    Controller
      CustomerRegistrationController.php
    Service
      SomeNonDomainService.php
  Domain
     Service
        MailService.php
     UseCase
       CustomerRegistration
         Command
           RegisterCustomer.php
         CommandHandler
           CustomerRegistrationCommandHandler.php
         Event
           CustomerWasRegistered.php
  Projection
    CustomerDetails
      CustomerDetails.php
      CustomerDetailsProjection.php
      CustomerDetailsFinder.php
    CustomerListItem
      CustomerListItem.php
      CustomerListItemProjection.php
      CustomerListItemFinder.php

But I'm not really confident with that yet.. Where would we put some UniqueUsername constraint/aggregate in that example? Also it still kinda mixes write & read (commands & events) and doesn't clearly distinguish API (public) and infrastructure (private) classes

bwaidelich commented 7 years ago

@robertlemke Can you share your POV (again)? Did you suggest just to go with the first suggestion but replace "Aggregate" by "Model"? I'd be fine with that, too. Regarding "process > structure" one could still achieve that like:

My.CrmContext
  Application
    Controller
      CustomerRegistrationController.php
    Service
      SomeNonDomainService.php
  Domain
     Service
        MailService.php
     Model
       CustomerRegistration
         Command
           RegisterCustomer.php
         CommandHandler
           CustomerRegistrationCommandHandler.php
         Event
           CustomerWasRegistered.php
  Projection
    CustomerDetails
      CustomerDetails.php
      CustomerDetailsProjection.php
      CustomerDetailsFinder.php
    CustomerListItem
      CustomerListItem.php
      CustomerListItemProjection.php
      CustomerListItemFinder.php
bwaidelich commented 7 years ago

One rule of thumb we should follow IMO is: Don't mix predefined and custom folders on one level (i.e. within one folder there should only be either pre-defined subfolders (like "Service", "Model", "Projection", ...) or app-specific ones (like "CustomerRegistration", "CustomerDetails", ...)

bwaidelich commented 7 years ago

Sorry for spamming, but I'm still "unhappy" with this (after thinking about it once again): The Domain folder doesn't make sense to me, as all of this is part of the domain really, even projections. And if they're not, they belong to a different package I'd say. Same with Model: Most of this is part of the domain model somehow – maybe not the command handler, but we put it in there anyways. I just stumbled upon this, because I try to create a separate (sub)package for every business capability (aka Bounded Context). And while this works out quite nicely, it feels tedious to have such a deep folder hierarchy. Also we don't really make the distinction between write vs read and/or public vs private classes very clear like this..

Maybe we can discuss this once again during our friday's weekly, now that we all have a bit more experience with this package

kitsunet commented 7 years ago

Can we drop the notion of subpackage please? I am really unhappy with it and IMHO there is no benefit (only constraints and possible problems) compared to just a separate package.

bwaidelich commented 7 years ago

Can we drop the notion of subpackage please?

Didn't we already have this discussion above? ;) (btw: I agree!)

albe commented 7 years ago

The Domain folder doesn't make sense to me, as all of this is part of the domain really, even projections. And if they're not, they belong to a different package I'd say.

Yes. I guess the actual intent is more "Core Domain" than generally "Domain".

And while this works out quite nicely, it feels tedious to have such a deep folder hierarchy.

We're currently having the same in our node.js project. I still haven't found a good convention/rule for how to group commands, events, models and command handlers - even though they clearly belong closely together. After all, they form the write side (see next point).

Also we don't really make the distinction between write vs read and/or public vs private classes very clear

Absolutely, and after playing again and updating my little pet example project for this package, I had the deep urge to create separate Controllers for the write and the read side. It just felt totally awkward, trying to create a CQRS architecture, where there's still a single controller for all reads and writes on a single model/aggregate. So I'd like to propagate that we also recommend a structure, where those are clearly separated.

I see two possiblities ways to achieve this clear separation:

My.CrmContext
  Command
    Controller
      CustomerRegistrationController.php
    Service
      SomeDomainService.php
    Model
      CustomerRegistration
        Command
          RegisterCustomer.php
        Event
          CustomerWasRegistered.php
        Customer(Constraints).php
        CustomerRepository.php
        CustomerRegistrationCommandHandler.php
    Process
      CustomerRegistrationProcess(Manager).php

  Query
    Controller
      CustomerController.php
    Service
      SomeNonCoreDomainService.php
    Projection
      CustomerDetails
        CustomerDetails.php
        CustomerDetailsProjection.php
        CustomerDetailsFinder.php
      CustomerListItem
        CustomerListItem.php
        CustomerListItemProjection.php
        CustomerListItemFinder.php
    Dto
      ...

The good thing about this structure is obviously, that it can very easily be split or joined to two/one package(s) (e.g. My.CrmContext.Command and My.CrmContext.Query). The bad is that the second structure has even deeper directory hierarchy.

Regarding private/public classes, I think this distinction is harder to propagate through folder structure. I see the public parts as: Commands, Events, Finders and Controllers (and maybe DTOs) - those form the public API of the application. Everything else is supposed to be private/implementation detail. My rule of thumb would be: Colocate things that change together. Obviously this is not the case for private and public classes.

dimaip commented 7 years ago

Just my 2c. Projections are also part of the domain, thus it's a bit illogical to put them outside of the Domain folder. I really like the separation into Command and Query. We should segregate them after all.

bwaidelich commented 7 years ago

@albe I like your suggestion, but some comments

I guess the actual intent is more "Core Domain" than generally "Domain"

I'm not sure about that. In my understanding the domain is everything about the business/world you're creating a solution for. The core domain is the part that the domain experts consider the fundamental and critical parts of the business, i.e. the competitive advantage. IMO that separation got nothing to do with the folder structure really (except for the fact that you might want to use existing packages for parts of the domain that are not part of that core domain)..

I think your idea of having Command and Query folders on the top level is worth considering – although I have cases where they share some DTOs etc – but maybe I should avoid that to begin with.

Also the idea of having separate controllers for read and write side is interesting. But I wouldn't force that and especially move it outside of the command / query part. The controller is just one possible gateway to interact with your domain, it belongs to the Application IMO.

I like your rule of colocating things that change together. It can be quite hard to tell though.. For example: It might be a common usecase that you add a new property to an event - so it needs to be added to the corresponding command, aggregate and maybe ReadModel, too ;)

bwaidelich commented 7 years ago

From today's weekly meeting the following idea came up: It will be very difficult to come up with a structure that fits all needs, so we should consider documenting the suggested structure(s) in multiple steps starting with the easiest case of a Service that publishes Events (and where to put them).

The final example could be a structure like the ones discussed above, including Application, Process Managers, Services, ... with the noticeable advice to split up complex projects into multiple packages (maybe even separating read/write in the application layer)

albe commented 7 years ago

In my understanding the domain is everything about the business/world you're creating a solution for. The core domain is the part that the domain experts consider the fundamental and critical parts of the business, i.e. the competitive advantage.

Check

IMO that separation got nothing to do with the folder structure really (except for the fact that you might want to use existing packages for parts of the domain that are not part of that core domain)..

Well, normally you should only be doing DDD and all the advanced mumbo jumbo on the core domain at all, because everything else is most likely time not spent optimally (by the schoolbook at least). But of course as soon as you implement your core domain you will be touching cross-cutting concerns and other parts of your domain, that are not core. So I would have considered the implementation of business rules (i.e. aggregates) and the things that domain experts care about (i.e. events and commands) to be the major part of the implementation of the core domain. Projections are part of the domain, but at the visualization and representation layer (unless your core domain is really a reporting/statistics UI - google analytics maybe?).

But at the bottom line, I'm not convinced either of having a folder named just "Domain" and then leaving parts out that clearly belong to the domain, just not the core.

But I wouldn't force that and especially move it outside of the command / query part.

No, not force of course. Just recommend, because it better carries the whole concept of CQRS and clearly separating write and read sides. The only reason we have Controllers is because it's a leftover from the MVC pattern, where the Controller is the central entry point to your application. There, Controllers were only separated per Use Case, but moving that further to read/write is just the logical next step when going CQRS IMO.

In a non-web/mvc environment, you don't have that at all, but you just directly talk to your single command dispatcher and then probably have some slim layer to run queries, either through an REST API or possibly some RPC or another dispatch mechanism.

I like your rule of colocating things that change together.

It's not originally by me. IIRC I grabbed that up from some tweet by Dan Abramov (aka Mr. Redux). So props there :)

It can be quite hard to tell though

Yes, absolutely and it also depends on your domain and work process. But I do believe there is a generic bottom line that whenever a command or event changes, the according ConsistencyModel (i.e. "Aggregate", if one exists) and CommandHandler needs to change too. Projections might need to change, but not necessarily. Otoh, if you do change a Projection, you do need to adjust the Projector, ReadModel and/or Finder together every time.

Otherwise I'm all in for the "growing" structure and documenting that incrementally. But we still need to find a "final" recommendation for the top complex projects.

robertlemke commented 7 years ago

Hi folks,

after the event-sourced-content-repository-week, here's the structure I'd like to propose. Still not fitting every case, but, in my opinion, better than what we currently suggest in our Readme:

My.CrmContext
  Application
    Controller
      CustomerController.php
    Service
      MailService.php
  Domain
     Service <--- This is not new, but well, could be used for cross-context services?
     Context <--- This is new (instead of "Aggregate")
       Customer
         Command
           RegisterCustomer.php
         CustomerCommandHandler.php <--- I put the command handler into the context namespace, but it's because I only have 1 usually
         Customer.php
         CustomerRepository.php
         Event
           CustomerRegistered.php
         EventListener <--- I think we should get rid of this, because different classes can be listeners
         Service
           CustomerOnboardingService.php (?)
    ProcessManager <--- this is new. "Process" would be fine too, but I tend to this
      CustomerSignUpProcessManager.php
      (you might also create a sub structure here if you have many process managers)
    Projection <--- Projection belongs to Domain
      CustomerDetails
        CustomerDetails.php
        CustomerDetailsProjection.php
        CustomerDetailsFinder.php
      CustomerListItem
        CustomerListItem.php
        CustomerListItemProjection.php
        CustomerListItemFinder.php

Please let me know if you like this more or less than what we currently suggest, so we can update the Readme accordingly.

albe commented 7 years ago

Just for clearity, this is our current recommendation (as of April 2017) for the structure as of the Readme:

Your.BoundedContextPackage.Command/
  Classes/
    Controller/
      [UseCase]Controller.php
      …
    Service/
      [SomeDomain]Service.php
      …
    Model/
      [YourAggregate]/
        Command/
          [DoSomething].php
          …
        Event/
          [SomethingHappened].php
          …
        [YourAggregate].php
        [YourAggregate]CommandHandler.php
        [YourAggregate]Repository.php
      …
    Process/
      [Some]ProcessManager.php
      …

Your.BoundedContextPackage.Query/
  Classes/
    Controller/
      [ReadUseCase]Controller.php
      …
    Dto/
      [SomeQueryDto].php
      …
    Service/
      [SomeNonDomain]Service.php
      …
    Projection/
      [ProjectionName]/
        [ProjectionName].php
        [ProjectionName]Finder.php
        [ProjectionName]Projector.php
      …

So a few points:

kitsunet commented 7 years ago

Again @robertlemke please no subpackages. So no (CLI) controllers in sub structures unless we introduce a first class concept for that to work correctly.