Repository: googleapis/api-linter
Pull request #1165 (2023-06-01 01:20:52)

fix(AIP-133): compare types instead for message fields
Change the filter for the resource message field to compare the field type to the response type (which should be the resource for a Standard Create), instead of a case sensitive derivation of the resource type name or singular name.

Add some tests for when `singular` is set.

Should be a fix for item 2 of http://yaqs/4607975166683643904 (internal link)


Patch:
diff --git a/rules/aip0133/request_required_fields.go b/rules/aip0133/request_required_fields.go
index c244736..6ba0229 100644
--- a/rules/aip0133/request_required_fields.go
+++ b/rules/aip0133/request_required_fields.go
@@ -45,8 +45,9 @@ var requestRequiredFields = &lint.MethodRule{
 			if !utils.GetFieldBehavior(f).Contains("REQUIRED") {
 				continue
 			}
-			// Skip the check with the field that is the body.
-			if t := f.GetMessageType(); t != nil && t.GetName() == resourceMsgName {
+			// Skip the check with the field that is the resource, which for
+			// Standard Create, is the output type.
+			if t := f.GetMessageType(); t != nil && t.GetName() == ot.GetName() {
 				continue
 			}
 			// Iterate remaining fields. If they're not in the allowed list,


Test patch:
diff --git a/rules/aip0133/request_required_fields_test.go b/rules/aip0133/request_required_fields_test.go
index c847638..2fe1ffc 100644
--- a/rules/aip0133/request_required_fields_test.go
+++ b/rules/aip0133/request_required_fields_test.go
@@ -26,24 +26,42 @@ func TestRequiredFieldTests(t *testing.T) {
 		name                 string
 		Fields               string
 		problematicFieldName string
+		Singular             string
 		problems             testutils.Problems
 	}{
 		{
 			"ValidNoExtraFields",
 			"",
 			"",
+			"",
+			nil,
+		},
+		{
+			"ValidWithSingularNoExtraFields",
+			"",
+			"",
+			"bookShelf",
+			nil,
+		},
+		{
+			"ValidWithSingularAndIdField",
+			"string book_shelf_id = 3 [(google.api.field_behavior) = OPTIONAL];",
+			"",
+			"bookShelf",
 			nil,
 		},
 		{
 			"ValidOptionalValidateOnly",
 			"string validate_only = 3 [(google.api.field_behavior) = OPTIONAL];",
 			"validate_only",
+			"",
 			nil,
 		},
 		{
 			"InvalidRequiredValidateOnly",
 			"bool validate_only = 3 [(google.api.field_behavior) = REQUIRED];",
 			"validate_only",
+			"",
 			testutils.Problems{
 				{Message: `Create RPCs must only require fields explicitly described in AIPs, not "validate_only"`},
 			},
@@ -52,10 +70,20 @@ func TestRequiredFieldTests(t *testing.T) {
 			"InvalidRequiredUnknownField",
 			"bool create_iam = 3 [(google.api.field_behavior) = REQUIRED];",
 			"create_iam",
+			"",
 			testutils.Problems{
 				{Message: `Create RPCs must only require fields explicitly described in AIPs, not "create_iam"`},
 			},
 		},
+		{
+			"InvalidRequiredUnknownMessageField",
+			"Foo foo = 3 [(google.api.field_behavior) = REQUIRED];",
+			"foo",
+			"",
+			testutils.Problems{
+				{Message: `Create RPCs must only require fields explicitly described in AIPs, not "foo"`},
+			},
+		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			f := testutils.ParseProto3Tmpl(t, `
@@ -75,10 +103,13 @@ func TestRequiredFieldTests(t *testing.T) {
 					option (google.api.resource) = {
 						type: "library.googleapis.com/BookShelf"
 						pattern: "publishers/{publisher}/bookShelves/{book_shelf}"
+						singular: "{{.Singular}}"
 					};
 					string name = 1;
 				}
 
+				message Foo {}
+
 				message CreateBookShelfRequest {
 					string parent = 1 [
 						(google.api.field_behavior) = REQUIRED