Split up the monolithic code blocks

[palant]

Search for duplicate feature request

• I already searched, and this feature request or improvement is not a duplicate.

Feature scope

Improve existing functionality

Feature request related to a problem

I wondered whether it would be possible to implement some improvements to make SWS more suitable for my needs. However, pretty much all the currently supported features seem to add to the following three areas:

1. Server initialization
2. Settings management
3. Request handling

While these features are called “modules,” the approach isn’t actually modular. It’s these three monolithic code blocks instead. This complexity makes adding functionality a rather tricky affair.

Describe the solution you'd like

I’d like actual modularization, where e.g. all functionality related to Basic Auth is contained in basic_auth.rs rather than added to large code blocks. A naive implementation would involve a trait like the following:

pub trait Module {
  fn priority() -> u32;
  fn server_init(&self) {}
  fn handle_request(&self) {}
}

The methods server_init() and handle_request() would be called for all registered modules during server initialization and request handling respectively. priority() would be a constant determining the order in which the modules are called (alternatively it could be specified during module registration).

I’m not sure whether this naive approach would have a notable performance impact due to Rust being unable to inline the module handlers. An alternative would be using macros to call all modules in a static fashion. That alternative has the disadvantage that it no longer allows trivially extending SWS functionality with third-party modules. For example, #97 would be a prime candidate for a third-party module.

Decentralizing settings should be more complicated and may be hard to achieve without changing config file format and CLI options. This is an optional second step however.

Describe alternatives you've considered

Things can stay as they are of course, but the complexity will only grow.

Build target

All targets

Additional context

I would try to help with these changes if these are considered desirable. The modularization can be implemented as a number of small changes, this doesn’t require one huge PR to do it all at once.

Pull requests so far

• Move health endpoint-related code into a separate file #344
• Move metrics endpoint-related code into a separate file #345
• Move all of Basic authentication logic into basic_auth module #346
• Move all redirect handling logic into the redirects module #348
• Move most of CORS-related code into the cors module #349
• Move most of maintenance mode logic into maintenance_mode module #350
• Move various code related to header handling to the respective modules #351
• Move all rewrite handling logic into the rewrites module #353
• Fix errors due to "unused code" when features are disabled #368
• Move all compression-related code into compression and compression_static modules #369
• Move all fallback page logic into the corresponding module #372
• Move directory listing initialization into the corresponding module #373

[joseluisq]

Thanks for your interest in improving SWS maintainability and reusability.

Below are some general thoughts.

Context

For SWS at least at the moment, modules are just functionality contained in a file that can be used when needed in the SWS scope.
But I guess what you meant about modularization is the reusability and composability of modules that can allow SWS to be used further in other projects interchangeably, fully or partially, not just internally. This is what SWS is not in v2 unfortunately as even stated in the crate description.
In fact, the current SWS v2 structure was even a decision taken from its inception. A binary project with a simple code base as possible. It was never meant to be a library to be reused or extended by users. But it does not mean that It can not be maintained or even scaled (like now). It can grow in functionality easily, for example. However, the cost of refactoring/complexity will just add up, in general. But this is not what I'm concerned about in v2 because we have enough control of the SWS code base (the so-called modules).

Although, the project has grown unavoidably with its pros and cons. I believe that it is also time to start thinking about the future of the project and a suitable design could help SWS to scale even more on different fronts, as a binary project as well as a library.

About modularization

Absolutely! SWS should improve in that regard for a variety of reasons. I (as a maintainer) for example, I'm aware of this and even I have a rough estimation of the effort needed which should be a challenge for a v3 at minimum, in my opinion.

About your proposed solution

This is a possibility to discuss further. However, I also made some minor steps in that respect like creating a middleware system that could be used as the foundation for a future reusable, composable and extensible SWS. That should also improve maintenance.

Personally, I'm in favor of a middleware system to better evolve the project and allow users to use SWS and extend it without limitations. Also for us, as maintainers for the sake of better maintenance.
This is not something simple to do of course, as it will require more discussion, evaluation and a final plan which can take quite a while.

Feel free to discuss here, or share any new ideas.

[joseluisq]

On the other hand, If you meant just to split up the monolithic code blocks (like the title says) then you could propose something or even start a draft PR or PoC to look at.
Feel free to share your thoughts on this.

[palant]

I was not in fact thinking that much about making SWS extensible – it’s an option if it is easy, but I don’t strictly need it myself. There is always the option of integrating the SWS handler in a larger and more complex server (I’ve got this working already).

This is a possibility to discuss further. However, I also made some minor steps in that respect like creating a middleware system that could be used as the foundation for a future reusable, composable and extensible SWS.

I don’t really have strong preferences here. This looks like it might do the job, but I don’t know the code good enough to tell whether it can cover all the necessary scenarios. I suspect that ordering will become an issue however, which is why I proposed to make it explicit by specifying priorities.

On the other hand, If you meant just to split up the monolithic code blocks (like the title says) then you could propose something or even start a draft PR or PoC to look at.

Yes, a logical first step would be moving code out of these code blocks and into the respective module files. Called explicitly for now, but that can be changed later easily. I’ll try creating a PR.

[joseluisq]

Yes, a logical first step would be moving code out of these code blocks and into the respective module files. Called explicitly for now, but that can be changed later easily. I’ll try creating a PR.

Great, go ahead then. It does not have to be perfect. I could also help to refine it. Take your time.
As long as the new ideas do not introduce breaking changes, I'm open to adopting them.

[palant]

#344 looks usable to me now. There is more shuffling code around than I would have hoped for, but the bright side is that subsequent changes will be simpler due to that.

[palant]

As long as the new ideas do not introduce breaking changes, I'm open to adopting them.

What do you consider breaking changes? CLI/config file changes only or also exposed library APIs?

[joseluisq]

#344 looks usable to me now. There is more shuffling code around than I would have hoped for, but the bright side is that subsequent changes will be simpler due to that.

Ok. In fact, there are several code splitting opportunities like handle.rs or server.rs.

Just keep it simple like one PR per functionality or feature.

[joseluisq]

What do you consider breaking changes? CLI/config file changes only or also exposed library APIs?

Both, for example if functionality that is not public gets moved to a new file then just keep the APIs at crate level. No need to expose them.

About CLI/config options. I would suggest to tackle them later once all the code splitting is done, it would be much simpler.

[joseluisq]

Just a suggestion, since you are planning to drive this task with successive PRs. It would be great to add a checkbox list with every PR link for completeness.

[palant]

Just a suggestion, since you are planning to drive this task with successive PRs. It would be great to add a checkbox list with every PR link for completeness.

Done. Hopefully after #345 it will also be possible to have more than one open PR at a time without introducing merge conflicts.

[palant]

This should do for now.

[joseluisq]

Great work @palant!