[full-ci] enhancement: add more kql spec tests and simplify ast normalization (#7254)

* enhancement: add more kql spec tests and simplify ast normalization

* enhancement: kql parser error if query starts with AND

* enhancement: add kql docs and support for date and time only dateTimeRestriction queries

* enhancement: add the ability to decide how kql nodes get connected

connecting nodes (with edges) seem straight forward when not using group, the default connection for nodes with the same node is always OR. THis only applies for first level nodes, for grouped nodes it is defined differently. The KQL docs are saying, nodes inside a grouped node, with the same key are connected by a AND edge.

* enhancement: explicit error handling for falsy group nodes and queries with leading binary operator

* enhancement: use optimized grammar for kql parser and toolify pigeon

* enhancement: simplify error handling

* fix: kql implicit 'AND' and 'OR' follows the ms html spec instead of the pdf spec

services/search/Makefile

@@ -29,7 +29,7 @@ ci-go-generate: $(PIGEON) $(MOCKERY) # CI runs ci-node-generate automatically be
 	$(MOCKERY) --dir pkg/content --output pkg/content/mocks --case underscore --name Extractor
 	$(MOCKERY) --dir pkg/content --output pkg/content/mocks --case underscore --name Retriever
 	$(MOCKERY) --dir pkg/search --output pkg/search/mocks --case underscore --name Searcher
-	$(PIGEON) -o pkg/query/kql/dictionary_gen.go pkg/query/kql/dictionary.peg
+	$(PIGEON) -optimize-grammar -optimize-parser -o pkg/query/kql/dictionary_gen.go pkg/query/kql/dictionary.peg
 
 .PHONY: ci-node-generate
 ci-node-generate:

services/search/pkg/query/ast/ast.go

@@ -73,3 +73,35 @@ type GroupNode struct {
 	Key   string
 	Nodes []Node
 }
+
+// NodeKey tries to return the node key
+func NodeKey(n Node) string {
+	switch node := n.(type) {
+	case *StringNode:
+		return node.Key
+	case *DateTimeNode:
+		return node.Key
+	case *BooleanNode:
+		return node.Key
+	case *GroupNode:
+		return node.Key
+	default:
+		return ""
+	}
+}
+
+// NodeValue tries to return the node key
+func NodeValue(n Node) interface{} {
+	switch node := n.(type) {
+	case *StringNode:
+		return node.Value
+	case *DateTimeNode:
+		return node.Value
+	case *BooleanNode:
+		return node.Value
+	case *GroupNode:
+		return node.Nodes
+	default:
+		return ""
+	}
+}

services/search/pkg/query/kql/cast.go

@@ -4,16 +4,11 @@ import (
 	"fmt"
 	"time"
 
+	"github.com/araddon/dateparse"
+
 	"github.com/owncloud/ocis/v2/services/search/pkg/query/ast"
 )
 
-func toIfaceSlice(in interface{}) []interface{} {
-	if in == nil {
-		return nil
-	}
-	return in.([]interface{})
-}
-
 func toNode[T ast.Node](in interface{}) (T, error) {
 	var t T
 	out, ok := in.(T)
@@ -25,25 +20,27 @@ func toNode[T ast.Node](in interface{}) (T, error) {
 }
 
 func toNodes[T ast.Node](in interface{}) ([]T, error) {
-
 	switch v := in.(type) {
+	case []T:
+		return v, nil
+	case T:
+		return []T{v}, nil
 	case []interface{}:
-		var nodes []T
-
-		for _, el := range toIfaceSlice(v) {
-			node, err := toNode[T](el)
+		var ts []T
+		for _, inter := range v {
+			n, err := toNodes[T](inter)
 			if err != nil {
 				return nil, err
 			}
 
-			nodes = append(nodes, node)
+			ts = append(ts, n...)
 		}
-
-		return nodes, nil
-	case []T:
-		return v, nil
+		return ts, nil
+	case nil:
+		return nil, nil
 	default:
-		return nil, fmt.Errorf("can't convert '%T' to []ast.Node", in)
+		var t T
+		return nil, fmt.Errorf("can't convert '%T' to '%T'", in, t)
 	}
 }
 
@@ -77,5 +74,5 @@ func toTime(in interface{}) (time.Time, error) {
 		return time.Time{}, err
 	}
 
-	return time.Parse(time.RFC3339Nano, ts)
+	return dateparse.ParseLocal(ts)
 }

services/search/pkg/query/kql/connect.go

@@ -0,0 +1,129 @@
+package kql
+
+import (
+	"strings"
+
+	"github.com/owncloud/ocis/v2/services/search/pkg/query/ast"
+)
+
+// connectNodes connects given nodes
+func connectNodes(c Connector, nodes ...ast.Node) []ast.Node {
+	var connectedNodes []ast.Node
+
+	for i := range nodes {
+		ri := len(nodes) - 1 - i
+		head := nodes[ri]
+
+		if connectionNodes := connectNode(c, head, connectedNodes...); len(connectionNodes) > 0 {
+			connectedNodes = append(connectionNodes, connectedNodes...)
+		}
+
+		connectedNodes = append([]ast.Node{head}, connectedNodes...)
+	}
+
+	return connectedNodes
+}
+
+// connectNode connects a tip node with the rest
+func connectNode(c Connector, headNode ast.Node, tailNodes ...ast.Node) []ast.Node {
+	var nearestNeighborNode ast.Node
+	var nearestNeighborOperators []*ast.OperatorNode
+
+l:
+	for _, tailNode := range tailNodes {
+		switch node := tailNode.(type) {
+		case *ast.OperatorNode:
+			nearestNeighborOperators = append(nearestNeighborOperators, node)
+		default:
+			nearestNeighborNode = node
+			break l
+		}
+	}
+
+	if nearestNeighborNode == nil {
+		return nil
+	}
+
+	return c.Connect(headNode, nearestNeighborNode, nearestNeighborOperators)
+}
+
+// Connector is responsible to decide what node connections are needed
+type Connector interface {
+	Connect(head ast.Node, neighbor ast.Node, connections []*ast.OperatorNode) []ast.Node
+}
+
+// DefaultConnector is the default node connector
+type DefaultConnector struct {
+	sameKeyOPValue string
+}
+
+// Connect implements the Connector interface and is used to connect the nodes using
+// the default logic defined by the kql spec.
+func (c DefaultConnector) Connect(head ast.Node, neighbor ast.Node, connections []*ast.OperatorNode) []ast.Node {
+	switch head.(type) {
+	case *ast.OperatorNode:
+		return nil
+	}
+
+	headKey := strings.ToLower(ast.NodeKey(head))
+	neighborKey := strings.ToLower(ast.NodeKey(neighbor))
+
+	connection := &ast.OperatorNode{
+		Base:  &ast.Base{Loc: &ast.Location{Source: &[]string{"implicitly operator"}[0]}},
+		Value: BoolAND,
+	}
+
+	// if the current node and the neighbor node have the same key
+	// the connection is of type OR
+	//
+	// spec: same
+	//		author:"John Smith" author:"Jane Smith"
+	//		author:"John Smith" OR author:"Jane Smith"
+	//
+	// if the nodes have NO key, the edge is a AND connection
+	//
+	// spec: same
+	//		cat dog
+	//		cat AND dog
+	// from the spec:
+	// 		To construct complex queries, you can combine multiple
+	// 		free-text expressions with KQL query operators.
+	// 		If there are multiple free-text expressions without any
+	// 		operators in between them, the query behavior is the same
+	// 		as using the AND operator.
+	//
+	// nodes inside of group node are handled differently,
+	// if no explicit operator given, it uses AND
+	//
+	// spec: same
+	// 		author:"John Smith" AND author:"Jane Smith"
+	// 		author:("John Smith" "Jane Smith")
+	if headKey == neighborKey && headKey != "" && neighborKey != "" {
+		connection.Value = c.sameKeyOPValue
+	}
+
+	// decisions based on nearest neighbor operators
+	for i, node := range connections {
+		// consider direct neighbor operator only
+		if i == 0 {
+			// no connection is necessary here because an `AND` or `OR` edge is already present
+			// exit
+			for _, skipValue := range []string{BoolOR, BoolAND} {
+				if node.Value == skipValue {
+					return nil
+				}
+			}
+
+			// if neighbor node negotiates, an AND edge is needed
+			//
+			// spec: same
+			// 		cat -dog
+			// 		cat AND NOT dog
+			if node.Value == BoolNOT {
+				connection.Value = BoolAND
+			}
+		}
+	}
+
+	return []ast.Node{connection}
+}

services/search/pkg/query/kql/const.go

@@ -1,8 +0,0 @@
-package kql
-
-// The operator node value definition
-const (
-	BoolAND = "AND"
-	BoolOR  = "OR"
-	BoolNOT = "NOT"
-)

services/search/pkg/query/kql/dictionary.peg

@@ -7,23 +7,22 @@
 ////////////////////////////////////////////////////////
 
 AST <-
-    _ nodes:Nodes _ {
-        return buildAST(nodes, c.text, c.pos)
+    n:Nodes {
+        return buildAST(n, c.text, c.pos)
     }
 
+////////////////////////////////////////////////////////
+// nodes
+////////////////////////////////////////////////////////
+
 Nodes <-
-    n:(
-        _
-        (
-            GroupNode /
-            PropertyRestrictionNodes /
-            OperatorBooleanNode /
-            FreeTextKeywordNodes
-        )
-        _
-    )+ {
-      return buildNodes(n)
-    }
+    (_ Node)+
+
+Node <-
+    GroupNode /
+    PropertyRestrictionNodes /
+    OperatorBooleanNodes /
+    FreeTextKeywordNodes
 
 ////////////////////////////////////////////////////////
 // nesting
@@ -49,7 +48,18 @@ YesNoPropertyRestrictionNode <-
     }
 
 DateTimeRestrictionNode <-
-    k:Char+ o:(OperatorGreaterOrEqualNode / OperatorLessOrEqualNode / OperatorGreaterNode / OperatorLessNode / OperatorEqualNode / OperatorColonNode) '"'? v:(FullDate "T" FullTime) '"'? {
+    k:Char+ o:(
+        OperatorGreaterOrEqualNode /
+        OperatorLessOrEqualNode /
+        OperatorGreaterNode /
+        OperatorLessNode /
+        OperatorEqualNode /
+        OperatorColonNode
+    ) '"'? v:(
+        DateTime /
+        FullDate /
+        FullTime
+    ) '"'? {
         return buildDateTimeNode(k, o, v, c.text, c.pos)
     }
 
@@ -80,8 +90,23 @@ WordNode <-
 // operators
 ////////////////////////////////////////////////////////
 
-OperatorBooleanNode <-
-    ("AND" / "OR" / "NOT") {
+OperatorBooleanNodes <-
+    OperatorBooleanAndNode /
+    OperatorBooleanNotNode /
+    OperatorBooleanOrNode
+
+OperatorBooleanAndNode <-
+    ("AND" / "+") {
+        return buildOperatorNode(c.text, c.pos)
+    }
+
+OperatorBooleanNotNode <-
+    ("NOT" / "-") {
+        return buildOperatorNode(c.text, c.pos)
+    }
+
+OperatorBooleanOrNode <-
+    ("OR") {
         return buildOperatorNode(c.text, c.pos)
     }
 
@@ -160,6 +185,11 @@ FullTime <-
         return c.text, nil
     }
 
+DateTime
+  = FullDate "T" FullTime {
+      return c.text, nil
+    }
+
 ////////////////////////////////////////////////////////
 // misc
 ////////////////////////////////////////////////////////
@@ -180,4 +210,6 @@ Digit <-
     }
 
 _ <-
-    [ \t]*
+    [ \t]* {
+       return nil, nil
+    }

services/search/pkg/query/kql/doc.go

@@ -0,0 +1,28 @@
+/*
+Package kql provides the ability to work with kql queries.
+
+Not every aspect of the spec is implemented yet.
+The language support will grow over time if needed.
+
+The following spec parts are supported and tested:
+  - 2.1.2 AND Operator
+  - 2.1.6 NOT Operator
+  - 2.1.8 OR Operator
+  - 2.1.12 Parentheses
+  - 2.3.5 Date Tokens
+  - Human tokens not implemented
+  - 3.1.11 Implicit Operator
+  - 3.1.12 Parentheses
+  - 3.1.2 AND Operator
+  - 3.1.6 NOT Operator
+  - 3.1.8 OR Operator
+  - 3.2.3 Implicit Operator for Property Restriction
+  - 3.3.1.1.1 Implicit AND Operator
+  - 3.3.5 Date Tokens
+
+References:
+  - https://learn.microsoft.com/en-us/sharepoint/dev/general-development/keyword-query-language-kql-syntax-reference
+  - https://learn.microsoft.com/en-us/openspecs/sharepoint_protocols/ms-kql/3bbf06cd-8fc1-4277-bd92-8661ccd3c9b0
+  - https://msopenspecs.azureedge.net/files/MS-KQL/%5bMS-KQL%5d.pdf
+*/
+package kql

services/search/pkg/query/kql/error.go

@@ -1,10 +1,30 @@
 package kql
 
+import (
+	"fmt"
+
+	"github.com/owncloud/ocis/v2/services/search/pkg/query/ast"
+)
+
 // StartsWithBinaryOperatorError records an error and the operation that caused it.
 type StartsWithBinaryOperatorError struct {
-	Op string
+	Node *ast.OperatorNode
 }
 
-func (e *StartsWithBinaryOperatorError) Error() string {
-	return "the expression can't begin from a binary operator: '" + e.Op + "'"
+func (e StartsWithBinaryOperatorError) Error() string {
+	return "the expression can't begin from a binary operator: '" + e.Node.Value + "'"
+}
+
+// NamedGroupInvalidNodesError records an error and the operation that caused it.
+type NamedGroupInvalidNodesError struct {
+	Node ast.Node
+}
+
+func (e NamedGroupInvalidNodesError) Error() string {
+	return fmt.Errorf(
+		"'%T' - '%v' - '%v' is not valid",
+		e.Node,
+		ast.NodeKey(e.Node),
+		ast.NodeValue(e.Node),
+	).Error()
 }

services/search/pkg/query/kql/factory.go

@@ -38,31 +38,16 @@ func buildAST(n interface{}, text []byte, pos position) (*ast.Ast, error) {
 		return nil, err
 	}
 
-	normalizedNodes, err := NormalizeNodes(nodes)
-	if err != nil {
+	a := &ast.Ast{
+		Base:  b,
+		Nodes: connectNodes(DefaultConnector{sameKeyOPValue: BoolOR}, nodes...),
+	}
+
+	if err := validateAst(a); err != nil {
 		return nil, err
 	}
 
-	return &ast.Ast{
-		Base:  b,
-		Nodes: normalizedNodes,
-	}, nil
-}
-
-func buildNodes(e interface{}) ([]ast.Node, error) {
-	maybeNodesGroups := toIfaceSlice(e)
-
-	nodes := make([]ast.Node, len(maybeNodesGroups))
-	for i, maybeNodesGroup := range maybeNodesGroups {
-		node, err := toNode[ast.Node](toIfaceSlice(maybeNodesGroup)[1])
-		if err != nil {
-			return nil, err
-		}
-
-		nodes[i] = node
-	}
-
-	return nodes, nil
+	return a, nil
 }
 
 func buildStringNode(k, v interface{}, text []byte, pos position) (*ast.StringNode, error) {
@@ -151,6 +136,13 @@ func buildOperatorNode(text []byte, pos position) (*ast.OperatorNode, error) {
 		return nil, err
 	}
 
+	switch value {
+	case "+":
+		value = BoolAND
+	case "-":
+		value = BoolNOT
+	}
+
 	return &ast.OperatorNode{
 		Base:  b,
 		Value: value,
@@ -170,9 +162,15 @@ func buildGroupNode(k, n interface{}, text []byte, pos position) (*ast.GroupNode
 		return nil, err
 	}
 
-	return &ast.GroupNode{
+	gn := &ast.GroupNode{
 		Base:  b,
 		Key:   key,
-		Nodes: nodes,
-	}, nil
+		Nodes: connectNodes(DefaultConnector{sameKeyOPValue: BoolOR}, nodes...),
+	}
+
+	if err := validateGroupNode(gn); err != nil {
+		return nil, err
+	}
+
+	return gn, nil
 }

services/search/pkg/query/kql/gen.go

@@ -1,3 +1,3 @@
 package kql
 
-//go:generate go run github.com/mna/pigeon -o dictionary_gen.go dictionary.peg
+//go:generate go run github.com/mna/pigeon -optimize-grammar -optimize-parser -o dictionary_gen.go dictionary.peg

services/search/pkg/query/kql/kql.go

@@ -2,9 +2,21 @@
 package kql
 
 import (
+	"errors"
+
 	"github.com/owncloud/ocis/v2/services/search/pkg/query/ast"
 )
 
+// The operator node value definition
+const (
+	// BoolAND connect two nodes with "AND"
+	BoolAND = "AND"
+	// BoolOR connect two nodes with "OR"
+	BoolOR = "OR"
+	// BoolNOT connect two nodes with "NOT"
+	BoolNOT = "NOT"
+)
+
 // Builder implements kql Builder interface
 type Builder struct{}
 
@@ -12,7 +24,21 @@ type Builder struct{}
 func (b Builder) Build(q string) (*ast.Ast, error) {
 	f, err := Parse("", []byte(q))
 	if err != nil {
-		return nil, err
+		var list errList
+		errors.As(err, &list)
+
+		for _, listError := range list {
+			var parserError *parserError
+			switch {
+			case errors.As(listError, &parserError):
+				if parserError.Inner != nil {
+					return nil, parserError.Inner
+				}
+
+				return nil, listError
+			}
+		}
 	}
+
 	return f.(*ast.Ast), nil
 }

services/search/pkg/query/kql/kql_test.go

@@ -5,23 +5,26 @@ import (
 
 	tAssert "github.com/stretchr/testify/assert"
 
+	"github.com/owncloud/ocis/v2/services/search/pkg/query/ast"
 	"github.com/owncloud/ocis/v2/services/search/pkg/query/kql"
 )
 
 func TestNewAST(t *testing.T) {
 	tests := []struct {
-		name        string
-		givenQuery  string
-		shouldError bool
+		name          string
+		givenQuery    string
+		expectedError error
 	}{
 		{
 			name:       "success",
 			givenQuery: "foo:bar",
 		},
 		{
-			name:        "error",
-			givenQuery:  "AND",
-			shouldError: true,
+			name:       "error",
+			givenQuery: kql.BoolAND,
+			expectedError: kql.StartsWithBinaryOperatorError{
+				Node: &ast.OperatorNode{Value: kql.BoolAND},
+			},
 		},
 	}
 
@@ -32,13 +35,20 @@ func TestNewAST(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			got, err := kql.Builder{}.Build(tt.givenQuery)
 
-			if tt.shouldError {
-				assert.NotNil(err)
+			if tt.expectedError != nil {
+				if tt.expectedError.Error() != "" {
+					assert.Equal(err.Error(), tt.expectedError.Error())
+				} else {
+					assert.NotNil(err)
+				}
+
 				assert.Nil(got)
-			} else {
-				assert.Nil(err)
-				assert.NotNil(got)
+
+				return
 			}
+
+			assert.Nil(err)
+			assert.NotNil(got)
 		})
 	}
 }

services/search/pkg/query/kql/normalize.go

@@ -1,128 +0,0 @@
-package kql
-
-import (
-	"github.com/owncloud/ocis/v2/services/search/pkg/query/ast"
-)
-
-var implicitOperatorNodeSource = "implicitly operator"
-var operatorNodeAnd = ast.OperatorNode{Base: &ast.Base{Loc: &ast.Location{Source: &implicitOperatorNodeSource}}, Value: BoolAND}
-var operatorNodeOr = ast.OperatorNode{Base: &ast.Base{Loc: &ast.Location{Source: &implicitOperatorNodeSource}}, Value: BoolOR}
-
-// NormalizeNodes Populate the implicit logical operators in the ast
-//
-// https://learn.microsoft.com/en-us/sharepoint/dev/general-development/keyword-query-language-kql-syntax-reference#constructing-free-text-queries-using-kql
-// If there are multiple free-text expressions without any operators in between them, the query behavior is the same as using the AND operator.
-// "John Smith" "Jane Smith"
-// This functionally is the same as using the AND Boolean operator, as follows:
-// "John Smith" AND "Jane Smith"
-//
-// https://learn.microsoft.com/en-us/sharepoint/dev/general-development/keyword-query-language-kql-syntax-reference#using-multiple-property-restrictions-within-a-kql-query
-// When you use multiple instances of the same property restriction, matches are based on the union of the property restrictions in the KQL query.
-// author:"John Smith" author:"Jane Smith"
-// This functionally is the same as using the OR Boolean operator, as follows:
-// author:"John Smith" OR author:"Jane Smith"
-//
-// When you use different property restrictions, matches are based on an intersection of the property restrictions in the KQL query, as follows:
-// author:"John Smith" filetype:docx
-// This is the same as using the AND Boolean operator, as follows:
-// author:"John Smith" AND filetype:docx
-//
-// https://learn.microsoft.com/en-us/sharepoint/dev/general-development/keyword-query-language-kql-syntax-reference#grouping-property-restrictions-within-a-kql-query
-// author:("John Smith" "Jane Smith")
-// This is the same as using the AND Boolean operator, as follows:
-// author:"John Smith" AND author:"Jane Smith"
-func NormalizeNodes(nodes []ast.Node) ([]ast.Node, error) {
-	res := make([]ast.Node, 0, len(nodes))
-	var currentNode ast.Node
-	var prevKey, currentKey *string
-	var operator *ast.OperatorNode
-	for _, node := range nodes {
-		switch n := node.(type) {
-		case *ast.StringNode:
-			if prevKey == nil {
-				prevKey = &n.Key
-				res = append(res, node)
-				continue
-			}
-			currentNode = n
-			currentKey = &n.Key
-		case *ast.DateTimeNode:
-			if prevKey == nil {
-				prevKey = &n.Key
-				res = append(res, node)
-				continue
-			}
-			currentNode = n
-			currentKey = &n.Key
-		case *ast.BooleanNode:
-			if prevKey == nil {
-				prevKey = &n.Key
-				res = append(res, node)
-				continue
-			}
-			currentNode = n
-			currentKey = &n.Key
-		case *ast.GroupNode:
-			var err error
-			n.Nodes, err = NormalizeNodes(n.Nodes)
-			if err != nil {
-				return nil, err
-			}
-			if prevKey == nil {
-				prevKey = &n.Key
-				res = append(res, n)
-				continue
-			}
-			currentNode = n
-			currentKey = &n.Key
-		case *ast.OperatorNode:
-			if n.Value == BoolNOT {
-				if prevKey == nil {
-					res = append(res, n)
-				} else {
-					operator = n
-				}
-			} else {
-				if prevKey == nil {
-					return nil, &StartsWithBinaryOperatorError{Op: n.Value}
-				}
-				prevKey = nil
-				res = append(res, node)
-			}
-		default:
-			prevKey = nil
-			res = append(res, node)
-		}
-		if prevKey != nil && currentKey != nil {
-			if *prevKey == *currentKey && *prevKey != "" {
-				res = append(res, &operatorNodeOr)
-			} else {
-				res = append(res, &operatorNodeAnd)
-			}
-			if operator != nil {
-				res = append(res, operator)
-				operator = nil
-			}
-			res = append(res, currentNode)
-
-			prevKey = currentKey
-			currentNode = nil
-			currentKey = nil
-			continue
-		}
-	}
-
-	return trimOrphan(res), nil
-}
-
-func trimOrphan(nodes []ast.Node) []ast.Node {
-	offset := len(nodes)
-	for i := len(nodes) - 1; i >= 0; i-- {
-		if _, ok := nodes[i].(*ast.OperatorNode); ok {
-			offset--
-		} else {
-			break
-		}
-	}
-	return nodes[:offset]
-}

services/search/pkg/query/kql/normalize_test.go

@@ -1,126 +0,0 @@
-package kql_test
-
-import (
-	"testing"
-	"time"
-
-	tAssert "github.com/stretchr/testify/assert"
-
-	"github.com/owncloud/ocis/v2/services/search/pkg/query/ast"
-	"github.com/owncloud/ocis/v2/services/search/pkg/query/ast/test"
-	"github.com/owncloud/ocis/v2/services/search/pkg/query/kql"
-)
-
-var now = time.Now()
-
-func TestNormalizeNodes(t *testing.T) {
-	tests := []struct {
-		name          string
-		givenNodes    []ast.Node
-		expectedNodes []ast.Node
-		fixme         bool
-		expectedError error
-	}{
-		{
-			name: "start with binary operator",
-			givenNodes: []ast.Node{
-				&ast.OperatorNode{Value: "OR"},
-			},
-			expectedError: &kql.StartsWithBinaryOperatorError{Op: "OR"},
-		},
-		{
-			name: "same key implicit OR",
-			givenNodes: []ast.Node{
-				&ast.StringNode{Key: "author", Value: "John Smith"},
-				&ast.StringNode{Key: "author", Value: "Jane Smith"},
-			},
-			expectedNodes: []ast.Node{
-				&ast.StringNode{Key: "author", Value: "John Smith"},
-				&ast.OperatorNode{Value: "OR"},
-				&ast.StringNode{Key: "author", Value: "Jane Smith"},
-			},
-		},
-		{
-			name: "no key implicit AND",
-			givenNodes: []ast.Node{
-				&ast.StringNode{Value: "John Smith"},
-				&ast.StringNode{Value: "Jane Smith"},
-			},
-			expectedNodes: []ast.Node{
-				&ast.StringNode{Value: "John Smith"},
-				&ast.OperatorNode{Value: "AND"},
-				&ast.StringNode{Value: "Jane Smith"},
-			},
-		},
-		{
-			name: "same key explicit AND",
-			givenNodes: []ast.Node{
-				&ast.StringNode{Key: "author", Value: "John Smith"},
-				&ast.OperatorNode{Value: "AND"},
-				&ast.StringNode{Key: "author", Value: "Jane Smith"},
-			},
-			expectedNodes: []ast.Node{
-				&ast.StringNode{Key: "author", Value: "John Smith"},
-				&ast.OperatorNode{Value: "AND"},
-				&ast.StringNode{Key: "author", Value: "Jane Smith"},
-			},
-		},
-		{
-			name: "key-group implicit AND",
-			// https://learn.microsoft.com/en-us/sharepoint/dev/general-development/keyword-query-language-kql-syntax-reference#grouping-property-restrictions-within-a-kql-query
-			fixme: true,
-			givenNodes: []ast.Node{
-				&ast.GroupNode{Key: "author", Nodes: []ast.Node{
-					&ast.StringNode{Key: "author", Value: "John Smith"},
-					&ast.StringNode{Key: "author", Value: "Jane Smith"},
-				}},
-			},
-			expectedNodes: []ast.Node{
-				&ast.GroupNode{Key: "author", Nodes: []ast.Node{
-					&ast.StringNode{Key: "author", Value: "John Smith"},
-					&ast.OperatorNode{Value: "AND"},
-					&ast.StringNode{Key: "author", Value: "Jane Smith"},
-				}},
-			},
-		},
-		{
-			name: "different key implicit AND",
-			givenNodes: []ast.Node{
-				&ast.StringNode{Key: "author", Value: "John Smith"},
-				&ast.StringNode{Key: "filetype", Value: "docx"},
-				&ast.DateTimeNode{Key: "mtime", Operator: &ast.OperatorNode{Value: "="}, Value: now},
-			},
-			expectedNodes: []ast.Node{
-				&ast.StringNode{Key: "author", Value: "John Smith"},
-				&ast.OperatorNode{Value: "AND"},
-				&ast.StringNode{Key: "filetype", Value: "docx"},
-				&ast.OperatorNode{Value: "AND"},
-				&ast.DateTimeNode{Key: "mtime", Operator: &ast.OperatorNode{Value: "="}, Value: now},
-			},
-		},
-	}
-
-	assert := tAssert.New(t)
-
-	for _, tt := range tests {
-		tt := tt
-		t.Run(tt.name, func(t *testing.T) {
-			if tt.fixme {
-				t.Skip("not implemented")
-			}
-
-			normalizedNodes, err := kql.NormalizeNodes(tt.givenNodes)
-
-			if tt.expectedError != nil {
-				assert.Equal(err, tt.expectedError)
-				assert.Nil(normalizedNodes)
-
-				return
-			}
-
-			if diff := test.DiffAst(tt.expectedNodes, normalizedNodes); diff != "" {
-				t.Fatalf("Nodes mismatch (-want +got): %s", diff)
-			}
-		})
-	}
-}

services/search/pkg/query/kql/validate.go

@@ -0,0 +1,37 @@
+package kql
+
+import (
+	"github.com/owncloud/ocis/v2/services/search/pkg/query/ast"
+)
+
+func validateAst(a *ast.Ast) error {
+	switch node := a.Nodes[0].(type) {
+	case *ast.OperatorNode:
+		switch node.Value {
+		case BoolAND, BoolOR:
+			return StartsWithBinaryOperatorError{Node: node}
+		}
+	}
+
+	return nil
+}
+
+func validateGroupNode(n *ast.GroupNode) error {
+	switch node := n.Nodes[0].(type) {
+	case *ast.OperatorNode:
+		switch node.Value {
+		case BoolAND, BoolOR:
+			return StartsWithBinaryOperatorError{Node: node}
+		}
+	}
+
+	if n.Key != "" {
+		for _, node := range n.Nodes {
+			if ast.NodeKey(node) != "" {
+				return NamedGroupInvalidNodesError{Node: node}
+			}
+		}
+	}
+
+	return nil
+}