Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AIP-0231 Batch methods: Get #260

Merged
merged 7 commits into from
Oct 29, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require (
github.com/golang/protobuf v1.3.2
github.com/google/go-cmp v0.3.1
github.com/jhump/protoreflect v1.5.1-0.20191024142022-1d0024204e58
github.com/kjk/inflect v0.0.0-20190213232030-bfc9dcd6393b
michaelwq marked this conversation as resolved.
Show resolved Hide resolved
github.com/lithammer/dedent v1.1.0
github.com/stoewer/go-strcase v1.0.2
github.com/urfave/cli v1.22.1
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ github.com/jhump/protoreflect v1.5.0 h1:NgpVT+dX71c8hZnxHof2M7QDK7QtohIJ7DYycjnk
github.com/jhump/protoreflect v1.5.0/go.mod h1:eaTn3RZAmMBcV0fifFvlm6VHNz3wSkYyXYWUh7ymB74=
github.com/jhump/protoreflect v1.5.1-0.20191024142022-1d0024204e58 h1:RHSRszt3DIDwIwV1fPXtZI74OL36BkDIzsp2IOdp+Xc=
github.com/jhump/protoreflect v1.5.1-0.20191024142022-1d0024204e58/go.mod h1:eaTn3RZAmMBcV0fifFvlm6VHNz3wSkYyXYWUh7ymB74=
github.com/kjk/inflect v0.0.0-20190213232030-bfc9dcd6393b h1:pSKZGMcdvyNmiHxmVni0zd28IL+xf+kKAixrsMdS3Mg=
github.com/kjk/inflect v0.0.0-20190213232030-bfc9dcd6393b/go.mod h1:dI+lsL14MLXYhREARMljP6+dati5FyM0eg9QEMUdUUU=
github.com/lithammer/dedent v1.1.0 h1:VNzHMVCBNG1j0fh3OrsFRkVUwStdDArbgBWoPAffktY=
github.com/lithammer/dedent v1.1.0/go.mod h1:jrXYCQtgg0nJiN+StA2KgR7w6CiQNv9Fd/Z9BP0jIOc=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
Expand Down
90 changes: 90 additions & 0 deletions rules/aip0231/aip0231.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// Copyright 2019 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 aip0231 contains rules defined in https://aip.dev/231.
package aip0231

import (
"regexp"

"github.com/googleapis/api-linter/lint"
"github.com/jhump/protoreflect/desc"
)

// AddRules accepts a register function and registers each of
// this AIP's rules to it.
func AddRules(r lint.RuleRegistry) {
r.Register(
pluralMethodResourceName,
inputName,
outputName,
httpBody,
httpUrl,
httpVerb,
parentField,
namesField,
resourceField,
)
}

var batchGetMethodRegexp = regexp.MustCompile("^BatchGet(?:[A-Za-z0-9]|$)")
var batchGetReqMessageRegexp = regexp.MustCompile("^BatchGet[A-Za-z0-9]*Request$")
var batchGetResMessageRegexp = regexp.MustCompile("^BatchGet[A-Za-z0-9]*Response$")
var batchGetURINameRegexp = regexp.MustCompile("\\{[a-zA-Z=/*]+\\}\\/[A-Za-z0-9-=/*]+:batchGet$")
var getReqMessageRegexp = regexp.MustCompile("^Get[A-Za-z0-9]*Request$")

// Returns true if this is a AIP-231 Get method, false otherwise.
func isBatchGetMethod(m *desc.MethodDescriptor) bool {
return batchGetMethodRegexp.MatchString(m.GetName())
}

// Returns true if this is an AIP-231 Get request message, false otherwise.
func isBatchGetRequestMessage(m *desc.MessageDescriptor) bool {
return batchGetReqMessageRegexp.MatchString(m.GetName())
}

// Returns true if this is an AIP-231 Get request message, false otherwise.
func isBatchGetResponseMessage(m *desc.MessageDescriptor) bool {
return batchGetResMessageRegexp.MatchString(m.GetName())
}

//// get resource message type name from method
michaelwq marked this conversation as resolved.
Show resolved Hide resolved
//func getResourceMsgName(m *desc.MethodDescriptor) string {
// // Usually the response message will be the resource message, and its name will
// // be part of method name (make a double check here to avoid the issue when
// // method or output naming doesn't follow the right principles)
// if strings.Contains(m.GetName()[6:], m.GetOutputType().GetName()) {
// return m.GetOutputType().GetName()
// }
// return m.GetName()[6:]
//}
//
//// get resource message type name from request message
//func getResourceMsgNameFromReq(m *desc.MessageDescriptor) string {
// // retrieve the string between the prefix "Create" and suffix "Request" from
// // the name "Create<XXX>Request", and this part will usually be the resource
// // message name(if its naming follows the right principle)
// resourceMsgName := m.GetName()[6 : len(m.GetName())-7]
//
// // Get the resource field of the request message if it exist, this part will
// // be exactly the resource message name (make a double check here to avoid the
// // issues when request message naming doesn't follow the right principles)
// for _, fieldDesc := range m.GetFields() {
// if msgDesc := fieldDesc.GetMessageType(); msgDesc != nil && strings.Contains(resourceMsgName, msgDesc.GetName()) {
// resourceMsgName = msgDesc.GetName()
// }
// }
//
// return resourceMsgName
//}
32 changes: 32 additions & 0 deletions rules/aip0231/aip0231_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright 2019 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 aip0231

import (
"strings"
"testing"

"github.com/googleapis/api-linter/lint"
)

func TestAddRules(t *testing.T) {
rules := make(lint.RuleRegistry)
AddRules(rules)
for ruleName := range rules {
if !strings.HasPrefix(string(ruleName), "core::0231") {
t.Errorf("Rule %s is not namespaced to core::0231.", ruleName)
}
}
}
143 changes: 143 additions & 0 deletions rules/aip0231/message_rules.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
// Copyright 2019 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 aip0231

import (
"fmt"

"github.com/googleapis/api-linter/lint"
"github.com/jhump/protoreflect/desc"
"github.com/jhump/protoreflect/desc/builder"
"github.com/kjk/inflect"
)

// The Batch Get request message should have parent field.
var parentField = &lint.MessageRule{
Name: lint.NewRuleName("core", "0231", "request-message", "parent-field"),
URI: "https://aip.dev/231#request-message",
OnlyIf: isBatchGetRequestMessage,
LintMessage: func(m *desc.MessageDescriptor) []lint.Problem {
// Rule check: Establish that a `parent` field is present.
parentField := m.FindFieldByName("parent")
if parentField == nil {
return []lint.Problem{{
Message: fmt.Sprintf("Message %q has no `parent` field", m.GetName()),
Descriptor: m,
}}
}

// Rule check: Establish that the parent field is a string.
if parentField.GetType() != builder.FieldTypeString().GetType() {
return []lint.Problem{{
Message: "`parent` field on create request message must be a string",
Descriptor: parentField,
}}
}

return nil
},
}

// The Batch Get standard method should have repeated name field or repeated
// standard get request message field, but the latter one is not suggested.
var namesField = &lint.MessageRule{
Name: lint.NewRuleName("core", "0231", "request-message", "name-field"),
URI: "https://aip.dev/231#request-message",
OnlyIf: isBatchGetRequestMessage,
LintMessage: func(m *desc.MessageDescriptor) (problems []lint.Problem) {
// Rule check: Establish that a name field is present.
names := m.FindFieldByName("names")
getReqMsg := m.FindFieldByName("requests")

// Rule check: Ensure that the names field is existed.
if names == nil && getReqMsg == nil {
problems = append(problems, lint.Problem{
Message: fmt.Sprintf(`Message %q has no "names" field`, m.GetName()),
Descriptor: m,
})
}

// Rule check: Ensure that only the suggested names field is existed.
if names != nil && getReqMsg != nil {
problems = append(problems, lint.Problem{
Message: fmt.Sprintf(`Message %q should delete "requests" field, only keep the "names" field`, m.GetName()),
Descriptor: getReqMsg,
})
}

// Rule check: Ensure that the names field is repeated.
if names != nil && !names.IsRepeated() {
problems = append(problems, lint.Problem{
Message: `The "names"" field should be repeated`,
Descriptor: names,
})
}

// Rule check: Ensure that the names field is the correct type.
if names != nil && names.GetType() != builder.FieldTypeString().GetType() {
problems = append(problems, lint.Problem{
Message: `"names" field on Batch Get Request should be a "string" type`,
Descriptor: names,
})
}

// Rule check: Ensure that the standard get request message field is repeated.
if getReqMsg != nil && !getReqMsg.IsRepeated() {
problems = append(problems, lint.Problem{
Message: `The "requests" field should be repeated`,
Descriptor: getReqMsg,
})
}
// Rule check: Ensure that the standard get request message field is the correct type.
rightTypeName := fmt.Sprintf("Get%sRequest", inflect.ToSingular(m.GetName()[8:len(m.GetName())-7]))
michaelwq marked this conversation as resolved.
Show resolved Hide resolved
if getReqMsg != nil && (getReqMsg.GetMessageType() == nil || getReqMsg.GetMessageType().GetName() != rightTypeName) {
problems = append(problems, lint.Problem{
Message: fmt.Sprintf(`The "requests" field on Batch Get Request should be a %q type`, rightTypeName),
Descriptor: getReqMsg,
})
}
return
},
}

// The Batch Get response message should have resource field.
var resourceField = &lint.MessageRule{
Name: lint.NewRuleName("core", "0231", "response-message", "resource-field"),
URI: "https://aip.dev/231#response-message",
OnlyIf: isBatchGetResponseMessage,
LintMessage: func(m *desc.MessageDescriptor) []lint.Problem {
// the singular form the resource name, the first letter is Capitalized
resourceMsgName := inflect.ToSingular(m.GetName()[8 : len(m.GetName())-9])
michaelwq marked this conversation as resolved.
Show resolved Hide resolved

for _, fieldDesc := range m.GetFields() {
if msgDesc := fieldDesc.GetMessageType(); msgDesc != nil && msgDesc.GetName() == resourceMsgName {
if !fieldDesc.IsRepeated() {
return []lint.Problem{{
Message: fmt.Sprintf("The %q type field on Batch Get Response message should be repeated", msgDesc.GetName()),
Descriptor: fieldDesc,
}}
} else {
michaelwq marked this conversation as resolved.
Show resolved Hide resolved
return nil
}
}
}

// Rule check: Establish that a resource field must be included.
return []lint.Problem{{
Message: fmt.Sprintf("Message %q has no %q type field", m.GetName(), resourceMsgName),
Descriptor: m,
}}
},
}
Loading