From 29e702949c26b6f3cb0c51eb7780fbe746d97015 Mon Sep 17 00:00:00 2001 From: Josh Giles Date: Tue, 28 May 2024 00:05:57 -0400 Subject: [PATCH] Fixes #3917: Add package openapi naming strategy. (#4372) * Fix 3917: Add package openapi naming strategy. Support passing "package" for openapi_naming_strategy to create in-package unique names by qualifying nested types. This strategy operates like "simple" except that we start by qualifying nested types, so that nested type names stay stable as new types are added in the same package. * Move regexp compilation to initialization of global var. * Tweak comment wording for consistency. * Add package option to protoc_gen_openapiv2 bazel rule. --- protoc-gen-openapiv2/defs.bzl | 4 +- .../internal/genopenapi/naming.go | 39 +++++++++++++++++-- .../internal/genopenapi/naming_test.go | 24 ++++++++---- protoc-gen-openapiv2/main.go | 2 +- 4 files changed, 55 insertions(+), 14 deletions(-) diff --git a/protoc-gen-openapiv2/defs.bzl b/protoc-gen-openapiv2/defs.bzl index 941a29c08c0..1ad31386af2 100644 --- a/protoc-gen-openapiv2/defs.bzl +++ b/protoc-gen-openapiv2/defs.bzl @@ -311,9 +311,9 @@ protoc_gen_openapiv2 = rule( "openapi_naming_strategy": attr.string( default = "", mandatory = False, - values = ["", "simple", "legacy", "fqn"], + values = ["", "simple", "package", "legacy", "fqn"], doc = "configures how OpenAPI names are determined." + - " Allowed values are `` (empty), `simple`, `legacy` and `fqn`." + + " Allowed values are `` (empty), `simple`, `package`, `legacy` and `fqn`." + " If unset, either `legacy` or `fqn` are selected, depending" + " on the value of the `fqn_for_openapi_name` setting", ), diff --git a/protoc-gen-openapiv2/internal/genopenapi/naming.go b/protoc-gen-openapiv2/internal/genopenapi/naming.go index 338ea2dcd0f..2a9ac069036 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/naming.go +++ b/protoc-gen-openapiv2/internal/genopenapi/naming.go @@ -2,6 +2,7 @@ package genopenapi import ( "reflect" + "regexp" "strings" ) @@ -17,6 +18,8 @@ func LookupNamingStrategy(strategyName string) func([]string) map[string]string return resolveNamesLegacy case "simple": return resolveNamesSimple + case "package": + return resolveNamesPackage } return nil } @@ -41,7 +44,7 @@ func resolveNamesFQN(messages []string) map[string]string { // E.g., if the fully qualified name is `.a.b.C.D`, and there are other messages with fully // qualified names ending in `.D` but not in `.C.D`, it assigns the unique name `bCD`. func resolveNamesLegacy(messages []string) map[string]string { - return resolveNamesUniqueWithContext(messages, 1, "") + return resolveNamesUniqueWithContext(messages, 1, "", false) } // resolveNamesSimple takes the names of all proto messages and generates unique references by using a simple @@ -52,20 +55,48 @@ func resolveNamesLegacy(messages []string) map[string]string { // E.g., if the fully qualified name is `.a.b.C.D`, and there are other messages with // fully qualified names ending in `.D` but not in `.C.D`, it assigns the unique name `C.D`. func resolveNamesSimple(messages []string) map[string]string { - return resolveNamesUniqueWithContext(messages, 0, ".") + return resolveNamesUniqueWithContext(messages, 0, ".", false) } +// resolveNamesPackage takes the names of all proto messages and generates unique references by +// starting with the package-scoped name (with nested message types qualified by their containing +// "parent" types), and then following the "simple" heuristic above to add package name components +// until each message has a unique name with a "." between each component. +// +// E.g., if the fully qualified name is `.a.b.C.D`, the name is `C.D` unless there is another +// package-scoped name ending in "C.D", in which case it would be `b.C.D` (unless that also +// conflicted, in which case the name would be the fully-qualified `a.b.C`). +func resolveNamesPackage(messages []string) map[string]string { + return resolveNamesUniqueWithContext(messages, 0, ".", true) +} + +// For the "package" naming strategy, we rely on the convention that package names are lowercase +// but message names are capitalized. +var pkgEndRegexp = regexp.MustCompile(`\.[A-Z]`) + // Take the names of every proto message and generates a unique reference by: // first, separating each message name into its components by splitting at dots. Then, // take the shortest suffix slice from each components slice that is unique among all // messages, and convert it into a component name by taking extraContext additional // components into consideration and joining all components with componentSeparator. -func resolveNamesUniqueWithContext(messages []string, extraContext int, componentSeparator string) map[string]string { +func resolveNamesUniqueWithContext(messages []string, extraContext int, componentSeparator string, qualifyNestedMessages bool) map[string]string { packagesByDepth := make(map[int][][]string) uniqueNames := make(map[string]string) hierarchy := func(pkg string) []string { - return strings.Split(pkg, ".") + if !qualifyNestedMessages { + return strings.Split(pkg, ".") + } + pkgEnd := pkgEndRegexp.FindStringIndex(pkg) + if pkgEnd == nil { + // Fall back to non-qualified behavior if search based on convention fails. + return strings.Split(pkg, ".") + } + // Return each package component as an element, followed by the full message name + // (potentially qualified, if nested) as a single element. + qualifiedPkgName := pkg[:pkgEnd[0]] + nestedTypeName := pkg[pkgEnd[0]+1:] + return append(strings.Split(qualifiedPkgName, "."), nestedTypeName) } for _, p := range messages { diff --git a/protoc-gen-openapiv2/internal/genopenapi/naming_test.go b/protoc-gen-openapiv2/internal/genopenapi/naming_test.go index c039510fc60..22fa5ae9b55 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/naming_test.go +++ b/protoc-gen-openapiv2/internal/genopenapi/naming_test.go @@ -4,15 +4,15 @@ import "testing" func TestNaming(t *testing.T) { type expectedNames struct { - fqn, legacy, simple string + fqn, legacy, simple, pkg string } messageNameToExpected := map[string]expectedNames{ - ".A": {"A", "A", "A"}, - ".a.B.C": {"a.B.C", "aBC", "B.C"}, - ".a.D.C": {"a.D.C", "aDC", "D.C"}, - ".a.E.F": {"a.E.F", "aEF", "a.E.F"}, - ".b.E.F": {"b.E.F", "bEF", "b.E.F"}, - ".c.G.H": {"c.G.H", "GH", "H"}, + ".A": {"A", "A", "A", "A"}, + ".a.B.C": {"a.B.C", "aBC", "B.C", "B.C"}, + ".a.D.C": {"a.D.C", "aDC", "D.C", "D.C"}, + ".a.E.F": {"a.E.F", "aEF", "a.E.F", "a.E.F"}, + ".b.E.F": {"b.E.F", "bEF", "b.E.F", "b.E.F"}, + ".c.G.H": {"c.G.H", "GH", "H", "G.H"}, } allMessageNames := make([]string, 0, len(messageNameToExpected)) @@ -50,4 +50,14 @@ func TestNaming(t *testing.T) { } } }) + t.Run("package", func(t *testing.T) { + uniqueNames := resolveNamesPackage(allMessageNames) + for _, msgName := range allMessageNames { + expected := messageNameToExpected[msgName].pkg + actual := uniqueNames[msgName] + if expected != actual { + t.Errorf("package unique name %q does not match expected name %q", actual, expected) + } + } + }) } diff --git a/protoc-gen-openapiv2/main.go b/protoc-gen-openapiv2/main.go index 958b77e8725..8552c7eb882 100644 --- a/protoc-gen-openapiv2/main.go +++ b/protoc-gen-openapiv2/main.go @@ -29,7 +29,7 @@ var ( _ = flag.Bool("allow_repeated_fields_in_body", true, "allows to use repeated field in `body` and `response_body` field of `google.api.http` annotation option. DEPRECATED: the value is ignored and always behaves as `true`.") includePackageInTags = flag.Bool("include_package_in_tags", false, "if unset, the gRPC service name is added to the `Tags` field of each operation. If set and the `package` directive is shown in the proto file, the package name will be prepended to the service name") useFQNForOpenAPIName = flag.Bool("fqn_for_openapi_name", false, "if set, the object's OpenAPI names will use the fully qualified names from the proto definition (ie my.package.MyMessage.MyInnerMessage). DEPRECATED: prefer `openapi_naming_strategy=fqn`") - openAPINamingStrategy = flag.String("openapi_naming_strategy", "", "use the given OpenAPI naming strategy. Allowed values are `legacy`, `fqn`, `simple`. If unset, either `legacy` or `fqn` are selected, depending on the value of the `fqn_for_openapi_name` flag") + openAPINamingStrategy = flag.String("openapi_naming_strategy", "", "use the given OpenAPI naming strategy. Allowed values are `legacy`, `fqn`, `simple`, `package`. If unset, either `legacy` or `fqn` are selected, depending on the value of the `fqn_for_openapi_name` flag") useGoTemplate = flag.Bool("use_go_templates", false, "if set, you can use Go templates in protofile comments") goTemplateArgs = utilities.StringArrayFlag(flag.CommandLine, "go_template_args", "provide a custom value that can override a key in the Go template. Requires the `use_go_templates` option to be set") ignoreComments = flag.Bool("ignore_comments", false, "if set, all protofile comments are excluded from output")