Skip to content

Commit

Permalink
fix(AIP-4232): support nested field of required field (#1339)
Browse files Browse the repository at this point in the history
  • Loading branch information
noahdietz authored Feb 20, 2024
1 parent f8127d7 commit e86a159
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 2 deletions.
9 changes: 8 additions & 1 deletion rules/aip4232/required_fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,14 @@ var requiredFields = &lint.MethodRule{
}
}
for i, sig := range sigs {
sigset := stringset.New(sig...)
sigset := stringset.New(sig...).Map(func(s string) string {
if !strings.Contains(s, ".") {
return s
}
// If signature contains nested fields, get the root field
// to use in top-level required field check.
return strings.Split(s, ".")[0]
})
if !sigset.Contains(requiredFields...) {
problems = append(problems, lint.Problem{
Message: fmt.Sprintf("Method signature %q missing at least one of the required fields: %q", strings.Join(sig, ","), strings.Join(requiredFields, ",")),
Expand Down
8 changes: 7 additions & 1 deletion rules/aip4232/required_fields_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ func TestRequiredFields(t *testing.T) {
FieldBehavior string
problems testutils.Problems
}{
{"Valid", "name,paperback_only", "REQUIRED", nil},
{"Valid", "name,paperback_only,shelf", "REQUIRED", nil},
{"ValidNested", "name,paperback_only,shelf.name", "REQUIRED", nil},
{"ValidNotRequired", "paperback_only", "OPTIONAL", nil},
{"Invalid", "paperback_only", "REQUIRED", testutils.Problems{{Message: "missing at least one"}}},
}
Expand All @@ -46,8 +47,13 @@ func TestRequiredFields(t *testing.T) {
string name = 1 [(google.api.field_behavior) = {{.FieldBehavior}}];
bool paperback_only = 2;
Shelf shelf = 3 [(google.api.field_behavior) = {{.FieldBehavior}}];
}
message ArchiveBookResponse {}
message Shelf {
string name = 1;
}
`, test)
method := f.GetServices()[0].GetMethods()[0]
if diff := test.problems.SetDescriptor(method).Diff(requiredFields.Lint(f)); diff != "" {
Expand Down

0 comments on commit e86a159

Please sign in to comment.