From a1f0871b93adf3c1e90660a47b5430de50a74b6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Gr=C3=B6hling?= Date: Tue, 8 Oct 2024 18:03:30 +0200 Subject: [PATCH 1/6] feat: add expand_slashed_path_patterns flag --- internal/descriptor/registry.go | 25 ++++++ .../internal/genopenapi/template.go | 83 ++++++++++++++++--- .../internal/genopenapi/template_test.go | 81 +++++++++++++++--- protoc-gen-openapiv2/main.go | 2 + 4 files changed, 167 insertions(+), 24 deletions(-) diff --git a/internal/descriptor/registry.go b/internal/descriptor/registry.go index 684fc5b60fc..c8b04b85399 100644 --- a/internal/descriptor/registry.go +++ b/internal/descriptor/registry.go @@ -163,6 +163,17 @@ type Registry struct { // enableRpcDeprecation whether to process grpc method's deprecated option enableRpcDeprecation bool + + // expandSlashedPathPatterns, if true, for a path parameter carying a sub-path, which is indicated via path parameter + // pattern (i.e. the parameter pattern contains forward slashes), this will expand the _pattern_ into the URI and will + // replace the parameter with wildcards from the _pattern_. + // + // Example: let's have a Google AIP style path "/v1/{name=projects/*/locations/*}/datasets/{dataset_id}" with a "name" + // parameter that contains sub-path. With "expandSlashedPathPatterns" turned on, the generated URI will be + // "/v1/projects/{project}/locations/{location}/datasets/{dataset_id}". Note that the original "name" parameter is + // replaced with "project" and "location" parameters. This leads to more compliant and readable OpenAPI suitable for + // documentation purposes, but may complicate client implementation if you want to pass the original "name" parameter. + expandSlashedPathPatterns bool } type repeatedFieldSeparator struct { @@ -435,6 +446,10 @@ func (r *Registry) SetStandalone(standalone bool) { r.standalone = standalone } +func (r *Registry) GetStandalone() bool { + return r.standalone +} + // SetRecursiveDepth records the max recursion count func (r *Registry) SetRecursiveDepth(count int) { r.recursiveDepth = count @@ -888,3 +903,13 @@ func (r *Registry) SetEnableRpcDeprecation(enable bool) { func (r *Registry) GetEnableRpcDeprecation() bool { return r.enableRpcDeprecation } + +// SetProto3OptionalNullable set proto3OtionalNullable +func (r *Registry) SetExpandSlashedPathPatterns(expandSlashedPathPatterns bool) { + r.expandSlashedPathPatterns = expandSlashedPathPatterns +} + +// SetProto3OptionalNullable set proto3OtionalNullable +func (r *Registry) GetExpandSlashedPathPatterns() bool { + return r.expandSlashedPathPatterns +} diff --git a/protoc-gen-openapiv2/internal/genopenapi/template.go b/protoc-gen-openapiv2/internal/genopenapi/template.go index bbb2480e772..9c3a09f2160 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/template.go +++ b/protoc-gen-openapiv2/internal/genopenapi/template.go @@ -9,6 +9,7 @@ import ( "os" "reflect" "regexp" + "slices" "sort" "strconv" "strings" @@ -979,7 +980,7 @@ 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`. -func templateToParts(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message) []string { +func templateToParts(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message, pathParams *[]descriptor.Parameter) []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 // /{my_field=prefix/*}/ and end up with 2 sections representing my_field. @@ -1004,14 +1005,72 @@ pathLoop: } // 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 = "" + paramName := "" + pattern := "" + if reg.GetExpandSlashedPathPatterns() { + paramPattern := strings.SplitN(buffer[1:], "=", 2) + if len(paramPattern) == 2 { + paramName = paramPattern[0] + pattern = paramPattern[1] + } + } + if pattern == "" || pattern == "*" { + 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 = "" + } + continue + } + + pathParamIndex := slices.IndexFunc(*pathParams, 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 := (*pathParams)[pathParamIndex] + patternParts := strings.Split(pattern, "/") + for i, part := range patternParts { + modifiedPart := part + if part == "*" { + paramName := strings.TrimSuffix(parts[len(parts)-1], "s") + if reg.GetUseJSONNamesForFields() { + paramName = casing.JSONCamelCase(paramName) + } + modifiedPart = "{" + 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, + } + *pathParams = append((*pathParams), newParam) + if pathParamIndex != -1 { + // the new parameter from the pattern replaces the old path parameter + *pathParams = append((*pathParams)[:pathParamIndex], (*pathParams)[pathParamIndex+1:]...) + pathParamIndex = -1 + } + } + isLastPart := i == len(patternParts)-1 + if isLastPart { + buffer = modifiedPart + } else { + parts = append(parts, modifiedPart) + } } case '/': if depth == 0 { @@ -1053,6 +1112,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 } @@ -1170,13 +1230,14 @@ func renderServices(services []*descriptor.Service, paths *openapiPathsObject, r operationFunc := operationForMethod(b.HTTPMethod) // Iterate over all the OpenAPI parameters parameters := openapiParametersObject{} + pathParams := b.PathParams // split the path template into its parts - parts := templateToParts(b.PathTmpl.Template, reg, meth.RequestType.Fields, msgs) + parts := templateToParts(b.PathTmpl.Template, reg, meth.RequestType.Fields, msgs, &pathParams) // 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{} diff --git a/protoc-gen-openapiv2/internal/genopenapi/template_test.go b/protoc-gen-openapiv2/internal/genopenapi/template_test.go index 35c6a512db6..317ea6771cb 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/template_test.go +++ b/protoc-gen-openapiv2/internal/genopenapi/template_test.go @@ -4045,7 +4045,7 @@ func TestTemplateWithJsonCamelCase(t *testing.T) { reg := descriptor.NewRegistry() reg.SetUseJSONNamesForFields(true) for _, data := range tests { - actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string)) + actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string), nil) if data.expected != actual { t.Errorf("Expected templateToOpenAPIPath(%v) = %v, actual: %v", data.input, data.expected, actual) } @@ -4073,7 +4073,7 @@ func TestTemplateWithoutJsonCamelCase(t *testing.T) { reg := descriptor.NewRegistry() reg.SetUseJSONNamesForFields(false) for _, data := range tests { - actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string)) + actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string), nil) if data.expected != actual { t.Errorf("Expected templateToOpenAPIPath(%v) = %v, actual: %v", data.input, data.expected, actual) } @@ -4105,20 +4105,75 @@ func TestTemplateToOpenAPIPath(t *testing.T) { reg := descriptor.NewRegistry() reg.SetUseJSONNamesForFields(false) for _, data := range tests { - actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string)) + actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string), nil) if data.expected != actual { t.Errorf("Expected templateToOpenAPIPath(%v) = %v, actual: %v", data.input, data.expected, actual) } } reg.SetUseJSONNamesForFields(true) for _, data := range tests { - actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string)) + actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string), nil) if data.expected != actual { t.Errorf("Expected templateToOpenAPIPath(%v) = %v, actual: %v", data.input, data.expected, actual) } } } +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) + actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string), &data.pathParams) + if data.expected != actual { + t.Errorf("Expected templateToOpenAPIPath(%v) = %v, actual: %v", data.input, data.expected, actual) + } + pathParamsNames := make([]string, 0) + for _, param := range data.pathParams { + 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" @@ -4127,7 +4182,7 @@ func BenchmarkTemplateToOpenAPIPath(b *testing.B) { reg.SetUseJSONNamesForFields(false) for i := 0; i < b.N; i++ { - _ = templateToOpenAPIPath(input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string)) + _ = templateToOpenAPIPath(input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string), nil) } }) @@ -4136,7 +4191,7 @@ func BenchmarkTemplateToOpenAPIPath(b *testing.B) { reg.SetUseJSONNamesForFields(true) for i := 0; i < b.N; i++ { - _ = templateToOpenAPIPath(input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string)) + _ = templateToOpenAPIPath(input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string), nil) } }) } @@ -4224,12 +4279,12 @@ func TestResolveFullyQualifiedNameToOpenAPIName(t *testing.T) { } } -func templateToOpenAPIPath(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message, pathParamNames map[string]string) string { - return partsToOpenAPIPath(templateToParts(path, reg, fields, msgs), pathParamNames) +func templateToOpenAPIPath(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message, pathParamNames map[string]string, pathParams *[]descriptor.Parameter) string { + return partsToOpenAPIPath(templateToParts(path, reg, fields, msgs, pathParams), pathParamNames) } -func templateToRegexpMap(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message) map[string]string { - return partsToRegexpMap(templateToParts(path, reg, fields, msgs)) +func templateToRegexpMap(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message, pathParams *[]descriptor.Parameter) map[string]string { + return partsToRegexpMap(templateToParts(path, reg, fields, msgs, pathParams)) } func TestFQMNToRegexpMap(t *testing.T) { @@ -4249,7 +4304,7 @@ func TestFQMNToRegexpMap(t *testing.T) { } reg := descriptor.NewRegistry() for _, data := range tests { - actual := templateToRegexpMap(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName()) + actual := templateToRegexpMap(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), nil) if !reflect.DeepEqual(data.expected, actual) { t.Errorf("Expected partsToRegexpMap(%v) = %v, actual: %v", data.input, data.expected, actual) } @@ -4272,14 +4327,14 @@ func TestFQMNtoOpenAPIName(t *testing.T) { reg := descriptor.NewRegistry() reg.SetUseJSONNamesForFields(false) for _, data := range tests { - actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string)) + actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string), nil) if data.expected != actual { t.Errorf("Expected templateToOpenAPIPath(%v) = %v, actual: %v", data.input, data.expected, actual) } } reg.SetUseJSONNamesForFields(true) for _, data := range tests { - actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string)) + actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string), nil) if data.expected != actual { t.Errorf("Expected templateToOpenAPIPath(%v) = %v, actual: %v", data.input, data.expected, actual) } diff --git a/protoc-gen-openapiv2/main.go b/protoc-gen-openapiv2/main.go index 58d19f17600..2e0b8b30273 100644 --- a/protoc-gen-openapiv2/main.go +++ b/protoc-gen-openapiv2/main.go @@ -50,6 +50,7 @@ var ( allowPatchFeature = flag.Bool("allow_patch_feature", true, "whether to hide update_mask fields in PATCH requests from the generated swagger file.") preserveRPCOrder = flag.Bool("preserve_rpc_order", false, "if true, will ensure the order of paths emitted in openapi swagger files mirror the order of RPC methods found in proto files. If false, emitted paths will be ordered alphabetically.") enableRpcDeprecation = flag.Bool("enable_rpc_deprecation", false, "whether to process grpc method's deprecated option.") + expandSlashedPathPatterns = flag.Bool("expand_slashed_path_patterns", false, "if set, expands path parameters with URI sub-paths into the URI. For example, \"/v1/{name=projects/*}/resource\" becomes \"/v1/projects/{project}/resource\".") _ = flag.Bool("logtostderr", false, "Legacy glog compatibility. This flag is a no-op, you can safely remove it") ) @@ -173,6 +174,7 @@ func main() { reg.SetAllowPatchFeature(*allowPatchFeature) reg.SetPreserveRPCOrder(*preserveRPCOrder) reg.SetEnableRpcDeprecation(*enableRpcDeprecation) + reg.SetExpandSlashedPathPatterns(*expandSlashedPathPatterns) if err := reg.SetRepeatedPathParamSeparator(*repeatedPathParamSeparator); err != nil { emitError(err) From 74da0f5310de8483723b9eea0719d5fe55128dbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Gr=C3=B6hling?= Date: Wed, 9 Oct 2024 07:47:59 +0000 Subject: [PATCH 2/6] chore: improve comments --- internal/descriptor/registry.go | 6 ------ .../internal/genopenapi/template.go | 14 ++++++++++++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/internal/descriptor/registry.go b/internal/descriptor/registry.go index c8b04b85399..d00f767b2d2 100644 --- a/internal/descriptor/registry.go +++ b/internal/descriptor/registry.go @@ -446,10 +446,6 @@ func (r *Registry) SetStandalone(standalone bool) { r.standalone = standalone } -func (r *Registry) GetStandalone() bool { - return r.standalone -} - // SetRecursiveDepth records the max recursion count func (r *Registry) SetRecursiveDepth(count int) { r.recursiveDepth = count @@ -904,12 +900,10 @@ func (r *Registry) GetEnableRpcDeprecation() bool { return r.enableRpcDeprecation } -// SetProto3OptionalNullable set proto3OtionalNullable func (r *Registry) SetExpandSlashedPathPatterns(expandSlashedPathPatterns bool) { r.expandSlashedPathPatterns = expandSlashedPathPatterns } -// SetProto3OptionalNullable set proto3OtionalNullable func (r *Registry) GetExpandSlashedPathPatterns() bool { return r.expandSlashedPathPatterns } diff --git a/protoc-gen-openapiv2/internal/genopenapi/template.go b/protoc-gen-openapiv2/internal/genopenapi/template.go index 9c3a09f2160..fb4bcabe17e 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/template.go +++ b/protoc-gen-openapiv2/internal/genopenapi/template.go @@ -978,8 +978,18 @@ 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 +// - pathParams: The path parameters of the service binding, only used when `expandSlashedPathPatterns` is true +// +// Returns: +// +// The path segments of the URL template. When `expandSlashedPathPatterns` is true, also mutates the pathParams slice. func templateToParts(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message, pathParams *[]descriptor.Parameter) []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 From 692d9f948fb26fa22b7d6333b3f03b167fc80ca2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Gr=C3=B6hling?= Date: Wed, 9 Oct 2024 10:04:06 +0200 Subject: [PATCH 3/6] chore: improve comments and code polishing --- internal/descriptor/registry.go | 17 +++++++++-------- .../internal/genopenapi/template_test.go | 12 ++++++------ 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/internal/descriptor/registry.go b/internal/descriptor/registry.go index d00f767b2d2..3a9be276769 100644 --- a/internal/descriptor/registry.go +++ b/internal/descriptor/registry.go @@ -164,15 +164,16 @@ type Registry struct { // enableRpcDeprecation whether to process grpc method's deprecated option enableRpcDeprecation bool - // expandSlashedPathPatterns, if true, for a path parameter carying a sub-path, which is indicated via path parameter - // pattern (i.e. the parameter pattern contains forward slashes), this will expand the _pattern_ into the URI and will - // replace the parameter with wildcards from the _pattern_. + // expandSlashedPathPatterns, if true, for a path parameter carying 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: let's have a Google AIP style path "/v1/{name=projects/*/locations/*}/datasets/{dataset_id}" with a "name" - // parameter that contains sub-path. With "expandSlashedPathPatterns" turned on, the generated URI will be - // "/v1/projects/{project}/locations/{location}/datasets/{dataset_id}". Note that the original "name" parameter is - // replaced with "project" and "location" parameters. This leads to more compliant and readable OpenAPI suitable for - // documentation purposes, but may complicate client implementation if you want to pass the original "name" parameter. + // 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 } diff --git a/protoc-gen-openapiv2/internal/genopenapi/template_test.go b/protoc-gen-openapiv2/internal/genopenapi/template_test.go index 317ea6771cb..8a56ce8229b 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/template_test.go +++ b/protoc-gen-openapiv2/internal/genopenapi/template_test.go @@ -4119,7 +4119,7 @@ func TestTemplateToOpenAPIPath(t *testing.T) { } } -func GetParameters(names []string) []descriptor.Parameter { +func getParameters(names []string) []descriptor.Parameter { params := make([]descriptor.Parameter, 0) for _, name := range names { params = append(params, descriptor.Parameter{ @@ -4149,11 +4149,11 @@ func TestTemplateToOpenAPIPathExpandSlashed(t *testing.T) { 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}, + {"/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) From fc2b47b4cd235bb4f35896392c2f886c8cece206 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Gr=C3=B6hling?= Date: Mon, 14 Oct 2024 09:40:38 +0000 Subject: [PATCH 4/6] fix: add expand_slashed_path_patterns into bazel --- protoc-gen-openapiv2/defs.bzl | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/protoc-gen-openapiv2/defs.bzl b/protoc-gen-openapiv2/defs.bzl index 0c09e80f3f6..509fa9100f1 100644 --- a/protoc-gen-openapiv2/defs.bzl +++ b/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, From c1f25b636cdcee49b48e3c3a32589e2f13e9f45d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Gr=C3=B6hling?= Date: Mon, 14 Oct 2024 12:04:20 +0000 Subject: [PATCH 5/6] refactor: extract logic to single function --- .../internal/genopenapi/template.go | 156 ++++++++++-------- .../internal/genopenapi/template_test.go | 32 ++-- 2 files changed, 104 insertions(+), 84 deletions(-) diff --git a/protoc-gen-openapiv2/internal/genopenapi/template.go b/protoc-gen-openapiv2/internal/genopenapi/template.go index fb4bcabe17e..3d6f9234a18 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/template.go +++ b/protoc-gen-openapiv2/internal/genopenapi/template.go @@ -985,12 +985,11 @@ var canRegexp = regexp.MustCompile("{([a-zA-Z][a-zA-Z0-9_.]*)([^}]*)}") // - 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 -// - pathParams: The path parameters of the service binding, only used when `expandSlashedPathPatterns` is true // // Returns: // -// The path segments of the URL template. When `expandSlashedPathPatterns` is true, also mutates the pathParams slice. -func templateToParts(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message, pathParams *[]descriptor.Parameter) []string { +// 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 // /{my_field=prefix/*}/ and end up with 2 sections representing my_field. @@ -1015,72 +1014,14 @@ pathLoop: } // Pop from the stack depth-- - paramName := "" - pattern := "" - if reg.GetExpandSlashedPathPatterns() { - paramPattern := strings.SplitN(buffer[1:], "=", 2) - if len(paramPattern) == 2 { - paramName = paramPattern[0] - pattern = paramPattern[1] - } - } - if pattern == "" || pattern == "*" { - 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 = "" - } - continue - } - - pathParamIndex := slices.IndexFunc(*pathParams, 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 := (*pathParams)[pathParamIndex] - patternParts := strings.Split(pattern, "/") - for i, part := range patternParts { - modifiedPart := part - if part == "*" { - paramName := strings.TrimSuffix(parts[len(parts)-1], "s") - if reg.GetUseJSONNamesForFields() { - paramName = casing.JSONCamelCase(paramName) - } - modifiedPart = "{" + 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, - } - *pathParams = append((*pathParams), newParam) - if pathParamIndex != -1 { - // the new parameter from the pattern replaces the old path parameter - *pathParams = append((*pathParams)[:pathParamIndex], (*pathParams)[pathParamIndex+1:]...) - pathParamIndex = -1 - } - } - isLastPart := i == len(patternParts)-1 - if isLastPart { - buffer = modifiedPart - } else { - parts = append(parts, modifiedPart) - } + 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 = "" } case '/': if depth == 0 { @@ -1215,6 +1156,78 @@ func renderServiceTags(services []*descriptor.Service, reg *descriptor.Registry) return tags } +// expandPathPatterns search 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, this slice will be mutated when the path pattern contains +// a sub-path with wildcard. +// +// Returns: +// +// The modified pathParts. Also mutates the pathParams slice. +func expandPathPatterns(pathParts []string, pathParams *[]descriptor.Parameter) []string { + expandedPathParts := []string{} + 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(*pathParams, 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 := (*pathParams)[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") + 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, + } + *pathParams = append((*pathParams), newParam) + if pathParamIndex != -1 { + // the new parameter from the pattern replaces the old path parameter + *pathParams = append((*pathParams)[:pathParamIndex], (*pathParams)[pathParamIndex+1:]...) + pathParamIndex = -1 + } + } + } + return expandedPathParts +} + 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 @@ -1242,7 +1255,10 @@ func renderServices(services []*descriptor.Service, paths *openapiPathsObject, r parameters := openapiParametersObject{} pathParams := b.PathParams // split the path template into its parts - parts := templateToParts(b.PathTmpl.Template, reg, meth.RequestType.Fields, msgs, &pathParams) + parts := templateToParts(b.PathTmpl.Template, reg, meth.RequestType.Fields, msgs) + if reg.GetExpandSlashedPathPatterns() { + parts = expandPathPatterns(parts, &pathParams) + } // extract any constraints specified in the path placeholders into ECMA regular expressions pathParamRegexpMap := partsToRegexpMap(parts) // Keep track of path parameter overrides diff --git a/protoc-gen-openapiv2/internal/genopenapi/template_test.go b/protoc-gen-openapiv2/internal/genopenapi/template_test.go index 8a56ce8229b..1e706a90d1f 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/template_test.go +++ b/protoc-gen-openapiv2/internal/genopenapi/template_test.go @@ -4045,7 +4045,7 @@ func TestTemplateWithJsonCamelCase(t *testing.T) { reg := descriptor.NewRegistry() reg.SetUseJSONNamesForFields(true) for _, data := range tests { - actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string), nil) + actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string)) if data.expected != actual { t.Errorf("Expected templateToOpenAPIPath(%v) = %v, actual: %v", data.input, data.expected, actual) } @@ -4073,7 +4073,7 @@ func TestTemplateWithoutJsonCamelCase(t *testing.T) { reg := descriptor.NewRegistry() reg.SetUseJSONNamesForFields(false) for _, data := range tests { - actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string), nil) + actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string)) if data.expected != actual { t.Errorf("Expected templateToOpenAPIPath(%v) = %v, actual: %v", data.input, data.expected, actual) } @@ -4105,14 +4105,14 @@ func TestTemplateToOpenAPIPath(t *testing.T) { reg := descriptor.NewRegistry() reg.SetUseJSONNamesForFields(false) for _, data := range tests { - actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string), nil) + actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string)) if data.expected != actual { t.Errorf("Expected templateToOpenAPIPath(%v) = %v, actual: %v", data.input, data.expected, actual) } } reg.SetUseJSONNamesForFields(true) for _, data := range tests { - actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string), nil) + actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string)) if data.expected != actual { t.Errorf("Expected templateToOpenAPIPath(%v) = %v, actual: %v", data.input, data.expected, actual) } @@ -4159,7 +4159,7 @@ func TestTemplateToOpenAPIPathExpandSlashed(t *testing.T) { reg.SetExpandSlashedPathPatterns(true) for _, data := range tests { reg.SetUseJSONNamesForFields(data.useJSONNames) - actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string), &data.pathParams) + actual := templateToExpandedPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), &data.pathParams) if data.expected != actual { t.Errorf("Expected templateToOpenAPIPath(%v) = %v, actual: %v", data.input, data.expected, actual) } @@ -4182,7 +4182,7 @@ func BenchmarkTemplateToOpenAPIPath(b *testing.B) { reg.SetUseJSONNamesForFields(false) for i := 0; i < b.N; i++ { - _ = templateToOpenAPIPath(input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string), nil) + _ = templateToOpenAPIPath(input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string)) } }) @@ -4191,7 +4191,7 @@ func BenchmarkTemplateToOpenAPIPath(b *testing.B) { reg.SetUseJSONNamesForFields(true) for i := 0; i < b.N; i++ { - _ = templateToOpenAPIPath(input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string), nil) + _ = templateToOpenAPIPath(input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string)) } }) } @@ -4279,12 +4279,16 @@ func TestResolveFullyQualifiedNameToOpenAPIName(t *testing.T) { } } -func templateToOpenAPIPath(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message, pathParamNames map[string]string, pathParams *[]descriptor.Parameter) string { - return partsToOpenAPIPath(templateToParts(path, reg, fields, msgs, pathParams), pathParamNames) +func templateToOpenAPIPath(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message, pathParamNames map[string]string) string { + return partsToOpenAPIPath(templateToParts(path, reg, fields, msgs), pathParamNames) } -func templateToRegexpMap(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message, pathParams *[]descriptor.Parameter) map[string]string { - return partsToRegexpMap(templateToParts(path, reg, fields, msgs, pathParams)) +func templateToRegexpMap(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message) map[string]string { + return partsToRegexpMap(templateToParts(path, reg, fields, msgs)) +} + +func templateToExpandedPath(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message, pathParams *[]descriptor.Parameter) string { + return partsToOpenAPIPath(expandPathPatterns(templateToParts(path, reg, fields, msgs), pathParams), make(map[string]string)) } func TestFQMNToRegexpMap(t *testing.T) { @@ -4304,7 +4308,7 @@ func TestFQMNToRegexpMap(t *testing.T) { } reg := descriptor.NewRegistry() for _, data := range tests { - actual := templateToRegexpMap(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), nil) + actual := templateToRegexpMap(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName()) if !reflect.DeepEqual(data.expected, actual) { t.Errorf("Expected partsToRegexpMap(%v) = %v, actual: %v", data.input, data.expected, actual) } @@ -4327,14 +4331,14 @@ func TestFQMNtoOpenAPIName(t *testing.T) { reg := descriptor.NewRegistry() reg.SetUseJSONNamesForFields(false) for _, data := range tests { - actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string), nil) + actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string)) if data.expected != actual { t.Errorf("Expected templateToOpenAPIPath(%v) = %v, actual: %v", data.input, data.expected, actual) } } reg.SetUseJSONNamesForFields(true) for _, data := range tests { - actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string), nil) + actual := templateToOpenAPIPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), make(map[string]string)) if data.expected != actual { t.Errorf("Expected templateToOpenAPIPath(%v) = %v, actual: %v", data.input, data.expected, actual) } From 526aee91584e1bf6b4066e8684313fb4e97073c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Gr=C3=B6hling?= Date: Fri, 18 Oct 2024 11:26:07 +0000 Subject: [PATCH 6/6] refactor: expandPathPatterns fn don't mutate input --- internal/descriptor/registry.go | 2 +- .../internal/genopenapi/template.go | 24 +++++++++---------- .../internal/genopenapi/template_test.go | 13 +++++----- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/internal/descriptor/registry.go b/internal/descriptor/registry.go index 3a9be276769..9f22b269d36 100644 --- a/internal/descriptor/registry.go +++ b/internal/descriptor/registry.go @@ -164,7 +164,7 @@ type Registry struct { // enableRpcDeprecation whether to process grpc method's deprecated option enableRpcDeprecation bool - // expandSlashedPathPatterns, if true, for a path parameter carying a sub-path, described via parameter pattern (i.e. + // 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. // diff --git a/protoc-gen-openapiv2/internal/genopenapi/template.go b/protoc-gen-openapiv2/internal/genopenapi/template.go index 3d6f9234a18..aee7653c03a 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/template.go +++ b/protoc-gen-openapiv2/internal/genopenapi/template.go @@ -1156,19 +1156,19 @@ func renderServiceTags(services []*descriptor.Service, reg *descriptor.Registry) return tags } -// expandPathPatterns search the URI parts for path parameters with pattern and when the pattern contains a sub-path, +// 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, this slice will be mutated when the path pattern contains -// a sub-path with wildcard. +// - pathParams: the path parameters of the service binding // // Returns: // -// The modified pathParts. Also mutates the pathParams slice. -func expandPathPatterns(pathParts []string, pathParams *[]descriptor.Parameter) []string { +// The modified pathParts and pathParams slice. +func expandPathPatterns(pathParts []string, pathParams []descriptor.Parameter) ([]string, []descriptor.Parameter) { expandedPathParts := []string{} + modifiedPathParams := pathParams for _, pathPart := range pathParts { if !strings.HasPrefix(pathPart, "{") || !strings.HasSuffix(pathPart, "}") { expandedPathParts = append(expandedPathParts, pathPart) @@ -1186,13 +1186,13 @@ func expandPathPatterns(pathParts []string, pathParams *[]descriptor.Parameter) expandedPathParts = append(expandedPathParts, pathPart) continue } - pathParamIndex := slices.IndexFunc(*pathParams, func(p descriptor.Parameter) bool { + 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 := (*pathParams)[pathParamIndex] + pathParam := modifiedPathParams[pathParamIndex] patternParts := strings.Split(pattern, "/") for _, patternPart := range patternParts { if patternPart != "*" { @@ -1217,15 +1217,15 @@ func expandPathPatterns(pathParts []string, pathParams *[]descriptor.Parameter) }}, Method: nil, } - *pathParams = append((*pathParams), newParam) + modifiedPathParams = append(modifiedPathParams, newParam) if pathParamIndex != -1 { // the new parameter from the pattern replaces the old path parameter - *pathParams = append((*pathParams)[:pathParamIndex], (*pathParams)[pathParamIndex+1:]...) + modifiedPathParams = append(modifiedPathParams[:pathParamIndex], modifiedPathParams[pathParamIndex+1:]...) pathParamIndex = -1 } } } - return expandedPathParts + return expandedPathParts, modifiedPathParams } func renderServices(services []*descriptor.Service, paths *openapiPathsObject, reg *descriptor.Registry, requestResponseRefs, customRefs refMap, msgs []*descriptor.Message, defs openapiDefinitionsObject) error { @@ -1253,11 +1253,11 @@ func renderServices(services []*descriptor.Service, paths *openapiPathsObject, r operationFunc := operationForMethod(b.HTTPMethod) // Iterate over all the OpenAPI parameters parameters := openapiParametersObject{} - pathParams := b.PathParams // split the path template into its parts parts := templateToParts(b.PathTmpl.Template, reg, meth.RequestType.Fields, msgs) + pathParams := b.PathParams if reg.GetExpandSlashedPathPatterns() { - parts = expandPathPatterns(parts, &pathParams) + parts, pathParams = expandPathPatterns(parts, pathParams) } // extract any constraints specified in the path placeholders into ECMA regular expressions pathParamRegexpMap := partsToRegexpMap(parts) diff --git a/protoc-gen-openapiv2/internal/genopenapi/template_test.go b/protoc-gen-openapiv2/internal/genopenapi/template_test.go index 1e706a90d1f..7c1823f497f 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/template_test.go +++ b/protoc-gen-openapiv2/internal/genopenapi/template_test.go @@ -4159,12 +4159,12 @@ func TestTemplateToOpenAPIPathExpandSlashed(t *testing.T) { reg.SetExpandSlashedPathPatterns(true) for _, data := range tests { reg.SetUseJSONNamesForFields(data.useJSONNames) - actual := templateToExpandedPath(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName(), &data.pathParams) - if data.expected != actual { - t.Errorf("Expected templateToOpenAPIPath(%v) = %v, actual: %v", data.input, data.expected, actual) + 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 data.pathParams { + for _, param := range actualParams { pathParamsNames = append(pathParamsNames, param.FieldPath[0].Name) } if !reflect.DeepEqual(data.expectedPathParams, pathParamsNames) { @@ -4287,8 +4287,9 @@ 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 { - return partsToOpenAPIPath(expandPathPatterns(templateToParts(path, reg, fields, msgs), pathParams), make(map[string]string)) +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) + return partsToOpenAPIPath(pathParts, make(map[string]string)), pathParams } func TestFQMNToRegexpMap(t *testing.T) {