Closed treibholzhq closed 3 months ago
Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
Makes sense to me Jo :v:
I think it makes sense to add an option to the input processor to turn includeComponents
or something similar.
Is it something you can help out with?
@jonaslagoni Absolutely. I will pick up your suggestion and just create a PR for a first review. Is that ok with you?
I would propose the same for the SwaggerInputProcessor as well.
Makes sense to me 👍
@jonaslagoni Phew, ok, this is going to be harder than expected.
First and foremost, modelina seems to be unable to handle large OpenAPI specs (ex. https://app.swaggerhub.com/apis/Plattform_i40/AssetAdministrationShellRepositoryServiceSpecification/V3.0.1_SSP-001).
It fails with a JS heap OOM. I gave it my full 16GB through the max old space size param, still crashing.
Second, it seems that handling $refs does not work properly. In my example above (User has a property called role which is a $ref to the UserRole enum) it generates the enum twice: once as UserRole, once as Role. For the Address it's the same. The Address model is generated twice. It seems that inferring the name of the schema does not respect $refs.
I can work around this if I define the property like this:
"role": {
"$ref": "#/components/schemas/UserRole",
"$id": "UserRole"
}
But this is against the OpenAPI specification I guess.
@treibholzhq generally it should not be a problem, I have tested it with 30k LOC AsyncAPI inputs that generate close to 2500 models without problems 🤨 Might be a bug with the OpenAPI input processor?
But this is against the OpenAPI specification I guess.
Yea, the naming behaviour might be lacking here with incorrect algorithm for finding the right names 🤔 Usually, with references, you loose the information about the name as
"role": {
"$ref": "#/components/schemas/UserRole",
"$id": "UserRole"
}
In Modelinas eyes becomes:
"role": {
...{
// Content of #/components/schemas/UserRole
}
}
With no information about UserRole
.
Yes I guess it's OpenAPI related, I didn't mean to generalize, sorry.
Regarding the naming algorithm, it might be sufficient to just detect the $ref and take the last part as the $id. But this is just a guess.
Just wanted to mention we use the same technique as @treibholzhq, in order to store event schemas as openapi docs without paths, as Stoplight.io didn't support AsyncAPI. So if modelina could handle it it could make the migration to a single generation tool easier.
Once https://github.com/asyncapi/modelina/pull/1984 is merged, it will enable this if you set the option processorOptions: { openapi: { includeComponentSchemas: true } }
, this will be part of the next
release of Modelina.
Make sure you have a valid OpenAPI document, paths
are required in v3.0, so simply include paths: {}
.
@treibholzhq I tested it with https://app.swaggerhub.com/domains/Plattform_i40/Part1-MetaModel-Schemas/V3.0.1 after setting paths: {}
and seem to work fine.
Why do we need this improvement?
Hello fellow model creators,
I realized that right now, Modelina is not able to generate Models from OpenAPI specs with an empty
paths
object.Part of our DDD (Domain Driven Design) strategy is defining domain models as OpenAPI specs, where we omit the paths object entirely to define a "metamodel" that can be used standalone for generating models for tooling/poc/documentation purpose or be referenced in another OpenAPI spec to be used as part of an API.
A complex example would be: https://app.swaggerhub.com/domains/Plattform_i40/Part1-MetaModel-Schemas/V3.0.1
A much simpler, generic example:
Using this with the Modelina generators results in no models being generated.
So I dived into the code and did a quick 3-liner that solves this, so I would happily share and create a pull request for that if this feature request is accepted.
Cheers Jo
How will this change help?
Enables Modelina to be used as part of a DDD strategy.
Screenshots
No response
How could it be implemented/designed?
It can be implemented with a simple
else if
condition here: https://github.com/asyncapi/modelina/blob/ff2d0cf54b53c3013e0e79013aa8f6cf4d8552d6/src/processors/OpenAPIInputProcessor.ts#L37using the same
includeSchema
function that is used in the regular processing: https://github.com/asyncapi/modelina/blob/ff2d0cf54b53c3013e0e79013aa8f6cf4d8552d6/src/processors/OpenAPIInputProcessor.ts#L239🚧 Breaking changes
No
👀 Have you checked for similar open issues?
🏢 Have you read the Contributing Guidelines?
Are you willing to work on this issue?
Yes I am willing to submit a PR!