-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
feat: add support for oneOf in sttp4 client
#22916
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 issues found across 8 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="modules/openapi-generator/src/main/resources/scala-sttp4/api.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/scala-sttp4/api.mustache:38">
P2: Request bodies are now always JSON‑serialized with `asJson(...)`, which breaks operations that consume non‑JSON media types by sending JSON instead of the raw body.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/scala-sttp4/model.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/scala-sttp4/model.mustache:52">
P2: json4s oneOf deserialization aborts on first mismatch because `Extraction.extract` throws, so later oneOf types are never tried.</violation>
<violation number="2" location="modules/openapi-generator/src/main/resources/scala-sttp4/model.mustache:74">
P2: Discriminator handling hardcodes the Scala class name as the wire value, ignoring OpenAPI discriminator mappings, so specs with mapped discriminator values will fail to deserialize/serialize correctly.</violation>
<violation number="3" location="modules/openapi-generator/src/main/resources/scala-sttp4/model.mustache:172">
P2: Inline oneOf member case classes are generated inside the parent model file, but the parent model’s import list isn’t augmented with the child models’ imports. If a oneOf member uses a type not referenced by the parent, the inline case class will compile without the necessary import.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| {{/vendorExtensions.x-isEnum}} | ||
| {{#vendorExtensions.x-isRegularModel}} | ||
| {{^isEnum}} | ||
| case class {{classname}}( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Inline oneOf member case classes are generated inside the parent model file, but the parent model’s import list isn’t augmented with the child models’ imports. If a oneOf member uses a type not referenced by the parent, the inline case class will compile without the necessary import.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/scala-sttp4/model.mustache, line 172:
<comment>Inline oneOf member case classes are generated inside the parent model file, but the parent model’s import list isn’t augmented with the child models’ imports. If a oneOf member uses a type not referenced by the parent, the inline case class will compile without the necessary import.</comment>
<file context>
@@ -15,46 +20,218 @@ import {{import}}
+{{/vendorExtensions.x-isEnum}}
+{{#vendorExtensions.x-isRegularModel}}
+{{^isEnum}}
+case class {{classname}}(
+ {{#vars}}
+ {{#description}}
</file context>
1ec344d to
c56b433
Compare
|
thanks for the PR cc @clasnake (2017/07), @shijinkui (2018/01), @ramzimaalej (2018/03), @chameleon82 (2020/03), @Bouillie (2020/04) @Fish86 (2023/06) |
|
https://github.com/OpenAPITools/openapi-generator/actions/runs/21774387501/job/62832048821?pr=22916 please follow step 3 to update the samples |
c56b433 to
4437a9e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="modules/openapi-generator/src/main/resources/scala-sttp4/model.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/scala-sttp4/model.mustache:107">
P2: `transformConstructorNames` is defined as a partial match over only `x-oneOfMembers`. Because `x-oneOfMembers` excludes shared oneOf subtypes, the function is not total and will throw `MatchError` when encoding/decoding those subtypes. Add a default case (identity) so unmapped constructor names don’t crash serialization.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="modules/openapi-generator/src/main/resources/scala-sttp4/model.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/scala-sttp4/model.mustache:110">
P2: Wildcard `case other` is emitted inside the oneOfMembers loop, so the first wildcard matches everything and makes subsequent member cases unreachable; only the first oneOf member can be encoded/decoded.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
modules/openapi-generator/src/main/resources/scala-sttp4/model.mustache
Outdated
Show resolved
Hide resolved
d05761d to
7aba995
Compare
Address #19891 for sttp4 clients.
@Fish86
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
Adds oneOf support to the Scala sttp4 client with sealed-trait parents and inline members, and aligns JSON and request/response handling with sttp4. Fixes discriminator mapping and makes circe constructor-name mapping total; adds a circe sample and updates to sttp 4.0.15 (fixes #19891).
New Features
Migration
Written for commit 7aba995. Summary will update on new commits.