diff --git a/docs/rules/0123/resource-pattern-plural.md b/docs/rules/0123/resource-pattern-plural.md new file mode 100644 index 00000000..7ddea19c --- /dev/null +++ b/docs/rules/0123/resource-pattern-plural.md @@ -0,0 +1,88 @@ +--- +rule: + aip: 123 + name: [core, '0123', resource-pattern-plural] + summary: Resource patterns must use the plural as the collection segment +permalink: /123/resource-pattern-plural +redirect_from: + - /0123/resource-pattern-plural +--- + +# Resource `pattern` use of `plural` + +This rule enforces that messages that have a `google.api.resource` annotation +use the `plural` form as the collection segment, as described in [AIP-123][]. + +## Details + +This rule scans messages with a `google.api.resource` annotation, and validates +the `plural` form of the resource type name is used as the collection segment +in every pattern. + +**Note:** Special consideration is given to type names of child collections that +stutter next to their parent collection, as described in +[AIP-122 Nested Collections][nested]. See AIP-122 for more details. + +**Important:** Do not accept the suggestion if it would produce a backwards +incompatible change. + +## Examples + +**Incorrect** code for this rule: + +```proto +// Incorrect. +message Book { + option (google.api.resource) = { + type: "library.googleapis.com/BookShelf" + // collection segment doesn't match the plural. + pattern: "publishers/{publisher}/shelves/{book_shelf}" + singular: "bookShelf" + plural: "bookShelves" + }; + + string name = 1; +} +``` + +**Correct** code for this rule: + +```proto +// Correct. +message Book { + option (google.api.resource) = { + type: "library.googleapis.com/BookShelf" + pattern: "publishers/{publisher}/bookShelves/{book_shelf}" + singular: "bookShelf" + plural: "bookShelves" + }; + + string name = 1; +} +``` + +## Disabling + +If you need to violate this rule, use a leading comment above the message. + +```proto +// (-- api-linter: core::0123::resource-pattern-plural=disabled +// aip.dev/not-precedent: We need to do this because reasons. --) +message Book { + option (google.api.resource) = { + type: "library.googleapis.com/BookShelf" + pattern: "publishers/{publisher}/shelves/{book_shelf}" + singular: "bookShelf" + plural: "bookShelves" + }; + + string name = 1; +} +``` + +If you need to violate this rule for an entire file, place the comment at the +top of the file. + +[aip-123]: http://aip.dev/123 +[aip.dev/not-precedent]: https://aip.dev/not-precedent +[nested]: https://aip.dev/122#nested-collections diff --git a/rules/aip0123/aip0123.go b/rules/aip0123/aip0123.go index ce484756..b9d201bc 100644 --- a/rules/aip0123/aip0123.go +++ b/rules/aip0123/aip0123.go @@ -46,6 +46,7 @@ func AddRules(r lint.RuleRegistry) error { resourceDefinitionPatterns, resourceDefinitionTypeName, resourcePatternSingular, + resourcePatternPlural, nameNeverOptional, ) } @@ -78,6 +79,18 @@ func getVariables(pattern string) []string { return answer } +// isRootLevelResource determines if the given resource is a root-level +// resource using the isRootLevelResourcePattern helper. +func isRootLevelResource(resource *apb.ResourceDescriptor) bool { + if len(resource.GetPattern()) == 0 { + return false + } + + pattern := resource.GetPattern()[0] + + return isRootLevelResourcePattern(pattern) +} + // isRootLevelResourcePattern determines if the given pattern is that of a // root-level resource by checking how many segments it has - root-level // resource patterns have only two segments, thus one delimeter. @@ -120,6 +133,26 @@ func nestedSingular(resource *apb.ResourceDescriptor) string { return strings.TrimPrefix(singularSnake, parentIDVar+"_") } +// nestedPlural returns the would be reduced plural form of a nested +// resource. Use isNestedName to check eligibility before using nestedPlural. +// This will return empty if the resource is not eligible for nested name +// reduction. +func nestedPlural(resource *apb.ResourceDescriptor) string { + if !isNestedName(resource) { + return "" + } + + // use the singular variable to trim the singular prefix of the compound + // child colleciton name that is pluralized e.g. + // "users/{user}/userEvents/{user_event}" the parent singular "user" is the + // prefix of the child collection "userEvents". + parentIDVar := getParentIDVariable(resource.GetPattern()[0]) + parentIDVar = strcase.LowerCamelCase(parentIDVar) + + plural := utils.GetResourcePlural(resource) + return strcase.LowerCamelCase(strings.TrimPrefix(plural, parentIDVar)) +} + // isNestedName determines if the resource naming could be reduced as a // repetitive, nested collection as per AIP-122 Nested Collections. It does this // by analyzing the first resource pattern defined, comparing the resource diff --git a/rules/aip0123/resource_pattern_plural.go b/rules/aip0123/resource_pattern_plural.go new file mode 100644 index 00000000..7d9dfab4 --- /dev/null +++ b/rules/aip0123/resource_pattern_plural.go @@ -0,0 +1,74 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package aip0123 contains rules defined in https://aip.dev/123. +package aip0123 + +import ( + "fmt" + "strings" + + "github.com/googleapis/api-linter/lint" + "github.com/googleapis/api-linter/locations" + "github.com/googleapis/api-linter/rules/internal/utils" + "github.com/jhump/protoreflect/desc" +) + +var resourcePatternPlural = &lint.MessageRule{ + Name: lint.NewRuleName(123, "resource-pattern-plural"), + OnlyIf: func(m *desc.MessageDescriptor) bool { + return utils.IsResource(m) && len(utils.GetResource(m).GetPattern()) > 0 && !utils.IsSingletonResource(m) + }, + LintMessage: func(m *desc.MessageDescriptor) []lint.Problem { + var problems []lint.Problem + res := utils.GetResource(m) + nested := isNestedName(res) + var nn string + if nested { + nn = nestedPlural(res) + } + + patterns := res.GetPattern() + plural := fmt.Sprintf("/%s/", utils.GetResourcePlural(res)) + if isRootLevelResource(res) { + plural = strings.TrimPrefix(plural, "/") + } + nn = fmt.Sprintf("/%s/", nn) + + // If the first pattern is reduced or non-compliant, but is nested name eligible, we want to recommend the nested name. + nestedFirstPattern := nested && (strings.Contains(patterns[0], nn) || !strings.Contains(patterns[0], plural)) + + for ndx, pattern := range patterns { + if !strings.Contains(pattern, plural) || ndx > 0 && nestedFirstPattern && !strings.Contains(pattern, nn) { + // allow the reduced, nested name instead if present + if nested && strings.Contains(pattern, nn) { + continue + } + + want := plural + if nestedFirstPattern { + want = nn + } + + problems = append(problems, lint.Problem{ + Message: fmt.Sprintf("resource pattern %q collection segment must be the resource plural %q", pattern, want), + Descriptor: m, + Location: locations.MessageResource(m), + }) + } + } + + return problems + }, +} diff --git a/rules/aip0123/resource_pattern_plural_test.go b/rules/aip0123/resource_pattern_plural_test.go new file mode 100644 index 00000000..687d33b7 --- /dev/null +++ b/rules/aip0123/resource_pattern_plural_test.go @@ -0,0 +1,108 @@ +// Copyright 2019 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package aip0123 + +import ( + "testing" + + "github.com/googleapis/api-linter/rules/internal/testutils" +) + +func TestResourcePatternPluralSimple(t *testing.T) { + for _, test := range []struct { + name string + Pattern string + problems testutils.Problems + }{ + {"Valid", "publishers/{publisher}/bookShelves/{book_shelf}", testutils.Problems{}}, + {"ValidRootLevel", "bookShelves/{book_shelf}", testutils.Problems{}}, + {"Invalid", "publishers/{publisher}/bookShelfs/{book_shelf}", testutils.Problems{{Message: "collection segment must be the resource plural"}}}, + } { + t.Run(test.name, func(t *testing.T) { + f := testutils.ParseProto3Tmpl(t, ` + import "google/api/resource.proto"; + + message BookShelf { + option (google.api.resource) = { + type: "library.googleapis.com/BookShelf" + singular: "bookShelf" + plural: "bookShelves" + pattern: "{{.Pattern}}" + }; + string name = 1; + } + `, test) + m := f.GetMessageTypes()[0] + if diff := test.problems.SetDescriptor(m).Diff(resourcePatternPlural.Lint(f)); diff != "" { + t.Errorf(diff) + } + }) + } +} + +func TestResourcePatternPluralNested(t *testing.T) { + for _, test := range []struct { + name string + FirstPattern string + SecondPattern string + problems testutils.Problems + }{ + { + name: "Valid", + FirstPattern: "publishers/{publisher}/credits/{credit}", + SecondPattern: "authors/{author}/credits/{credit}", + problems: testutils.Problems{}, + }, + { + name: "ValidFull", + FirstPattern: "publishers/{publisher}/publisherCredits/{publisher_credit}", + SecondPattern: "authors/{author}/publisherCredits/{publisher_credit}", + problems: testutils.Problems{}, + }, + { + name: "InvalidSecondWithFirstNestedName", + FirstPattern: "publishers/{publisher}/credits/{credit}", + SecondPattern: "authors/{author}/publisherCredits/{credit}", + problems: testutils.Problems{{Message: `collection segment must be the resource plural "/credits/"`}}, + }, + { + name: "InvalidFirstWithReducedSecond", + FirstPattern: "publishers/{publisher}/pubCredits/{credit}", + SecondPattern: "authors/{author}/credits/{credit}", + problems: testutils.Problems{{Message: `collection segment must be the resource plural "/credits/"`}}, + }, + } { + t.Run(test.name, func(t *testing.T) { + f := testutils.ParseProto3Tmpl(t, ` + import "google/api/resource.proto"; + + message PublisherCredit { + option (google.api.resource) = { + type: "library.googleapis.com/PublisherCredit" + singular: "publisherCredit" + plural: "publisherCredits" + pattern: "{{.FirstPattern}}" + pattern: "{{.SecondPattern}}" + }; + string name = 1; + } + `, test) + m := f.GetMessageTypes()[0] + if diff := test.problems.SetDescriptor(m).Diff(resourcePatternPlural.Lint(f)); diff != "" { + t.Errorf(diff) + } + }) + } +} diff --git a/rules/aip0123/resource_pattern_singular_test.go b/rules/aip0123/resource_pattern_singular_test.go index 31cc6344..b025cc5e 100644 --- a/rules/aip0123/resource_pattern_singular_test.go +++ b/rules/aip0123/resource_pattern_singular_test.go @@ -28,6 +28,7 @@ func TestResourcePatternSingularSimple(t *testing.T) { }{ {"Valid", "publishers/{publisher}/bookShelves/{book_shelf}", testutils.Problems{}}, {"ValidSingleton", "publishers/{publisher}/bookShelf", testutils.Problems{}}, + {"ValidRootLevel", "bookShelves/{book_shelf}", testutils.Problems{}}, {"Invalid", "publishers/{publisher}/bookShelves/{shelf}", testutils.Problems{{Message: "final segment must include the resource singular"}}}, {"InvalidSingleton", "publishers/{publisher}/shelf", testutils.Problems{{Message: "final segment must include the resource singular"}}}, } {