Carapacik / swagger_parser

Dart package that takes an OpenApi definition file and generates REST clients based on retrofit and data classes for your project.
https://pub.dev/packages/swagger_parser
MIT License
94 stars 43 forks source link

API method name should use operationId #35

Closed walsha2 closed 1 year ago

walsha2 commented 1 year ago

Typically, the operationId is used as the method name in the generated client. Right now, swagger_parser does not use this property at all. Instead, the method name is generated as follows:

https://github.com/Carapacik/swagger_parser/blob/18ceacbcde15579af35b415cfae459ffff38931e/swagger_parser/lib/src/parser/parser.dart#L333-L334

This is using the API path as the method name which is not necessarily indicative of the operation. Also, this arbitrary name definition (HTTP method (key) + path) is essentially defining a new specification for how method names should be defined... which is kind of the opposite of the OpenAPI spec which is meant to define a common standard.

Further, if you reference the source documentation of the API you will likely notice that the operationId is what is listed in the docs for that operation. So using that for consistency makes it easier to map the client generated by swagger_parser to the source API and any accompanying docs.

Options

This is a breaking change to anyone already using swagger_parser because it will change the API client definition method names. However, this is how most OpenAPI spec parsers work and it should at least be implemented in the package. There are two options here:

  1. Make operationId the new method name - will break users
  2. Make operationId the default and allow it to be opt-in via configuration file option - will not break users

I am partial to option 1 because this is how I have seen nearly every other parser, for every language, work. operationId is used as the method name. Although it is not a requirement of the OpenAPI spec, it is a common assumption that most parsers make.

I can push up a PR with option 1 as I have this working right now.