Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(AIP-162): handle LRO in response name rules #1431

Merged
merged 1 commit into from
Sep 20, 2024
Merged

fix(AIP-162): handle LRO in response name rules #1431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions rules/aip0162/commit_response_message_name.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ 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"
)

Expand All @@ -28,8 +29,19 @@ var commitResponseMessageName = &lint.MethodRule{
LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
// Rule check: Establish that for methods such as `CommitBook`, the response
// message is `Book`.
response := utils.GetResponseType(m)
if response == nil {
return nil
}
got := response.GetName()
want := commitMethodRegexp.FindStringSubmatch(m.GetName())[1]
got := m.GetOutputType().GetName()
loc := locations.MethodResponseType(m)
suggestion := want

if utils.GetOperationInfo(m) != nil {
loc = locations.MethodOperationInfo(m)
suggestion = "" // We cannot offer a precise enough location to make a suggestion.
}

// Return a problem if we did not get the expected return name.
if got != want {
Expand All @@ -39,9 +51,9 @@ var commitResponseMessageName = &lint.MethodRule{
want,
got,
),
Suggestion: want,
Suggestion: suggestion,
Descriptor: m,
Location: locations.MethodResponseType(m),
Location: loc,
}}
}
return nil
Expand Down
35 changes: 34 additions & 1 deletion rules/aip0162/commit_response_message_name_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ func TestCommitResponseMessageName(t *testing.T) {
} {
t.Run(test.name, func(t *testing.T) {
f := testutils.ParseProto3Tmpl(t, `
import "google/longrunning/operations.proto";
service Library {
rpc {{.Method}}(CommitBookRequest) returns ({{.ResponseType}});
}
Expand All @@ -47,3 +46,37 @@ func TestCommitResponseMessageName(t *testing.T) {
})
}
}

func TestCommitResponseMessageNameLRO(t *testing.T) {
for _, test := range []struct {
name string
Method string
ResponseType string
problems testutils.Problems
}{
{"Valid", "CommitBook", "Book", nil},
{"Invalid", "CommitBook", "CommitBookResponse", testutils.Problems{{Message: "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}}(CommitBookRequest) returns (google.longrunning.Operation) {
option (google.longrunning.operation_info) = {
response_type: "{{.ResponseType}}"
metadata_type: "OperationMetadata"
};
};
}
message CommitBookRequest {}
message {{.ResponseType}} {}
message OperationMetadata{}
`, test)
m := f.GetServices()[0].GetMethods()[0]
if diff := test.problems.SetDescriptor(m).Diff(commitResponseMessageName.Lint(f)); diff != "" {
t.Errorf(diff)
}
})
}
}
20 changes: 17 additions & 3 deletions rules/aip0162/delete_revision_response_message_name.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ 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"
)

Expand All @@ -28,14 +29,27 @@ var deleteRevisionResponseMessageName = &lint.MethodRule{
Name: lint.NewRuleName(162, "delete-revision-response-message-name"),
OnlyIf: isDeleteRevisionMethod,
LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
got := m.GetOutputType().GetName()
response := utils.GetResponseType(m)
if response == nil {
return nil
}
got := response.GetName()
want := strings.TrimPrefix(strings.TrimSuffix(m.GetName(), "Revision"), "Delete")

loc := locations.MethodResponseType(m)
suggestion := want

if utils.GetOperationInfo(m) != nil {
loc = locations.MethodOperationInfo(m)
suggestion = "" // We cannot offer a precise enough location to make a suggestion.
}

if got != want {
return []lint.Problem{{
Message: fmt.Sprintf("Delete Revision methods should return the resource itself (`%s`), not `%s`.", want, got),
Suggestion: want,
Suggestion: suggestion,
Descriptor: m,
Location: locations.MethodResponseType(m),
Location: loc,
}}
}
return nil
Expand Down
36 changes: 36 additions & 0 deletions rules/aip0162/delete_revision_response_message_name_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,39 @@ func TestDeleteRevisionResponseMessageName(t *testing.T) {
})
}
}

func TestDeleteRevisionResponseMessageNameLRO(t *testing.T) {
for _, test := range []struct {
name string
MethodName string
ResponseType string
problems testutils.Problems
}{
{"Valid", "DeleteBookRevision", "Book", nil},
{"Invalid", "DeleteBookRevision", "DeleteBookRevisionResponse", testutils.Problems{{Message: "Book"}}},
{"Irrelevant", "DeleteBook", "DeleteBookResponse", nil},
} {
t.Run(test.name, func(t *testing.T) {
file := testutils.ParseProto3Tmpl(t, `
import "google/longrunning/operations.proto";
service Library {
rpc {{.MethodName}}({{.MethodName}}Request) returns (google.longrunning.Operation) {
option (google.longrunning.operation_info) = {
response_type: "{{.ResponseType}}"
metadata_type: "OperationMetadata"
};
};
}
message {{.MethodName}}Request {}
message {{.ResponseType}} {}
message OperationMetadata {}
`, test)

method := file.GetServices()[0].GetMethods()[0]
problems := deleteRevisionResponseMessageName.Lint(file)
if diff := test.problems.SetDescriptor(method).Diff(problems); diff != "" {
t.Error(diff)
}
})
}
}
17 changes: 9 additions & 8 deletions rules/aip0162/rollback_response_message_name.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package aip0162

import (
"fmt"
"strings"

"github.com/googleapis/api-linter/lint"
"github.com/googleapis/api-linter/locations"
Expand All @@ -31,15 +30,17 @@ var rollbackResponseMessageName = &lint.MethodRule{
// Rule check: Establish that for methods such as `RollbackBook`, the response
// message is `Book`.
want := rollbackMethodRegexp.FindStringSubmatch(m.GetName())[1]
got := m.GetOutputType().GetName()
response := utils.GetResponseType(m)
if response == nil {
return nil
}
got := response.GetName()
loc := locations.MethodResponseType(m)
suggestion := want

// 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:]
if utils.GetOperationInfo(m) != nil {
loc = locations.MethodOperationInfo(m)
suggestion = "" // We cannot offer a precise enough location to make a suggestion.
}

// Return a problem if we did not get the expected return name.
Expand All @@ -50,7 +51,7 @@ var rollbackResponseMessageName = &lint.MethodRule{
want,
got,
),
Suggestion: want,
Suggestion: suggestion,
Descriptor: m,
Location: loc,
}}
Expand Down
3 changes: 1 addition & 2 deletions rules/aip0162/rollback_response_message_name_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ func TestRollbackResponseMessageName(t *testing.T) {
} {
t.Run(test.name, func(t *testing.T) {
f := testutils.ParseProto3Tmpl(t, `
import "google/longrunning/operations.proto";
service Library {
rpc {{.Method}}(RollbackBookRequest) returns ({{.ResponseType}});
}
Expand All @@ -56,7 +55,7 @@ func TestRollbackOperationResponse(t *testing.T) {
problems testutils.Problems
}{
{"Valid", "RollbackBook", "Book", nil},
{"Invalid", "RollbackBook", "RollbackBookResponse", testutils.Problems{{Suggestion: "Book"}}},
{"Invalid", "RollbackBook", "RollbackBookResponse", testutils.Problems{{Message: "Book"}}},
{"Irrelevant", "AcquireBook", "PurgeBooksResponse", nil},
} {
t.Run(test.name, func(t *testing.T) {
Expand Down
18 changes: 15 additions & 3 deletions rules/aip0162/tag_revision_response_message_name.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ 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"
)

Expand All @@ -29,7 +30,18 @@ var tagRevisionResponseMessageName = &lint.MethodRule{
// Rule check: Establish that for methods such as `TagBookRevision`, the response
// message is `Book`.
want := tagRevisionMethodRegexp.FindStringSubmatch(m.GetName())[1]
got := m.GetOutputType().GetName()
response := utils.GetResponseType(m)
if response == nil {
return nil
}
got := response.GetName()
loc := locations.MethodResponseType(m)
suggestion := want

if utils.GetOperationInfo(m) != nil {
loc = locations.MethodOperationInfo(m)
suggestion = "" // We cannot offer a precise enough location to make a suggestion.
}

// Return a problem if we did not get the expected return name.
if got != want {
Expand All @@ -39,9 +51,9 @@ var tagRevisionResponseMessageName = &lint.MethodRule{
want,
got,
),
Suggestion: want,
Suggestion: suggestion,
Descriptor: m,
Location: locations.MethodResponseType(m),
Location: loc,
}}
}
return nil
Expand Down
35 changes: 34 additions & 1 deletion rules/aip0162/tag_revision_response_message_name_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ func TestTagRevisionResponseMessageName(t *testing.T) {
} {
t.Run(test.name, func(t *testing.T) {
f := testutils.ParseProto3Tmpl(t, `
import "google/longrunning/operations.proto";
service Library {
rpc {{.Method}}(TagBookRevisionRequest) returns ({{.ResponseType}});
}
Expand All @@ -47,3 +46,37 @@ func TestTagRevisionResponseMessageName(t *testing.T) {
})
}
}

func TestTagRevisionResponseMessageNameLRO(t *testing.T) {
for _, test := range []struct {
name string
Method string
ResponseType string
problems testutils.Problems
}{
{"Valid", "TagBookRevision", "Book", nil},
{"Invalid", "TagBookRevision", "TagBookRevisionResponse", testutils.Problems{{Message: "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}}(TagBookRevisionRequest) returns (google.longrunning.Operation) {
option (google.longrunning.operation_info) = {
response_type: "{{.ResponseType}}"
metadata_type: "OperationMetadata"
};
};
}
message TagBookRevisionRequest {}
message {{.ResponseType}} {}
message OperationMetadata {}
`, test)
m := f.GetServices()[0].GetMethods()[0]
if diff := test.problems.SetDescriptor(m).Diff(tagRevisionResponseMessageName.Lint(f)); diff != "" {
t.Errorf(diff)
}
})
}
}
Loading