Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add expand_slashed_path_patterns flag #4813

Merged
Prev Previous commit
Next Next commit
refactor: expandPathPatterns fn don't mutate input
  • Loading branch information
czabaj committed Oct 18, 2024
commit 526aee91584e1bf6b4066e8684313fb4e97073c1
2 changes: 1 addition & 1 deletion internal/descriptor/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down
24 changes: 12 additions & 12 deletions protoc-gen-openapiv2/internal/genopenapi/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 != "*" {
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
13 changes: 7 additions & 6 deletions protoc-gen-openapiv2/internal/genopenapi/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down