Skip to content

Commit

Permalink
fix(AIP-132): lint OnlyIf the response message is a resource (#973)
Browse files Browse the repository at this point in the history
  • Loading branch information
noahdietz authored May 17, 2022
1 parent 15657fd commit 8873ed7
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 14 deletions.
11 changes: 9 additions & 2 deletions rules/aip0132/resource_reference_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,23 @@ import (
"github.com/googleapis/api-linter/locations"
"github.com/googleapis/api-linter/rules/internal/utils"
"github.com/jhump/protoreflect/desc"
"google.golang.org/genproto/googleapis/api/annotations"
)

// List methods should reference the target resource via `child_type` or the
// parent directly via `type`.
var resourceReferenceType = &lint.MethodRule{
Name: lint.NewRuleName(132, "resource-reference-type"),
OnlyIf: func(m *desc.MethodDescriptor) bool {
repeated := utils.GetRepeatedMessageFields(m.GetOutputType())
p := m.GetInputType().FindFieldByName("parent")
return isListMethod(m) && p != nil && utils.GetResourceReference(p) != nil && len(repeated) > 0

repeated := utils.GetRepeatedMessageFields(m.GetOutputType())
var resource *annotations.ResourceDescriptor
if len(repeated) > 0 {
resource = utils.GetResource(repeated[0].GetMessageType())
}

return isListMethod(m) && p != nil && utils.GetResourceReference(p) != nil && resource != nil
},
LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
// The first repeated message field must be the paginated resource.
Expand Down
30 changes: 18 additions & 12 deletions rules/aip0132/resource_reference_type_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,26 @@ import (
)

func TestResourceReferenceType(t *testing.T) {
bookResource := `
option (google.api.resource) = {
type: "library.googleapis.com/Book"
pattern: "shelves/{shelf}/books/{book}"
};
`

// Set up testing permutations.
tests := []struct {
testName string
TypeName string
RefType string
problems testutils.Problems
testName string
TypeName string
RefType string
ResourceAnnotation string
problems testutils.Problems
}{
{"ValidChildType", "library.googleapis.com/Book", "child_type", nil},
{"ValidType", "library.googleapis.com/Shelf", "type", nil},
{"InvalidType", "library.googleapis.com/Book", "type", testutils.Problems{{Message: "not a `type`"}}},
{"InvalidChildType", "library.googleapis.com/Shelf", "child_type", testutils.Problems{{Message: "`child_type`"}}},
{"ValidChildType", "library.googleapis.com/Book", "child_type", bookResource, nil},
{"ValidType", "library.googleapis.com/Shelf", "type", bookResource, nil},
{"InvalidType", "library.googleapis.com/Book", "type", bookResource, testutils.Problems{{Message: "not a `type`"}}},
{"InvalidChildType", "library.googleapis.com/Shelf", "child_type", bookResource, testutils.Problems{{Message: "`child_type`"}}},
{"SkipNonResource", "library.googleapis.com/Book", "child_type", "", nil},
}

// Run each test.
Expand All @@ -50,10 +59,7 @@ func TestResourceReferenceType(t *testing.T) {
repeated Book books = 1;
}
message Book {
option (google.api.resource) = {
type: "library.googleapis.com/Book"
pattern: "shelves/{shelf}/books/{book}"
};
{{ .ResourceAnnotation }}
string name = 1;
}
`, test)
Expand Down

0 comments on commit 8873ed7

Please sign in to comment.