Edits to comply with code review

This commit is contained in:
Leland Batey 2016-10-14 16:41:15 -07:00
Родитель 79ecc77422
Коммит f69d8be272
4 изменённых файлов: 95 добавлений и 50 удалений

Просмотреть файл

@ -11,13 +11,13 @@ import (
"github.com/pkg/errors"
)
// ConsolidateHTTP accepts a SvcDef and the io.Readers for the proto files
// consolidateHTTP accepts a SvcDef and the io.Readers for the proto files
// comprising the definition. It modifies the SvcDef so that HTTPBindings and
// their associated HTTPParamters are added to each ServiceMethod. After this,
// each `HTTPBinding` will have a populated list of all the http parameters
// that that binding requires, where that parameter should be located, and the
// type of each parameter.
func ConsolidateHTTP(sd *Svcdef, protoFiles []io.Reader) error {
func consolidateHTTP(sd *Svcdef, protoFiles []io.Reader) error {
for _, pfile := range protoFiles {
lex := svcparse.NewSvcLexer(pfile)
protosvc, err := svcparse.ParseService(lex)
@ -35,6 +35,9 @@ func ConsolidateHTTP(sd *Svcdef, protoFiles []io.Reader) error {
return nil
}
// assembleHTTPParams will use the output of the service parser to create
// HTTPParams for each service RequestType field indicating that parameters
// location, and the field to which it refers.
func assembleHTTPParams(svc *Service, httpsvc *svcparse.Service) error {
getMethNamed := func(name string) *ServiceMethod {
for _, m := range svc.Methods {
@ -45,6 +48,8 @@ func assembleHTTPParams(svc *Service, httpsvc *svcparse.Service) error {
return nil
}
// This logic has been broken out of the for loop below to flatten
// this function and avoid difficult to read nesting
createParams := func(meth *ServiceMethod, parsedbind *svcparse.HTTPBinding) {
msg := meth.RequestType.Message
bind := HTTPBinding{}
@ -61,6 +66,8 @@ func assembleHTTPParams(svc *Service, httpsvc *svcparse.Service) error {
meth.Bindings = append(meth.Bindings, &bind)
}
// Iterate through every HTTPBinding on every ServiceMethod, and create the
// HTTPParameters for that HTTPBinding.
for _, hm := range httpsvc.Methods {
m := getMethNamed(hm.Name)
if m == nil {

Просмотреть файл

@ -1,19 +1,28 @@
package svcdef
// After the initial creation of Svcdef, each FieldType will have it's name
// set to the name of the type that it represents, but it won't have a pointer
// to the full instance of the type with that name. Type resolution is the
// process of taking the name of each FieldType, then searching for the
// enum/message with that name and setting the respective *Message/*Enum
// property of our FieldType to point to that enum/message.
// typeBox holds either a Message or an Enum; used only in resolveTypes() to
// associate FieldTypes with their underlying data.
type typeBox struct {
Message *Message
Enum *Enum
}
// resolveTypes accepts a pointer to a Svcdef and modifies that Svcdef, and
// it's child structs, in place.
// newTypeMap returns a map of type (Message/Enum) Names to correlated
// typeBoxes
func newTypeMap(sd *Svcdef) map[string]typeBox {
rv := make(map[string]typeBox)
for _, m := range sd.Messages {
rv[m.Name] = typeBox{Message: m}
}
for _, e := range sd.Enums {
rv[e.Name] = typeBox{Enum: e}
}
return rv
}
// resolveTypes sets the underlying types for all of our service methods
// Fields, RequestTypes, and ResponseTypes. Since Enums have no Fields which
// may refer to other types, they are ignored by resolveTypes.
func resolveTypes(sd *Svcdef) {
tmap := newTypeMap(sd)
for _, m := range sd.Messages {
@ -29,6 +38,7 @@ func resolveTypes(sd *Svcdef) {
}
}
// setType unpacks typeBox value into corresponding FieldType
func setType(f *FieldType, tmap map[string]typeBox) {
// Special case maps with valuetypes pointing to messages
if f.Map != nil {
@ -47,15 +57,3 @@ func setType(f *FieldType, tmap map[string]typeBox) {
f.Message = entry.Message
}
}
// newTypeMap returns a map from
func newTypeMap(sd *Svcdef) map[string]typeBox {
rv := make(map[string]typeBox)
for _, m := range sd.Messages {
rv[m.Name] = typeBox{Message: m}
}
for _, e := range sd.Enums {
rv[e.Name] = typeBox{Enum: e}
}
return rv
}

Просмотреть файл

@ -1,6 +1,20 @@
package svcdef
/*
Package svcdef provides a straightforward view of the Go code for a gRPC
service defined using protocol buffers. Information is distilled from the
.proto files comprising the definition, as well as the Go source files
generated from those same .proto files.
// consider renaming to census, pbregistry, or just to "essence"
Since svcdef is only meant to be used to generate Go code, svcdef has a limited
view of the definition of the gRPC service.
Additionally, since svcdef only parses Go code generated by protoc-gen-go, all
methods accept only ast types with structures created by protoc-gen-go. See
NewTYPE functions such as NewMap for details on the relevant conventions.
Note that svcdef does not support embedding sub-fields of nested messages into
the path of an HTTP annotation.
*/
package svcdef
import (
"go/ast"
@ -12,6 +26,7 @@ import (
"github.com/pkg/errors"
)
// Svcdef is the top-level struct for the definition of a service.
type Svcdef struct {
// PkgName will be the pacakge name of the go file(s) analyzed. So if a
// Go file contained "package authz", then PkgName will be "authz". If
@ -24,6 +39,7 @@ type Svcdef struct {
Service *Service
}
// Message represents a protobuf Message, though greatly simplified.
type Message struct {
Name string
Fields []*Field
@ -34,7 +50,7 @@ type Enum struct {
}
type Map struct {
Name string
// KeyType will always be a basetype, e.g. string, int64, etc.
KeyType *FieldType
ValueType *FieldType
}
@ -53,11 +69,15 @@ type ServiceMethod struct {
Bindings []*HTTPBinding
}
// Field represents a field on a protobuf message.
type Field struct {
Name string
Type *FieldType
}
// FieldType contains information about the type of one Field on a message,
// such as if that Field is a slice or if it's a pointer, as well as a
// reference to the definition of the type of this Field.
type FieldType struct {
// Name will contain the name of the type, for example "string" or "bool"
Name string
@ -82,7 +102,8 @@ type FieldType struct {
type HTTPBinding struct {
Verb string
Path string
// There is one HTTPParamter for each of the parent service methods Fields.
// There is one HTTPParamter for each of the fields on parent service
// methods RequestType.
Params []*HTTPParameter
}
@ -112,6 +133,8 @@ func retrieveTypeSpecs(f *ast.File) ([]*ast.TypeSpec, error) {
return rv, nil
}
// New creates a Svcdef by parsing the provided Go and Protobuf source files to
// derive type information, gRPC service data, and HTTP annotations.
func New(goFiles []io.Reader, protoFiles []io.Reader) (*Svcdef, error) {
rv := Svcdef{}
@ -124,7 +147,6 @@ func New(goFiles []io.Reader, protoFiles []io.Reader) (*Svcdef, error) {
typespecs, _ := retrieveTypeSpecs(fileAst)
for _, t := range typespecs {
//sp.Dump(t)
switch typdf := t.Type.(type) {
case *ast.Ident:
if typdf.Name == "int32" {
@ -157,7 +179,7 @@ func New(goFiles []io.Reader, protoFiles []io.Reader) (*Svcdef, error) {
}
}
resolveTypes(&rv)
err := ConsolidateHTTP(&rv, protoFiles)
err := consolidateHTTP(&rv, protoFiles)
if err != nil {
return nil, errors.Wrap(err, "failed to consolidate HTTP")
}
@ -165,7 +187,6 @@ func New(goFiles []io.Reader, protoFiles []io.Reader) (*Svcdef, error) {
return &rv, nil
}
// NewEnum returns a new Enum struct derived from an *ast.TypeSpec
func NewEnum(e *ast.TypeSpec) (*Enum, error) {
return &Enum{
Name: e.Name.Name,
@ -231,7 +252,7 @@ func NewMap(m ast.Expr) (*Map, error) {
}
// NewService returns a new Service struct derived from an *ast.TypeSpec with a
// Type of *ast.InterfaceType.
// Type of *ast.InterfaceType representing an "{SVCNAME}Client" interface.
func NewService(s *ast.TypeSpec) (*Service, error) {
rv := &Service{
Name: s.Name.Name,
@ -247,10 +268,9 @@ func NewService(s *ast.TypeSpec) (*Service, error) {
return rv, nil
}
// NewServiceMethod returns a new ServiceMethod derived from the provided
// *ast.Field. The given *ast.Field is intended to have a Type of *ast.FuncType
// from an *ast.InterfaceType's Methods.List attribute. Providing an *ast.Field
// with a different structure may return an error.
// NewServiceMethod returns a new ServiceMethod derived from a method of a
// Service interface. This is accepted in the form of an *ast.Field which
// contains the name of the method.
func NewServiceMethod(m *ast.Field) (*ServiceMethod, error) {
rv := &ServiceMethod{
Name: m.Names[0].Name,
@ -264,7 +284,14 @@ func NewServiceMethod(m *ast.Field) (*ServiceMethod, error) {
output := ft.Results.List
// Zero'th param of a serverMethod is Context.context, while first param is
// this methods RequestType
// this methods RequestType. Example:
//
// GetMap(context.Context, *MapTypeRequest) (*MapTypeResponse, error)
// └────────────┘ └─────────────┘
// RequestType ResponseType
// └──────────────────────────────┘ └─────────────────────┘
// input output
rq := input[1]
rs := output[0]
@ -297,7 +324,8 @@ func NewServiceMethod(m *ast.Field) (*ServiceMethod, error) {
}
// NewField returns a Field struct with information distilled from an
// *ast.Field.
// *ast.Field. If the provided *ast.Field does not match the conventions of
// code generated by protoc-gen-go, an error will be returned.
func NewField(f *ast.Field) (*Field, error) {
// The following is an informational table of how the proto-to-go
// concepts map to the Types of an ast.Field. An arrow indicates "nested

Просмотреть файл

@ -7,7 +7,7 @@ import (
"testing"
)
func TestNewCatalog(t *testing.T) {
func TestSvcdef(t *testing.T) {
gf, err := os.Open("./test-go.txt")
if err != nil {
t.Fatal(err)
@ -97,17 +97,17 @@ type NestedTypeRequest struct {
}
}
// Test that type resolution of map values functions correctly. So if a message
// has a map field, and that map field has values that are some other message
// type, then the type of the key will be correct.
func TestMapTypeResolution(t *testing.T) {
// Test that after calling New(), type resolution of map values functions
// correctly. So if a message has a map field, and that map field has values
// that are some other message type, then the type of the key will be correct.
func TestNewMapTypeResolution(t *testing.T) {
caseCode := `
package TEST
type NestedMessageC struct {
A int64
}
type MapNestedMsg struct {
type MsgWithMap struct {
Beta map[int64]*NestedMessageC
}
`
@ -115,16 +115,28 @@ type MapNestedMsg struct {
if err != nil {
t.Fatal(err)
}
tmap := newTypeMap(sd)
expected, ok := tmap["MapNestedMsg"]
if !ok {
t.Fatal("Couldn't find message 'MapNestedMsg'")
// findMsg defined here for brevity
findMsg := func(name string) *Message {
for _, m := range sd.Messages {
if m.Name == name {
return m
}
}
return nil
}
beta := expected.Message.Fields[0].Type.Map
msg := findMsg("MsgWithMap")
if msg == nil {
t.Fatal("Couldn't find message 'MsgWithMap'")
}
expected := findMsg("NestedMessageC")
if expected == nil {
t.Fatal("Couldn't find message 'NestedMessageC'")
}
if beta.ValueType.Message != tmap["NestedMessageC"].Message {
beta := msg.Fields[0].Type.Map
if beta.ValueType.Message != expected {
t.Fatalf("Expected beta ValueType to be 'NestedMessageC', is %q", beta.ValueType.Message.Name)
}