Skip to content
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

Optional path parameters generate two operations in the OpenAPI spec #1960

Open
loicmathieu opened this issue Feb 4, 2025 · 11 comments · May be fixed by #1964
Open

Optional path parameters generate two operations in the OpenAPI spec #1960

loicmathieu opened this issue Feb 4, 2025 · 11 comments · May be fixed by #1964

Comments

@loicmathieu
Copy link

Expected Behavior

Optional path parameters generate only one operation in the OpenAPI spec with an optional path parameter.

Actual Behaviour

Using the following controller which has an optional tenantId path parameter:

@Controller("/openapi-issue{/tenantId}")
public class OpenapiIssueController {

    @Get(uri="/", produces="text/plain")
    public String index() {
        return "Example Response";
    }
}

The following OpenAPI spec is generated with 2 operations:

openapi: 3.0.1
info:
  title: openapi-issue
  version: "0.0"
paths:
  /openapi-issue:
    get:
      operationId: index
      responses:
        "200":
          description: index 200 response
          content:
            text/plain:
              schema:
                type: string
  /openapi-issue/{tenantId}:
    get:
      operationId: index_1
      parameters:
      - name: tenantId
        in: path
        required: true
        schema:
          type: string
      responses:
        "200":
          description: index_1 200 response
          content:
            text/plain:
              schema:
                type: string

Steps To Reproduce

No response

Environment Information

No response

Example Application

No response

Version

4.4.4

@loicmathieu
Copy link
Author

Example application:

openapi-issue.zip

@altro3
Copy link
Collaborator

altro3 commented Feb 4, 2025

@loicmathieu It's correct. In openapi specification - you can't set optional path variables

Image

@altro3 altro3 closed this as completed Feb 4, 2025
@altro3
Copy link
Collaborator

altro3 commented Feb 4, 2025

I mean, you haven't another way to describe optional path variable. Only by two endpoints - with it and without it

@loicmathieu
Copy link
Author

It's annoying as the operationId is generated with an incremented counter which is not very good when we generate a client based on the OpenAPI spec.

Is there a way to customize the operationId generated name?

@altro3
Copy link
Collaborator

altro3 commented Feb 4, 2025

Hmm.. I understand what you mean. Okay, if you want a different operationId in that case, how do you propose to do this? The only way I see is to come up with some internal extension for the Operation annotation, in which to specify an explicit operation ID, but there is a problem, there can be many optional variables, then there will be many operations.

Your suggestions?

@altro3 altro3 reopened this Feb 4, 2025
@altro3
Copy link
Collaborator

altro3 commented Feb 4, 2025

@loicmathieu another way, you can describe 2 different endpoints

@loicmathieu
Copy link
Author

Yes, the problem is that in case a single methods generates two operations, having an operationId in the code (which can already be customized using OpenAPI extension annotation) will not works.

We would need a pluggable operationId naming strategy for that.

another way, you can describe 2 different endpoints

I don't think so, we have approx a hundred endpoints on 15 different controllers, all can take an optional tenantId in the path for multi-tenancy so we add it in the @Controller annotation so we never forgot about it.
Then we have some automatic tenant switching code (deep inside the stack).

Worst is that the counter for the operationId is shared for the whole application, it is not for a controller nor an endpoint.
As we have hundreds of endpoints, we end of with operationId like find42.

@altro3
Copy link
Collaborator

altro3 commented Feb 4, 2025

Ok, one possible solution: I can add a naming strategy for these endpoints. For example:

  1. As now - a common counter
  2. Counter at the controller level or at the level of similar endpoints with optional path variables
  3. Can add a postfix by name variable. According to your example it will be index and indexWithTenantId

@loicmathieu WDYT?

altro3 added a commit to altro3/micronaut-openapi that referenced this issue Feb 5, 2025
@loicmathieu
Copy link
Author

This would be great!

For us, the third option is the best, but you can also make it customizable if you don't want to support the third option so we an implement it in our code base.

altro3 added a commit to altro3/micronaut-openapi that referenced this issue Feb 5, 2025
altro3 added a commit to altro3/micronaut-openapi that referenced this issue Feb 6, 2025
altro3 added a commit to altro3/micronaut-openapi that referenced this issue Feb 6, 2025
altro3 added a commit to altro3/micronaut-openapi that referenced this issue Feb 6, 2025
@altro3
Copy link
Collaborator

altro3 commented Feb 6, 2025

I thought again and decided that there is no point in doing the other 2 options. As a result, operationId will now always be generated as in the third option.

Wait for the next release

altro3 added a commit to altro3/micronaut-openapi that referenced this issue Feb 6, 2025
@loicmathieu
Copy link
Author

Great, thank you a lot.

altro3 added a commit to altro3/micronaut-openapi that referenced this issue Feb 6, 2025
altro3 added a commit to altro3/micronaut-openapi that referenced this issue Feb 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants