[kotlin-server] Fix class names for useTags#23441
[kotlin-server] Fix class names for useTags#23441feczkob wants to merge 7 commits intoOpenAPITools:masterfrom
Conversation
|
I used this repo for reproduction, thanks to @lbilger: https://github.com/lbilger/openapi-generator-path-shadowing-reproducer JAXRS-SPEC,
|
945ed35 to
7a365ce
Compare
lbilger
left a comment
There was a problem hiding this comment.
@feczkob I think I see more or less what you're doing here now. You want to align kotlin-server/jaxrs-spec with the "original" jaxrs-spec. If that is the main goal, the PR mostly achieves it as far as I can see. Only the docs should also state that the useTags parameter is only supported for this library, just like useMutiny already does.
This will however be a breaking change for everyone using this generator/library. Also, it deliberately excludes all the other libraries from using this parameter even though it would be no additional effort to support it for all libraries. So I suggest to at least consider making useTags=true the default, even though it is not completely in line with jaxrs-spec.
| Class | Method | HTTP request | Description | ||
| ------------ | ------------- | ------------- | ------------- | ||
| {{#apiInfo}}{{#apis}}{{#operations}}{{#operation}}*{{classname}}* | [**{{operationId}}**]({{apiDocPath}}{{classname}}.md#{{operationIdLowerCase}}) | **{{httpMethod}}** {{#useTags}}{{commonPath}}{{/useTags}}{{path}} | {{{summary}}} | ||
| {{#apiInfo}}{{#apis}}{{#operations}}{{#operation}}*{{classname}}* | [**{{operationId}}**]({{apiDocPath}}{{classname}}.md#{{operationIdLowerCase}}) | **{{httpMethod}}** {{commonPath}}{{path}} | {{{summary}}} |
There was a problem hiding this comment.
It seems operationIdLowerCase is not set in the useTags=false case - probably because the super method that sets it is never called.
aef988e to
c3e559b
Compare
|
Hey @lbilger, I added a new commit c3e559b with your suggested changes. The default for As for the future steps, I can think of a few:
When my time permits, I'd pick up the last one in a separate PR, to finally make |
There was a problem hiding this comment.
3 issues found across 29 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="samples/server/petstore/kotlin-server/ktor2-usetags-false/src/main/resources/logback.xml">
<violation number="1" location="samples/server/petstore/kotlin-server/ktor2-usetags-false/src/main/resources/logback.xml:4">
P2: Log timestamp pattern uses week-based year (`YYYY`) instead of calendar year (`yyyy`), which can log incorrect years near New Year.</violation>
</file>
<file name="samples/server/petstore/kotlin-server/ktor2-usetags-false/src/main/kotlin/org/openapitools/server/infrastructure/ApiKeyAuth.kt">
<violation number="1" location="samples/server/petstore/kotlin-server/ktor2-usetags-false/src/main/kotlin/org/openapitools/server/infrastructure/ApiKeyAuth.kt:67">
P2: Auth provider registers without validating required config, allowing empty `apiKeyName` (and default throwing validator) to fail at runtime instead of startup.</violation>
</file>
<file name="samples/server/petstore/kotlin-server/ktor2-usetags-false/src/main/kotlin/org/openapitools/server/apis/PetApi.kt">
<violation number="1" location="samples/server/petstore/kotlin-server/ktor2-usetags-false/src/main/kotlin/org/openapitools/server/apis/PetApi.kt:100">
P1: Array response examples are deserialized as a map type (`empty::class.java`), which breaks array-root payloads and can cause runtime JSON parse failures.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
...ore/kotlin-server/ktor2-usetags-false/src/main/kotlin/org/openapitools/server/apis/PetApi.kt
Outdated
Show resolved
Hide resolved
samples/server/petstore/kotlin-server/ktor2-usetags-false/src/main/resources/logback.xml
Outdated
Show resolved
Hide resolved
| ) | ||
| } | ||
|
|
||
| var apiKeyName: String = "" |
There was a problem hiding this comment.
P2: Auth provider registers without validating required config, allowing empty apiKeyName (and default throwing validator) to fail at runtime instead of startup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/server/petstore/kotlin-server/ktor2-usetags-false/src/main/kotlin/org/openapitools/server/infrastructure/ApiKeyAuth.kt, line 67:
<comment>Auth provider registers without validating required config, allowing empty `apiKeyName` (and default throwing validator) to fail at runtime instead of startup.</comment>
<file context>
@@ -0,0 +1,102 @@
+ )
+ }
+
+ var apiKeyName: String = ""
+
+ var apiKeyLocation: ApiKeyLocation = ApiKeyLocation.QUERY
</file context>
There was a problem hiding this comment.
Changing this is outside of the scope of this PR. It'd be a behavioral change that should probably be first discussed with users of the generator.
|
After all I could not resist the temptation to enable |






Follow-up of #23379.
Adds support of
useTagsfor allkotlin-servergenerators. The default value istruein order to avoid a breaking change. Adds a new sample forktor2withuseTags=false.Fixes deserialization into
Anyclass instead of aMapthat'd fail in case the response is an array forktor2. Fixes the week-baed year to calendar year forktorandktor2.PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master(upcoming7.x.0minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)"fixes #123"present in the PR description)Summary by cubic
Fixes
kotlin-serverJAX-RS class naming whenuseTags=false, standardizes class-level JAX-RS@Pathvia a computed common path, and enablesuseTagsacross allkotlin-servergenerators with a default of true. Also fixes Ktor example deserialization and logback date patterns, and adds aktor2sample withuseTags=false.Bug Fixes
jaxrs-spec): computecommonPathper API; set class-level@Path(empty string for root) and add method@Pathonly for sub-resources.useTags=false, group operations (and class names) by the first path segment; fallback todefaultfor root or templated-only paths.ktor/ktor2: deserialize example JSON toAny(Gson); fix logback pattern to use calendar year (yyyy).Refactors
useTagsto true across allkotlin-serverlibraries; docs updated; removeuseTagsfromkotlin-server-jaxrs-spec.yaml; addbin/configs/kotlin-server-ktor2-usetags-false.yaml.{{commonPath}}{{path}}; guard method@PathwithsubresourceOperation.samples/server/petstore/kotlin-server/ktor2-usetags-false; update tests and regenerate samples.Written for commit 878b96b. Summary will update on new commits.