diff --git a/rules/aip0136/response_message_name_test.go b/rules/aip0136/response_message_name_test.go index 78bcd7f7..d7f04bd5 100644 --- a/rules/aip0136/response_message_name_test.go +++ b/rules/aip0136/response_message_name_test.go @@ -31,6 +31,7 @@ func TestResponseMessageName(t *testing.T) { }{ {"Valid", "ArchiveBook", "ArchiveBookResponse", testutils.Problems{}}, {"Invalid", "ArchiveBook", "ArchiveBookResp", testutils.Problems{{Message: "not \"ArchiveBookResp\"."}}}, + {"SkipRevisionMethod", "CommitBook", "Book", testutils.Problems{}}, } for _, test := range tests { diff --git a/rules/aip0162/aip0162.go b/rules/aip0162/aip0162.go index 2f0884b1..6aea1ec5 100644 --- a/rules/aip0162/aip0162.go +++ b/rules/aip0162/aip0162.go @@ -68,64 +68,40 @@ func AddRules(r lint.RuleRegistry) error { } var ( - tagRevisionMethodRegexp = regexp.MustCompile(`^Tag([A-Za-z0-9]+)Revision$`) tagRevisionReqMessageRegexp = regexp.MustCompile(`^Tag(?:[A-Za-z0-9]+)RevisionRequest$`) tagRevisionURINameRegexp = regexp.MustCompile(`:tagRevision$`) ) -// Returns true if this is an AIP-162 Tag Revision method, false otherwise. -func isTagRevisionMethod(m *desc.MethodDescriptor) bool { - return tagRevisionMethodRegexp.MatchString(m.GetName()) -} - // Returns true if this is an AIP-162 Tag Revision request message, false otherwise. func isTagRevisionRequestMessage(m *desc.MessageDescriptor) bool { return tagRevisionReqMessageRegexp.MatchString(m.GetName()) } var ( - commitMethodRegexp = regexp.MustCompile(`^Commit([A-Za-z0-9]+)$`) commitReqMessageRegexp = regexp.MustCompile(`^Commit(?:[A-Za-z0-9]+)Request$`) commitURINameRegexp = regexp.MustCompile(`:commit$`) ) -// Returns true if this is an AIP-162 Commit method, false otherwise. -func isCommitMethod(m *desc.MethodDescriptor) bool { - return commitMethodRegexp.MatchString(m.GetName()) -} - // Returns true if this is an AIP-162 Commit request message, false otherwise. func isCommitRequestMessage(m *desc.MessageDescriptor) bool { return commitReqMessageRegexp.MatchString(m.GetName()) } var ( - rollbackMethodRegexp = regexp.MustCompile(`^Rollback([A-Za-z0-9]+)$`) rollbackReqMessageRegexp = regexp.MustCompile(`^Rollback(?:[A-Za-z0-9]+)Request$`) rollbackURINameRegexp = regexp.MustCompile(`:rollback$`) ) -// Returns true if this is an AIP-162 Rollback method, false otherwise. -func isRollbackMethod(m *desc.MethodDescriptor) bool { - return rollbackMethodRegexp.MatchString(m.GetName()) -} - // Returns true if this is an AIP-162 Rollback request message, false otherwise. func isRollbackRequestMessage(m *desc.MessageDescriptor) bool { return rollbackReqMessageRegexp.MatchString(m.GetName()) } var ( - deleteRevisionMethodRegexp = regexp.MustCompile(`^Delete(?:[A-Za-z0-9]+)Revision$`) deleteRevisionReqMessageRegexp = regexp.MustCompile(`^Delete(?:[A-Za-z0-9]+)RevisionRequest$`) deleteRevisionURINameRegexp = regexp.MustCompile(`:deleteRevision$`) ) -// Returns true if this is an AIP-162 Delete Revision method, false otherwise. -func isDeleteRevisionMethod(m *desc.MethodDescriptor) bool { - return deleteRevisionMethodRegexp.MatchString(m.GetName()) -} - // Returns true if this is an AIP-162 Delete Revision request message, false otherwise. func isDeleteRevisionRequestMessage(m *desc.MessageDescriptor) bool { return deleteRevisionReqMessageRegexp.MatchString(m.GetName()) diff --git a/rules/aip0162/commit_http_body.go b/rules/aip0162/commit_http_body.go index 0d16827d..4d28b2b5 100644 --- a/rules/aip0162/commit_http_body.go +++ b/rules/aip0162/commit_http_body.go @@ -22,6 +22,6 @@ import ( // Commit methods should have "*" as the HTTP body. var commitHTTPBody = &lint.MethodRule{ Name: lint.NewRuleName(162, "commit-http-body"), - OnlyIf: isCommitMethod, + OnlyIf: utils.IsCommitRevisionMethod, LintMethod: utils.LintWildcardHTTPBody, } diff --git a/rules/aip0162/commit_http_method.go b/rules/aip0162/commit_http_method.go index 0abb7b51..0de1b02e 100644 --- a/rules/aip0162/commit_http_method.go +++ b/rules/aip0162/commit_http_method.go @@ -22,6 +22,6 @@ import ( // Commit methods should use the HTTP POST method. var commitHTTPMethod = &lint.MethodRule{ Name: lint.NewRuleName(162, "commit-http-method"), - OnlyIf: isCommitMethod, + OnlyIf: utils.IsCommitRevisionMethod, LintMethod: utils.LintHTTPMethod("POST"), } diff --git a/rules/aip0162/commit_http_uri_suffix.go b/rules/aip0162/commit_http_uri_suffix.go index f4745f2e..f48b837f 100644 --- a/rules/aip0162/commit_http_uri_suffix.go +++ b/rules/aip0162/commit_http_uri_suffix.go @@ -24,7 +24,7 @@ import ( // Commit methods should have a proper HTTP pattern. var commitHTTPURISuffix = &lint.MethodRule{ Name: lint.NewRuleName(162, "commit-http-uri-suffix"), - OnlyIf: isCommitMethod, + OnlyIf: utils.IsCommitRevisionMethod, LintMethod: func(m *desc.MethodDescriptor) []lint.Problem { for _, httpRule := range utils.GetHTTPRules(m) { if !commitURINameRegexp.MatchString(httpRule.URI) { diff --git a/rules/aip0162/commit_request_message_name.go b/rules/aip0162/commit_request_message_name.go index b56c89bb..154254b4 100644 --- a/rules/aip0162/commit_request_message_name.go +++ b/rules/aip0162/commit_request_message_name.go @@ -22,6 +22,6 @@ import ( // Commit messages should have a properly named request message. var commitRequestMessageName = &lint.MethodRule{ Name: lint.NewRuleName(162, "commit-request-message-name"), - OnlyIf: isCommitMethod, + OnlyIf: utils.IsCommitRevisionMethod, LintMethod: utils.LintMethodHasMatchingRequestName, } diff --git a/rules/aip0162/commit_response_message_name.go b/rules/aip0162/commit_response_message_name.go index 99876929..b5a263db 100644 --- a/rules/aip0162/commit_response_message_name.go +++ b/rules/aip0162/commit_response_message_name.go @@ -19,16 +19,20 @@ import ( "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 commitResponseMessageName = &lint.MethodRule{ Name: lint.NewRuleName(162, "commit-response-message-name"), - OnlyIf: isCommitMethod, + OnlyIf: utils.IsCommitRevisionMethod, LintMethod: func(m *desc.MethodDescriptor) []lint.Problem { // Rule check: Establish that for methods such as `CommitBook`, the response // message is `Book`. - want := commitMethodRegexp.FindStringSubmatch(m.GetName())[1] + want, ok := utils.ExtractRevisionResource(m) + if !ok { + return nil + } got := m.GetOutputType().GetName() // Return a problem if we did not get the expected return name. diff --git a/rules/aip0162/delete_revision_http_body.go b/rules/aip0162/delete_revision_http_body.go index 522d5724..7fa8a2a7 100644 --- a/rules/aip0162/delete_revision_http_body.go +++ b/rules/aip0162/delete_revision_http_body.go @@ -22,6 +22,6 @@ import ( // Delete Revision methods should have no HTTP body. var deleteRevisionHTTPBody = &lint.MethodRule{ Name: lint.NewRuleName(162, "delete-revision-http-body"), - OnlyIf: isDeleteRevisionMethod, + OnlyIf: utils.IsDeleteRevisionMethod, LintMethod: utils.LintNoHTTPBody, } diff --git a/rules/aip0162/delete_revision_http_method.go b/rules/aip0162/delete_revision_http_method.go index 8c9f5d8d..2638d4d9 100644 --- a/rules/aip0162/delete_revision_http_method.go +++ b/rules/aip0162/delete_revision_http_method.go @@ -22,6 +22,6 @@ import ( // Delete Revision methods should use the HTTP DELETE method. var deleteRevisionHTTPMethod = &lint.MethodRule{ Name: lint.NewRuleName(162, "delete-revision-http-method"), - OnlyIf: isDeleteRevisionMethod, + OnlyIf: utils.IsDeleteRevisionMethod, LintMethod: utils.LintHTTPMethod("DELETE"), } diff --git a/rules/aip0162/delete_revision_http_uri_suffix.go b/rules/aip0162/delete_revision_http_uri_suffix.go index ca6aed83..b5569968 100644 --- a/rules/aip0162/delete_revision_http_uri_suffix.go +++ b/rules/aip0162/delete_revision_http_uri_suffix.go @@ -24,7 +24,7 @@ import ( // Delete Revision methods should have a proper HTTP pattern. var deleteRevisionHTTPURISuffix = &lint.MethodRule{ Name: lint.NewRuleName(162, "delete-revision-http-uri-suffix"), - OnlyIf: isDeleteRevisionMethod, + OnlyIf: utils.IsDeleteRevisionMethod, LintMethod: func(m *desc.MethodDescriptor) []lint.Problem { for _, httpRule := range utils.GetHTTPRules(m) { if !deleteRevisionURINameRegexp.MatchString(httpRule.URI) { diff --git a/rules/aip0162/delete_revision_request_message_name.go b/rules/aip0162/delete_revision_request_message_name.go index 3fa65bb8..0039277f 100644 --- a/rules/aip0162/delete_revision_request_message_name.go +++ b/rules/aip0162/delete_revision_request_message_name.go @@ -22,6 +22,6 @@ import ( // Delete Revision messages should have a properly named request message. var deleteRevisionRequestMessageName = &lint.MethodRule{ Name: lint.NewRuleName(162, "delete-revision-request-message-name"), - OnlyIf: isDeleteRevisionMethod, + OnlyIf: utils.IsDeleteRevisionMethod, LintMethod: utils.LintMethodHasMatchingRequestName, } diff --git a/rules/aip0162/delete_revision_response_message_name.go b/rules/aip0162/delete_revision_response_message_name.go index b574a06e..14e877a8 100644 --- a/rules/aip0162/delete_revision_response_message_name.go +++ b/rules/aip0162/delete_revision_response_message_name.go @@ -16,20 +16,23 @@ package aip0162 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" ) // Delete Revision methods should return the resource itself. var deleteRevisionResponseMessageName = &lint.MethodRule{ Name: lint.NewRuleName(162, "delete-revision-response-message-name"), - OnlyIf: isDeleteRevisionMethod, + OnlyIf: utils.IsDeleteRevisionMethod, LintMethod: func(m *desc.MethodDescriptor) []lint.Problem { got := m.GetOutputType().GetName() - want := strings.TrimPrefix(strings.TrimSuffix(m.GetName(), "Revision"), "Delete") + want, ok := utils.ExtractRevisionResource(m) + if !ok { + return nil + } if got != want { return []lint.Problem{{ Message: fmt.Sprintf("Delete Revision methods should return the resource itself (`%s`), not `%s`.", want, got), diff --git a/rules/aip0162/rollback_http_body.go b/rules/aip0162/rollback_http_body.go index 643b7be4..7668858c 100644 --- a/rules/aip0162/rollback_http_body.go +++ b/rules/aip0162/rollback_http_body.go @@ -22,6 +22,6 @@ import ( // Rollback methods should have "*" as the HTTP body. var rollbackHTTPBody = &lint.MethodRule{ Name: lint.NewRuleName(162, "rollback-http-body"), - OnlyIf: isRollbackMethod, + OnlyIf: utils.IsRollbackRevisionMethod, LintMethod: utils.LintWildcardHTTPBody, } diff --git a/rules/aip0162/rollback_http_method.go b/rules/aip0162/rollback_http_method.go index 3da5f24a..c3336378 100644 --- a/rules/aip0162/rollback_http_method.go +++ b/rules/aip0162/rollback_http_method.go @@ -22,6 +22,6 @@ import ( // Rollback methods should use the HTTP POST method. var rollbackHTTPMethod = &lint.MethodRule{ Name: lint.NewRuleName(162, "rollback-http-method"), - OnlyIf: isRollbackMethod, + OnlyIf: utils.IsRollbackRevisionMethod, LintMethod: utils.LintHTTPMethod("POST"), } diff --git a/rules/aip0162/rollback_http_uri_suffix.go b/rules/aip0162/rollback_http_uri_suffix.go index c13b21fd..9e38e469 100644 --- a/rules/aip0162/rollback_http_uri_suffix.go +++ b/rules/aip0162/rollback_http_uri_suffix.go @@ -24,7 +24,7 @@ import ( // Rollback methods should have a proper HTTP pattern. var rollbackHTTPURISuffix = &lint.MethodRule{ Name: lint.NewRuleName(162, "rollback-http-uri-suffix"), - OnlyIf: isRollbackMethod, + OnlyIf: utils.IsRollbackRevisionMethod, LintMethod: func(m *desc.MethodDescriptor) []lint.Problem { for _, httpRule := range utils.GetHTTPRules(m) { if !rollbackURINameRegexp.MatchString(httpRule.URI) { diff --git a/rules/aip0162/rollback_request_message_name.go b/rules/aip0162/rollback_request_message_name.go index 2de5b45f..7a5f900e 100644 --- a/rules/aip0162/rollback_request_message_name.go +++ b/rules/aip0162/rollback_request_message_name.go @@ -22,6 +22,6 @@ import ( // Rollback messages should have a properly named request message. var rollbackRequestMessageName = &lint.MethodRule{ Name: lint.NewRuleName(162, "rollback-request-message-name"), - OnlyIf: isRollbackMethod, + OnlyIf: utils.IsRollbackRevisionMethod, LintMethod: utils.LintMethodHasMatchingRequestName, } diff --git a/rules/aip0162/rollback_response_message_name.go b/rules/aip0162/rollback_response_message_name.go index c778923f..b10aec91 100644 --- a/rules/aip0162/rollback_response_message_name.go +++ b/rules/aip0162/rollback_response_message_name.go @@ -26,11 +26,14 @@ import ( var rollbackResponseMessageName = &lint.MethodRule{ Name: lint.NewRuleName(162, "rollback-response-message-name"), - OnlyIf: isRollbackMethod, + OnlyIf: utils.IsRollbackRevisionMethod, LintMethod: func(m *desc.MethodDescriptor) []lint.Problem { // Rule check: Establish that for methods such as `RollbackBook`, the response // message is `Book`. - want := rollbackMethodRegexp.FindStringSubmatch(m.GetName())[1] + want, ok := utils.ExtractRevisionResource(m) + if !ok { + return nil + } got := m.GetOutputType().GetName() loc := locations.MethodResponseType(m) diff --git a/rules/aip0162/tag_revision_http_body.go b/rules/aip0162/tag_revision_http_body.go index daa054ef..490f04b8 100644 --- a/rules/aip0162/tag_revision_http_body.go +++ b/rules/aip0162/tag_revision_http_body.go @@ -22,6 +22,6 @@ import ( // Tag Revision methods should have "*" as the HTTP body. var tagRevisionHTTPBody = &lint.MethodRule{ Name: lint.NewRuleName(162, "tag-revision-http-body"), - OnlyIf: isTagRevisionMethod, + OnlyIf: utils.IsTagRevisionMethod, LintMethod: utils.LintWildcardHTTPBody, } diff --git a/rules/aip0162/tag_revision_http_method.go b/rules/aip0162/tag_revision_http_method.go index b1c0a2b0..5959d55e 100644 --- a/rules/aip0162/tag_revision_http_method.go +++ b/rules/aip0162/tag_revision_http_method.go @@ -22,6 +22,6 @@ import ( // Tag Revision methods should use the HTTP POST method. var tagRevisionHTTPMethod = &lint.MethodRule{ Name: lint.NewRuleName(162, "tag-revision-http-method"), - OnlyIf: isTagRevisionMethod, + OnlyIf: utils.IsTagRevisionMethod, LintMethod: utils.LintHTTPMethod("POST"), } diff --git a/rules/aip0162/tag_revision_http_uri_suffix.go b/rules/aip0162/tag_revision_http_uri_suffix.go index 907ab088..f7471f82 100644 --- a/rules/aip0162/tag_revision_http_uri_suffix.go +++ b/rules/aip0162/tag_revision_http_uri_suffix.go @@ -24,7 +24,7 @@ import ( // Tag Revision methods should have a proper HTTP pattern. var tagRevisionHTTPURISuffix = &lint.MethodRule{ Name: lint.NewRuleName(162, "tag-revision-http-uri-suffix"), - OnlyIf: isTagRevisionMethod, + OnlyIf: utils.IsTagRevisionMethod, LintMethod: func(m *desc.MethodDescriptor) []lint.Problem { for _, httpRule := range utils.GetHTTPRules(m) { if !tagRevisionURINameRegexp.MatchString(httpRule.URI) { diff --git a/rules/aip0162/tag_revision_request_message_name.go b/rules/aip0162/tag_revision_request_message_name.go index aac28ceb..29add926 100644 --- a/rules/aip0162/tag_revision_request_message_name.go +++ b/rules/aip0162/tag_revision_request_message_name.go @@ -22,6 +22,6 @@ import ( // Tag Revision messages should have a properly named request message. var tagRevisionRequestMessageName = &lint.MethodRule{ Name: lint.NewRuleName(162, "tag-revision-request-message-name"), - OnlyIf: isTagRevisionMethod, + OnlyIf: utils.IsTagRevisionMethod, LintMethod: utils.LintMethodHasMatchingRequestName, } diff --git a/rules/aip0162/tag_revision_response_message_name.go b/rules/aip0162/tag_revision_response_message_name.go index 9c01214d..5408d97f 100644 --- a/rules/aip0162/tag_revision_response_message_name.go +++ b/rules/aip0162/tag_revision_response_message_name.go @@ -19,16 +19,20 @@ import ( "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 tagRevisionResponseMessageName = &lint.MethodRule{ Name: lint.NewRuleName(162, "tag-revision-response-message-name"), - OnlyIf: isTagRevisionMethod, + OnlyIf: utils.IsTagRevisionMethod, LintMethod: func(m *desc.MethodDescriptor) []lint.Problem { // Rule check: Establish that for methods such as `TagBookRevision`, the response // message is `Book`. - want := tagRevisionMethodRegexp.FindStringSubmatch(m.GetName())[1] + want, ok := utils.ExtractRevisionResource(m) + if !ok { + return nil + } got := m.GetOutputType().GetName() // Return a problem if we did not get the expected return name. diff --git a/rules/internal/utils/method.go b/rules/internal/utils/method.go index ac43f409..19b1cf51 100644 --- a/rules/internal/utils/method.go +++ b/rules/internal/utils/method.go @@ -21,15 +21,20 @@ import ( ) var ( - createMethodRegexp = regexp.MustCompile("^Create(?:[A-Z]|$)") - getMethodRegexp = regexp.MustCompile("^Get(?:[A-Z]|$)") - listMethodRegexp = regexp.MustCompile("^List(?:[A-Z]|$)") + createMethodRegexp = regexp.MustCompile("^Create(?:[A-Z]|$)") + getMethodRegexp = regexp.MustCompile("^Get(?:[A-Z]|$)") + listMethodRegexp = regexp.MustCompile("^List(?:[A-Z]|$)") + updateMethodRegexp = regexp.MustCompile("^Update(?:[A-Z]|$)") + deleteMethodRegexp = regexp.MustCompile("^Delete(?:[A-Z]|$)") + standardMethodRegexp = regexp.MustCompile("^(Batch(Get|Create|Update|Delete))|(Get|Create|Update|Delete|List)(?:[A-Z]|$)") + + // AIP-162 Resource revision methods listRevisionsMethodRegexp = regexp.MustCompile(`^List(?:[A-Za-z0-9]+)Revisions$`) - updateMethodRegexp = regexp.MustCompile("^Update(?:[A-Z]|$)") - deleteMethodRegexp = regexp.MustCompile("^Delete(?:[A-Z]|$)") - deleteRevisionMethodRegexp = regexp.MustCompile("^Delete[A-Za-z0-9]*Revision$") legacyListRevisionsURINameRegexp = regexp.MustCompile(`:listRevisions$`) - standardMethodRegexp = regexp.MustCompile("^(Batch(Get|Create|Update|Delete))|(Get|Create|Update|Delete|List)(?:[A-Z]|$)") + commitRevisionMethodRegexp = regexp.MustCompile(`^Commit([A-Za-z0-9]+)$`) + deleteRevisionMethodRegexp = regexp.MustCompile(`^Delete([A-Za-z0-9]+)Revision$`) + rollbackRevisionMethodRegexp = regexp.MustCompile(`^Rollback([A-Za-z0-9]+)$`) + tagRevisionMethodRegexp = regexp.MustCompile(`^Tag([A-Za-z0-9]+)Revision$`) ) // IsCreateMethod returns true if this is a AIP-133 Create method. @@ -121,7 +126,65 @@ func IsStandardMethod(m *desc.MethodDescriptor) bool { return standardMethodRegexp.MatchString(m.GetName()) } -// IsCustomMethod returns true if this is a AIP-130 Custom Method +// IsCustomMethod returns true if this is a AIP-136 Custom Method func IsCustomMethod(m *desc.MethodDescriptor) bool { - return !IsStandardMethod(m) + return !IsStandardMethod(m) && !isRevisionMethod(m) +} + +// isRevisionMethod returns true if the given method is any of the documented +// Revision methods. At the moment, this is only relevant for excluding revision +// methods from other method type checks. +func isRevisionMethod(m *desc.MethodDescriptor) bool { + return IsDeleteRevisionMethod(m) || + IsTagRevisionMethod(m) || + IsCommitRevisionMethod(m) || + IsRollbackRevisionMethod(m) +} + +// IsDeleteRevisionMethod returns true if this is an AIP-162 Delete Revision +// method, false otherwise. +func IsDeleteRevisionMethod(m *desc.MethodDescriptor) bool { + return deleteRevisionMethodRegexp.MatchString(m.GetName()) +} + +// IsTagRevisionMethod returns true if this is an AIP-162 Tag Revision method, +// false otherwise. +func IsTagRevisionMethod(m *desc.MethodDescriptor) bool { + return tagRevisionMethodRegexp.MatchString(m.GetName()) +} + +// IsCommitRevisionMethod returns true if this is an AIP-162 Commit method, +// false otherwise. +func IsCommitRevisionMethod(m *desc.MethodDescriptor) bool { + return commitRevisionMethodRegexp.MatchString(m.GetName()) +} + +// IsRollbackRevisionMethod returns true if this is an AIP-162 Rollback method, +// false otherwise. +func IsRollbackRevisionMethod(m *desc.MethodDescriptor) bool { + return rollbackRevisionMethodRegexp.MatchString(m.GetName()) +} + +// ExtractRevisionResource uses the appropriate revision method regular +// expression to capture and extract the resource noun in the method name. +// If the given method is not one of the standard revision RPCs, it returns +// empty string and false. +func ExtractRevisionResource(m *desc.MethodDescriptor) (string, bool) { + if !isRevisionMethod(m) { + return "", false + } + + n := m.GetName() + + if IsCommitRevisionMethod(m) { + return commitRevisionMethodRegexp.FindStringSubmatch(n)[1], true + } else if IsTagRevisionMethod(m) { + return tagRevisionMethodRegexp.FindStringSubmatch(n)[1], true + } else if IsRollbackRevisionMethod(m) { + return rollbackRevisionMethodRegexp.FindStringSubmatch(n)[1], true + } else if IsDeleteRevisionMethod(m) { + return deleteRevisionMethodRegexp.FindStringSubmatch(n)[1], true + } + + return "", false } diff --git a/rules/internal/utils/method_test.go b/rules/internal/utils/method_test.go index 78e03f4b..9c97484b 100644 --- a/rules/internal/utils/method_test.go +++ b/rules/internal/utils/method_test.go @@ -17,6 +17,7 @@ import ( "testing" "github.com/googleapis/api-linter/rules/internal/testutils" + "github.com/jhump/protoreflect/desc" ) func TestIsCreateMethod(t *testing.T) { @@ -385,3 +386,115 @@ func TestIsStandardMethod(t *testing.T) { }) } } + +func TestIsRevisionMethod(t *testing.T) { + for _, test := range []struct { + name string + MethodName string + want bool + is func(m *desc.MethodDescriptor) bool + }{ + { + "IsRollbackRevisionMethod", + "RollbackBook", + true, + IsRollbackRevisionMethod, + }, + { + "IsCommitRevisionMethod", + "CommitBook", + true, + IsCommitRevisionMethod, + }, + { + "IsTagRevisionMethod", + "TagBookRevision", + true, + IsTagRevisionMethod, + }, + { + "IsDeleteRevisionMethod", + "DeleteBookRevision", + true, + IsDeleteRevisionMethod, + }, + { + "NotRevisionMethod", + "GetBook", + false, + isRevisionMethod, + }, + } { + t.Run(test.name, func(t *testing.T) { + file := testutils.ParseProto3Tmpl(t, ` + service Foo { + rpc {{.MethodName}}(Book) returns (Book); + } + + // This is the request and response, which is irrelevant for + // asserting if the rpc is a standard method or not. We just + // check the naming of the rpc against a regex + message Book {} + `, test) + method := file.GetServices()[0].GetMethods()[0] + got := test.is(method) + + if got != test.want { + t.Errorf("got %v want %v", got, test.want) + } + }) + } +} + +func TestExtractRevisionResource(t *testing.T) { + for _, test := range []struct { + name string + MethodName string + want string + }{ + { + "RollbackRevisionMethod", + "RollbackBook", + "Book", + }, + { + "CommitRevisionMethod", + "CommitBook", + "Book", + }, + { + "TagRevisionMethod", + "TagBookRevision", + "Book", + }, + { + "DeleteRevisionMethod", + "DeleteBookRevision", + "Book", + }, + { + "NotRevisionMethod", + "GetBook", + "", + }, + } { + t.Run(test.name, func(t *testing.T) { + file := testutils.ParseProto3Tmpl(t, ` + service Foo { + rpc {{.MethodName}}(Book) returns (Book); + } + + // This is the request and response, which is irrelevant for + // asserting if the rpc is a standard method or not. We just + // check the naming of the rpc against a regex + message Book {} + `, test) + method := file.GetServices()[0].GetMethods()[0] + got, _ := ExtractRevisionResource(method) + + if got != test.want { + t.Errorf("got %q want %q", got, test.want) + } + }) + } +}