feat(AIP-162): lint operation_info response_type (#1137)

googleapis · Apr 13, 2023 · 6ea9f20 · 6ea9f20
1 parent 81262a4
commit 6ea9f20
Show file tree

Hide file tree

Showing 11 changed files with 59 additions and 9 deletions.
diff --git a/rules/aip0133/response_message_name.go b/rules/aip0133/response_message_name.go
@@ -33,7 +33,7 @@ var outputName = &lint.MethodRule{
 		// If this is an LRO, then use the annotated response type instead of
 		// the actual RPC return type.
 		got := m.GetOutputType().GetName()
-		if m.GetOutputType().GetFullyQualifiedName() == "google.longrunning.Operation" {
+		if utils.IsOperation(m.GetOutputType()) {
 			got = utils.GetOperationInfo(m).GetResponseType()
 		}
 

diff --git a/rules/aip0134/response_message_name.go b/rules/aip0134/response_message_name.go
@@ -35,7 +35,7 @@ var responseMessageName = &lint.MethodRule{
 		got := m.GetOutputType().GetName()
 
 		// If the return type is an LRO, use the annotated response type instead.
-		if m.GetOutputType().GetFullyQualifiedName() == "google.longrunning.Operation" {
+		if utils.IsOperation(m.GetOutputType()) {
 			got = utils.GetOperationInfo(m).GetResponseType()
 		}
 

diff --git a/rules/aip0135/response_message_name.go b/rules/aip0135/response_message_name.go
@@ -48,7 +48,7 @@ var responseMessageName = &lint.MethodRule{
 
 		// If the return type is an Operation, use the annotated response type.
 		lro := false
-		if got == "google.longrunning.Operation" {
+		if utils.IsOperation(m.GetOutputType()) {
 			got = utils.GetOperationInfo(m).GetResponseType()
 			lro = true
 		}

diff --git a/rules/aip0162/rollback_response_message_name.go b/rules/aip0162/rollback_response_message_name.go
@@ -16,9 +16,11 @@ 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"
 )
 
@@ -30,6 +32,15 @@ var rollbackResponseMessageName = &lint.MethodRule{
 		// message is `Book`.
 		want := rollbackMethodRegexp.FindStringSubmatch(m.GetName())[1]
 		got := m.GetOutputType().GetName()
+		loc := locations.MethodResponseType(m)
+
+		// If LRO, check the response_type short name.
+		if utils.IsOperation(m.GetOutputType()) {
+			t := utils.GetOperationInfo(m).GetResponseType()
+			ndx := strings.LastIndex(t, ".")
+			got = t[ndx+1:]
+			loc = locations.MethodOperationInfo(m)
+		}
 
 		// Return a problem if we did not get the expected return name.
 		if got != want {
@@ -41,7 +52,7 @@ var rollbackResponseMessageName = &lint.MethodRule{
 				),
 				Suggestion: want,
 				Descriptor: m,
-				Location:   locations.MethodResponseType(m),
+				Location:   loc,
 			}}
 		}
 		return nil

diff --git a/rules/aip0162/rollback_response_message_name_test.go b/rules/aip0162/rollback_response_message_name_test.go
@@ -47,3 +47,37 @@ func TestRollbackResponseMessageName(t *testing.T) {
 		})
 	}
 }
+
+func TestRollbackOperationResponse(t *testing.T) {
+	for _, test := range []struct {
+		name         string
+		Method       string
+		ResponseType string
+		problems     testutils.Problems
+	}{
+		{"Valid", "RollbackBook", "Book", nil},
+		{"Invalid", "RollbackBook", "RollbackBookResponse", testutils.Problems{{Suggestion: "Book"}}},
+		{"Irrelevant", "AcquireBook", "PurgeBooksResponse", nil},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			f := testutils.ParseProto3Tmpl(t, `
+				import "google/longrunning/operations.proto";
+				service Library {
+					rpc {{.Method}}(RollbackBookRequest) returns (google.longrunning.Operation) {
+						option (google.longrunning.operation_info) = {
+							response_type: "{{.ResponseType}}"
+							metadata_type: "OperationMetadata"
+						};
+					}
+				}
+				message RollbackBookRequest {}
+				message OperationMetadata {}
+				message {{.ResponseType}} {}
+			`, test)
+			m := f.GetServices()[0].GetMethods()[0]
+			if diff := test.problems.SetDescriptor(m).Diff(rollbackResponseMessageName.Lint(f)); diff != "" {
+				t.Errorf(diff)
+			}
+		})
+	}
+}
diff --git a/rules/aip0164/response_message_name.go b/rules/aip0164/response_message_name.go
@@ -36,7 +36,7 @@ var responseMessageName = &lint.MethodRule{
 		got := m.GetOutputType().GetName()
 
 		// If the return type is an LRO, use the annotated response type instead.
-		if m.GetOutputType().GetFullyQualifiedName() == "google.longrunning.Operation" {
+		if utils.IsOperation(m.GetOutputType()) {
 			got = utils.GetOperationInfo(m).GetResponseType()
 		}
 

diff --git a/rules/aip0233/response_message_name.go b/rules/aip0233/response_message_name.go
@@ -35,7 +35,7 @@ var responseMessageName = &lint.MethodRule{
 		// If this is an LRO, then use the annotated response type instead of
 		// the actual RPC return type.
 		got := m.GetOutputType().GetName()
-		if m.GetOutputType().GetFullyQualifiedName() == "google.longrunning.Operation" {
+		if utils.IsOperation(m.GetOutputType()) {
 			got = utils.GetOperationInfo(m).GetResponseType()
 		}
 

diff --git a/rules/aip0234/response_message_name.go b/rules/aip0234/response_message_name.go
@@ -35,7 +35,7 @@ var responseMessageName = &lint.MethodRule{
 		// If this is an LRO, then use the annotated response type instead of
 		// the actual RPC return type.
 		got := m.GetOutputType().GetName()
-		if m.GetOutputType().GetFullyQualifiedName() == "google.longrunning.Operation" {
+		if utils.IsOperation(m.GetOutputType()) {
 			got = utils.GetOperationInfo(m).GetResponseType()
 		}
 

diff --git a/rules/aip0235/response_message_name.go b/rules/aip0235/response_message_name.go
@@ -30,7 +30,7 @@ var responseMessageName = &lint.MethodRule{
 	OnlyIf: isBatchDeleteMethod,
 	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
 		got := m.GetOutputType().GetFullyQualifiedName()
-		if got == "google.longrunning.Operation" {
+		if utils.IsOperation(m.GetOutputType()) {
 			got = utils.GetOperationInfo(m).GetResponseType()
 		} else if got != "google.protobuf.Empty" {
 			got = m.GetOutputType().GetName()

diff --git a/rules/internal/utils/declarative_friendly.go b/rules/internal/utils/declarative_friendly.go
@@ -83,7 +83,7 @@ func DeclarativeFriendlyResource(d desc.Descriptor) *desc.MessageDescriptor {
 
 		// If the method is an LRO, then get the response type from the
 		// operation_info annotation.
-		if response.GetFullyQualifiedName() == "google.longrunning.Operation" {
+		if IsOperation(response) {
 			if opInfo := GetOperationInfo(m); opInfo != nil {
 				response = FindMessage(m.GetFile(), opInfo.GetResponseType())
 

diff --git a/rules/internal/utils/type_name.go b/rules/internal/utils/type_name.go
@@ -35,3 +35,8 @@ func GetTypeName(f *desc.FieldDescriptor) string {
 	}
 	return strings.ToLower(f.GetType().String()[len("TYPE_"):])
 }
+
+// IsOperation returns if the message is a longrunning Operation or not.
+func IsOperation(m *desc.MessageDescriptor) bool {
+	return m.GetFullyQualifiedName() == "google.longrunning.Operation"
+}