feat: add expand_slashed_path_patterns flag (#4813)

* feat: add expand_slashed_path_patterns flag

* chore: improve comments

* chore: improve comments and code polishing

* fix: add expand_slashed_path_patterns into bazel

* refactor: extract logic to single function

* refactor: expandPathPatterns fn don't mutate input

* fix: templateToParts camel-cases also path-patterns

* fix: inferred path-params not camel-cased

* docs: document the new compiler option

* fix: failing test

* docs: reword documentation about the option

Author: czabaj

docs/docs/mapping/customizing_openapi_output.md

@@ -539,6 +539,51 @@ Note that path parameters in OpenAPI does not support values with `/`, as discus
 so tools as Swagger UI will URL encode any `/` provided as parameter value. A possible workaround for this is to write
 a custom post processor for your OAS file to replace any path parameter with `/` into multiple parameters.
 
+#### Expand path parameters containing sub-path segments
+
+Alternative to the above, you can enable the `expand_slashed_path_patterns` compiler option to expand path parameters containing sub-path segments into the URI.
+
+For example, consider:
+```protobuf
+rpc GetBook(GetBookRequest) returns (Book) {
+  option (google.api.http) = {
+    get: "/v1/{name=publishers/*/books/*}"
+  };
+}
+```
+
+Where the `GetBook` has a path parameter `name` with a pattern `publishers/*/books/*`. When you enable the `expand_slashed_path_patterns=true` option the path pattern is expanded into the URI and each wildcard in the pattern is transformed into new path parameter. The generated schema for previous protobuf is:
+
+```JSON
+{
+ "/v1/publishers/{publisher}/books/{book}": {
+      "get": {
+        "parameters": [
+          {
+            "name": "publisher",
+            "in": "path",
+            "required": true,
+            "type": "string",
+          },
+          {
+            "name": "book",
+            "in": "path",
+            "required": true,
+            "type": "string",
+          }
+        ]
+      }
+    }
+}
+```
+
+The URI is now pretty descriptive and there are two path parameters `publisher` and `book` instead of one `name`. The name of the new parameters is derived from the path segment before the wildcard in the pattern.
+
+Caveats:
+
+- the fact that the original `name` parameter is missing might complicate the usage of the API if you intend to pass in the `name` parameters from the resources,
+- when the `expand_slashed_path_patterns` compiler flag is enabled, the [`path_param_name`](#path-parameters) field annotation is ignored.
+
 ### Output format
 
 By default the output format is JSON, but it is possible to configure it using the `output_format` option. Allowed values are: `json`, `yaml`. The output format will also change the extension of the output files.
@@ -1043,4 +1088,6 @@ definitions:
         type: string
 ```
 
+
 {% endraw %}
+

internal/descriptor/registry.go

@@ -163,6 +163,18 @@ type Registry struct {
 
 	// enableRpcDeprecation whether to process grpc method's deprecated option
 	enableRpcDeprecation bool
+
+	// expandSlashedPathPatterns, if true, for a path parameter carrying a sub-path, described via parameter pattern (i.e.
+	// the pattern contains forward slashes), this will expand the _pattern_ into the URI and will _replace_ the parameter
+	// with new path parameters inferred from patterns wildcards.
+	//
+	// Example: a Google AIP style path "/v1/{name=projects/*/locations/*}/datasets/{dataset}" with a "name" parameter
+	// containing sub-path will generate "/v1/projects/{project}/locations/{location}/datasets/{dataset}" path in OpenAPI.
+	// Note that the original "name" parameter is replaced with "project" and "location" parameters.
+	//
+	// This leads to more compliant and readable OpenAPI suitable for documentation, but may complicate client
+	// implementation if you want to pass the original "name" parameter.
+	expandSlashedPathPatterns bool
 }
 
 type repeatedFieldSeparator struct {
@@ -888,3 +900,11 @@ func (r *Registry) SetEnableRpcDeprecation(enable bool) {
 func (r *Registry) GetEnableRpcDeprecation() bool {
 	return r.enableRpcDeprecation
 }
+
+func (r *Registry) SetExpandSlashedPathPatterns(expandSlashedPathPatterns bool) {
+	r.expandSlashedPathPatterns = expandSlashedPathPatterns
+}
+
+func (r *Registry) GetExpandSlashedPathPatterns() bool {
+	return r.expandSlashedPathPatterns
+}

protoc-gen-openapiv2/defs.bzl

@@ -75,7 +75,8 @@ def _run_proto_gen_openapi(
         visibility_restriction_selectors,
         use_allof_for_refs,
         disable_default_responses,
-        enable_rpc_deprecation):
+        enable_rpc_deprecation,
+        expand_slashed_path_patterns):
     args = actions.args()
 
     args.add("--plugin", "protoc-gen-openapiv2=%s" % protoc_gen_openapiv2.path)
@@ -152,6 +153,9 @@ def _run_proto_gen_openapi(
     if enable_rpc_deprecation:
         args.add("--openapiv2_opt", "enable_rpc_deprecation=true")
 
+    if expand_slashed_path_patterns:
+        args.add("--openapiv2_opt", "expand_slashed_path_patterns=true")
+
     args.add("--openapiv2_opt", "repeated_path_param_separator=%s" % repeated_path_param_separator)
 
     proto_file_infos = _direct_source_infos(proto_info)
@@ -260,6 +264,7 @@ def _proto_gen_openapi_impl(ctx):
                     use_allof_for_refs = ctx.attr.use_allof_for_refs,
                     disable_default_responses = ctx.attr.disable_default_responses,
                     enable_rpc_deprecation = ctx.attr.enable_rpc_deprecation,
+                    expand_slashed_path_patterns = ctx.attr.expand_slashed_path_patterns,
                 ),
             ),
         ),
@@ -420,6 +425,13 @@ protoc_gen_openapiv2 = rule(
             mandatory = False,
             doc = "whether to process grpc method's deprecated option.",
         ),
+        "expand_slashed_path_patterns": attr.bool(
+            default = False,
+            mandatory = False,
+            doc = "if set, expands path patterns containing slashes into URI." +
+                  " It also creates a new path parameter for each wildcard in " +
+                  " the path pattern.",
+        ),
         "_protoc": attr.label(
             default = "@com_google_protobuf//:protoc",
             executable = True,

protoc-gen-openapiv2/internal/genopenapi/template.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"reflect"
 	"regexp"
+	"slices"
 	"sort"
 	"strconv"
 	"strings"
@@ -984,8 +985,17 @@ func resolveFullyQualifiedNameToOpenAPINames(messages []string, namingStrategy s
 
 var canRegexp = regexp.MustCompile("{([a-zA-Z][a-zA-Z0-9_.]*)([^}]*)}")
 
-// templateToParts will split a URL template as defined by https://github.com/googleapis/googleapis/blob/master/google/api/http.proto
-// into a string slice with each part as an element of the slice for use by `partsToOpenAPIPath` and `partsToRegexpMap`.
+// templateToParts splits a URL template into path segments for use by `partsToOpenAPIPath` and `partsToRegexpMap`.
+//
+// Parameters:
+//   - path:	The URL template as defined by https://github.com/googleapis/googleapis/blob/master/google/api/http.proto
+//   - reg:	The descriptor registry used to read compiler flags
+//   - fields:	The fields of the request message, only used when `useJSONNamesForFields` is true
+//   - msgs:	The Messages of the service binding, only used when `useJSONNamesForFields` is true
+//
+// Returns:
+//
+//	The path segments of the URL template.
 func templateToParts(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message) []string {
 	// It seems like the right thing to do here is to just use
 	// strings.Split(path, "/") but that breaks badly when you hit a url like
@@ -995,31 +1005,26 @@ func templateToParts(path string, reg *descriptor.Registry, fields []*descriptor
 	var parts []string
 	depth := 0
 	buffer := ""
-	jsonBuffer := ""
 pathLoop:
 	for i, char := range path {
 		switch char {
 		case '{':
 			// Push on the stack
 			depth++
 			buffer += string(char)
-			jsonBuffer = ""
-			jsonBuffer += string(char)
 		case '}':
 			if depth == 0 {
 				panic("Encountered } without matching { before it.")
 			}
 			// Pop from the stack
 			depth--
-			buffer += string(char)
-			if reg.GetUseJSONNamesForFields() &&
-				len(jsonBuffer) > 1 {
-				jsonSnakeCaseName := jsonBuffer[1:]
-				jsonCamelCaseName := lowerCamelCase(jsonSnakeCaseName, fields, msgs)
-				prev := buffer[:len(buffer)-len(jsonSnakeCaseName)-2]
-				buffer = strings.Join([]string{prev, "{", jsonCamelCaseName, "}"}, "")
-				jsonBuffer = ""
+			if !reg.GetUseJSONNamesForFields() {
+				buffer += string(char)
+				continue
 			}
+			paramNameProto := strings.SplitN(buffer[1:], "=", 2)[0]
+			paramNameCamelCase := lowerCamelCase(paramNameProto, fields, msgs)
+			buffer = strings.Join([]string{"{", paramNameCamelCase, buffer[len(paramNameProto)+1:], "}"}, "")
 		case '/':
 			if depth == 0 {
 				parts = append(parts, buffer)
@@ -1029,7 +1034,6 @@ pathLoop:
 				continue
 			}
 			buffer += string(char)
-			jsonBuffer += string(char)
 		case ':':
 			if depth == 0 {
 				// As soon as we find a ":" outside a variable,
@@ -1039,10 +1043,8 @@ pathLoop:
 				break pathLoop
 			}
 			buffer += string(char)
-			jsonBuffer += string(char)
 		default:
 			buffer += string(char)
-			jsonBuffer += string(char)
 		}
 	}
 
@@ -1060,6 +1062,7 @@ pathLoop:
 func partsToOpenAPIPath(parts []string, overrides map[string]string) string {
 	for index, part := range parts {
 		part = canRegexp.ReplaceAllString(part, "{$1}")
+
 		if override, ok := overrides[part]; ok {
 			part = override
 		}
@@ -1152,6 +1155,81 @@ func renderServiceTags(services []*descriptor.Service, reg *descriptor.Registry)
 	return tags
 }
 
+// expandPathPatterns searches the URI parts for path parameters with pattern and when the pattern contains a sub-path,
+// it expands the pattern into the URI parts and adds the new path parameters to the pathParams slice.
+//
+// Parameters:
+//   - pathParts:	the URI parts parsed from the path template with `templateToParts` function
+//   - pathParams: the path parameters of the service binding
+//
+// Returns:
+//
+//	The modified pathParts and pathParams slice.
+func expandPathPatterns(pathParts []string, pathParams []descriptor.Parameter, reg *descriptor.Registry) ([]string, []descriptor.Parameter) {
+	expandedPathParts := []string{}
+	modifiedPathParams := pathParams
+	for _, pathPart := range pathParts {
+		if !strings.HasPrefix(pathPart, "{") || !strings.HasSuffix(pathPart, "}") {
+			expandedPathParts = append(expandedPathParts, pathPart)
+			continue
+		}
+		woBraces := pathPart[1 : len(pathPart)-1]
+		paramPattern := strings.SplitN(woBraces, "=", 2)
+		if len(paramPattern) != 2 {
+			expandedPathParts = append(expandedPathParts, pathPart)
+			continue
+		}
+		paramName := paramPattern[0]
+		pattern := paramPattern[1]
+		if pattern == "*" {
+			expandedPathParts = append(expandedPathParts, pathPart)
+			continue
+		}
+		pathParamIndex := slices.IndexFunc(modifiedPathParams, func(p descriptor.Parameter) bool {
+			return p.FieldPath.String() == paramName
+		})
+		if pathParamIndex == -1 {
+			panic(fmt.Sprintf("Path parameter %q not found in path parameters", paramName))
+		}
+		pathParam := modifiedPathParams[pathParamIndex]
+		patternParts := strings.Split(pattern, "/")
+		for _, patternPart := range patternParts {
+			if patternPart != "*" {
+				expandedPathParts = append(expandedPathParts, patternPart)
+				continue
+			}
+			lastPart := expandedPathParts[len(expandedPathParts)-1]
+			paramName := strings.TrimSuffix(lastPart, "s")
+			if reg.GetUseJSONNamesForFields() {
+				paramName = casing.JSONCamelCase(paramName)
+			}
+			expandedPathParts = append(expandedPathParts, "{"+paramName+"}")
+			newParam := descriptor.Parameter{
+				Target: &descriptor.Field{
+					FieldDescriptorProto: &descriptorpb.FieldDescriptorProto{
+						Name: proto.String(paramName),
+					},
+					Message:           pathParam.Target.Message,
+					FieldMessage:      pathParam.Target.FieldMessage,
+					ForcePrefixedName: pathParam.Target.ForcePrefixedName,
+				},
+				FieldPath: []descriptor.FieldPathComponent{{
+					Name:   paramName,
+					Target: nil,
+				}},
+				Method: nil,
+			}
+			modifiedPathParams = append(modifiedPathParams, newParam)
+			if pathParamIndex != -1 {
+				// the new parameter from the pattern replaces the old path parameter
+				modifiedPathParams = append(modifiedPathParams[:pathParamIndex], modifiedPathParams[pathParamIndex+1:]...)
+				pathParamIndex = -1
+			}
+		}
+	}
+	return expandedPathParts, modifiedPathParams
+}
+
 func renderServices(services []*descriptor.Service, paths *openapiPathsObject, reg *descriptor.Registry, requestResponseRefs, customRefs refMap, msgs []*descriptor.Message, defs openapiDefinitionsObject) error {
 	// Correctness of svcIdx and methIdx depends on 'services' containing the services in the same order as the 'file.Service' array.
 	svcBaseIdx := 0
@@ -1179,11 +1257,15 @@ func renderServices(services []*descriptor.Service, paths *openapiPathsObject, r
 				parameters := openapiParametersObject{}
 				// split the path template into its parts
 				parts := templateToParts(b.PathTmpl.Template, reg, meth.RequestType.Fields, msgs)
+				pathParams := b.PathParams
+				if reg.GetExpandSlashedPathPatterns() {
+					parts, pathParams = expandPathPatterns(parts, pathParams, reg)
+				}
 				// extract any constraints specified in the path placeholders into ECMA regular expressions
 				pathParamRegexpMap := partsToRegexpMap(parts)
 				// Keep track of path parameter overrides
 				var pathParamNames = make(map[string]string)
-				for _, parameter := range b.PathParams {
+				for _, parameter := range pathParams {
 
 					var paramType, paramFormat, desc, collectionFormat string
 					var defaultValue interface{}

protoc-gen-openapiv2/internal/genopenapi/template_test.go

@@ -4119,6 +4119,61 @@ func TestTemplateToOpenAPIPath(t *testing.T) {
 	}
 }
 
+func getParameters(names []string) []descriptor.Parameter {
+	params := make([]descriptor.Parameter, 0)
+	for _, name := range names {
+		params = append(params, descriptor.Parameter{
+			Target: &descriptor.Field{
+				FieldDescriptorProto: &descriptorpb.FieldDescriptorProto{
+					Name: proto.String(name),
+				},
+				Message:           &descriptor.Message{},
+				FieldMessage:      nil,
+				ForcePrefixedName: false,
+			},
+			FieldPath: []descriptor.FieldPathComponent{{
+				Name:   name,
+				Target: nil,
+			}},
+			Method: nil,
+		})
+	}
+	return params
+}
+
+func TestTemplateToOpenAPIPathExpandSlashed(t *testing.T) {
+	var tests = []struct {
+		input              string
+		expected           string
+		pathParams         []descriptor.Parameter
+		expectedPathParams []string
+		useJSONNames       bool
+	}{
+		{"/v1/{name=projects/*/documents/*}:exportResults", "/v1/projects/{project}/documents/{document}:exportResults", getParameters([]string{"name"}), []string{"project", "document"}, true},
+		{"/test/{name=*}", "/test/{name}", getParameters([]string{"name"}), []string{"name"}, true},
+		{"/test/{name=*}/", "/test/{name}/", getParameters([]string{"name"}), []string{"name"}, true},
+		{"/test/{name=test_cases/*}/", "/test/test_cases/{testCase}/", getParameters([]string{"name"}), []string{"testCase"}, true},
+		{"/test/{name=test_cases/*}/", "/test/test_cases/{test_case}/", getParameters([]string{"name"}), []string{"test_case"}, false},
+	}
+	reg := descriptor.NewRegistry()
+	reg.SetExpandSlashedPathPatterns(true)
+	for _, data := range tests {
+		reg.SetUseJSONNamesForFields(data.useJSONNames)
+		actualParts, actualParams := templateToExpandedPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), data.pathParams)
+		if data.expected != actualParts {
+			t.Errorf("Expected templateToOpenAPIPath(%v) = %v, actual: %v", data.input, data.expected, actualParts)
+		}
+		pathParamsNames := make([]string, 0)
+		for _, param := range actualParams {
+			pathParamsNames = append(pathParamsNames, param.FieldPath[0].Name)
+		}
+		if !reflect.DeepEqual(data.expectedPathParams, pathParamsNames) {
+			t.Errorf("Expected mutated path params in templateToOpenAPIPath(%v) = %v, actual: %v", data.input, data.expectedPathParams, data.pathParams)
+		}
+
+	}
+}
+
 func BenchmarkTemplateToOpenAPIPath(b *testing.B) {
 	const input = "/{user.name=prefix1/*/prefix2/*}:customMethod"
 
@@ -4232,6 +4287,11 @@ func templateToRegexpMap(path string, reg *descriptor.Registry, fields []*descri
 	return partsToRegexpMap(templateToParts(path, reg, fields, msgs))
 }
 
+func templateToExpandedPath(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message, pathParams []descriptor.Parameter) (string, []descriptor.Parameter) {
+	pathParts, pathParams := expandPathPatterns(templateToParts(path, reg, fields, msgs), pathParams, reg)
+	return partsToOpenAPIPath(pathParts, make(map[string]string)), pathParams
+}
+
 func TestFQMNToRegexpMap(t *testing.T) {
 	var tests = []struct {
 		input    string