Code API inconsistencies

[RobertoPrevato]

There are inconsistencies in the code API. Some features can be enabled with a dedicated method in the Application class (e.g., use_sessions, use_cors), while others are not. Maybe it is best to remove the methods that were added to the Application class, since covering all scenarios cannot scale well.

[tyzhnenko]

It sounds reasonable.
How about making a separate package with all middlewares?
Another bonus that I also see everything related to this new package can be put in one place in the documentation.
But in the other hand such changes can break projects that is using blacksheep. It seems that only one chance to do it in right time is before release version 2.

Btw, if you think the project needs a volunteer or some other help just tell me. Currently I have free time between jobs and would like to spend it with profit.

[RobertoPrevato]

@tyzhnenko, I apologize for replying late. I work on these open source projects in my private time, and never during my work hours, so sometimes it takes me days to reply.
I don't like the idea of separating middlewares into different packages, for a few reasons:

• I like BlackSheep to be "battery included" for things that don't require extra dependencies, like the Gzip middleware you contributed to recently
• I like separating features into dedicated packages whenever I see the opportunity to reuse functionalities outside the context of the web framework. For example I put guardpost into a dedicated library, rodi for dependency injection, etc. I already maintain about 20 projects and I wouldn't split blacksheep into several packages, unless it's for the good reason to not add extra dependencies (example: blacksheep-sqlalchemy has its own package because of this reason).

Btw, if you think the project needs a volunteer or some other help just tell me. Currently I have free time between jobs and would like to spend it with profit.

I welcome contributions if you like these projects and find them useful. But please be aware that I don't earn any money from these projects and I cannot compensate monetarily. I even disabled sponsors on my GitHub account.

Going back to the subject of this ticket, please consider it low priority. Python itself has several code inconsistencies, since years (defaultdict, OrderedDict, logging.getLogger, etc.). I am just considering to put the features that currently are attached to the Application class such as use_cors, use_authentication, use_authorization into separate modules inside the same library, to make them consistent with other things that were not attached to the Application (like use_oidc). Because it doesn't scale very well putting everything together in the Application class. 😄

[nkhitrov]

+1, I like the idea of separate packages to handle cors, auth, etc.

For more flexibility, we can try to implement event system like in sqlalchemy, or look at plug lib from elixir. I think plug is really powerful feature because it's like middleware per route(s) and similar to fastapi depends (but not so ugly)

What you think about it @RobertoPrevato ?

[RobertoPrevato]

@nkhitrov thanks for sharing your ideas. After the improvement of middlewares at #624, it became easy to remove the code API inconsistencies I had in mind when creating this issue. The use_sessions, use_cors, use_authentication, and use_authorization methods can be removed from the Application class and placed into their respective namespaces.

I start the work on a development branch.

[RobertoPrevato]

For the record, this is GitHub Copilot's recommendation (🤔...):

Me:
Do you think this change is a good idea, for the sake of Code API consistency (#328)? c4c730a

GitHub Copilot:

I'll analyze the commit you've referenced to understand the change and provide my opinion on whether it's a good idea for Code API consistency.Let me get a better view of the commit changes:Based on issue #328 and the commit you've shared, I can provide some perspective on this approach:

Context

Issue #328 highlights an inconsistency in BlackSheep's API design: some features have dedicated methods in the Application class (like use_sessions, use_cors), while others don't. The issue notes that this approach "cannot scale well" since it's impractical to cover all scenarios with dedicated methods.

The Trade-offs

Arguments in favor of the change:

1. Consistency: A uniform approach (removing convenience methods) creates a more predictable API
2. Scalability: Avoids the problem of deciding which features "deserve" a dedicated method
3. Simplicity: One clear way to configure features reduces mental overhead
4. Maintainability: Less surface area in the Application class means fewer methods to document and maintain

Arguments against:

1. Developer Experience: Convenience methods like app.use_cors() are more discoverable and readable than manual middleware registration
2. Breaking Changes: Removing these methods would break existing code
3. Common Use Cases: For frequently-used features (sessions, CORS), the convenience is valuable
4. Framework Conventions: Many web frameworks (Express.js, ASP.NET Core) use similar use_* patterns successfully

My Take

This is a good architectural question but perhaps an extreme solution. Consider a middle ground:

• Keep convenience methods for the most common features (CORS, sessions, authentication)
• Document clearly that not all features will have dedicated methods
• Provide excellent documentation for the general middleware/configuration approach

The "PoC" nature of your commit suggests you're exploring this - I'd recommend gathering feedback from your users before fully committing to removing widely-used convenience methods.

[RobertoPrevato]

After further questions, Copilot recommended against adopting a Builder pattern, and finally recommended as good idea to reorganize the code like in the PoC commit I created c4c730a.