[Enhancement] Better naming for method names

[wing328]

Currently method name is directly sourced from "operationId" without any conversion. I would suggest we convert the method name based on the style guide of the respective programming languages. Here are some examples:

Ruby: addPet => add_pet Python: addPet => add_pet PHP: add_pet => addPet Java: add_pet => addPet Scala: add_pet => addPet Android: add_pet => addPet Objc: add_pet => addPet CSharp: addPet => AddPet

The auto-generated SDK will then follow the style guide of the respective programming languages in naming the method/function.

If anyone has any concern with the proposed change, please kindly share with us. Otherwise, I'll file a PR to make the change.

[webron]

@wing328 - that may be a very nice feature. One of the reasons we decided to make operationId not mandatory in 2.0 was to make the consumers (in this case, the codegen) responsible for the naming. If it can be controlled in a reasonable manner, that would be great, though there may be edge cases.

On top of that, I'd add a configuration option that allows ignoring operationId in the generation process and generate the code as if it weren't there. This is probably a separate enhancement.

[wing328]

@ron, totally agree there will be edge cases.

What about adding a configuration option to update method name based on the programming language's style guide ?

[webron]

It's an option, though I'm worried we'll end up with a configuration options overload. We can still manage it differently in the future.

if you do end up adding a configuration option, please make sure to update the relevant documentation as well.

[wing328]

I also share your concern about configuration options overload.

To be clear, my suggestion also applies when operationId is not supplied. For codegen, I believe the default operationId is determined based on the URL (https://github.com/swagger-api/swagger-codegen/blob/e8ae9178af5a62276f09deac9805ca3ff8b2d6e8/modules/swagger-codegen/src/main/java/com/wordnik/swagger/codegen/DefaultCodegen.java#L661). My proposed change simply converts the method name (default or supplied via operationId) into underscore or camelize style depending on the language.

[webron]

In that case, by all means, do it. I believe that was the original intent. Not sure I'd use the full path for method name generation, but that's a different issue.

[wing328]

OK.

If you don't mind, may I also seek your opinion on how you would name the method? I believe HTTP verb (GET, POST, etc) would also be helpful to come up with the method name.

[webron]

That may also depend on the language/implementation possibly. Normally HTTP verbs translate to CRUD operations, but not everyone follows that standards. Did you have anything specific in mind?

[wing328]

To me, it's a hard problem to come up with a meaningful method name based on URL and/or HTTP verbs. We can still try to "guess" the method name and I would consider the guess a successful one if 80% of the users find it meaningful/acceptable.

If the users do not like the default, they can always supply the operationId.

Another thing we need to check when generating default method name is to make sure the method name is unique within the same class. Otherwise, the auto-generated source code likley won't compile due to the same method defined more than once

[webron]

The 'meaningful' name would probably end up being a bit of a trial and error. We can mark the feature as experimental and set a flag to use it as you previously suggested.

With the current version of Swagger, uniqueness should not be an issue since at most you can have one operation with the combination of HTTP verb and a path. This may change (to some extent) in the future, but for now we can definitely rely on it.

[who]

Currently the logic for function names is

1. Use the operationId if it is present
2. If the operationId is not present, use the full URI

I think that @wing328's proposal would indeed work well in happy-case scenarios, but I feel that the 2 items above are insufficient for creating good names, and we should do some additional work to make the name generator more sophisticated.

For example, given an API that exposes an HTTP GET on /library/v1/magazines/{magazine}/articles/{article}, assuming no operationId is present in the provided spec, the generated function name, at least in the Java realm, would be libraryV1MagazinesMagazineArticlesArticleGet(). Most devs would call this function getMagazineArticle().

Instead of using just operationId or path for generating the names, I think that checking vendor extensions would also be a viable approach.

Proposal: Because the generated function name is so specific to the language it lies in, we should look for a vendor extension in the path spec for naming hints. If an API spec contains x-generator-fn vendor extention inside a path definition, use it in the code generator function naming logic.

Below is an example spec snippet that would use vendor extensions to guide the generator's behavior.

{
  ...
  "paths": {
    "/books": {
      "post": {
        "x-generator-fn" : {"default":"create_book", "java":"createBook", "perl":"!@%!@#!:)"},
        "summary": "Creates a book",
        "description": "Creates a book so that it can be managed by this api.",
        "schemes": [],
        "consumes": [
          "application/json"
        ],
        "produces": [
          "application/json"
        ],
        "parameters": [
          {
            "in": "body",
            "name": "body",
            "description": "Book object",
            "required": true,
            "schema": {
              "$ref": "#/definitions/Book"
            }
          }
        ],
        "responses": {
          ...
        }
      }
    }
  }
  ...
}

The value of x-generator-fn could be just a simple string, such as "create_book", or it could be an object such as {"default":"create_book", "java":"makeBookPlease"}, in which case the default value would be passed through the language specific logic proposed by @wing328 , and any specified language keys, such as the "makeBookPlease" one above, would override any generated outputs for that specific language.

[webron]

For the current and near releases we should not rely on any vendor extensions. We need to expose a mechanism to allow people to extend the code generator using vendor extensions but this is not the place to do so.

For now, we should stick to the information available and rely on that. We'll never get 100% of the users satisfied and that's fine. The name generation can be improved over time, I don't think we need to over think it right now.

[who]

@webron Because the vendor extensions are optional, we wouldn't be relying on them. We'd simply be using them if they were present.

[webron]

right, but since vender extensions are by definition extensions, we should not use it at all in the capacity of this feature. it would be an extension that could be implemented in general. we're not going to define here an name for a vender extension name.

[who]

Got it @webron. Sorry for the thread hijack. Another alternative would be to, instead of defining a specific vendor extension name here, make the vendor extension name configurable at runtime, and then load any codegen options from inside the resulting json payload.

@wing328 , Going back to the original thread, I think your proposal is good for cases where operationId is available.

Unfortunately, in our specific case, where we generate the Swagger 2.0 spec with springfox and then generate clients from that, we do not have the option of using operationId, so we are stuck with URI-path function names until we can figure out a more elegant solution.

[webron]

@who - really, it's a healthy discussion. I very much appreciate it, because it also hints as to possible changes we may want to do in the spec in the future (among other things).

In general, vendor extensions are meant for vendors to extend the specification. Relying on that in the base tools we provide is not wise, imho. I do think we need to provide an extension mechanism, but that should be a general implementation which would have aspects on various aspects of the codegen. For now, we should concentrate on utilizing whatever is available and later work on the extension mechanism which could definitely be used for what you suggest here.

And we may not agree on that point, but generated code will never be as elegant as a developer would right, but the idea here is functionality and clarity. Eventually, libraryV1MagazinesMagazineArticlesArticleGet() is clearer than method1(). To be clear, I'm not saying that's what it should be, just that there's a limit to what we can reasonably achieve given some very obvious constraints.

[fehguy]

I do agree that the naming convention varies by language and with a personal twist on top of it. That's why we made operationId optional. Each language should probably expose it's own naming strategy, and that my override the explicit operationId

getPetById // java
get_pet_by_id // python / php
petByIdWithCompletionBlock  // objc
GetPetById  // c#

[who]

@fehguy

Assuming only the URI path is passed in to swagger-codegen, are you intending that the generator will parse the path, pick out the significant segments, and generate a camel/snake/other case string depending on the language?

Input

HTTP GET /pet/{petId}

Output

getPetById // java
get_pet_by_id // python / php
petByIdWithCompletionBlock  // objc
GetPetById  // c#

This output is fairly easy to generate, as the input is basic.

However, the following inputs would also have to be accounted for:

Input

HTTP GET /pet/{petId}/collars

HTTP GET /pet/findByStatus

Output ?

getCollarsForPet // java
get_collars_for_pet // python / php
collarsForPetWithCompletionBlock  // objc
GetCollarsByPet  // c#

findPetByStatus // java
find_pet_by_status // python / php
petByStatusWithCompletionBlock  // objc
FindPetByStatus  // c#

More complex input:

HTTP POST /v1/admin/magazines/{magazineId}/articles/{articleId}

Output

addMagazineArticle // java
add_magazine_article // python / php
addMagazineArticleWithCompletionBlock  // objc
AddMagazineArticle  // c#

[fehguy]

@who it is tricky. I'm expecting we follow some simple rules:

• http method is the prefix
• path params will be converted to "by" + pathParam name
• language depending, we use mixed case or underscores for either path separators or case changes.

After thinking about it more, it would produce the following:

HTTP GET /pet/{petId}

# java-ish
getPetByPetId

# python / php
get_pet_by_pet_id

# objc
petByPetIdWithCompletionBlock

# c-sharp
GetPetByPetId

and this: HTTP GET /pet/{petId}/collars

getPetByPetIdCollars
get_pet_by_pet_id_collars
petByPetIdCollarsWithCompletionBlock
GetPetByPetIdCollars

and this: HTTP GET /pet/findByStatus

getPetFindByStatus
get_pet_find_by_status
petFindByStatusWithCompletionBlock
GetPetFindByStatus

and this: HTTP POST /v1/admin/magazines/{magazineId}/articles/{articleId}

addV1AdminMagazinesByMagazineIdArticlesByArticleId
add_v1_admin_magazines_by_magazine_id_articles_by_article_id
v1AdminMagazinesByMagazineIdArticlesByArticleIdWithCompletionBlock
AddV1AdminMagazinesByMagazineIdArticlesByArticleId

It does get a bit ugly. We can create some simple rules for common path params but not sure if that suffices. What do you think?

[wing328]

@who is it correct to say that the current springfox swaggger plug-in does not allow customized operationId? if yes, would it be difficult to update the plug-in to support customized operationId ?

@fehguy I would suggest underscores for separators. I agree with @webron that the default would never meet everyone's requirement and those who do not like the default operationId should provide the value for operationId in the spec.

[who]

@fehguy

This solution really only works well for short, simple URIs.

Also, not everyone uses the singular naming convention to name their "things", so HTTP GET /things/{thingId} would resolve to getThingsByThingId, which really doesn't make sense.

[who]

@wing328

That is correct, yes. Springfox completely removed operationId from the output it renders.

[fehguy]

@wing328 I think that going "all underscores" will make one faction happy and piss off another (objective c folks for example won't like it).

[wing328]

@fehguy I was not clear. If the default operationId is in underscore style, then I can file a PR to customize the language code generator (e.g. ObjcClientCodegen.java) to easily output the method name following the language-specified method naming style, which is what I originally proposed in this thread.

[wing328]

@who, do you agree that no matter what the default method naming mechanism is, it would never meet everyone's requirement? in other words, there will always be edge cases.

Looking at the bigger picture, there are other swagger plug-ins (e.g. https://github.com/tim-vandecasteele/grape-swagger) that may have similar problems. Having the plug-in to optionally provide the method name based on user's input seems to me good enough to address those edge cases.

[who]

@wing328 I most definitely agree that you can't make everyone happy, and no matter what the default is, someone will want something different. However, this can be mitigated by providing a means by which naysayers can easily override the default mechanism with whatever they chose.

[wing328]

@who If springfox adds back the operationId to the output (so that users can customize the method name), would that meet your requirement ?

[who]

@wing328 Most definately, yes. If they add the operationId back in, my team will use operationIds that have underscores, so that they can be parsed by the logic you proposed at the top of this thread.

However, I do understand where Springfox and other producers are coming from; they should concern themselves with creating solid Swagger spec files, and should not have to worry about how the spec is consumed.