Generated OpenAPI document fails validation on parameters in path

[someone1]

🐛 Bug Report

Take the example a little bit of everything swagger file and pop it into swagger editor/UI - it will fail validation checks

This feels like the issues raised in #720 and #1015 though I didn't want to bump an older issue or hijack another issue that seemed more specific.

Sometimes the path annotations generate fine in the OpenAPI document and other times not. For example, the following annotrations seem to work:

option (google.api.http) = {
      get: "/v1alpha/{user=users/*}/noun:verb"
    };

option (google.api.http) = {
      post: "/v1alpha/{user=users/*}/noun2:verb"
      body: "*"
    };

Generates:

"/v1alpha/{user}/noun:verb": {
      "get": {
        "parameters": [
          {
            "name": "user",
            "in": "path",
            "required": true,
            "type": "string"
          }
        ],
    }
}
"/v1alpha/{user}/advice:generate": {
      "post": {
        "parameters": [
          {
            "name": "user",
            "in": "path",
            "required": true,
            "type": "string"
          },
          {
            "name": "body",
            "in": "body",
            "required": true,
            "schema": {
              "$ref": "#/definitions/v1alphaVerbRequest"
            }
          }
        ],
    }
}

However, the following does not:

    option (google.api.http) = {
      get: "/v1alpha/{parent=users/*}/accounts"
    };
    option (google.api.http) = {
      get: "/v1alpha/{name=users/*/accounts/*}"
    };

Generates:

 "/v1alpha/{parent=users/*}/accounts": {
      "get": {
        "parameters": [
          {
            "name": "parent",
            "in": "path",
            "required": true,
            "type": "string"
          }
        ],
    }
}
 "/v1alpha/{name=users/*/accounts/*}": {
      "get": {
        "parameters": [
          {
            "name": "name",
            "in": "path",
            "required": true,
            "type": "string"
          }
        ],
    }
}

Interestingly, in the example above that doesn't' work, renaming parent= to user= generates properly!

    option (google.api.http) = {
      get: "/v1alpha/{user=users/*}/accounts"
    };

  "/v1alpha/{user}/accounts": {
      "get": {
        "parameters": [
          {
            "name": "user",
            "in": "path",
            "required": true,
            "type": "string"
          }
        ],
   }
}

Does the generator expect specific values in which it will trigger the correct generation in the OpenAPI spec for the path annotations? Switching from parent to user to fix one such case of issues sounds buggy to me. I'm not certain how the second example /v1alpha/{name=users/*/accounts/*} should map to query params in the OpenAPI spec, but the URL should probably look like /v1alpha/users/*/accounts/* - I think the gateway itself processes all this okay, its just the generated OpenAPI spec that's falling short.

Generated w/ v2.3.0

[someone1]

Some light digging and it seems that name and parent are explicitly flagged as resource names and behave differently.

This is probably the right check, but maybe there should be a different outcome? Replacing /v1alpha/{name=users/*/accounts/*} with /v1alpha/{name} is wrong, and I don't think there's a version where you can get the path and named parameter to both exist. I think the OpenAPI spec will have to be generated where the path parameter is broken up for the sake of the path itself, otherwise you run the risk of duplicate paths (e.g. /v1/{name=projects/*/subscription/*} vs /v1/{name=projects/*/topics/*} would be indistinguishable).

This is my first time looking at the code so apologies if this sounds appalling, but what do you think about the following?

• If the resource name has 1 part, then this can just become the named part of the URL: /v1alpha/{parent=users/*}/accounts -> /v1alpha/users/{parent}/accounts
• If there are more than 2 parts, then each part is a named parameter: /v1alpha/{account.name=users/*/accounts/*} -> /v1alpha/users/{name_users}/accounts/{name_accounts}

We sacrifice the parameter's intended format in exchange for being able to generate an OpenAPI file.

[johanbrandhorst]

Hi @someone1, thanks for the detailed issue! This area of the code is somewhat unfamiliar to me as well, so I would be happy to discuss changing it so that it works as expected in the generator. We'll want to be wary that whatever changes we make actually correspond to how it's being parsed by the gateway, but I think you suggestion makes sense. Then the question becomes whether our existing parser is clever enough to be able to make that translation, or if we need to rework it.

Would you be interested in contributing a fix for this?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[tenevdev]

I'd like to contribute a fix if the draft I referenced is a few commits away from an acceptable solution.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.