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

feat(AIP-123): enforce singular as ID segment #1403

Merged
merged 10 commits into from
Jul 24, 2024
88 changes: 88 additions & 0 deletions docs/rules/0123/resource-pattern-singular.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
---
rule:
aip: 123
name: [core, '0123', resource-pattern-singular]
summary: Resource patterns must use the singular as the resource ID segment
permalink: /123/resource-pattern-singular
redirect_from:
- /0123/resource-pattern-singular
---

# Resource `pattern` use of `singular`

This rule enforces that messages that have a `google.api.resource` annotation
use the `singular` form as the resource ID segment, as described in [AIP-123][].

## Details

This rule scans messages with a `google.api.resource` annotation, and validates
the `singular` form of the resource type name is used as the resource ID 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"
// resource ID segment doesn't match the singular.
pattern: "publishers/{publisher}/bookShelves/{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-singular=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}/bookShelves/{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
4 changes: 2 additions & 2 deletions docs/rules/0123/resource-singular.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ redirect_from:
- /0123/resource-singular
---

# Resource type name
# Resource `singular`

This rule enforces that messages that have a `google.api.resource` annotation,
have a properly formatted `singular`, as described in [AIP-123][].
Expand Down Expand Up @@ -60,7 +60,7 @@ If you need to violate this rule, use a leading comment above the message.
// aip.dev/not-precedent: We need to do this because reasons. --)
message Book {
option (google.api.resource) = {
type: "library.googleapis.com/Genre/Mystery/Book"
type: "library.googleapis.com/Book"
pattern: "publishers/{publisher}/books/{book}"
singular: "shelf",
};
Expand Down
87 changes: 87 additions & 0 deletions rules/aip0123/aip0123.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/googleapis/api-linter/rules/internal/utils"
"github.com/jhump/protoreflect/desc"
"github.com/stoewer/go-strcase"
apb "google.golang.org/genproto/googleapis/api/annotations"
)

// AddRules accepts a register function and registers each of
Expand All @@ -44,6 +45,7 @@ func AddRules(r lint.RuleRegistry) error {
resourceDefinitionVariables,
resourceDefinitionPatterns,
resourceDefinitionTypeName,
resourcePatternSingular,
nameNeverOptional,
)
}
Expand Down Expand Up @@ -76,6 +78,91 @@ func getVariables(pattern string) []string {
return answer
}

// 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.
func isRootLevelResourcePattern(pattern string) bool {
return strings.Count(pattern, "/") <= 1
}

// getParentIDVariable is a helper that returns the parent resource ID segment
// for a given pattern. Returns empty string if the pattern has no variables or
// is a top-level resource.
func getParentIDVariable(pattern string) string {
variables := getVariables(pattern)
// no variables shouldn't happen but should be safe
if len(variables) == 0 || isRootLevelResourcePattern(pattern) {
return ""
}

// TODO: handle if singleton is a *parent*.
if utils.IsSingletonResourcePattern(pattern) {
// Last variable is the parent's for a singleton child.
return variables[len(variables)-1]
}

return variables[len(variables)-2]
}

// nestedSingular returns the would be reduced singular form of a nested
// resource. Use isNestedName to check eligibility before using nestedSingular.
// This will return empty if the resource is not eligible for nested name
// reduction.
func nestedSingular(resource *apb.ResourceDescriptor) string {
if !isNestedName(resource) {
return ""
}
parentIDVar := getParentIDVariable(resource.GetPattern()[0])

singular := utils.GetResourceSingular(resource)
singularSnake := strcase.SnakeCase(singular)

return strings.TrimPrefix(singularSnake, 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
// singular to the resource ID segment of the parent portion of the pattern. If
// that is a prefix of resource singular (in snake_case form as well), then the
// resource name could be reduced. For example, given `singular: "userEvent"`
// and `pattern: "users/{user}/userEvents/{user_event}"`, isNestedName would
// return `true`, because the `pattern` could be reduced to
// `"users/{user}/events/{event}"`.
func isNestedName(resource *apb.ResourceDescriptor) bool {
if len(resource.GetPattern()) == 0 {
return false
}
// only evaluate the first pattern b.c patterns cannot be reordered
// and nested names must be used consistently, thus from the beginning
pattern := resource.GetPattern()[0]

// Can't be a nested collection if it is a top level resource.
if isRootLevelResourcePattern(pattern) {
return false
}

singular := utils.GetResourceSingular(resource)

// If the resource type's singular is not camelCase then it is not a
// multi-word type and we do not need to check for nestedness in naming.
singularSnake := strcase.SnakeCase(singular)
if strings.Count(singularSnake, "_") < 1 {
return false
}

// Second to last resource ID variable will be the parent's to compare the
// child resource's singular form against. This prevents us from needing to
// deal with pluralizations due to the child resource typically being
// pluralized on its own noun, not the parent noun.
parentIDVar := getParentIDVariable(pattern)

// For example:
// publisher_credit starts with publisher --> nested
// book_shelf does not start with library --> not nested re:naming
return strings.HasPrefix(singularSnake, parentIDVar)
}

// getPlainPattern returns the pattern with all variables replaced with "*".
//
// For example, a pattern of "publishers/{publisher}/books/{book}" would
Expand Down
Loading
Loading