Skip to content

Commit

Permalink
feat: Add HTTP body and method rules for AIP-132 (#703)
Browse files Browse the repository at this point in the history
  • Loading branch information
apasel422 committed Nov 2, 2020
1 parent 40fc0a3 commit 274d2e2
Show file tree
Hide file tree
Showing 7 changed files with 307 additions and 0 deletions.
71 changes: 71 additions & 0 deletions docs/rules/0132/http-body.md
@@ -0,0 +1,71 @@
---
rule:
aip: 132
name: [core, '0132', http-body]
summary: List methods must not have an HTTP body.
permalink: /132/http-body
redirect_from:
- /0132/http-body
---

# List methods: No HTTP body

This rule enforces that all `List` RPCs omit the HTTP `body`, as mandated in
[AIP-132][].

## Details

This rule looks at any message beginning with `List`, and complains if
the HTTP `body` field is set.

## Examples

**Incorrect** code for this rule:

```proto
// Incorrect.
rpc ListBooks(ListBooksRequest) returns (ListBooksResponse) {
option (google.api.http) = {
get: "/v1/{parent=publishers/*}/books"
body: "*" // This should be absent.
};
}
```

**Correct** code for this rule:

```proto
// Correct.
rpc ListBooks(ListBooksRequest) returns (ListBooksResponse) {
option (google.api.http) = {
get: "/v1/{parent=publishers/*}/books"
};
}
```

## Disabling

If you need to violate this rule, use a leading comment above the method.
Remember to also include an [aip.dev/not-precedent][] comment explaining why.

```proto
// (-- api-linter: core::0132::http-body=disabled
// api-linter: core::0132::http-method=disabled
// aip.dev/not-precedent: We need to do this because reasons. --)
rpc ListBooks(ListBooksRequest) returns (ListBooksResponse) {
option (google.api.http) = {
post: "/v1/{parent=publishers/*}/books"
body: "*"
};
}
```

**Important:** HTTP `GET` requests are unable to have an HTTP body, due to the
nature of the protocol. The only valid way to include a body is to also use a
different HTTP method (as depicted above).

If you need to violate this rule for an entire file, place the comment at the
top of the file.

[aip-132]: https://aip.dev/132
[aip.dev/not-precedent]: https://aip.dev/not-precedent
65 changes: 65 additions & 0 deletions docs/rules/0132/http-method.md
@@ -0,0 +1,65 @@
---
rule:
aip: 132
name: [core, '0132', http-method]
summary: List methods must use the GET HTTP verb.
permalink: /132/http-method
redirect_from:
- /0132/http-method
---

# List methods: GET HTTP verb

This rule enforces that all `List` RPCs use the `GET` HTTP verb, as mandated in
[AIP-132][].

## Details

This rule looks at any message beginning with `List`, and complains if
the HTTP verb is anything other than `GET`. It _does_ check additional bindings
if they are present.

## Examples

**Incorrect** code for this rule:

```proto
// Incorrect.
rpc ListBooks(ListBooksRequest) returns (ListBooksResponse) {
option (google.api.http) = {
post: "/v1/{parent=publishers/*}/books" // Should be `get:`.
};
}
```

**Correct** code for this rule:

```proto
// Correct.
rpc ListBooks(ListBooksRequest) returns (ListBooksResponse) {
option (google.api.http) = {
get: "/v1/{parent=publishers/*}/books"
};
}
```

## Disabling

If you need to violate this rule, use a leading comment above the method.
Remember to also include an [aip.dev/not-precedent][] comment explaining why.

```proto
// (-- api-linter: core::0132::http-method=disabled
// aip.dev/not-precedent: We need to do this because reasons. --)
rpc ListBooks(ListBooksRequest) returns (ListBooksResponse) {
option (google.api.http) = {
post: "/v1/{parent=publishers/*}/books"
};
}
```

If you need to violate this rule for an entire file, place the comment at the
top of the file.

[aip-132]: https://aip.dev/132
[aip.dev/not-precedent]: https://aip.dev/not-precedent
2 changes: 2 additions & 0 deletions rules/aip0132/aip0132.go
Expand Up @@ -26,6 +26,8 @@ import (
func AddRules(r lint.RuleRegistry) error {
return r.Register(
132,
httpBody,
httpMethod,
methodSignature,
requestFieldTypes,
requestMessageName,
Expand Down
27 changes: 27 additions & 0 deletions rules/aip0132/http_body.go
@@ -0,0 +1,27 @@
// 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 aip0132

import (
"github.com/googleapis/api-linter/lint"
"github.com/googleapis/api-linter/rules/internal/utils"
)

// List methods should not have an HTTP body.
var httpBody = &lint.MethodRule{
Name: lint.NewRuleName(132, "http-body"),
OnlyIf: isListMethod,
LintMethod: utils.LintNoHTTPBody,
}
57 changes: 57 additions & 0 deletions rules/aip0132/http_body_test.go
@@ -0,0 +1,57 @@
// 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 aip0132

import (
"testing"

"github.com/googleapis/api-linter/rules/internal/testutils"
)

func TestHttpBody(t *testing.T) {
tests := []struct {
testName string
Body string
MethodName string
problems testutils.Problems
}{
{"Valid", "", "ListBooks", nil},
{"Invalid", "*", "ListBooks", testutils.Problems{{Message: "HTTP body"}}},
{"Irrelevant", "*", "AcquireBook", nil},
}

for _, test := range tests {
t.Run(test.testName, func(t *testing.T) {
file := testutils.ParseProto3Tmpl(t, `
import "google/api/annotations.proto";
service Library {
rpc {{.MethodName}}({{.MethodName}}Request) returns ({{.MethodName}}Response) {
option (google.api.http) = {
get: "/v1/{parent=publishers/*}/books"
body: "{{.Body}}"
};
}
}
message {{.MethodName}}Request {}
message {{.MethodName}}Response {}
`, test)
method := file.GetServices()[0].GetMethods()[0]
problems := httpBody.Lint(file)
if diff := test.problems.SetDescriptor(method).Diff(problems); diff != "" {
t.Error(diff)
}
})
}
}
27 changes: 27 additions & 0 deletions rules/aip0132/http_method.go
@@ -0,0 +1,27 @@
// 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 aip0132

import (
"github.com/googleapis/api-linter/lint"
"github.com/googleapis/api-linter/rules/internal/utils"
)

// List methods should use the HTTP GET verb.
var httpMethod = &lint.MethodRule{
Name: lint.NewRuleName(132, "http-method"),
OnlyIf: isListMethod,
LintMethod: utils.LintHTTPMethod("GET"),
}
58 changes: 58 additions & 0 deletions rules/aip0132/http_method_test.go
@@ -0,0 +1,58 @@
// 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 aip0132

import (
"testing"

"github.com/googleapis/api-linter/rules/internal/testutils"
)

func TestHttpMethod(t *testing.T) {
// Set up testing permutations.
tests := []struct {
testName string
Method string
MethodName string
problems testutils.Problems
}{
{"Valid", "get", "ListBooks", nil},
{"Invalid", "post", "ListBooks", testutils.Problems{{Message: "HTTP GET"}}},
{"Irrelevant", "post", "AcquireBook", nil},
}

// Run each test.
for _, test := range tests {
t.Run(test.testName, func(t *testing.T) {
file := testutils.ParseProto3Tmpl(t, `
import "google/api/annotations.proto";
service Library {
rpc {{.MethodName}}({{.MethodName}}Request) returns ({{.MethodName}}Response) {
option (google.api.http) = {
{{.Method}}: "/v1/{parent=publishers/*}/books"
};
}
}
message {{.MethodName}}Request {}
message {{.MethodName}}Response {}
`, test)
method := file.GetServices()[0].GetMethods()[0]
problems := httpMethod.Lint(file)
if diff := test.problems.SetDescriptor(method).Diff(problems); diff != "" {
t.Error(diff)
}
})
}
}

0 comments on commit 274d2e2

Please sign in to comment.