Improve architecture

[jean-michelet]

Currently, we have a couple of problems:

1. Controllers that handle business logic.
2. Plugins lack clear modularity, making it harder to distinguish between core infrastructure (like db) and domain-specific logic.
3. The structure is organized by technical concerns (e.g., routes, plugins) rather than by business domains, which can make it challenging to locate and manage related functionalities as the codebase grows.

It is ok for a very small demo, but it becomes inconvenient to maintain if we add more features.

1. I think controllers should be dedicated to HTTP handling (e.g., request/response handling, parameter validation...).

2. We can rename the plugins folder to something more representative, like infrastructure or shared, to reflect that these are reusable components providing core capabilities across the project.

3. Maybe we should rename routes to domains or modules to represent distinct business areas (e.g., auth, tasks, etc.).

src/
├── infrastructure/           # Core reusable logic
│   ├── authorization.ts
│   └── scrypt.ts
├── domains/                  # Business domains
│   ├── auth/                 
│   │   ├── controllers/      
│   │   ├── services/         
│   │   ├── schemas.ts     
│   │   └── routes.ts        
│   ├── tasks/                
│   │   ├── controllers/   
│   │   ├── services/  
│   │   ├── schemas.ts  
│   │   └── routes.ts 
│   └── ...
└── app.ts     

Wdyt? Do you want to work on it?

[jean-michelet]

Ping for feedback: @climba03003 @melroy89 @Fdawgs

[Fdawgs]

I'd keep it as routes and plugins as then the language is consistent with how the official Fastify documentation and plugin docs refer to these concepts, which should make it easier to understand.

[Turn0xx]

I fully support this initiative to reorganize the codebase, and i can heavily help on that.
Currently, controllers are handling too much, including business logic, which makes file navigation and code comprehension more challenging.
Also the reorganization by "buisness" domains can be very beneficial, so each component purpose is much clearer, intuitive...

[Turn0xx]

I'd keep it as routes and plugins as then the language is consistent with how the official Fastify documentation and plugin docs refer to these concepts, which should make it easier to understand.

For routes, certainly easier to understand since it's standard/present on most demos.. Imo it is not really representative of the real purpose of the components inside.

[jean-michelet]

You can take inspiration from this repo created by Matteo. https://github.com/mcollina/modular_monolith/tree/main/modular

…

On Sat, Nov 2, 2024, 15:20 Mouad ***@***.***> wrote: I'd keep it as routes and plugins as then the language is consistent with how the official Fastify documentation and plugin docs refer to these concepts, which should make it easier to understand. For routes, certainly easier to understand since it's standard/present on most demos.. Imo it is not really representative of the real purpose of the components inside. — Reply to this email directly, view it on GitHub <#52 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/A2J2324KOIZYYSDCNDAX4QTZ6TNRZAVCNFSM6AAAAABRBXKBWGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINJTGAYDMMBWGE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[climba03003]

For the project structure that I currently using,

src/
├── plugins/                      # shared / global plugins (e.g. database)
│   ├── database.ts
│   └── config.ts
├── routes/                       # business logic with routes
│   ├── auth/
│   │   ├── index.controller.ts   # controller for database operation
│   │   ├── index.route.ts        # routes
│   │   ├── index.route.schema.ts # schema for routes - used to prevent cyclic import
│   │   └── index.schema.ts       # schema for database object
│   ├── tasks/
│   │   ├── index.controller.ts   # controller for database operation
│   │   ├── index.function.ts     # complicated logic that would reuse
│   │   ├── index.route.ts        # routes
│   │   ├── index.route.schema.ts # schema for routes - used to prevent cyclic import
│   │   └── index.schema.ts       # schema for database object
│   └── ...
├── app.ts                        # fastify application
└── index.ts                      # entrypoint

Basically, the domain you mention is scoped inside the route, and you can follow the routes name to find which files you need to edit. The actual business logic should be inside index.route.ts unless some reusable function is extracted to index.function.ts.
Personally, I do not use @fastify/autoload and routes should be all encapsulated.

[jean-michelet]

Personally, I do not use @fastify/autoload

Weird, I see you contributing to it, why?

[jean-michelet]

I think if this PR is accepted, we should implement the changes here.
Fastify need to improve dependency injection I think.

[melroy89]

My setup is currently looking like this (and yes I'm using autoload for sure):

tree -d src
src
├── handlers
├── interfaces
├── plugins
├── routes
│   └── api
│       └── v1
└── schemas

Autoloads are used for plugins and routes.

I call it plugins because I use FastifyPlugin(Async), so I think it fit the purpose and meaning of the directory structure.

My handlers are the routes with the business logical, which I believe was also a good practice by Matteo himself, since decoupling the routes too far away from the logic can increase technical depth.

The file (index.ts) in the routes directory is currently only registering handlers with a prefixes, using the register() method of Fastify. Eg. fastify.register(userHandler, { prefix: '/users' })

Then again, in my case Fastify is used as backend. The handlers are only doing database calls. If you have much more logical in the handles, I would advice to split that further. But again, in my use-case this level of abstraction is sufficient enough.

DB Pagination, advanced DB queries or payload validation is all done with plugins like preHandlerHookHandler or decorate or just using addHook() method. So here is currently more advance logical.

[jean-michelet]

Fastify community is too anarchic, we should create a fastify-opiniated package 😁

[Mathieuka]

IMHO

I don't yet know all the technical details of Fastify's internal workings, but integrating it as a component of the network layer in a domain-driven architecture shouldn't be an issue.


I see two levels of improvement and separation of concerns: at the folder and file level, and at the module (code) level.

From an architectural perspective, it would be preferable to organize endpoints into domain-specific folders, each containing its own use cases. 



This approach would encapsulate the technical details of Fastify's network layer.


The controllers, consumed by this network layer, will receive the database client through dependency injection, to avoid coupling the domain with this type of detail.


Initially, we could adopt the Transaction Script model as we do today, and once its limitations are reached, we can start creating entities.

Looking at the testing perspective can be interesting.


If I understand correctly, the philosophy is that, we do not mock the external services like database, so the controller layer can serve as an orchestrator

At the code level, it might be beneficial to create entities, only if the business complexity increase and or if we want to test the business rules without relying on the database, this declarative approach also allows us to see things more clearly but brings complexity in the development because you have to be familiar with this model

This doesn't prevent us from maintaining the philosophy of systematically testing use cases with database persistence.
However, it is very useful for achieving a clear separation of responsibilities, with expressive entities that are easy to test in isolation if needed.

The first step could be to organize the folders to separate the network layer from the domain by creating the first controllers and evaluate if the transaction script model is sufficient. From there, we could start creating the first entities.

I welcome any other opinions.

src/
├── controllers/           // Inject the usecase in the controller 
│   ├── __tests__/  
│   ├── index.ts  
│   ├── authController.ts  
│   └── userController.ts
├── routes/                // Consume the Controller<DBClient>
│   ├── index.ts
│   ├── auth/
│   │   ├── index.ts   
│   │   ├── login.ts        
│   │   └── logout.ts
│   └── user/
│       ├── index.ts
│       ├── updatePassword.ts
│       └── deleteAccount.ts
├── services/
│   ├── index.ts
│   ├── authService.ts
│   └── userService.ts
├── usecases/             // Entities are instantiated here
│   ├── index/
│   ├── auth/
│   │   ├── index.ts
│   │   ├── login/
│   │   └── logout/
│   └── user/
│       ├── updatePassworUseCase.ts
│       └── index.ts

[Turn0xx]

@Mathieuka IMO for a demo, I believe wrapping each 'action' (command or query) in a separate use case is excessive. This adds another layer (so + mental load and "complexity")—especially for something meant to be "straightforward". As @jean-michelet mentioned, Fastify lacks built-in dependency injection, but (correct me if i'm wrong) we can achieve a similar effect with the plugin mechanism.
routes -> controllers -> service -> output (e.g., database or file handling).

Using Fastify plugins, we can consider services like plugins, so we can injecte then into controllers via routes that have access to the app instance.

We could also instead of separating each route into its own file, consolidate all routes in one file per domain and handle separation at the controller layer or not at all.

However, I believe keeping a file per route still has its advantages. It gives us a clear, quick overview of each domain’s actions and makes it easy to locate and modify specific routes as needed.

Other thoughts on this approach?

src/
├── lib - infrastructure - shared .../
│   ├── authorization.ts # I think that also each domain should have his own authorization.
│   └── scrypt.ts
├── domains/
│   ├── auth/                 
│   │   ├── auth.controller.ts
│   │   ├── auth.service.ts
│   │   ├── routes/ 
│   │   │   ├── index.ts
│   │   │   ├── login.ts
│   │   │   ├── logout.ts     
│   ├── user-account/                 
│   │   ├── user-account.controller.ts
│   │   ├── user-account.service.ts
│   │   ├── routes/ 
│   │   │   ├── index.ts
│   │   │   ├── register.ts     
│   │   │   ├── update-account.ts     
│   │   │   ├── update-password.ts
│   │   │   ├── delete-account.ts
│   │   │   ├── ...
│   ├── tasks/                 
│   │   ├── tasks.controller.ts
│   │   ├── tasks.service.ts
│   │   ├── routes/ 
│   │   │   ├── index.ts
│   │   │   ├── create-task.ts
│   │   │   ├── assign-task.ts
│   │   │   ├── upload-image.ts
│   │   │   ├── delete-image.ts
│   │   │   ├── ...
│   └── ...
└── app.ts    

[Mathieuka]

@Turn0xx

For a demo intended to be "simple," I agree that the complexity might indeed be unnecessarily high.

I don't yet know the plugin mechanism well enough.

Consolidating all the routes into a file by domain and managing the separation at the controller level seems like a good idea.

This approach mitigates complexity and offers interesting cohesion from a readability standpoint.

[Mathieuka]

@Turn0xx

My question is whether controllers, routes, and services should be part of the domain.

Shouldn't they belong to the application layer instead ?

The domain should be self-sufficient through its modules and entities, if necessary ?

My point is that if we want to bring about the separation of responsibilities, it will be done through the entities, otherwise we will end up with the same problem of controllers who manage the state, apply the business rules, and persist the data.

auth/
├── domain/
│   ├── entities/
│   ├── services/
│   └── authorization.ts
└── application/
    ├── auth.controller.ts
    ├── auth.service.ts
    └── routes/
        ├── index.ts
        ├── login.ts
        └── logout.ts

However, the trade-off you propose might be sufficient for the needs of the demo

[jean-michelet]

As @jean-michelet mentioned, Fastify lacks built-in dependency injection

This is not what I said, I said it could be improved.

A lot of developers think about dependency injection trough constructor injection:

@Injectable()
class DateFormatter {
  format(date: Date): string {
    return date.toISOString();
  }
}

@Controller('some-prefix') // prefix routes path defined in the class
class AppController {
  constructor(
    private readonly dateFormatter: DateFormatter,
  ) {}

  @Get('/log-date')
  logCurrentDate(): { message: string } {
    const currentDate = new Date();
    const formattedDate = this.dateFormatter.format(currentDate);
    return { message: `${this.prefix} ${formattedDate}` };
  }
}

But the same is achievable via a functional paradigm:

function formatDate (date) {
  return date.toISOString()
}

fastify.register(function (instance, deps) {
  instance.get('/log-date', (request, reply) => {
    const currentDate = new Date()
    const formattedDate = deps.formatDate(currentDate)

    reply.send({ message: `Format date: ${formattedDate}` })
  })
}, {
  formatDate, // Function injection
  prefix: '/some-prefix' // prefix routes path defined in the plugin
})

[Turn0xx]

@Mathieuka

Shouldn't they belong to the application layer instead ?
The domain should be self-sufficient through its modules and entities, if necessary ?

Totally agree with you,all actual components that i cited should reside in the application layer. In my example, I intended 'domains' simply to represent areas of work or functionality, rather than fully self-sufficient domains.

My point is that if we want to bring about the separation of responsibilities, it will be done through the entities, otherwise we will end up with the same problem of controllers who manage the state, apply the business rules, and persist the data.

Ideally, controllers won’t manage any of this. This should be handled by the service layer, and I don’t think the demo has enough business complexity to introduce entities as it add another another layer..

However, as you mentioned, I don’t think this level of separation is necessary for the demo, especially since it’s an "introductory" demo (I guess xD). So it should be accessible for all potential users, who may find to much layering / separation overwhelming or complex at this stage.

[Mathieuka]

@Turn0xx fully agree about the demo has not enough business complexity to go in this direction

I took the time to read the plugin system documentation and I understand your arguments better.

Using Fastify plugins, we can consider services like plugins, so we can injecte then into controllers via routes that have access to the app instance.

[jean-michelet]

I think a more modular approach could be beneficial, and it will be easily achievable when the PR I mentioned in this comment will be merged.

Example of what could be achieved:

export const deepPluginDependency = createPlugin((instance) =>
  instance
    .decorateReply('deep_dependency_reply', true))

export const pluginDependecy = createPlugin((instance) => instance
  .decorateRequest('dependency_request', true), { dependencies: [deepPluginDependency] })

export const dependantPlugin = createPlugin((instance) => {
  return instance.get('/', (req, res) => {
    expectType<boolean>(req.dependency_request)
    expectType<boolean>(res.deep_dependency_reply)
  })
}, { dependencies: [pluginDependecy] })

[jean-michelet]

It would be plugin injection, and the injected plugin decorate the fastify instance, request and reply with features in the scope it is injected (encapsulation).