fix broken queryPart sorting (#16)

also fixed coverage upload

.circleci/config.yml

@@ -39,5 +39,5 @@ jobs:
       - run:
           name: Upload coverage
           command: |
-            ./tmp/cc-test-reporter after-build -t gocov --prefix github.com/nuts-foundation/go-leia
+            ./tmp/cc-test-reporter after-build -t gocov --prefix github.com/nuts-foundation/go-leia/v2
 

index.go

@@ -52,10 +52,10 @@ type Index interface {
 
 	// Sort the query so its parts align with the index parts.
 	// includeMissing, if true, the sort will append queryParts not matched by an index at the end.
-	Sort(query Query, includeMissing bool) ([]QueryPart, error)
+	Sort(query Query, includeMissing bool) []QueryPart
 
-	// QueryPartsOutsideIndex selects the queryParts that are not convered by the index.
-	QueryPartsOutsideIndex(query Query) ([]QueryPart, error)
+	// QueryPartsOutsideIndex selects the queryParts that are not covered by the index.
+	QueryPartsOutsideIndex(query Query) []QueryPart
 
 	// Depth returns the number of indexed fields
 	Depth() int
@@ -213,10 +213,7 @@ func removeRefFromBucket(bucket *bbolt.Bucket, key Key, ref Reference) error {
 func (i *index) IsMatch(query Query) float64 {
 	hitcount := 0
 
-	parts, err := i.Sort(query, false)
-	if err != nil {
-		return 0.0
-	}
+	parts := i.Sort(query, false)
 
 outer:
 	for thc, ip := range i.indexParts {
@@ -234,59 +231,60 @@ outer:
 	return float64(hitcount) / float64(len(i.indexParts))
 }
 
-func (i *index) Sort(query Query, includeMissing bool) ([]QueryPart, error) {
-	var sorted = make([]QueryPart, len(query.Parts()))
-	var missing = make([]QueryPart, 0)
-	hits := 0
+func (i *index) Sort(query Query, includeMissing bool) []QueryPart {
+	var sorted = make([]QueryPart, len(i.indexParts))
 
 outer:
 	for _, qp := range query.Parts() {
 		for j, ip := range i.indexParts {
 			if ip.Name() == qp.Name() {
-				if j >= len(sorted) {
-					return nil, errors.New("invalid query part")
-				}
-				sorted[hits] = qp
-				hits++
+				sorted[j] = qp
 				continue outer
 			}
 		}
-		missing = append(missing, qp)
+	}
+
+	// only use till the first nil value
+	for i, s := range sorted {
+		if s == nil {
+			sorted = sorted[:i]
+			break
+		}
 	}
 
 	if includeMissing {
-		for i, qp := range missing {
-			sorted[hits+i] = qp
+		// now include all params not in the sorted list
+	outerMissing:
+		for _, qp := range query.Parts() {
+			for _, sp := range sorted {
+				if sp.Name() == qp.Name() {
+					continue outerMissing
+				}
+			}
+			// missing so append
+			sorted = append(sorted, qp)
 		}
-	} else {
-		sorted = sorted[:hits]
 	}
 
-	return sorted, nil
+	return sorted
 }
 
-func (i *index) QueryPartsOutsideIndex(query Query) ([]QueryPart, error) {
+func (i *index) QueryPartsOutsideIndex(query Query) []QueryPart {
 	hits := 0
-	parts, err := i.Sort(query, true)
-	if err != nil {
-		return nil, err
-	}
+	parts := i.Sort(query, true)
 
-outer:
-	for _, qp := range parts {
-		for _, ip := range i.indexParts {
-			if ip.Name() == qp.Name() {
-				hits++
-				continue outer
-			}
+	for j, qp := range parts {
+		if j >= len(i.indexParts) || qp.Name() != i.indexParts[j].Name() {
+			break
 		}
+		hits++
 	}
 
 	if hits == len(parts) {
-		return []QueryPart{}, nil
+		return []QueryPart{}
 	}
 
-	return parts[hits:], nil
+	return parts[hits:]
 }
 
 func (i *index) Iterate(bucket *bbolt.Bucket, query Query, fn iteratorFn) error {
@@ -298,9 +296,10 @@ func (i *index) Iterate(bucket *bbolt.Bucket, query Query, fn iteratorFn) error
 	}
 
 	// Sort the parts of the Query to conform to the index key building order
-	sortedQueryParts, err := i.Sort(query, false)
-	if err != nil {
-		return err
+	sortedQueryParts := i.Sort(query, false)
+
+	if len(sortedQueryParts) == 0 {
+		return errors.New("unable to iterate over index without matching keys")
 	}
 
 	// extract tokenizer and transform to here

index_test.go

@@ -447,3 +447,98 @@ func TestIndex_addRefToBucket(t *testing.T) {
 		})
 	})
 }
+
+func TestIndex_Sort(t *testing.T) {
+	i := NewIndex(t.Name(),
+		NewFieldIndexer("path.part", AliasOption("key")),
+		NewFieldIndexer("path.more.#.parts", AliasOption("key2")),
+	)
+
+	t.Run("returns correct order when given in reverse", func(t *testing.T) {
+		sorted := i.Sort(
+			New(Eq("key2", "value")).
+				And(Eq("key", "value")), false)
+
+		if !assert.Len(t, sorted, 2) {
+			return
+		}
+		assert.Equal(t, "key", sorted[0].Name())
+		assert.Equal(t, "key2", sorted[1].Name())
+	})
+
+	t.Run("returns correct order when given in correct order", func(t *testing.T) {
+		sorted := i.Sort(
+			New(Eq("key", "value")).
+				And(Eq("key2", "value")), false)
+
+		if !assert.Len(t, sorted, 2) {
+			return
+		}
+		assert.Equal(t, "key", sorted[0].Name())
+		assert.Equal(t, "key2", sorted[1].Name())
+	})
+
+	t.Run("does not include any keys when primary key is missing", func(t *testing.T) {
+		sorted := i.Sort(
+			New(Eq("key2", "value")), false)
+
+		assert.Len(t, sorted, 0)
+	})
+
+	t.Run("includes all keys when includeMissing option is given", func(t *testing.T) {
+		sorted := i.Sort(
+			New(Eq("key3", "value")).
+				And(Eq("key2", "value")), true)
+
+		if !assert.Len(t, sorted, 2) {
+			return
+		}
+		assert.Equal(t, "key3", sorted[0].Name())
+		assert.Equal(t, "key2", sorted[1].Name())
+	})
+
+	t.Run("includes additional keys when includeMissing option is given", func(t *testing.T) {
+		sorted := i.Sort(
+			New(Eq("key3", "value")).
+				And(Eq("key", "value")), true)
+
+		if !assert.Len(t, sorted, 2) {
+			return
+		}
+		assert.Equal(t, "key", sorted[0].Name())
+		assert.Equal(t, "key3", sorted[1].Name())
+	})
+}
+
+func TestIndex_QueryPartsOutsideIndex(t *testing.T) {
+	i := NewIndex(t.Name(),
+		NewFieldIndexer("path.part", AliasOption("key")),
+		NewFieldIndexer("path.more.#.parts", AliasOption("key2")),
+	)
+
+	t.Run("returns empty list when all parts in index", func(t *testing.T) {
+		additional := i.QueryPartsOutsideIndex(
+			New(Eq("key2", "value")).
+				And(Eq("key", "value")))
+
+		assert.Len(t, additional, 0)
+	})
+
+	t.Run("returns all parts when none match index", func(t *testing.T) {
+		additional := i.QueryPartsOutsideIndex(
+			New(Eq("key2", "value")))
+
+		assert.Len(t, additional, 1)
+	})
+
+	t.Run("returns correct params on partial index match", func(t *testing.T) {
+		additional := i.QueryPartsOutsideIndex(
+			New(Eq("key3", "value")).
+				And(Eq("key", "value")))
+
+		if !assert.Len(t, additional, 1) {
+			return
+		}
+		assert.Equal(t, "key3", additional[0].Name())
+	})
+}

plan.go

@@ -90,10 +90,7 @@ func (f fullTableScanQueryPlan) execute(walker DocumentWalker) error {
 }
 
 func (i indexScanQueryPlan) execute(walker ReferenceScanFn) error {
-	queryParts, err := i.index.QueryPartsOutsideIndex(i.query)
-	if err != nil {
-		return err
-	}
+	queryParts := i.index.QueryPartsOutsideIndex(i.query)
 	if len(queryParts) != 0 {
 		return errors.New("no index with exact match to query found")
 	}
@@ -114,10 +111,7 @@ func (i indexScanQueryPlan) execute(walker ReferenceScanFn) error {
 }
 
 func (i resultScanQueryPlan) execute(walker DocumentWalker) error {
-	queryParts, err := i.index.QueryPartsOutsideIndex(i.query)
-	if err != nil {
-		return err
-	}
+	queryParts := i.index.QueryPartsOutsideIndex(i.query)
 
 	// do the IndexScan
 	return i.collection.db.View(func(tx *bbolt.Tx) error {