Skip to content

Conversation

@mbarrien
Copy link

Closes #28

Adds support for 2 related cases in URLs:

  • Support path segments like in https://google.aip.dev/127, where the
    URL contains a pattern like post: "/v1/{parent=publishers/*}/books"
  • Support nested field names in the URL, where the URL is structured
    like:
      option (google.api.http) = {
        patch: "/v3/{intent.name=projects/*/locations/*/agents/*/intents/*}"
        body: "intent"
      };
    
    This gets translated to /v3/${req["intent"]["name"]}

While here, use the newer protoc-gen-go-grpc plugin for generating
server test code, due to deprecated usage of plugin=grpc (see
https://github.com/protocolbuffers/protobuf-go/releases/tag/v1.20.0#v1.20-grpc-support)

@mbarrien mbarrien force-pushed the nested-fields-path-segments branch 8 times, most recently from a9d09c1 to 3b14d8a Compare March 30, 2022 04:16
Closes grpc-ecosystem#28

Adds support for 2 related cases in URLs:
* Support path segments like in https://google.aip.dev/127, where the
  URL contains a pattern like `post: "/v1/{parent=publishers/*}/books"`
* Support nested field names in the URL, where the URL is structured
  like:
  ```
    option (google.api.http) = {
      patch: "/v3/{intent.name=projects/*/locations/*/agents/*/intents/*}"
      body: "intent"
    };
  ```
  This gets translated to `/v3/${req["intent"]["name"]}`

While here, use the newer protoc-gen-go-grpc plugin for generating
server test code, due to deprecated usage of plugin=grpc (see
https://github.com/protocolbuffers/protobuf-go/releases/tag/v1.20.0#v1.20-grpc-support)
@mbarrien mbarrien force-pushed the nested-fields-path-segments branch from 3b14d8a to 07fb69d Compare March 30, 2022 04:20
@mbarrien mbarrien marked this pull request as ready for review March 30, 2022 04:23
Copy link

@mckelveygreg mckelveygreg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got this to generate a lovely client with your changes!
One suggestion is in order to make strict typescript happy, you'll need to do a null check on the nested fields because they are optional.

Copy link

@mckelveygreg mckelveygreg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now things compile! 🚀

}
fieldName := strings.Join(subFieldNames, ".")
part := fmt.Sprintf(`${req%s}`, strings.Join(partNames, ""))
part := fmt.Sprintf(`${req%s}`, strings.Join(partNames, "?."))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This worked great for me!

@hcwang512
Copy link

This fix works perfect for our use case, please merge it. @mbarrien

Copy link
Collaborator

@lyonlai lyonlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mbarrien Thanks for the contribution. Can you take a loot at the failed CI pipeline and fix it first? also I don't see the client proto generated I suspect that's one of the reason that failed the CI pipeline.

@yeluyang
Copy link

yeluyang commented Oct 8, 2022

we really need this patch, can someone do some magic and make ci happy ? 😭

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Google-recommended way of structuring resource path is not supported (https://google.aip.dev/127)

5 participants