From 08ddeac03fd3832d44a3dfb48ee85ecd95d2b388 Mon Sep 17 00:00:00 2001 From: Ondrej Fabry Date: Tue, 21 Aug 2018 22:32:11 +0200 Subject: Make the warnings for validating services more obvious - there is currently simple validation for services, which checks type of services and its name for request/reply - there is one known warning in dhcp package for dhcp_client_config, since it is single case for normal requests subscribing to event Change-Id: I504a52b9a1823ced841b2ead712318ef5e5477b1 Signed-off-by: Ondrej Fabry --- cmd/binapi-generator/generate.go | 35 ++++++------ cmd/binapi-generator/main.go | 21 +++++-- cmd/binapi-generator/objects.go | 120 +++++++++++++++++++++++++++++++++++++++ cmd/binapi-generator/parse.go | 118 ++++++-------------------------------- 4 files changed, 169 insertions(+), 125 deletions(-) create mode 100644 cmd/binapi-generator/objects.go diff --git a/cmd/binapi-generator/generate.go b/cmd/binapi-generator/generate.go index 251d39d..e29c84d 100644 --- a/cmd/binapi-generator/generate.go +++ b/cmd/binapi-generator/generate.go @@ -504,13 +504,7 @@ func generateService(ctx *context, w io.Writer, svc *Service) { reqTyp := camelCaseName(svc.RequestType) // method name is same as parameter type name by default - method := reqTyp - if svc.Stream { - // use Dump as prefix instead of suffix for stream services - if m := strings.TrimSuffix(method, "Dump"); method != m { - method = "Dump" + m - } - } + method := svc.MethodName() params := fmt.Sprintf("*%s", reqTyp) returns := "error" if replyTyp := camelCaseName(svc.ReplyType); replyTyp != "" { @@ -521,25 +515,28 @@ func generateService(ctx *context, w io.Writer, svc *Service) { } // generateMessageNameGetter generates getter for original VPP message name into the provider writer -func generateMessageNameGetter(w io.Writer, structName string, msgName string) { - fmt.Fprintln(w, "func (*"+structName+") GetMessageName() string {") - fmt.Fprintln(w, "\treturn \""+msgName+"\"") - fmt.Fprintln(w, "}") +func generateMessageNameGetter(w io.Writer, structName, msgName string) { + fmt.Fprintf(w, `func (*%s) GetMessageName() string { + return %q +} +`, structName, msgName) } // generateTypeNameGetter generates getter for original VPP type name into the provider writer -func generateTypeNameGetter(w io.Writer, structName string, msgName string) { - fmt.Fprintln(w, "func (*"+structName+") GetTypeName() string {") - fmt.Fprintln(w, "\treturn \""+msgName+"\"") - fmt.Fprintln(w, "}") +func generateTypeNameGetter(w io.Writer, structName, msgName string) { + fmt.Fprintf(w, `func (*%s) GetTypeName() string { + return %q +} +`, structName, msgName) } // generateCrcGetter generates getter for CRC checksum of the message definition into the provider writer -func generateCrcGetter(w io.Writer, structName string, crc string) { +func generateCrcGetter(w io.Writer, structName, crc string) { crc = strings.TrimPrefix(crc, "0x") - fmt.Fprintln(w, "func (*"+structName+") GetCrcString() string {") - fmt.Fprintln(w, "\treturn \""+crc+"\"") - fmt.Fprintln(w, "}") + fmt.Fprintf(w, `func (*%s) GetCrcString() string { + return %q +} +`, structName, crc) } // generateMessageTypeGetter generates message factory for the generated message into the provider writer diff --git a/cmd/binapi-generator/main.go b/cmd/binapi-generator/main.go index 8045212..b73a699 100644 --- a/cmd/binapi-generator/main.go +++ b/cmd/binapi-generator/main.go @@ -20,13 +20,13 @@ import ( "flag" "fmt" "io/ioutil" - "log" "os" "os/exec" "path/filepath" "strings" "github.com/bennyscetbun/jsongo" + "github.com/sirupsen/logrus" ) var ( @@ -38,15 +38,26 @@ var ( continueOnError = flag.Bool("continue-onerror", false, "Wheter to continue with next file on error.") ) +func init() { + flag.Parse() + if *debug { + logrus.SetLevel(logrus.DebugLevel) + } +} + func logf(f string, v ...interface{}) { if *debug { - log.Printf(f, v...) + logrus.Debugf(f, v...) } } -func main() { - flag.Parse() +var log = logrus.Logger{ + Level: logrus.InfoLevel, + Formatter: &logrus.TextFormatter{}, + Out: os.Stdout, +} +func main() { if *inputFile == "" && *inputDir == "" { fmt.Fprintln(os.Stderr, "ERROR: input-file or input-dir must be specified") os.Exit(1) @@ -143,7 +154,7 @@ func generateFromFile(inputFile, outputDir string) error { // count number of lines in generated output file cmd = exec.Command("wc", "-l", ctx.outputFile) if output, err := cmd.CombinedOutput(); err != nil { - log.Printf("wc command failed: %v\n%s", err, string(output)) + log.Warnf("wc command failed: %v\n%s", err, string(output)) } else { logf("generated lines: %s", output) } diff --git a/cmd/binapi-generator/objects.go b/cmd/binapi-generator/objects.go new file mode 100644 index 0000000..2681085 --- /dev/null +++ b/cmd/binapi-generator/objects.go @@ -0,0 +1,120 @@ +package main + +import "strings" + +// Package represents collection of objects parsed from VPP binary API JSON data +type Package struct { + APIVersion string + Enums []Enum + Unions []Union + Types []Type + Messages []Message + Services []Service + RefMap map[string]string +} + +// MessageType represents the type of a VPP message +type MessageType int + +const ( + requestMessage MessageType = iota // VPP request message + replyMessage // VPP reply message + eventMessage // VPP event message + otherMessage // other VPP message +) + +// Message represents VPP binary API message +type Message struct { + Name string + CRC string + Fields []Field +} + +// Type represents VPP binary API type +type Type struct { + Name string + CRC string + Fields []Field +} + +// Union represents VPP binary API union +type Union struct { + Name string + CRC string + Fields []Field +} + +// Field represents VPP binary API object field +type Field struct { + Name string + Type string + Length int + SizeFrom string +} + +func (f *Field) IsArray() bool { + return f.Length > 0 || f.SizeFrom != "" +} + +// Enum represents VPP binary API enum +type Enum struct { + Name string + Type string + Entries []EnumEntry +} + +// EnumEntry represents VPP binary API enum entry +type EnumEntry struct { + Name string + Value interface{} +} + +// Service represents VPP binary API service +type Service struct { + Name string + RequestType string + ReplyType string + Stream bool + Events []string +} + +func (s Service) MethodName() string { + reqTyp := camelCaseName(s.RequestType) + + // method name is same as parameter type name by default + method := reqTyp + if s.Stream { + // use Dump as prefix instead of suffix for stream services + if m := strings.TrimSuffix(method, "Dump"); method != m { + method = "Dump" + m + } + } + + return method +} + +func (s Service) IsDumpService() bool { + return s.Stream +} + +func (s Service) IsEventService() bool { + return len(s.Events) > 0 +} + +func (s Service) IsRequestService() bool { + // some binapi messages might have `null` reply (for example: memclnt) + return s.ReplyType != "" && s.ReplyType != "null" // not null +} + +func getSizeOfType(typ *Type) (size int) { + for _, field := range typ.Fields { + if n := getBinapiTypeSize(field.Type); n > 0 { + if field.Length > 0 { + size += n * field.Length + } else { + size += n + } + } + } + return size +} diff --git a/cmd/binapi-generator/parse.go b/cmd/binapi-generator/parse.go index 7f7880b..2d6fdd4 100644 --- a/cmd/binapi-generator/parse.go +++ b/cmd/binapi-generator/parse.go @@ -17,101 +17,12 @@ package main import ( "errors" "fmt" - "log" "sort" "strings" "github.com/bennyscetbun/jsongo" ) -// Package represents collection of objects parsed from VPP binary API JSON data -type Package struct { - APIVersion string - Enums []Enum - Unions []Union - Types []Type - Messages []Message - Services []Service - RefMap map[string]string -} - -// MessageType represents the type of a VPP message -type MessageType int - -const ( - requestMessage MessageType = iota // VPP request message - replyMessage // VPP reply message - eventMessage // VPP event message - otherMessage // other VPP message -) - -// Message represents VPP binary API message -type Message struct { - Name string - CRC string - Fields []Field -} - -// Type represents VPP binary API type -type Type struct { - Name string - CRC string - Fields []Field -} - -// Union represents VPP binary API union -type Union struct { - Name string - CRC string - Fields []Field -} - -// Field represents VPP binary API object field -type Field struct { - Name string - Type string - Length int - SizeFrom string -} - -func (f *Field) IsArray() bool { - return f.Length > 0 || f.SizeFrom != "" -} - -// Enum represents VPP binary API enum -type Enum struct { - Name string - Type string - Entries []EnumEntry -} - -// EnumEntry represents VPP binary API enum entry -type EnumEntry struct { - Name string - Value interface{} -} - -// Service represents VPP binary API service -type Service struct { - RequestType string - ReplyType string - Stream bool - Events []string -} - -func getSizeOfType(typ *Type) (size int) { - for _, field := range typ.Fields { - if n := getBinapiTypeSize(field.Type); n > 0 { - if field.Length > 0 { - size += n * field.Length - } else { - size += n - } - } - } - return size -} - func getTypeByRef(ctx *context, ref string) *Type { for _, typ := range ctx.packageData.Types { if ref == toApiType(typ.Name) { @@ -409,6 +320,7 @@ func parseMessage(ctx *context, msgNode *jsongo.JSONNode) (*Message, error) { } msgCRC, ok := msgNode.At(msgNode.Len() - 1).At("crc").Get().(string) if !ok { + return nil, fmt.Errorf("message crc invalid or missing") } @@ -478,6 +390,7 @@ func parseService(ctx *context, svcName string, svcNode *jsongo.JSONNode) (*Serv } svc := Service{ + Name: ctx.moduleName + "." + svcName, RequestType: svcName, } @@ -486,7 +399,6 @@ func parseService(ctx *context, svcName string, svcNode *jsongo.JSONNode) (*Serv if !ok { return nil, fmt.Errorf("service reply is %T, not a string", replyNode.Get()) } - // some binapi messages might have `null` reply (for example: memclnt) if reply != "null" { svc.ReplyType = reply } @@ -510,20 +422,24 @@ func parseService(ctx *context, svcName string, svcNode *jsongo.JSONNode) (*Serv } // validate service - if svc.Stream { + if svc.IsEventService() { + if !strings.HasPrefix(svc.RequestType, "want_") { + log.Warnf("Unusual EVENTS SERVICE: %+v\n"+ + "- events service %q does not have 'want_' prefix in request.", + svc, svc.Name) + } + } else if svc.IsDumpService() { if !strings.HasSuffix(svc.RequestType, "_dump") || !strings.HasSuffix(svc.ReplyType, "_details") { - fmt.Printf("Invalid STREAM SERVICE: %+v\n", svc) - } - } else if len(svc.Events) > 0 { - if (!strings.HasSuffix(svc.RequestType, "_events") && - !strings.HasSuffix(svc.RequestType, "_stats")) || - !strings.HasSuffix(svc.ReplyType, "_reply") { - fmt.Printf("Invalid EVENTS SERVICE: %+v\n", svc) + log.Warnf("Unusual STREAM SERVICE: %+v\n"+ + "- stream service %q does not have '_dump' suffix in request or reply does not have '_details' suffix.", + svc, svc.Name) } - } else if svc.ReplyType != "" { + } else if svc.IsRequestService() { if !strings.HasSuffix(svc.ReplyType, "_reply") { - fmt.Printf("Invalid SERVICE: %+v\n", svc) + log.Warnf("Unusual REQUEST SERVICE: %+v\n"+ + "- service %q does not have '_reply' suffix in reply.", + svc, svc.Name) } } @@ -540,7 +456,7 @@ func convertToGoType(ctx *context, binapiType string) (typ string) { typ = camelCaseName(r) } else { // fallback type - log.Printf("found unknown VPP binary API type %q, using byte", binapiType) + log.Warnf("found unknown VPP binary API type %q, using byte", binapiType) typ = "byte" } return typ -- cgit 1.2.3-korg