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

openapi: Improve operationId/request/response naming strategy #19319

Merged
merged 29 commits into from
Apr 4, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
857ec3f
add prefix & suffix display attributes
averche Feb 21, 2023
beed664
add DisplayAttrs to PathParameters
averche Feb 22, 2023
df763a5
add constructOperationID func
averche Feb 22, 2023
72c5c47
Fixes & comments
averche Feb 22, 2023
ce4ca4a
Add test and fix logic
averche Feb 23, 2023
fcdd2d3
fix existing test data
averche Feb 23, 2023
f3a4dbe
ommitempty
averche Feb 23, 2023
5c72769
changelog
averche Feb 23, 2023
d3e16aa
better suffix disambiguation
averche Feb 26, 2023
22a1e74
Update comment
averche Feb 26, 2023
7003717
hyphenate instead of TitleCase
averche Feb 27, 2023
4713d81
fmt
averche Feb 27, 2023
1b5afe3
User OperationVerb since Action conflicts
averche Feb 27, 2023
54ad97a
reorder vars
averche Feb 27, 2023
1644e43
Merge branch 'main' into ui/openapi-naming-strategy
averche Feb 28, 2023
26b144e
allow verb-only
averche Mar 1, 2023
49eda18
better comments
averche Mar 1, 2023
72c4acd
more comments, better example
averche Mar 3, 2023
d6c2a45
better name for helper
averche Mar 12, 2023
99c30f4
Merge branch 'main' into ui/openapi-naming-strategy
averche Mar 13, 2023
af41a73
allow empty multi-field suffixes
averche Mar 22, 2023
23f731d
Merge branch 'main' into ui/openapi-naming-strategy
averche Mar 23, 2023
243c898
add withoutOperationHints
averche Mar 23, 2023
d37caaa
nil check
averche Mar 23, 2023
cb5aa04
empty obj check
averche Mar 23, 2023
7aacc2c
write -> create-or-update
averche Mar 30, 2023
f05e5e1
Merge branch 'main' into ui/openapi-naming-strategy
averche Mar 30, 2023
ed3a56d
Revert "write -> create-or-update"
averche Mar 31, 2023
0748a1a
title case response/request names
averche Apr 4, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/19319.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
openapi: Improve operationId/request/response naming strategy
```
158 changes: 126 additions & 32 deletions sdk/framework/openapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import (
"github.com/hashicorp/vault/sdk/helper/wrapping"
"github.com/hashicorp/vault/sdk/logical"
"github.com/mitchellh/mapstructure"
"golang.org/x/text/cases"
"golang.org/x/text/language"
)

// OpenAPI specification (OAS): https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md
Expand Down Expand Up @@ -244,7 +242,7 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st
}
}

for _, path := range paths {
for pathIndex, path := range paths {
// Construct a top level PathItem which will be populated as the path is processed.
pi := OASPathItem{
Description: cleanString(p.HelpSynopsis),
Expand Down Expand Up @@ -331,9 +329,19 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st

op := NewOASOperation()

operationID := constructOperationID(
path,
pathIndex,
p.DisplayAttrs,
opType,
props.DisplayAttrs,
requestResponsePrefix,
)

op.Summary = props.Summary
op.Description = props.Description
op.Deprecated = props.Deprecated
op.OperationID = operationID

// Add any fields not present in the path as body parameters for POST.
if opType == logical.CreateOperation || opType == logical.UpdateOperation {
Expand Down Expand Up @@ -381,7 +389,7 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st

// Set the final request body. Only JSON request data is supported.
if len(s.Properties) > 0 || s.Example != nil {
requestName := constructRequestResponseName(path, requestResponsePrefix, "Request")
requestName := operationID + "-request"
doc.Components.Schemas[requestName] = s
op.RequestBody = &OASRequestBody{
Required: true,
Expand Down Expand Up @@ -488,7 +496,7 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st
}

if len(resp.Fields) != 0 {
responseName := constructRequestResponseName(path, requestResponsePrefix, "Response")
responseName := operationID + "-response"
doc.Components.Schemas[responseName] = responseSchema
content = OASContent{
"application/json": &OASMediaTypeObject{
Expand Down Expand Up @@ -520,33 +528,6 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st
return nil
}

// constructRequestResponseName joins the given path with prefix & suffix into
// a CamelCase request or response name.
//
// For example, path=/config/lease/{name}, prefix="secret", suffix="request"
// will result in "SecretConfigLeaseRequest"
func constructRequestResponseName(path, prefix, suffix string) string {
var b strings.Builder

title := cases.Title(language.English)

b.WriteString(title.String(prefix))

// split the path by / _ - separators
for _, token := range strings.FieldsFunc(path, func(r rune) bool {
return r == '/' || r == '_' || r == '-'
}) {
// exclude request fields
if !strings.ContainsAny(token, "{}") {
b.WriteString(title.String(token))
}
}

b.WriteString(suffix)

return b.String()
}

func specialPathMatch(path string, specialPaths []string) bool {
// Test for exact or prefix match of special paths.
for _, sp := range specialPaths {
Expand All @@ -558,6 +539,112 @@ func specialPathMatch(path string, specialPaths []string) bool {
return false
}

// constructOperationID joins the given inputs into a hyphen-separated
// lower-case operation id, which is also used as a prefix for request and
// response names.
//
// The OperationPrefix / -Verb / -Suffix found in display attributes will be
// used, if provided. Otherwise, the function falls back to using the path and
// the operation.
//
// Examples of generated operation identifiers:
// - kvv2-write
// - kvv2-read
// - google-cloud-login
// - google-cloud-write-role
func constructOperationID(
path string,
pathIndex int,
pathAttributes *DisplayAttributes,
operation logical.Operation,
operationAttributes *DisplayAttributes,
defaultPrefix string,
) string {
var (
prefix string
verb string
suffix string
)

if operationAttributes != nil {
prefix = operationAttributes.OperationPrefix
verb = operationAttributes.OperationVerb
suffix = operationAttributes.OperationSuffix
}

if pathAttributes != nil {
if prefix == "" {
prefix = pathAttributes.OperationPrefix
}
if verb == "" {
verb = pathAttributes.OperationVerb
}
if suffix == "" {
suffix = pathAttributes.OperationSuffix
}
}

// A single suffix string can contain multiple pipe-delimited strings. To
// determine the actual suffix, we attempt to match it by the index of the
// paths returned from `expandPattern(...)`. For example:
//
// aws/
// Pattern: `^(creds|sts)/(?P<name>\w(([\w-.@]+)?\w)?)$`
// DisplayAttrs: {
// OperationSuffix: "credentials|sts-credentials"
// }
//
// Will expand into two paths and corresponding suffixes:
//
// path 0: "creds/{name}" suffix: credentials
// path 1: "sts/{name}" suffix: sts-credentials
//
if suffixes := strings.Split(suffix, "|"); len(suffixes) > 1 || pathIndex > 0 {
averche marked this conversation as resolved.
Show resolved Hide resolved
// if the index is out of bounds, fall back to the old logic
if pathIndex >= len(suffixes) {
suffix = ""
} else {
suffix = suffixes[pathIndex]
}
}

// hyphenate is a helper that hyphenates the given slice except the empty elements
hyphenate := func(parts []string) string {
filtered := make([]string, 0, len(parts))
for _, e := range parts {
if e != "" {
filtered = append(filtered, e)
}
}
return strings.ToLower(strings.Join(filtered, "-"))
}

// fall back to using the path + operation to construct the operation id
var (
needPrefix = prefix == "" && verb == ""
needVerb = verb == ""
needSuffix = suffix == "" && (verb == "" || pathIndex > 0)
)

if needPrefix {
prefix = defaultPrefix
}

if needVerb {
if operation == logical.UpdateOperation {
verb = "write"
} else {
verb = string(operation)
}
}

if needSuffix {
suffix = hyphenate(nonWordRe.Split(strings.ToLower(path), -1))
}

return hyphenate([]string{prefix, verb, suffix})
}

// expandPattern expands a regex pattern by generating permutations of any optional parameters
// and changing named parameters into their {openapi} equivalents.
func expandPattern(pattern string) ([]string, error) {
Expand Down Expand Up @@ -883,6 +970,9 @@ func cleanResponse(resp *logical.Response) *cleanedResponse {
// postSysToolsRandomUrlbytes_2
//
// An optional user-provided suffix ("context") may also be appended.
//
// Deprecated: operationID's are now populated using `constructOperationID`.
// This function is here for backwards compatibility with older plugins.
func (d *OASDocument) CreateOperationIDs(context string) {
opIDCount := make(map[string]int)
var paths []string
Expand Down Expand Up @@ -910,6 +1000,10 @@ func (d *OASDocument) CreateOperationIDs(context string) {
continue
}

if oasOperation.OperationID != "" {
continue
}

// Discard "_mount_path" from any {thing_mount_path} parameters
path = strings.Replace(path, "_mount_path", "", 1)

Expand Down
Loading