Repository: goadesign/goa
Pull request #3123 (2022-08-19 19:57:43)

Add `operationId` templating
This PR adds template support for the `operationId` generated from the Goa DSL.

## Usage

The `operationId` may be modified at three levels, the `API()`, `Service()`, or `Method()`. The closest template to the operation will be used. For example, a custom template in `API()` will be used globally except in cases where an operation's `Service()` or `Method()` defines a template.

To specify a template in any of the supported blocks, use `Meta`.

```go
// This is the default template
Meta("openapi:operationId", "{service}#{method}(#{routeIndex})")
```

## Variables

The following replacement variables are available:

### `{service}`

This variable is replaced with the name of the Service that the operation belongs to.

### `{method}`

This variable is replaced with the name of the Method documented by the operation.

### `(#{routeIndex})`

This variable looks a bit gnarly but can be broken down into two distinct parts:

The `{routeIndex}` component is replaced with the index of the route in cases where more than one path matches the same method.

The outer component, `(# )`, indicates that the `{routeIndex}` should only be replaced with the actual *index* if its greater than 0. The `#` character, which is the default, may be replaced with any content that you wish to render as a spacer between preceding tokens and the `{routeIndex}` replacement.

The following are all valid:

- `(#{routeIndex})` - The default behavior (eg. `#1`, `#2`, etc)
- `(.{routeIndex})` - Places a `.` before the index
- `(someWords{routeIndex})` - Places `someWords` before the index
- `({routeIndex})` - No separating content between preceding tokens and the index.
- `(spaces are also supported{routeIndex})` - Places `spaces are also supported` before the index.

**Important note!** If the `routeIndex` variable is not provided and multiple paths are defined on a single operation, the template will act as if `(#{routeIndex})` were appended to the end of the `operationId` template.

## Static Content

In addition the the above replacement variables, you may also specify static content which will be rendered literally if no replacement variables are found. For example, the following would be acceptable:

```go
// operationId will be: listPosts
Meta("openapi:operationId", "listPosts")
```


raphael: Awesome PR, thank you!

Patch:
diff --git a/http/codegen/openapi/v3/builder.go b/http/codegen/openapi/v3/builder.go
index 57d68d7d..b2aead15 100644
--- a/http/codegen/openapi/v3/builder.go
+++ b/http/codegen/openapi/v3/builder.go
@@ -2,8 +2,10 @@ package openapiv3
 
 import (
 	"fmt"
+	"regexp"
 	"sort"
 	"strconv"
+	"strings"
 
 	"goa.design/goa/v3/expr"
 	"goa.design/goa/v3/http/codegen/openapi"
@@ -12,6 +14,14 @@ import (
 // OpenAPIVersion is the OpenAPI specification version targeted by this package.
 const OpenAPIVersion = "3.0.3"
 
+var (
+	routeIndexReplacementRegExp = regexp.MustCompile(`\((.*){routeIndex}\)`)
+)
+
+const (
+	defaultOperationIDFormat = "{service}#{method}(#{routeIndex})"
+)
+
 // New returns the OpenAPI v3 specification for the given API.
 // It returns nil if the design does not define HTTP endpoints.
 func New(root *expr.RootExpr) *OpenAPI {
@@ -182,24 +192,6 @@ func buildOperation(key string, r *expr.RouteExpr, bodies *EndpointBodies, rand
 	m := e.MethodExpr
 	svc := e.Service
 
-	// operation ID
-	var opID string
-	{
-		opID = fmt.Sprintf("%s#%s", svc.Name(), e.Name())
-		// An endpoint can have multiple routes. If there are multiple routes for
-		// the endpoint suffix the operation ID with the route index.
-		index := 0
-		for i, rt := range r.Endpoint.Routes {
-			if rt == r {
-				index = i
-				break
-			}
-		}
-		if index > 0 {
-			opID = fmt.Sprintf("%s#%d", opID, index)
-		}
-	}
-
 	// OpenAPI summary
 	var summary string
 	setSummary := func(meta expr.MetaExpr) {
@@ -213,6 +205,7 @@ func buildOperation(key string, r *expr.RouteExpr, bodies *EndpointBodies, rand
 			}
 		}
 	}
+
 	{
 		summary = fmt.Sprintf("%s %s", e.Name(), svc.Name())
 		setSummary(expr.Root.API.Meta)
@@ -220,6 +213,24 @@ func buildOperation(key string, r *expr.RouteExpr, bodies *EndpointBodies, rand
 		setSummary(m.Meta)
 	}
 
+	// OpenAPI operationId
+	var operationIDFormat string
+	setOperationIDFormat := func(meta expr.MetaExpr) {
+		for n, mdata := range meta {
+			if (n == "openapi:operationId") && len(mdata) > 0 {
+				operationIDFormat = mdata[0]
+			}
+		}
+	}
+
+	{
+		operationIDFormat = defaultOperationIDFormat
+		setOperationIDFormat(expr.Root.API.Meta)
+		setOperationIDFormat(m.Service.Meta)
+		setOperationIDFormat(r.Endpoint.Meta)
+		setOperationIDFormat(m.Meta)
+	}
+
 	// request body
 	var requestBody *RequestBodyRef
 	if e.Body.Type != expr.Empty {
@@ -295,11 +306,21 @@ func buildOperation(key string, r *expr.RouteExpr, bodies *EndpointBodies, rand
 		}
 	}
 
+	// An endpoint can have multiple routes, so we need to be able to build a unique
+	// operationId for each route.
+	var routeIndex int
+	for i, rt := range e.Routes {
+		if rt == r {
+			routeIndex = i
+			break
+		}
+	}
+
 	return &Operation{
 		Tags:         tagNames,
 		Summary:      summary,
 		Description:  e.Description(),
-		OperationID:  opID,
+		OperationID:  parseOperationIDTemplate(operationIDFormat, svc.Name(), e.Name(), routeIndex),
 		Parameters:   params,
 		RequestBody:  requestBody,
 		Responses:    responses,
@@ -364,6 +385,23 @@ func buildFileServerOperation(key string, fs *expr.HTTPFileServerExpr, api *expr
 		}
 	}
 
+	// OpenAPI operationId
+	var operationIDFormat string
+	setOperationIDFormat := func(meta expr.MetaExpr) {
+		for n, mdata := range meta {
+			if n == "openapi:operationId" && len(mdata) > 0 {
+				operationIDFormat = mdata[0]
+			}
+		}
+	}
+
+	{
+		operationIDFormat = defaultOperationIDFormat
+		setOperationIDFormat(api.Meta)
+		setOperationIDFormat(svc.Meta)
+		setOperationIDFormat(fs.Meta)
+	}
+
 	// tag names
 	var tagNames []string
 	{
@@ -375,7 +413,7 @@ func buildFileServerOperation(key string, fs *expr.HTTPFileServerExpr, api *expr
 	}
 
 	return &Operation{
-		OperationID:  fmt.Sprintf("%s#%s", svc.Name(), key),
+		OperationID:  parseOperationIDTemplate(operationIDFormat, svc.Name(), key, 0),
 		Description:  fs.Description,
 		Summary:      summary,
 		Parameters:   params,
@@ -388,6 +426,33 @@ func buildFileServerOperation(key string, fs *expr.HTTPFileServerExpr, api *expr
 	}
 }
 
+func parseOperationIDTemplate(template, service, method string, routeIndex int) string {
+	// Early return if no replacement is needed for the template.
+	if !strings.Contains(template, "{") && routeIndex == 0 {
+		return template
+	}
+
+	// The template replacer
+	repl := strings.NewReplacer(
+		"{service}", service,
+		"{method}", method,
+	)
+
+	operationID := repl.Replace(template)
+
+	if routeIndex == 0 {
+		return routeIndexReplacementRegExp.ReplaceAllString(operationID, "")
+	}
+
+	// If the routeIndex is greater than 0, we need to add the routeIndex to the operationId.
+	if sep := routeIndexReplacementRegExp.FindStringSubmatch(template); sep != nil {
+		return routeIndexReplacementRegExp.ReplaceAllString(operationID, fmt.Sprintf("%s%d", sep[1], routeIndex))
+	}
+
+	// Fallback in the event that the operationId doesn't contain the routeIndex placeholder.
+	return fmt.Sprintf("%s#%d", operationID, routeIndex)
+}
+
 // buildServers builds the OpenAPI Server objects from the given server
 // expressions.
 func buildServers(servers []*expr.ServerExpr) []*Server {


Test patch:
diff --git a/http/codegen/openapi/v3/builder_test.go b/http/codegen/openapi/v3/builder_test.go
index 67184b4d..d9c7d1e1 100644
--- a/http/codegen/openapi/v3/builder_test.go
+++ b/http/codegen/openapi/v3/builder_test.go
@@ -231,6 +231,85 @@ func TestBuildOperation(t *testing.T) {
 	}
 }
 
+func TestBuildOperationID(t *testing.T) {
+	const svcName = "test service"
+
+	cases := []struct {
+		Name string
+		DSL  func()
+
+		ExpectedOperationIDs []string
+	}{
+		{
+			Name:                 "template_in_method",
+			DSL:                  dsls.OperationIDMethod(svcName, "template_in_method", "{method}"),
+			ExpectedOperationIDs: []string{"template_in_method"},
+		}, {
+			Name:                 "template_in_service",
+			DSL:                  dsls.OperationIDService(svcName, "template_in_service", "{service}"),
+			ExpectedOperationIDs: []string{"test service"},
+		}, {
+			Name:                 "template_in_api",
+			DSL:                  dsls.OperationIDAPI(svcName, "template_in_api", defaultOperationIDFormat),
+			ExpectedOperationIDs: []string{"test service#template_in_api"},
+		}, {
+			Name:                 "multiple_routes",
+			DSL:                  dsls.OperationIDMultipleRoutes(svcName, "multiple_routes", defaultOperationIDFormat),
+			ExpectedOperationIDs: []string{"test service#multiple_routes", "test service#multiple_routes#1"},
+		}, {
+			Name:                 "multiple_routes_custom_separator",
+			DSL:                  dsls.OperationIDMultipleRoutes(svcName, "multiple_routes_custom_separator", "{service}.{method}(.{routeIndex})"),
+			ExpectedOperationIDs: []string{"test service.multiple_routes_custom_separator", "test service.multiple_routes_custom_separator.1"},
+		}, {
+			Name:                 "multiple_routes_custom_separator_without_routeIndex",
+			DSL:                  dsls.OperationIDMultipleRoutes(svcName, "multiple_routes_custom_separator_without_routeIndex", "{service}.{method}"),
+			ExpectedOperationIDs: []string{"test service.multiple_routes_custom_separator_without_routeIndex", "test service.multiple_routes_custom_separator_without_routeIndex#1"},
+		}, {
+			Name:                 "multiple_routes_no_routeIndex_separator",
+			DSL:                  dsls.OperationIDMultipleRoutes(svcName, "multiple_routes_no_routeIndex_separator", "{service}.{method}({routeIndex})"),
+			ExpectedOperationIDs: []string{"test service.multiple_routes_no_routeIndex_separator", "test service.multiple_routes_no_routeIndex_separator1"},
+		}, {
+			Name:                 "multiple_routes_long_separator",
+			DSL:                  dsls.OperationIDMultipleRoutes(svcName, "multiple_routes_long_separator", "{service}.{method}(someWordsHere and maybe some spaces{routeIndex})"),
+			ExpectedOperationIDs: []string{"test service.multiple_routes_long_separator", "test service.multiple_routes_long_separatorsomeWordsHere and maybe some spaces1"},
+		}, {
+			Name:                 "custom_static_operation_id",
+			DSL:                  dsls.OperationIDMethod(svcName, "custom_static_operation_id", "listThings"),
+			ExpectedOperationIDs: []string{"listThings"},
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.Name, func(t *testing.T) {
+			api := codegen.RunDSL(t, c.DSL).API
+
+			if len(api.HTTP.Services) == 0 {
+				t.Error("no HTTP service created from DSL")
+			}
+
+			for _, s := range api.HTTP.Services {
+				if s.Name() == svcName {
+					for _, e := range s.HTTPEndpoints {
+						for i, r := range e.Routes {
+							op := buildOperation(c.Name, r, &EndpointBodies{}, expr.NewRandom(c.Name))
+
+							if len(c.ExpectedOperationIDs) == 0 {
+								t.Error("no expected operation IDs")
+								return
+							}
+
+							if op.OperationID != c.ExpectedOperationIDs[i] {
+								t.Errorf("got operation ID %q, expected %q", op.OperationID, c.ExpectedOperationIDs[i])
+								return
+							}
+						}
+					}
+				}
+			}
+		})
+	}
+}
+
 func matchesParameter(t *testing.T, p *ParameterRef, types map[string]*openapi.Schema, expected param) {
 	matchesParameterHeader(t, p, types, expected, "parameter")
 }
diff --git a/http/codegen/openapi/v3/testdata/dsls/operations.go b/http/codegen/openapi/v3/testdata/dsls/operations.go
index 6321a175..eb3336cd 100644
--- a/http/codegen/openapi/v3/testdata/dsls/operations.go
+++ b/http/codegen/openapi/v3/testdata/dsls/operations.go
@@ -158,3 +158,76 @@ var ResponseRecursiveArrayUserType = func(svc, met string) func() {
 		})
 	}
 }
+
+var OperationIDStatic = func(svc, met string) func() {
+	return func() {
+		var _ = Service(svc, func() {
+			Method(met, func() {
+				Meta("openapi:operationId", "staticOperationId")
+
+				HTTP(func() {
+					GET("/")
+				})
+			})
+		})
+	}
+}
+
+var OperationIDMethod = func(svc, met, tmpl string) func() {
+	return func() {
+		var _ = Service(svc, func() {
+			Method(met, func() {
+				Meta("openapi:operationId", tmpl)
+
+				HTTP(func() {
+					GET("/")
+				})
+			})
+		})
+	}
+}
+
+var OperationIDService = func(svc, met, tmpl string) func() {
+	return func() {
+		var _ = Service(svc, func() {
+			Meta("openapi:operationId", tmpl)
+
+			Method(met, func() {
+				HTTP(func() {
+					GET("/")
+				})
+			})
+		})
+	}
+}
+
+var OperationIDAPI = func(svc, met, tmpl string) func() {
+	return func() {
+		var _ = API("test api", func() {
+			Meta("openapi:operationId", tmpl)
+		})
+
+		var _ = Service(svc, func() {
+			Method(met, func() {
+				HTTP(func() {
+					GET("/")
+				})
+			})
+		})
+	}
+}
+
+var OperationIDMultipleRoutes = func(svc, met, tmpl string) func() {
+	return func() {
+		var _ = Service(svc, func() {
+			Method(met, func() {
+				Meta("openapi:operationId", tmpl)
+
+				HTTP(func() {
+					GET("/")
+					POST("/another")
+				})
+			})
+		})
+	}
+}