Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a different OpenAPI Handler for each "tag" #691

Open
MahdiBM opened this issue Dec 8, 2024 · 10 comments
Open

Create a different OpenAPI Handler for each "tag" #691

MahdiBM opened this issue Dec 8, 2024 · 10 comments
Labels
kind/feature New feature. status/triage Collecting information required to triage the issue.

Comments

@MahdiBM
Copy link
Contributor

MahdiBM commented Dec 8, 2024

Motivation

The generator currently only creates one big OpenAPI Handler protocol.
This makes it hard to organize the routes.

Proposed solution

Create a different OpenAPI Handler for each "tag" so we have something close to Vapor "controllers".
These are quite useful for decluttering the code.

The generator could also generate each tag's "controller protocol" in it's own file. There are known issues in the Swift compiler where it struggles to handle one big file, or makes builds potentially extremely slower.

Alternatives considered

Manually handling this by different methods, including manually creating controllers and redirecting the routes in the main openapi handler to those controllers, or using multiple targets and filtering a shared openapi file based on tag in each controller.

Additional information

No response

@MahdiBM MahdiBM added kind/feature New feature. status/triage Collecting information required to triage the issue. labels Dec 8, 2024
@czechboy0
Copy link
Contributor

czechboy0 commented Dec 8, 2024

Hi @MahdiBM,

can you elaborate on why "using multiple targets and filtering a shared openapi file based on tag in each controller" isn't a viable option here? Tags, paths, operationIds are all reasonable ways to group operations, and filtering gives you great flexibility to use them all and even combine them.

Using Swift modules here allows you to have full control over the fully qualified protocol name MyModuleA.APIProtocol, MyModuleB.APIProtocol. If we generated multiple protocols, naming them would become a new challenge, as we'd have to derive some non-conflicting name from the combination of tags/paths/operationIds.

Are there specific issues with using filtering + modules? And if there are issues with Swift modules, should we instead work on addressing those in SwiftPM/the compiler?

@MahdiBM
Copy link
Contributor Author

MahdiBM commented Dec 8, 2024

o hmm sorry for 3 different issues. That was network problem and likely GitHub reattempting the creation 3 times.

can you elaborate on why "using multiple targets and filtering a shared openapi file based on tag in each controller" isn't a viable option here?

It is an option, It's just cumbersome.

If we generated multiple protocols, naming them would become a new challenge, as we'd have to derive some non-conflicting name from the combination of tags/paths/operationIds.

I guess that could be a theoretical problem, but I think we can just make it work in that case? e.g. add numbers to the names.
Realistically this should be very rare, but i understand OpenAPI Generator needs to assume worst case scenario.

Are there specific issues with using filtering + modules? ...

I mean, not really, aside from being a bit cumbersome to set up, and requiring a fine amount of knowledge to know how to make it work properly (e.g. I don't presume most newbies know what "symlinks" are, so this will look even more insane to them).

@MahdiBM
Copy link
Contributor Author

MahdiBM commented Dec 8, 2024

This should also help for the namings: #683

We could have a "group by" option in the config file and users will be able to declare how to group their routes?
We can make sure that only accepts 1 field, so name conflicts become basically non-existent.

@MahdiBM
Copy link
Contributor Author

MahdiBM commented Dec 8, 2024

having multiple modules can also make it a bit weird for the "types".
will we have to have a single target only for the types as well? Then @_exported import the types target in all the controller targets?

All possible, but ... harder than it should be?

@czechboy0
Copy link
Contributor

Is it? I'm not sure that doing this in the generator would result in a significantly better workflow, and we'd be reimplementing features present in the language and package manager. Can you list some of the concrete issues that exist when using the existing language features to achieve the decomposition today? Might make it easier to figure out what we're trying to solve here.

@MahdiBM
Copy link
Contributor Author

MahdiBM commented Dec 8, 2024

Is it?

for 1- it requires someone good at things to figure this out. I don't think any of my colleagues at work could come up with the idea, and 2- figure out how to do it (symlinks, @_exported, etc...).
Maybe you're overestimating the competence of an avg. user/developer 😅.
I can make it work, yes, but I don't think many developers can.

Even if the language already provides the tools and all for this, OpenAPI generator should at the minimum have some docs about it, and optimally have some kind of commands (command plugin?) to trivially to this.
It should be even easier with Swift 6's support of adding targets and all.
Edit: ... adding targets and all through CLI.

(To be clear, OpenAPI Generator has pretty good docs already, and I appreciate that 🙂)

@czechboy0
Copy link
Contributor

Right, that makes sense - I'm happy for us to document this workflow better. Would you want to open a PR? Or at least describe the workflow you're using in more detail, so that we could write a how-to?

@MahdiBM
Copy link
Contributor Author

MahdiBM commented Dec 9, 2024

@czechboy0

To be clear I have not done this yet anywhere.

What I'm thinking about the process, is that it would be something like:

  • Create a OpenAPI document file.
  • Create a target named xxxTypes.
    • Add types generation to the openapi yaml file
    • Symlink the openapi document in the target.
  • Create targets with names xxxTag for each tag.
    • @_exported import the xxxTypes targets.
    • Add server generation to the openapi yaml file with tag filter.
    • Symlink the openapi document in the target.

The problem here is that I'd like this to be automated. Perhaps with a command plugin.

  • The command plugin will need to take some parameters (probably depend on the ArgumentsParser package).
  • Will need to take arguments for a minimum level of control over name creations. For example to take a prefix or suffix for target names.
  • Will need to create targets in the package in an automated way. Then add dependencies to those targets.
    • This won't be too hard as we have some Swift CLI commands in Swift 6 and above to do that.
  • Should somehow be able to add plugins (The OpenAPI Generator plugin) to those targets as well?
    • I actually would be fine with not needing this.
    • I don't see how this can be properly done without asking users for some manual work.

Long story short, I can see this turning out to be its own thing in its own package.
Perhaps even the current command plugin could be merged with this at some point (v2 ? since we might need breaking changes).

In the end, I still think having a config option in the yaml file like group-by: tag is still much better.
I understand that you'd want to not bloat this package with things that can otherwise be implemented outside it, but I also think the command plugin will be way harder to use than having a simple field in the yaml file.

As someone who has tried selling OpenAPI to multiple people, as I believe in its value specially for bigger projects and in the longer term, the worst parts OpenAPIGenerator is how harder it is to use compared to just creating the routes right away in the Vapor / Hummingbird app. This will make things even more complicated and harder to use, as you'd need to learn yet another thing, as opposed to just getting everything for free after using the group-by field.
Part of the convenience problem is due to the nature of OpenAPI and the document not being "Swift" code, and having to learn about OpenAPI, but there are other parts that we can make easier, like this.

@czechboy0
Copy link
Contributor

czechboy0 commented Dec 9, 2024

Thanks for the thoughtful description of the workflow. I'm even more convinced now that it should be its own separate tool, as it builds purely on top of Swift OpenAPI Generator, it doesn't use any of its internals. So it should be at least a separate module/plugin, but arguably a separate package.

It also sounds like package generation, I wonder if the Vapor CLI wouldn't be a good place for this? As a flavor of a generated package, one that integrates OpenAPI and splits per-tag into separate modules?

I think one advantage of splitting your service into subcomponents, like controllers, is that you could iterate on them separately, test them separately, and they should be allowed to have distinct dependencies.

That's why I think separate modules for each tag is the right way to scale a service, and you're right that configuring a package this way could use some automation. Unclear for now whether this must be in this project, or could be done separately. What do you think, would this fit into Vapor's CLI?

@MahdiBM
Copy link
Contributor Author

MahdiBM commented Dec 9, 2024

would this fit into Vapor's CLI?

For 1, the Vapor CLI is pretty much deprecated. It used to have some usecases but Swift itself has grown to support those.
The only usecase for the Vapor CLI currently, is to start new Vapor projects.

Also this doesn't really seem like Vapor related anyway ... Like Hummingbird users might also want to do this?

Another thing is that we'd want to have some kind of support for adding plugins to packages through Swift CLI (e.g. swift package add-plugin), although this is not a deal breaker. Should create an issue for this.

Also Swift's current situation for working with sub-processes is a little bit suboptimal until those new Subprocess APIs in Foundation land, which is a hurdle but I guess still not a deal breaker. Because such a tool will need a lot of communication with Swift itself though CLI to retrieve information about the package.

So ... I guess doable although it'll take a lot of work?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature. status/triage Collecting information required to triage the issue.
Projects
None yet
Development

No branches or pull requests

2 participants