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 <ofabry@cisco.com>

4 files changed, 169 insertions, 125 deletions

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