Fixes #407 and #1700 - google.api.http path parameter constraints and…

… multiple HTTP path/method endpoint support (#2461) * Support standard OpenAPI paths for all valid path params for gRPC endpoints in protoc-gen-openapiv2 (#407) * Fix issue with URL encoded path segments not being split when unescaping mode is legacy, default or all characters * Adding docs for path template syntax and new functionality (#1700) * Removed unused old code * Additional tests for regular expressions * Formatting code * Fix lint error * Removing duplicate test * Adding additional test for path escaping * Fix failing test because of path change * Fix failing tests in code coverage * Change determination of unescaping by supporting default mode change * Regenerated files from changes * Remove unused parameters in test * Remove code warnings in test * Implement changes from PR #2461 comments * Correcting incorrect documentation * Adding test case to show change in docs is warranted
grpc-ecosystem · Dec 15, 2021 · 58634e4 · 58634e4
1 parent 65c0585
commit 58634e4
Show file tree

Hide file tree

Showing 9 changed files with 779 additions and 61 deletions.
diff --git a/README.md b/README.md
@@ -429,6 +429,19 @@ Alternatively, see the section on remotely managed plugin versions below.
    Note that this plugin also supports generating OpenAPI definitions for unannotated methods;
    use the `generate_unbound_methods` option to enable this.
 
+   It is possible with the HTTP mapping for a gRPC service method to create duplicate mappings 
+   with the only difference being constraints on the path parameter.
+
+   `/v1/{name=projects/*}` and `/v1/{name=organizations/*}` both become `/v1/{name}`.  When 
+   this occurs the plugin will rename the path parameter with a "_1" (or "_2" etc) suffix
+   to differentiate the different operations. So in the above example, the 2nd path would become
+   `/v1/{name_1=organizations/*}`.  This can also cause OpenAPI clients to URL encode the "/" that is
+   part of the path parameter as that is what OpenAPI defines in the specification.  To allow gRPC gateway to  
+   accept the URL encoded slash and still route the request, use the UnescapingModeAllCharacters or  
+   UnescapingModeLegacy (which is the default currently though may change in future versions). See 
+   [Customizing Your Gateway](https://grpc-ecosystem.github.io/grpc-gateway/docs/mapping/customizing_your_gateway/) 
+   for more information.
+
 ## Usage with remote plugins
 
 As an alternative to all of the above, you can use `buf` with
@@ -564,13 +577,15 @@ But patches are welcome.
 - HTTP request host is added as `X-Forwarded-Host` gRPC request header.
 - HTTP `Authorization` header is added as `authorization` gRPC request header.
 - Remaining Permanent HTTP header keys (as specified by the IANA
-  [here](http://www.iana.org/assignments/message-headers/message-headers.xhtml)
+  [here](http://www.iana.org/assignments/message-headers/message-headers.xhtml))
   are prefixed with `grpcgateway-` and added with their values to gRPC request
   header.
 - HTTP headers that start with 'Grpc-Metadata-' are mapped to gRPC metadata
   (prefixed with `grpcgateway-`).
 - While configurable, the default {un,}marshaling uses
   [protojson](https://pkg.go.dev/google.golang.org/protobuf/encoding/protojson).
+- The path template used to map gRPC service methods to HTTP endpoints supports the [google.api.http](https://github.com/googleapis/googleapis/blob/master/google/api/http.proto)
+  path template syntax. For example, `/api/v1/{name=projects/*/topics/*}` or `/prefix/{path=organizations/**}`.
 
 ## Contribution
 

diff --git a/examples/internal/clients/abe/api/swagger.yaml b/examples/internal/clients/abe/api/swagger.yaml
@@ -158,6 +158,7 @@ paths:
         in: "query"
         required: false
         type: "string"
+        pattern: "[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}"
         x-exportParamName: "Uuid"
         x-optionalDataType: "String"
       - name: "floatValue"
@@ -470,6 +471,7 @@ paths:
         in: "query"
         required: false
         type: "string"
+        pattern: "[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}"
         x-exportParamName: "Uuid"
         x-optionalDataType: "String"
       - name: "floatValue"
@@ -773,6 +775,7 @@ paths:
         in: "query"
         required: false
         type: "string"
+        pattern: "[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}"
         x-exportParamName: "Uuid"
         x-optionalDataType: "String"
       - name: "floatValue"
@@ -1445,6 +1448,7 @@ paths:
         in: "path"
         required: true
         type: "string"
+        pattern: "strprefix/[^/]+"
         x-exportParamName: "StringValue"
       - name: "uint32Value"
         in: "path"
@@ -1544,6 +1548,7 @@ paths:
         in: "query"
         required: false
         type: "string"
+        pattern: "[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}"
         x-exportParamName: "Uuid"
         x-optionalDataType: "String"
       - name: "bytesValue"
@@ -2076,7 +2081,7 @@ paths:
           description: "An unexpected error response."
           schema:
             $ref: "#/definitions/rpcStatus"
-  /v1/{book.name=publishers/*/books/*}:
+  /v1/{book.name}:
     patch:
       tags:
       - "ABitOfEverythingService"
@@ -2088,6 +2093,7 @@ paths:
           \nExample: `publishers/1257894000000000000/books/my-book`"
         required: true
         type: "string"
+        pattern: "publishers/[^/]+/books/[^/]+"
         x-exportParamName: "BookName"
       - in: "body"
         name: "body"
@@ -2138,7 +2144,7 @@ paths:
           description: "An unexpected error response."
           schema:
             $ref: "#/definitions/rpcStatus"
-  /v1/{parent=publishers/*}/books:
+  /v1/{parent}/books:
     post:
       tags:
       - "ABitOfEverythingService"
@@ -2151,6 +2157,7 @@ paths:
           \nExample: `publishers/1257894000000000000`"
         required: true
         type: "string"
+        pattern: "publishers/[^/]+"
         x-exportParamName: "Parent"
       - in: "body"
         name: "body"

diff --git a/examples/internal/clients/abe/api_a_bit_of_everything_service.go b/examples/internal/clients/abe/api_a_bit_of_everything_service.go
@@ -1943,7 +1943,7 @@ func (a *ABitOfEverythingServiceApiService) ABitOfEverythingServiceCreateBook(ct
 	)
 
 	// create path and map variables
-	localVarPath := a.client.cfg.BasePath + "/v1/{parent=publishers/*}/books"
+	localVarPath := a.client.cfg.BasePath + "/v1/{parent}/books"
 	localVarPath = strings.Replace(localVarPath, "{"+"parent"+"}", fmt.Sprintf("%v", parent), -1)
 
 	localVarHeaderParams := make(map[string]string)
@@ -4080,7 +4080,7 @@ func (a *ABitOfEverythingServiceApiService) ABitOfEverythingServiceUpdateBook(ct
 	)
 
 	// create path and map variables
-	localVarPath := a.client.cfg.BasePath + "/v1/{book.name=publishers/*/books/*}"
+	localVarPath := a.client.cfg.BasePath + "/v1/{book.name}"
 	localVarPath = strings.Replace(localVarPath, "{"+"book.name"+"}", fmt.Sprintf("%v", bookName), -1)
 
 	localVarHeaderParams := make(map[string]string)

diff --git a/examples/internal/proto/examplepb/a_bit_of_everything.swagger.json b/examples/internal/proto/examplepb/a_bit_of_everything.swagger.json
@@ -238,7 +238,8 @@
             "name": "uuid",
             "in": "query",
             "required": false,
-            "type": "string"
+            "type": "string",
+            "pattern": "[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}"
           },
           {
             "name": "floatValue",
@@ -579,7 +580,8 @@
             "name": "uuid",
             "in": "query",
             "required": false,
-            "type": "string"
+            "type": "string",
+            "pattern": "[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}"
           },
           {
             "name": "floatValue",
@@ -913,7 +915,8 @@
             "name": "uuid",
             "in": "query",
             "required": false,
-            "type": "string"
+            "type": "string",
+            "pattern": "[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}"
           },
           {
             "name": "floatValue",
@@ -1652,7 +1655,8 @@
             "name": "stringValue",
             "in": "path",
             "required": true,
-            "type": "string"
+            "type": "string",
+            "pattern": "strprefix/[^/]+"
           },
           {
             "name": "uint32Value",
@@ -1766,7 +1770,8 @@
             "name": "uuid",
             "in": "query",
             "required": false,
-            "type": "string"
+            "type": "string",
+            "pattern": "[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}"
           },
           {
             "name": "bytesValue",
@@ -2615,7 +2620,7 @@
         ]
       }
     },
-    "/v1/{book.name=publishers/*/books/*}": {
+    "/v1/{book.name}": {
       "patch": {
         "operationId": "ABitOfEverythingService_UpdateBook",
         "responses": {
@@ -2661,7 +2666,8 @@
             "description": "The resource name of the book.\n\nFormat: `publishers/{publisher}/books/{book}`\n\nExample: `publishers/1257894000000000000/books/my-book`",
             "in": "path",
             "required": true,
-            "type": "string"
+            "type": "string",
+            "pattern": "publishers/[^/]+/books/[^/]+"
           },
           {
             "name": "body",
@@ -2692,7 +2698,7 @@
         ]
       }
     },
-    "/v1/{parent=publishers/*}/books": {
+    "/v1/{parent}/books": {
       "post": {
         "summary": "Create a book.",
         "operationId": "ABitOfEverythingService_CreateBook",
@@ -2739,7 +2745,8 @@
             "description": "The publisher in which to create the book.\n\nFormat: `publishers/{publisher}`\n\nExample: `publishers/1257894000000000000`",
             "in": "path",
             "required": true,
-            "type": "string"
+            "type": "string",
+            "pattern": "publishers/[^/]+"
           },
           {
             "name": "body",