feat: (OpenAPI) Fix class name generation for inline-object of //component/response schemas
#835
Conversation
//component/response schemas//component/response schemas
...ain/java/com/sap/cloud/sdk/datamodel/openapi/generator/GenerationConfigurationConverter.java
Show resolved
Hide resolved
| mediaContent.forEach(( mediaType, content ) -> { | ||
| final Schema<?> schema = content.getSchema(); | ||
| if( schema != null && schema.getTitle() == null ) { | ||
| schema.setTitle(key + " " + (mediaContent.size() > 1 ? mediaType : "Content")); |
There was a problem hiding this comment.
(Minor)
I don't like adding "Content" to the title if there is only one mediaContent. I would rather always use the mediaType in the title. If in the future a second mediaType is added in the spec then using "Content" will lead to avoidable breaking changes: the already existing title (and thus the existing classname) would change from XYZ Content to e.g. XYZ plain/text.
There was a problem hiding this comment.
Generally I agree to that concern. However in 99% of cases there is only one media-type: application/json.
And this would result in many (many) classes being generated with ApplicationJson name suffix. That looks really bad.
We might consider the following suffix-options:
1️⃣ $mediaType
2️⃣ n=1 -> "Content", else -> $mediaType
3️⃣ n=1 -> "Payload", else -> $mediaType
4️⃣ n=1 -> "", else -> $mediaType
There was a problem hiding this comment.
If we are fairly certain that this will not come up often (which I am not able to asses, so I believe you), then I am fine with this approach. I also see that having ApplicationJson appended to many class names does not add much readability. I would then, however probably use version 4️⃣ since if we assume that there will mostly be only one media type, then we do not need any special suffix IMO.
There was a problem hiding this comment.
I assumed 4️⃣ was not possible due to naming conflicts but it works! No conflicts.
Context
https://github.com/SAP/ai-sdk-java-backlog/issues/273
Offers an opt-in feature to enable preprocessing of OpenAPI:
When
fixResponseSchemaTitles=truethen the followingtitleproperties are added dynamically.components: responses: NotFound: description: The specified resource was not found content: application/json: schema: + title: NotFound Content type: object properties: message: type: string example: Resource not found ServiceUnavailable: description: The service is currently unavailable content: application/json: schema: + title: ServiceUnavailable application/json type: object properties: message: type: string example: Resource not found application/xml: schema: + title: ServiceUnavailable application/xml type: object properties: message: type: string example: Resource not foundThe result is class names being fixed:
The important side-effect:
Feature scope:
Definition of Done
Error handling created / updated & covered by the tests aboveDocumentation updatedRelease notes updated