Default Operation ID clashes

[wafs]

I'm submitting a...

[ ] Regression 
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

Two controllers with the same operation will produce the same operation ID if no ID is provided when using tsc to compile.

@Controller('a')
class A {
    @ApiOperation()
    getOne(){...}
}

@Controller('b')
class B {
    @ApiOperation()
    getOne(){...}
}

// Output truncated 
{
    "paths" : {
        "a" : {"get" : {"operationId": "getOne"}},
        "b" : {"get" : {"operationId": "getOne"}},
    }
}

Expected behavior

Take the name of the controller also, or append a hash

// Output truncated 
{
    "paths" : {
        "a" : {"get" : {"operationId": "a__getOne"}},
        "b" : {"get" : {"operationId": "b__getOne"}},
        // or         
        "a" : {"get" : {"operationId": "getOne_r@nd0mH4$h"}},
        "b" : {"get" : {"operationId": "getOne_abcd12345"}},

    }
}

Minimal reproduction of the problem with instructions

1. Create two controllers with the same name for a method
2. Expose the same named method with @ApiOperation() decorator, do not provide an operationId
3. Use TSC to compile (untested if nest build command works)
4. View the JSON output

What is the motivation / use case for changing the behavior?

This is causing issues where operationId fields are expected to be globally unique but the value of them isn't important. I am using the JSON output in an OpenAPI UI generator called redoc. Without the unique operationIds, the links it produces are broken because all links with the same name point to the same endpoint, and this is frequent for my project as I'm using generic controllers.

Environment

Nest version: 6.10.14

For Tooling issues:

• Node version: 11.15
• Platform: Mac

[wafs]

Currently using this as a bandaid fix after the json doc compiles, it overrides all regardless since I'm not doing any smart checking for duplicates:

Object.entries(document.paths).forEach(([path, obj]) => {
    ['get', 'patch', 'put', 'post', 'delete'].forEach(method => {
      if (obj[method]) {
        obj[method].operationId = `${path.split('/')[1]}__${obj[method].operationId}`;
      }
    });
  });

[kamilmysliwiec]

Fixed in 4.1.14

[maxfriedmann]

Just upgraded to the latest version and now my method names are unnecessary long when I use e.g. the open api generator. Before, it generated a nice designService.updateImages method based on the operationId, now it is called designService.frontendDesignsControllerUpdateImages ?

Is it possible to turn that off again and use the not-so-unique version? I understand that problem with doubled operationIds (been there, done that) but since you can easily work around that issue by scoping your swagger spec via nestjs modules, I'd really like to have the possibility to get back to the shorter method names...

So for me, #487 is clearly a breaking change, have to update all projects that are using our generated api clients!

[tocosastalo]

I totally aggre with @maxfriedmann. It is a breaking change, i have to downgrade. Existed client API calls are all invalid now.

[Syarx]

I agree with @maxfriedmann. It is a breaking change and now I have to update 100 of backend calls in my client. This should either have an option to disable it or be reversed completely. I have to downgrade too.

[maxfriedmann]

@kamilmysliwiec, am too far away from @nestjs/swagger development, do you think there could be an easy option to disable this behavior?

[wafs]

Looking at the patch, I feel like this could be customisable behaviour if it were exposed in the settings.

https://github.com/nestjs/swagger/pull/487/files#diff-bb845d0d85e92f24987732c111594fb0R208 is how it decides what the default operation id is if there wasn't one.

I envision something like

type SwaggerOperationIdStrategy = 
    | 'FUNCTION_NAME' // old behaviour
    | 'CONTROLLER_FUNCTION_NAME'     // default, current behaviour
    | (instance: object, method: Function) => string  // custom behaviour, make your own Id

 new DocumentBuilder()
    .setTitle('Cats example')
    .setDefaultOperationIdStrategy('FUNCTION_NAME')

[dennisameling]

Had the same issue with my Angular client libraries after updating to >= 4.1.14. Just provided a PR to make this behavior configurable: #747