Skip to content

Commit

Permalink
refactor: expandPathPatterns fn don't mutate input
Browse files Browse the repository at this point in the history
  • Loading branch information
czabaj committed Oct 18, 2024
1 parent c1f25b6 commit 526aee9
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 19 deletions.
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

0 comments on commit 526aee9

Please sign in to comment.