Skip to content

Fix incorrect Uni<Set<Enum>> generation due to path interference#1464

Open
pavelkryl wants to merge 1 commit intoquarkiverse:mainfrom
pavelkryl:issue-1456
Open

Fix incorrect Uni<Set<Enum>> generation due to path interference#1464
pavelkryl wants to merge 1 commit intoquarkiverse:mainfrom
pavelkryl:issue-1456

Conversation

@pavelkryl
Copy link
Copy Markdown

Summary

Test plan

  • Added integration test in client/integration-tests/reuse-enums/ to verify correct enum type generation
  • Verify existing tests pass

🤖 Generated with Claude Code

@pavelkryl pavelkryl requested a review from a team as a code owner February 17, 2026 21:40
Copy link
Copy Markdown
Member

@ricardozanini ricardozanini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems ok to fix your specific use case. I'm asking a few questions since it can impact other users as well, especially if you are restricting the checks for 200 http code and application/json content type.

OperationsMap result = super.postProcessOperationsWithModels(objs, allModels);

Map<String, Object> operations = (Map<String, Object>) result.get("operations");
List<org.openapitools.codegen.CodegenOperation> os = (List<org.openapitools.codegen.CodegenOperation>) operations
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you import this class instead?

private Operation findOperation(String path, String httpMethod) {
if (this.openAPI == null)
return null;
io.swagger.v3.oas.models.PathItem pathItem = this.openAPI.getPaths().get(path);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
io.swagger.v3.oas.models.PathItem pathItem = this.openAPI.getPaths().get(path);
PathItem pathItem = this.openAPI.getPaths().get(path);

Can you do the imports here too?

&& openApiOperation.getResponses().get("200").getContent() != null && openApiOperation
.getResponses().get("200").getContent().get("application/json") != null) {
Schema<?> responseSchema = openApiOperation.getResponses().get("200").getContent()
.get("application/json").getSchema();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it only apply to 200? Perhaps add it for 2xx. I also found it curious why enforcing the schema to only JSON.

// DEBUG: returnContainer = {op.returnContainer}
{#if op.imports}
// DEBUG: imports = {op.imports}
{/if}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove these comments please?

super.postProcessModelProperty(model, property);
if ("set".equals(property.containerType)) {
model.imports.add("Arrays");
public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List<ModelMap> allModels) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this method override the feature of postProcessModelProperty? Asking out of curiosity - it's been a long time since I did this.

@mcruzdev
Copy link
Copy Markdown
Member

mcruzdev commented Mar 5, 2026

Hello @pavelkryl I would love to have this one merge, are you minding to solve the @ricardozanini comments?

@pavelkryl
Copy link
Copy Markdown
Author

Sorry @mcruzdev I am completely buried :/ I have no time in the next 2 weeks

@mcruzdev
Copy link
Copy Markdown
Member

mcruzdev commented Mar 5, 2026

No worry, you did a great work!

Could you give me access to your repo to address these comments?

@pavelkryl
Copy link
Copy Markdown
Author

sure! would be nice if you could finish this, thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrectly generated instead of Uni<Set<Enum>> due to path interference

3 participants