diff --git a/docs/rules/0134/response-lro.md b/docs/rules/0134/response-lro.md new file mode 100644 index 000000000..46f1ab7d6 --- /dev/null +++ b/docs/rules/0134/response-lro.md @@ -0,0 +1,79 @@ +--- +rule: + aip: 134 + name: [core, '0134', response-lro] + summary: | + Declarative-friendly Update methods should use long-running operations. +permalink: /134/response-lro +redirect_from: + - /0134/response-lro +--- + +# Long-running Update + +This rule enforces that declarative-friendly update methods use long-running +operations, as mandated in [AIP-134][]. + +## Details + +This rule looks at any `Update` method connected to a resource with a +`google.api.resource` annotation that includes `style: DECLARATIVE_FRIENDLY`, +and complains if it does not use long-running operations. + +## Examples + +**Incorrect** code for this rule: + +```proto +// Incorrect. +// Assuming that Book is styled declarative-friendly, UpdateBook should +// return a long-running operation. +rpc UpdateBook(UpdateBookRequest) returns (Book) { + option (google.api.http) = { + patch: "/v1/{book.name=publishers/*/books/*}" + body: "book" + }; +} +``` + +**Correct** code for this rule: + +```proto +// Correct. +// Assuming that Book is styled declarative-friendly... +rpc UpdateBook(UpdateBookRequest) returns (google.longrunning.Operation) { + option (google.api.http) = { + patch: "/v1/{book.name=publishers/*/books/*}" + body: "book" + }; + option (google.longrunning.operation_info) = { + response_type: "Book" + metadata_type: "OperationMetadata" + }; +} +``` + +## Disabling + +If you need to violate this rule, use a leading comment above the message. +Remember to also include an [aip.dev/not-precedent][] comment explaining why. + +```proto +// (-- api-linter: core::0134::response-lro=disabled +// aip.dev/not-precedent: We need to do this because reasons. --) +rpc UpdateBook(UpdateBookRequest) returns (Book) { + option (google.api.http) = { + patch: "/v1/{book.name=publishers/*/books/*}" + body: "book" + }; +} +``` + +**Note:** Violations of declarative-friendly rules should be rare, as tools are +likely to expect strong consistency. + +If you need to violate this rule for an entire file, place the comment at the +top of the file. + +[aip-134]: https://aip.dev/134 +[aip.dev/not-precedent]: https://aip.dev/not-precedent diff --git a/rules/aip0134/aip0134.go b/rules/aip0134/aip0134.go index ae123282c..ae850f554 100644 --- a/rules/aip0134/aip0134.go +++ b/rules/aip0134/aip0134.go @@ -40,6 +40,7 @@ func AddRules(r lint.RuleRegistry) error { requestMessageName, requestResourceField, requestResourceRequired, + responseLRO, synonyms, unknownFields, ) diff --git a/rules/aip0134/response_lro.go b/rules/aip0134/response_lro.go new file mode 100644 index 000000000..e9b779c79 --- /dev/null +++ b/rules/aip0134/response_lro.go @@ -0,0 +1,40 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package aip0134 + +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 responseLRO = &lint.MethodRule{ + Name: lint.NewRuleName(134, "response-lro"), + OnlyIf: func(m *desc.MethodDescriptor) bool { + return isUpdateMethod(m) && utils.IsDeclarativeFriendlyMethod(m) + }, + LintMethod: func(m *desc.MethodDescriptor) []lint.Problem { + if m.GetOutputType().GetFullyQualifiedName() != "google.longrunning.Operation" { + return []lint.Problem{{ + Message: "Declarative-friendly update methods should use an LRO.", + Descriptor: m, + Location: locations.MethodResponseType(m), + Suggestion: "google.longrunning.Operation", + }} + } + return nil + }, +} diff --git a/rules/aip0134/response_lro_test.go b/rules/aip0134/response_lro_test.go new file mode 100644 index 000000000..b8e17bb3a --- /dev/null +++ b/rules/aip0134/response_lro_test.go @@ -0,0 +1,60 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package aip0134 + +import ( + "testing" + + "github.com/googleapis/api-linter/rules/internal/testutils" +) + +func TestResponseLRO(t *testing.T) { + problems := testutils.Problems{{Suggestion: "google.longrunning.Operation"}} + for _, test := range []struct { + name string + Style string + ResponseType string + problems testutils.Problems + }{ + {"ValidNotDF", "", "Book", nil}, + {"ValidLRO", "style: DECLARATIVE_FRIENDLY", "google.longrunning.Operation", nil}, + {"InvalidDFSync", "style: DECLARATIVE_FRIENDLY", "Book", problems}, + } { + t.Run(test.name, func(t *testing.T) { + f := testutils.ParseProto3Tmpl(t, ` + import "google/api/resource.proto"; + import "google/longrunning/operations.proto"; + + service Library { + rpc UpdateBook(UpdateBookRequest) returns ({{.ResponseType}}); + } + + message Book { + option (google.api.resource) = { + type: "library.googleapis.com/Book" + pattern: "publishers/{publisher}/books/{book}" + {{.Style}} + }; + } + + message UpdateBookRequest {} + `, test) + m := f.GetServices()[0].GetMethods()[0] + if diff := test.problems.SetDescriptor(m).Diff(responseLRO.Lint(f)); diff != "" { + t.Errorf(diff) + } + }) + } +}