Fix combination generator in CompoundMultiIndex (#108)

* Fixes CompoundMultiIndex.FromObject not generating correct value
combinations.
* Add test for sub-indexer case
* Use updated Go image
* Add a benchmark and property test
* Fix a bug when the index is 0 length, found by the property test
* Cleanup the existing test a little bit by using the '\x00' literal

Co-authored-by: Wing <wing@stirista.com>
Co-authored-by: Daniel Nephin <dnephin@gmail.com>

.circleci/config.yml

@@ -2,7 +2,7 @@ version: 2.1
 
 references:
   images:
-    go: &GOLANG_IMAGE docker.mirror.hashicorp.services/circleci/golang:1.15.3
+    go: &GOLANG_IMAGE docker.mirror.hashicorp.services/circleci/golang:1.16.7
 
 # reusable 'executor' object for jobs
 executors:

index.go

@@ -767,8 +767,6 @@ type CompoundMultiIndex struct {
 func (c *CompoundMultiIndex) FromObject(raw interface{}) (bool, [][]byte, error) {
 	// At each entry, builder is storing the results from the next index
 	builder := make([][][]byte, 0, len(c.Indexes))
-	// Start with something higher to avoid resizing if possible
-	out := make([][]byte, 0, len(c.Indexes)^3)
 
 forloop:
 	// This loop goes through each indexer and adds the value(s) provided to the next
@@ -810,6 +808,9 @@ forloop:
 		}
 	}
 
+	// Start with something higher to avoid resizing if possible
+	out := make([][]byte, 0, len(c.Indexes)^3)
+
 	// We are walking through the builder slice essentially in a depth-first fashion,
 	// building the prefix and leaves as we go. If AllowMissing is false, we only insert
 	// these full paths to leaves. Otherwise, we also insert each prefix along the way.
@@ -818,10 +819,16 @@ forloop:
 	// field specified as "abc", it is valid to call FromArgs with just "abc".
 	var walkVals func([]byte, int)
 	walkVals = func(currPrefix []byte, depth int) {
+		if depth >= len(builder) {
+			return
+		}
+
 		if depth == len(builder)-1 {
 			// These are the "leaves", so append directly
 			for _, v := range builder[depth] {
-				out = append(out, append(currPrefix, v...))
+				outcome := make([]byte, len(currPrefix))
+				copy(outcome, currPrefix)
+				out = append(out, append(outcome, v...))
 			}
 			return
 		}

index_test.go

@@ -5,9 +5,13 @@ import (
 	crand "crypto/rand"
 	"encoding/binary"
 	"fmt"
+	"math/rand"
+	"reflect"
 	"sort"
 	"strings"
 	"testing"
+	"testing/quick"
+	"time"
 )
 
 type TestObject struct {
@@ -1314,3 +1318,135 @@ func TestCompoundIndex_PrefixFromArgs(t *testing.T) {
 		t.Fatalf("expected an error when passing too many arguments")
 	}
 }
+
+func TestCompoundMultiIndex_FromObject(t *testing.T) {
+	// handle sub-indexer case unique to MultiIndexer
+	obj := &TestObject{
+		ID:       "obj1-uuid",
+		Foo:      "Foo1",
+		Baz:      "yep",
+		Qux:      []string{"Test", "Test2"},
+		QuxEmpty: []string{"Qux", "Qux2"},
+	}
+	indexer := &CompoundMultiIndex{
+		Indexes: []Indexer{
+			&StringFieldIndex{Field: "Foo"},
+			&StringSliceFieldIndex{Field: "Qux"},
+			&StringSliceFieldIndex{Field: "QuxEmpty"},
+		},
+	}
+
+	ok, vals, err := indexer.FromObject(obj)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if !ok {
+		t.Fatalf("should be ok")
+	}
+	want := []string{
+		"Foo1\x00Test\x00Qux\x00",
+		"Foo1\x00Test\x00Qux2\x00",
+		"Foo1\x00Test2\x00Qux\x00",
+		"Foo1\x00Test2\x00Qux2\x00",
+	}
+	got := make([]string, len(vals))
+	for i, v := range vals {
+		got[i] = string(v)
+	}
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("\ngot:  %+v\nwant: %+v\n", got, want)
+	}
+}
+
+func TestCompoundMultiIndex_FromObject_IndexUniquenessProperty(t *testing.T) {
+	indexPermutations := [][]string{
+		{"Foo", "Qux", "QuxEmpty"},
+		{"Foo", "QuxEmpty", "Qux"},
+		{"QuxEmpty", "Qux", "Foo"},
+		{"QuxEmpty", "Foo", "Qux"},
+		{"Qux", "QuxEmpty", "Foo"},
+		{"Qux", "Foo", "QuxEmpty"},
+	}
+
+	fn := func(o TestObject) bool {
+		for _, perm := range indexPermutations {
+			indexer := indexerFromFieldNameList(perm)
+			ok, vals, err := indexer.FromObject(o)
+			if err != nil {
+				t.Logf("err: %v", err)
+				return false
+			}
+			if !ok {
+				t.Logf("should be ok")
+				return false
+			}
+			if !assertAllUnique(t, vals) {
+				return false
+			}
+		}
+		return true
+	}
+	seed := time.Now().UnixNano()
+	t.Logf("Using seed %v", seed)
+	cfg := quick.Config{Rand: rand.New(rand.NewSource(seed))}
+	if err := quick.Check(fn, &cfg); err != nil {
+		t.Fatalf("property not held: %v", err)
+	}
+}
+
+func assertAllUnique(t *testing.T, vals [][]byte) bool {
+	t.Helper()
+	s := make(map[string]struct{}, len(vals))
+	for _, index := range vals {
+		s[string(index)] = struct{}{}
+	}
+
+	if l := len(s); l != len(vals) {
+		t.Logf("expected %d unique indexes, got %v", len(vals), l)
+		return false
+	}
+	return true
+}
+
+func indexerFromFieldNameList(keys []string) *CompoundMultiIndex {
+	indexer := &CompoundMultiIndex{AllowMissing: true}
+	for _, key := range keys {
+		if key == "Foo" || key == "Baz" {
+			indexer.Indexes = append(indexer.Indexes, &StringFieldIndex{Field: key})
+			continue
+		}
+		indexer.Indexes = append(indexer.Indexes, &StringSliceFieldIndex{Field: key})
+	}
+	return indexer
+}
+
+func BenchmarkCompoundMultiIndex_FromObject(b *testing.B) {
+	obj := &TestObject{
+		ID:       "obj1-uuid",
+		Foo:      "Foo1",
+		Baz:      "yep",
+		Qux:      []string{"Test", "Test2"},
+		QuxEmpty: []string{"Qux", "Qux2"},
+	}
+	indexer := &CompoundMultiIndex{
+		Indexes: []Indexer{
+			&StringFieldIndex{Field: "Foo"},
+			&StringSliceFieldIndex{Field: "Qux"},
+			&StringSliceFieldIndex{Field: "QuxEmpty"},
+		},
+	}
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		ok, vals, err := indexer.FromObject(obj)
+		if err != nil {
+			b.Fatalf("expected no error, got: %v", err)
+		}
+		if !ok {
+			b.Fatalf("should be ok")
+		}
+		if l := len(vals); l != 4 {
+			b.Fatalf("expected 4 indexes, got %v", l)
+		}
+	}
+}