Overriding path parameter names not helpful with additional_bindings

[yinzara]

🚀 Feature

This is a follow-up to PR #2562

After attempting to implement this change in our codebase, unfortunately this doesn't help my team very much.

We use the additional_mappings attribute of the google.api.http extension to provide alternative path parameter formats for a required parameter of the entity which means depending on the additional_mappings the name of the parameter should be different.

In the below example we have a Foo resource that has a parent property of which a bing/* or bash/* resource name is allowed for the parent.

// Creates a new Foo given a parent
    rpc CreateFoo(CreateFooRequest) returns (Foo) {
        option (google.api.http)              = {
          post: "/v1/{parent=bing/*}/Foo"
          body: "foo"
          additional_bindings: {
              post: "/v1/{parent=bash/*}/Foo"
              body: "foo"
          }
        };
    }

This generates two different OpenAPI path mappings, one for POST /v1/{parent}/Foo and one for POST /v1/{parent_1}/Foo. Unfortunately one of them has the path parameter regex restriction for "bash/" and the other "bing/" and there is nothing visible in the Swagger UI of a difference between them (you just have very confused users why they can use one and not the other).

There are two acceptable outputs IMHO:

Either two endpoints POST /v1/{bingParent}/Foo and POST /v1/{bashParent}/Foo. This is unfortunately impossible with the current design for the path name replacements as you annotate the field on the Message which is the same between both.

OR

One endpoint POST /v1/{parent}/Foo with the parent parameter being some kind of regular expression combination that allows either bing/* or bash/*. This would require some really smart processing to determine that everything about the URLs is identical except the restriction on the path parameter and combine the two into a single OpenAPI endpoint. This seems like the "more correct" solution and would greatly simplify our output. This would be a change to my prior PR that adds "_1" suffixes on the path parameters to instead merge together two identical endpoints and make the path parameter regex support both standards.

Thoughts?

[johanbrandhorst]

I'm not entirely sure I follow, but does option 2 imply parsing the path parameters and then constructing a regex that will match all of them?

CC @oyvindwe

[oyvindwe]

I see the problem - we got the same problem in one of our own methods. It is also confusing because the parameter documentation is shared. Merging the paths and the regexes would be a very nice solution.

Merging the regexes can probably simply be done by or-ing all the individual patterns, e.g. "pattern":"(bing/[^/]+)|(bash/[^/]+)" (need to check correct syntax with the spec).

Note that this only applies to additional bindings that share paths, e.g. regexes should not be merged for cases like this:

        post: "/v1/{parent=bing/*}/Foo"
          body: "foo"
          additional_bindings: {
              post: "/v1/{parent=bash/*}/Bar"
              body: "foo"
          }
        };

[oyvindwe]

@yinzara Btw, we use readme.com for API browsing, which does show pattern (if set) when you enter the input field.

However, as pointed out here https://github.com/grpc-ecosystem/grpc-gateway/issues/720#issuecomment-1123584733 / isn't allowed in path parameters, and Swagger UI will URL encode it. So even though I agree that your approach is desirable, it looks like OpenAPI must updated with support for RFC 6570 first, which currently is a "Post 3.0 Proposal" :(

I suggest closing this issue and revisit when (if) OpenAPI supports / in path parameters.

[yinzara]

I only see that as a "nice to have" as it only states they aren't allowed unescaped. Since there is a configuration for grpc-gateway that allows you to accept escaped path parameters, then this is still viable even without OpenAPI support.

[oyvindwe]

there is a configuration for grpc-gateway that allows you to accept escaped path parameters,

That’s interesting. We use ESP, which doesn’t provide that: https://cloud.google.com/endpoints/docs/grpc/specify-esp-v2-startup-options#transcoding

[yinzara]

there is a configuration for grpc-gateway that allows you to accept escaped path parameters,

That’s interesting. We use ESP, which doesn’t provide that: https://cloud.google.com/endpoints/docs/grpc/specify-esp-v2-startup-options#transcoding

:-) you could always switch.

[stale[bot]]

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.