Skip to content

Commit

Permalink
feat(AIP-133): add request-id-field for non-df API (#1155)
Browse files Browse the repository at this point in the history
Corresponds to the change to aip-dev/google.aip.dev#1019.

resource_id is now required regardless of declarative_friendly.
  • Loading branch information
toumorokoshi authored May 22, 2023
1 parent 9fd6d24 commit 2787792
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 27 deletions.
21 changes: 7 additions & 14 deletions docs/rules/0133/request-id-field.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@
rule:
aip: 133
name: [core, '0133', request-id-field]
summary: |
Declarative-friendly create methods should have a client-specified
ID field.
summary: create methods should have a client-specified ID field.
permalink: /133/request-id-field
redirect_from:
- /0133/request-id-field
Expand All @@ -17,17 +15,15 @@ client-specified ID field, as mandated in [AIP-133][].

## Details

This rule looks at any `Create` method connected to a resource with a
`google.api.resource` annotation that includes `style: DECLARATIVE_FRIENDLY`,
and complains if it lacks a client-specified ID (e.g. `string book_id`) field.
This rule looks at any `Create` method connected to a resource and complains if
it lacks a client-specified ID (e.g. `string book_id`) field.

## Examples

**Incorrect** code for this rule:

```proto
// Incorrect.
// Assuming that Book is styled declarative-friendly...
message CreateBookRequest {
string parent = 1 [(google.api.resource_reference) = {
child_type: "library.googleapis.com/Book"
Expand All @@ -43,15 +39,15 @@ message CreateBookRequest {

```proto
// Correct.
// Assuming that Book is styled declarative-friendly...
message CreateBookRequest {
string parent = 1 [(google.api.resource_reference) = {
child_type: "library.googleapis.com/Book"
}];
Book book = 2;
string book_id = 2;
Book book = 3;
string book_id = 3;
}
```

Expand All @@ -72,11 +68,8 @@ message CreateBookRequest {
}
```

**Note:** Violations of declarative-friendly rules should be rare, as tools are
likely to expect strong consistency.

If you need to violate this rule for an entire file, place the comment at the
top of the file.

[aip-133]: https://aip.dev/133
[aip.dev/not-precedent]: https://aip.dev/not-precedent
[aip.dev/not-precedent]: https://aip.dev/not-precedent
8 changes: 3 additions & 5 deletions rules/aip0133/request_id_field.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,13 @@ import (
)

var requestIDField = &lint.MessageRule{
Name: lint.NewRuleName(133, "request-id-field"),
OnlyIf: func(m *desc.MessageDescriptor) bool {
return isCreateRequestMessage(m) && utils.IsDeclarativeFriendlyMessage(m)
},
Name: lint.NewRuleName(133, "request-id-field"),
OnlyIf: isCreateRequestMessage,
LintMessage: func(m *desc.MessageDescriptor) []lint.Problem {
idField := strcase.SnakeCase(strings.TrimPrefix(strings.TrimSuffix(m.GetName(), "Request"), "Create")) + "_id"
if field := m.FindFieldByName(idField); field == nil || utils.GetTypeName(field) != "string" || field.IsRepeated() {
return []lint.Problem{{
Message: fmt.Sprintf("Declarative-friendly create methods should contain a singular `string %s` field.", idField),
Message: fmt.Sprintf("create methods should contain a singular `string %s` field.", idField),
Descriptor: m,
}}
}
Expand Down
13 changes: 5 additions & 8 deletions rules/aip0133/request_id_field_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,13 @@ func TestRequestIDField(t *testing.T) {
problems := testutils.Problems{{Message: "`string book_id`"}}
for _, test := range []struct {
name string
Style string
IDField string
problems testutils.Problems
}{
{"ValidNotDF", "", "", nil},
{"ValidClientSpecified", "style: DECLARATIVE_FRIENDLY", "string book_id = 3;", nil},
{"InvalidDF", "style: DECLARATIVE_FRIENDLY", "", problems},
{"InvalidType", "style: DECLARATIVE_FRIENDLY", "bytes book_id = 3;", problems},
{"InvalidRepeated", "style: DECLARATIVE_FRIENDLY", "repeated string book_id = 3;", problems},
{"Valid", "string book_id = 2;", nil},
{"InvalidMissing", "", problems},
{"InvalidType", "bytes book_id = 2;", problems},
{"InvalidRepeated", "repeated string book_id = 2;", problems},
} {
t.Run(test.name, func(t *testing.T) {
f := testutils.ParseProto3Tmpl(t, `
Expand All @@ -46,14 +44,13 @@ func TestRequestIDField(t *testing.T) {
option (google.api.resource) = {
type: "library.googleapis.com/Book"
pattern: "publishers/{publisher}/books/{book}"
{{.Style}}
};
}
message CreateBookRequest {
string parent = 1;
Book book = 2;
{{.IDField}}
Book book = 3;
}
`, test)
m := f.FindMessage("CreateBookRequest")
Expand Down

0 comments on commit 2787792

Please sign in to comment.