Skip to content

Commit

Permalink
feat(AIP-134): update_mask must be OPTIONAL (#1394)
Browse files Browse the repository at this point in the history
* feat(AIP-134): update_mask must be OPTIONAL

* chore(AIP-203): use locations.FieldBehavior helper

* update license header year

* chore: fix typos
  • Loading branch information
noahdietz authored Jul 10, 2024
1 parent 4acf04c commit 9fc0d05
Show file tree
Hide file tree
Showing 6 changed files with 173 additions and 1 deletion.
72 changes: 72 additions & 0 deletions docs/rules/0134/update-mask-optional-behavior.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
---
rule:
aip: 134
name: [core, '0134', update-mask-optional-behavior]
summary: Standard Update `update_mask` field must be `OPTIONAL`.
permalink: /134/update-mask-optional-behavior
redirect_from:
- /0134/update-mask-optional-behavior
---

# Update methods: Mask field expected behavior

This rule enforces that the `update_mask` field of a Standard `Update` request
uses `google.api.field_behavior = OPTIONAL`, as mandated in [AIP-134][].

## Details

This rule looks at any field named `update_mask` that's in an `Update*Request`
and complains if the field is not annotated with
`google.api.field_behavior = OPTIONAL`.

## Examples

**Incorrect** code for this rule:

```proto
message UpdateBookRequest {
Book book = 1;
// Incorrect. Must be `OPTIONAL`.
google.protobuf.FieldMask update_mask = 2 [
(google.api.field_behavior) = REQUIRED
];
}
```


**Correct** code for this rule:

```proto
message UpdateBookRequest {
Book book = 1;
// Correct.
google.protobuf.FieldMask update_mask = 2 [
(google.api.field_behavior) = OPTIONAL
];
}
```

## 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
message UpdateBookRequest {
Book book = 1;
// (-- api-linter: core::0134::update-mask-optional-behavior=disabled
// aip.dev/not-precedent: We need to do this because reasons. --)
google.protobuf.FieldMask update_mask = 2 [
(google.api.field_behavior) = REQUIRED
];
}
```

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
6 changes: 6 additions & 0 deletions locations/field_locations.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ func FieldResourceReference(f *desc.FieldDescriptor) *dpb.SourceCodeInfo_Locatio
return pathLocation(f, 8, int(apb.E_ResourceReference.TypeDescriptor().Number())) // FieldDescriptor.options == 8
}

// FieldBehavior returns the precise location for a field's
// field_behavior annotation.
func FieldBehavior(f *desc.FieldDescriptor) *dpb.SourceCodeInfo_Location {
return pathLocation(f, 8, int(apb.E_FieldBehavior.TypeDescriptor().Number())) // FieldDescriptor.options == 8
}

// FieldType returns the precise location for a field's type.
func FieldType(f *desc.FieldDescriptor) *dpb.SourceCodeInfo_Location {
if f.GetMessageType() != nil || f.GetEnumType() != nil {
Expand Down
1 change: 1 addition & 0 deletions rules/aip0134/aip0134.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func AddRules(r lint.RuleRegistry) error {
responseLRO,
synonyms,
unknownFields,
updateMaskOptionalBehavior,
)
}

Expand Down
42 changes: 42 additions & 0 deletions rules/aip0134/update_mask_optional_behavior.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright 2024 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 updateMaskOptionalBehavior = &lint.FieldRule{
Name: lint.NewRuleName(134, "update-mask-optional-behavior"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
return f.GetName() == "update_mask" && utils.IsUpdateRequestMessage(f.GetOwner())
},
LintField: func(f *desc.FieldDescriptor) []lint.Problem {
behaviors := utils.GetFieldBehavior(f)
if !behaviors.Contains("OPTIONAL") {
return []lint.Problem{
{
Message: "Standard Update field `update_mask` must have `OPTIONAL` behavior",
Descriptor: f,
Location: locations.FieldBehavior(f),
},
}
}
return nil
},
}
51 changes: 51 additions & 0 deletions rules/aip0134/update_mask_optional_behavior_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright 2024 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 TestUpdateMaskOptionalBehavior(t *testing.T) {
tests := []struct {
name string
Behavior string
problems testutils.Problems
}{
{"Valid", "[(google.api.field_behavior) = OPTIONAL]", nil},
{"InvalidWrong", "[(google.api.field_behavior) = REQUIRED]", testutils.Problems{{Message: "must have `OPTIONAL`"}}},
{"InvalidMissing", "", testutils.Problems{{Message: "must have `OPTIONAL`"}}},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
file := testutils.ParseProto3Tmpl(t, `
import "google/api/field_behavior.proto";
import "google/protobuf/field_mask.proto";
message UpdateBookRequest {
Book book = 1;
google.protobuf.FieldMask update_mask = 2 {{.Behavior}};
}
message Book {}
`, test)
field := file.GetMessageTypes()[0].FindFieldByName("update_mask")
problems := updateMaskOptionalBehavior.Lint(file)
if diff := test.problems.SetDescriptor(field).Diff(problems); diff != "" {
t.Errorf(diff)
}
})
}
}
2 changes: 1 addition & 1 deletion rules/aip0203/resource_name_identifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ var resourceNameIdentifier = &lint.MessageRule{
return []lint.Problem{{
Message: "resource name field must have field_behavior IDENTIFIER",
Descriptor: f,
Location: locations.FieldOption(f, fpb.E_FieldBehavior),
Location: locations.FieldBehavior(f),
}}
}

Expand Down

0 comments on commit 9fc0d05

Please sign in to comment.