diff --git a/rules/aip0134/request_allow_missing_field.go b/rules/aip0134/request_allow_missing_field.go index 76768379f..0fe528499 100644 --- a/rules/aip0134/request_allow_missing_field.go +++ b/rules/aip0134/request_allow_missing_field.go @@ -23,7 +23,11 @@ import ( var allowMissing = &lint.MessageRule{ Name: lint.NewRuleName(134, "request-allow-missing-field"), OnlyIf: func(m *desc.MessageDescriptor) bool { - return isUpdateRequestMessage(m) && utils.IsDeclarativeFriendlyMessage(m) + if !isUpdateRequestMessage(m) { + return false + } + r := utils.DeclarativeFriendlyResource(m) + return r != nil && !utils.IsSingletonResource(r) }, LintMessage: func(m *desc.MessageDescriptor) []lint.Problem { for _, field := range m.GetFields() { diff --git a/rules/aip0134/request_allow_missing_field_test.go b/rules/aip0134/request_allow_missing_field_test.go index 095880b1d..c5de83068 100644 --- a/rules/aip0134/request_allow_missing_field_test.go +++ b/rules/aip0134/request_allow_missing_field_test.go @@ -21,18 +21,22 @@ import ( ) func TestAllowMissing(t *testing.T) { + const singletonPattern = `books/{book}/settings` + const nonSingletonPattern = `books/{book}` problems := testutils.Problems{{Message: "include a singular `bool allow_missing`"}} for _, test := range []struct { name string Style string + Pattern string AllowMissing string problems testutils.Problems }{ - {"IgnoredNotDF", "", "", nil}, - {"ValidIncluded", "style: DECLARATIVE_FRIENDLY", "bool allow_missing = 2;", nil}, - {"Invalid", "style: DECLARATIVE_FRIENDLY", "", problems}, - {"InvalidWrongType", "style: DECLARATIVE_FRIENDLY", "string allow_missing = 2;", problems}, - {"InvalidRepeated", "style: DECLARATIVE_FRIENDLY", "repeated bool allow_missing = 2;", problems}, + {"IgnoredNotDF", "", nonSingletonPattern, "", nil}, + {"ValidIncluded", "style: DECLARATIVE_FRIENDLY", nonSingletonPattern, "bool allow_missing = 2;", nil}, + {"Invalid", "style: DECLARATIVE_FRIENDLY", nonSingletonPattern, "", problems}, + {"InvalidWrongType", "style: DECLARATIVE_FRIENDLY", nonSingletonPattern, "string allow_missing = 2;", problems}, + {"InvalidRepeated", "style: DECLARATIVE_FRIENDLY", nonSingletonPattern, "repeated bool allow_missing = 2;", problems}, + {"IgnoredSingleton", "style: DECLARATIVE_FRIENDLY", singletonPattern, "", nil}, } { t.Run(test.name, func(t *testing.T) { f := testutils.ParseProto3Tmpl(t, ` @@ -50,6 +54,7 @@ func TestAllowMissing(t *testing.T) { message Book { option (google.api.resource) = { type: "library.googleapis.com/Book" + pattern: "{{.Pattern}}" {{.Style}} }; } diff --git a/rules/aip0148/declarative_friendly_fields.go b/rules/aip0148/declarative_friendly_fields.go index d55db53fe..49d74c98a 100644 --- a/rules/aip0148/declarative_friendly_fields.go +++ b/rules/aip0148/declarative_friendly_fields.go @@ -33,7 +33,7 @@ var declarativeFriendlyRequired = &lint.MessageRule{ return false }, LintMessage: func(m *desc.MessageDescriptor) []lint.Problem { - singleton := isSingletonResource(m) + singleton := utils.IsSingletonResource(m) // Define the fields that are expected. missingFields := stringset.New() for name, typ := range reqFields { @@ -79,17 +79,3 @@ var singletonExceptions = stringset.New( "delete_time", "uid", ) - -func isSingletonResource(m *desc.MessageDescriptor) bool { - // If the pattern ends in something other than "}", that indicates that this is a singleton. - // - // For example: - // publishers/{publisher}/books/{book} -- not a singleton, many books - // publishers/*/settings -- a singleton; one settings object per publisher - for _, pattern := range utils.GetResource(m).GetPattern() { - if !strings.HasSuffix(pattern, "}") { - return true - } - } - return false -} diff --git a/rules/internal/utils/extension.go b/rules/internal/utils/extension.go index ff3646e6a..4c0412c14 100644 --- a/rules/internal/utils/extension.go +++ b/rules/internal/utils/extension.go @@ -80,6 +80,22 @@ func IsResource(m *desc.MessageDescriptor) bool { return false } +// IsSingletonResource returns true if the given message is a singleton +// resource according to its pattern. +func IsSingletonResource(m *desc.MessageDescriptor) bool { + // If the pattern ends in something other than "}", that indicates that this is a singleton. + // + // For example: + // publishers/{publisher}/books/{book} -- not a singleton, many books + // publishers/*/settings -- a singleton; one settings object per publisher + for _, pattern := range GetResource(m).GetPattern() { + if !strings.HasSuffix(pattern, "}") { + return true + } + } + return false +} + // GetResourceDefinitions returns the google.api.resource_definition annotations // for a file. func GetResourceDefinitions(f *desc.FileDescriptor) []*apb.ResourceDescriptor {