merge 4.0.0-RC

1 files changed, 59 insertions, 49 deletions

diff --git a/internal/mapr/query.go b/internal/mapr/query.go
index 01852da..d70675f 100644
--- a/internal/mapr/query.go
+++ b/internal/mapr/query.go
@@ -6,8 +6,6 @@ import (
 	"strconv"
 	"strings"
 	"time"
-
-	"github.com/mimecast/dtail/internal/io/logger"
 )
 
 const (
@@ -34,7 +32,9 @@ type Query struct {
 }
 
 func (q Query) String() string {
-	return fmt.Sprintf("Query(Select:%v,Table:%s,Where:%v,Set:%vGroupBy:%v,GroupKey:%s,OrderBy:%v,ReverseOrder:%v,Interval:%v,Limit:%d,Outfile:%s,RawQuery:%s,tokens:%v,LogFormat:%s)",
+	return fmt.Sprintf("Query(Select:%v,Table:%s,Where:%v,Set:%vGroupBy:%v,"+
+		"GroupKey:%s,OrderBy:%v,ReverseOrder:%v,Interval:%v,Limit:%d,Outfile:%s,"+
+		"RawQuery:%s,tokens:%v,LogFormat:%s)",
 		q.Select,
 		q.Table,
 		q.Where,
@@ -56,20 +56,14 @@ func NewQuery(queryStr string) (*Query, error) {
 	if queryStr == "" {
 		return nil, nil
 	}
-
 	tokens := tokenize(queryStr)
-
 	q := Query{
 		RawQuery: queryStr,
 		tokens:   tokens,
 		Interval: time.Second * 5,
 		Limit:    -1,
 	}
-
-	err := q.parse(tokens)
-
-	logger.Debug(q)
-	return &q, err
+	return &q, q.parse(tokens)
 }
 
 // HasOutfile returns true if query result will be written to a CVS output file.
@@ -83,8 +77,45 @@ func (q *Query) Has(what string) bool {
 }
 
 func (q *Query) parse(tokens []token) error {
-	var found []token
+	tokens, err := q.parseTokens(tokens)
+	if err != nil {
+		return err
+	}
+
+	if len(q.Select) < 1 {
+		return errors.New(invalidQuery + "Expected at least one field in 'select' " +
+			"clause but got none")
+	}
+
+	if len(q.GroupBy) == 0 {
+		field := q.Select[0].Field
+		q.GroupBy = append(q.GroupBy, field)
+	}
+
+	if q.OrderBy != "" {
+		var orderFieldIsValid bool
+		for _, sc := range q.Select {
+			if q.OrderBy == sc.FieldStorage {
+				orderFieldIsValid = true
+				break
+			}
+		}
+		if !orderFieldIsValid {
+			return errors.New(invalidQuery + fmt.Sprintf("Can not '(r)order by' '%s',"+
+				"must be present in 'select' clause", q.OrderBy))
+		}
+	}
+
+	return nil
+}
+
+// One can argue that this function is too large (as reported by automatic tools such
+// as SonarQube). However, refactoring this method into several smaller ones would make
+// the code as a matter of fact less readable. Also, I want to have at least one issue
+// reported in SonarQube, just to make sure that SonarQube still works ;-)
+func (q *Query) parseTokens(tokens []token) ([]token, error) {
 	var err error
+	var found []token
 
 	for tokens != nil && len(tokens) > 0 {
 		switch strings.ToLower(tokens[0].str) {
@@ -92,53 +123,53 @@ func (q *Query) parse(tokens []token) error {
 			tokens, found = tokensConsume(tokens[1:])
 			q.Select, err = makeSelectConditions(found)
 			if err != nil {
-				return err
+				return tokens, err
 			}
 		case "from":
 			tokens, found = tokensConsume(tokens[1:])
 			if len(found) == 0 {
-				return errors.New(invalidQuery + "expected table name after 'from'")
+				return tokens, errors.New(invalidQuery + "expected table name after 'from'")
 			}
 			if len(found) > 1 {
-				return errors.New(invalidQuery + "expected only one table name after 'from'")
+				return tokens, errors.New(invalidQuery + "expected only one table name after 'from'")
 			}
 			q.Table = strings.ToUpper(found[0].str)
 		case "where":
 			tokens, found = tokensConsume(tokens[1:])
 			if q.Where, err = makeWhereConditions(found); err != nil {
-				return err
+				return tokens, err
 			}
 		case "set":
 			tokens, found = tokensConsume(tokens[1:])
 			if q.Set, err = makeSetConditions(found); err != nil {
-				return err
+				return tokens, err
 			}
 		case "group":
 			tokens = tokensConsumeOptional(tokens[1:], "by")
 			if tokens == nil || len(tokens) < 1 {
-				return errors.New(invalidQuery + unexpectedEnd)
+				return tokens, errors.New(invalidQuery + unexpectedEnd)
 			}
 			tokens, q.GroupBy = tokensConsumeStr(tokens)
 			q.GroupKey = strings.Join(q.GroupBy, ",")
 		case "rorder":
 			tokens = tokensConsumeOptional(tokens[1:], "by")
 			if tokens == nil || len(tokens) < 1 {
-				return errors.New(invalidQuery + unexpectedEnd)
+				return tokens, errors.New(invalidQuery + unexpectedEnd)
 			}
 			tokens, found = tokensConsume(tokens)
 			if len(found) == 0 {
-				return errors.New(invalidQuery + unexpectedEnd)
+				return tokens, errors.New(invalidQuery + unexpectedEnd)
 			}
 			q.OrderBy = found[0].str
 			q.ReverseOrder = true
 		case "order":
 			tokens = tokensConsumeOptional(tokens[1:], "by")
 			if tokens == nil || len(tokens) < 1 {
-				return errors.New(invalidQuery + unexpectedEnd)
+				return tokens, errors.New(invalidQuery + unexpectedEnd)
 			}
 			tokens, found = tokensConsume(tokens)
 			if len(found) == 0 {
-				return errors.New(invalidQuery + unexpectedEnd)
+				return tokens, errors.New(invalidQuery + unexpectedEnd)
 			}
 			q.OrderBy = found[0].str
 		case "interval":
@@ -146,57 +177,36 @@ func (q *Query) parse(tokens []token) error {
 			if len(found) > 0 {
 				i, err := strconv.Atoi(found[0].str)
 				if err != nil {
-					return errors.New(invalidQuery + err.Error())
+					return tokens, errors.New(invalidQuery + err.Error())
 				}
 				q.Interval = time.Second * time.Duration(i)
 			}
 		case "limit":
 			tokens, found = tokensConsume(tokens[1:])
 			if len(found) == 0 {
-				return errors.New(invalidQuery + unexpectedEnd)
+				return tokens, errors.New(invalidQuery + unexpectedEnd)
 			}
 			i, err := strconv.Atoi(found[0].str)
 			if err != nil {
-				return errors.New(invalidQuery + err.Error())
+				return tokens, errors.New(invalidQuery + err.Error())
 			}
 			q.Limit = i
 		case "outfile":
 			tokens, found = tokensConsume(tokens[1:])
 			if len(found) == 0 {
-				return errors.New(invalidQuery + unexpectedEnd)
+				return tokens, errors.New(invalidQuery + unexpectedEnd)
 			}
 			q.Outfile = found[0].str
 		case "logformat":
 			tokens, found = tokensConsume(tokens[1:])
 			if len(found) == 0 {
-				return errors.New(invalidQuery + unexpectedEnd)
+				return tokens, errors.New(invalidQuery + unexpectedEnd)
 			}
 			q.LogFormat = found[0].str
 		default:
-			return errors.New(invalidQuery + "Unexpected keyword " + tokens[0].str)
-		}
-	}
-
-	if len(q.Select) < 1 {
-		return errors.New(invalidQuery + "Expected at least one field in 'select' clause but got none")
-	}
-	if len(q.GroupBy) == 0 {
-		field := q.Select[0].Field
-		q.GroupBy = append(q.GroupBy, field)
-	}
-
-	if q.OrderBy != "" {
-		var orderFieldIsValid bool
-		for _, sc := range q.Select {
-			if q.OrderBy == sc.FieldStorage {
-				orderFieldIsValid = true
-				break
-			}
-		}
-		if !orderFieldIsValid {
-			return errors.New(invalidQuery + fmt.Sprintf("Can not '(r)order by' '%s', must be present in 'select' clause", q.OrderBy))
+			return tokens, errors.New(invalidQuery + "Unexpected keyword " + tokens[0].str)
 		}
 	}
 
-	return nil
+	return tokens, nil
 }