Skip to content

Commit

Permalink
fix(AIP-121): strict check of resource type (#1235)
Browse files Browse the repository at this point in the history
Previous code assumed that if the standard method checks responded positively that the resulting message retrieved would be a resource. This is not always the case when the API has some non-compliant RPCs that look like Standard Methods.
  • Loading branch information
noahdietz committed Aug 14, 2023
1 parent efaced9 commit 3764f3c
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 2 deletions.
4 changes: 2 additions & 2 deletions rules/aip0121/resource_must_support_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ var resourceMustSupportList = &lint.ServiceRule{
// resources which have a List method, and which ones do not.
for _, m := range s.GetMethods() {
if utils.IsListMethod(m) {
if msg := utils.GetListResourceMessage(m); msg != nil {
if msg := utils.GetListResourceMessage(m); msg != nil && utils.IsResource(msg) {
t := utils.GetResource(msg).GetType()
resourcesWithList.Add(t)
}
} else if utils.IsCreateMethod(m) || utils.IsUpdateMethod(m) || utils.IsGetMethod(m) {
if msg := utils.GetResponseType(m); msg != nil {
if msg := utils.GetResponseType(m); msg != nil && utils.IsResource(msg) {
// Skip tracking Singleton resources, they do not need List.
if utils.IsSingletonResource(msg) {
continue
Expand Down
7 changes: 7 additions & 0 deletions rules/aip0121/resource_must_support_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ func TestResourceMustSupportList(t *testing.T) {
{"ValidIgnoreSingleton", `
rpc GetBookCover(GetBookCoverRequest) returns (BookCover) {};
`, nil},
{"ValidIgnoreNonResource", `
rpc GetBookCover(GetBookCoverRequest) returns (Other) {};
`, nil},
} {
t.Run(test.name, func(t *testing.T) {
file := testutils.ParseProto3Tmpl(t, `
Expand Down Expand Up @@ -139,6 +142,10 @@ func TestResourceMustSupportList(t *testing.T) {
repeated Book books = 1;
string next_page_token = 2;
}
message Other {
string other = 1;
}
`, test)
s := file.GetServices()[0]
got := resourceMustSupportList.Lint(file)
Expand Down

0 comments on commit 3764f3c

Please sign in to comment.