From 1ad59090719d4bb58f303d0995e48a1a67c1bcf9 Mon Sep 17 00:00:00 2001 From: Evan Culver Date: Tue, 12 Oct 2021 15:01:29 -0700 Subject: [PATCH 1/4] Add test for sub-indexer case --- index_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/index_test.go b/index_test.go index 56b9cd5..b3d77f6 100644 --- a/index_test.go +++ b/index_test.go @@ -5,6 +5,7 @@ import ( crand "crypto/rand" "encoding/binary" "fmt" + "reflect" "sort" "strings" "testing" @@ -1314,3 +1315,44 @@ 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") + } + // this goes in between each index prefix to compare them correctly + nilb := string([]byte{0}) + want := []string{ + "Foo1" + nilb + "Test" + nilb + "Qux" + nilb, + "Foo1" + nilb + "Test" + nilb + "Qux2" + nilb, + "Foo1" + nilb + "Test2" + nilb + "Qux" + nilb, + "Foo1" + nilb + "Test2" + nilb + "Qux2" + nilb, + } + 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) + } +} From 63b9ac20019eca465e78c54079c5fe7a29422843 Mon Sep 17 00:00:00 2001 From: Wing Date: Tue, 5 Oct 2021 04:58:38 -0700 Subject: [PATCH 2/4] Fixes CompoundMultiIndex::FromObject not generating correct value combinations. --- index.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/index.go b/index.go index 3b87d94..8c27e46 100644 --- a/index.go +++ b/index.go @@ -821,7 +821,9 @@ forloop: 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 } From 77bd1ee43a75c355dcbcbbbdc902cf9447985ece Mon Sep 17 00:00:00 2001 From: Evan Culver Date: Wed, 27 Oct 2021 13:10:01 -0700 Subject: [PATCH 3/4] Use updated Go image --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index ca8822a..d89441c 100644 --- a/.circleci/config.yml +++ b/.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: From abc550368b953b73dbac88e503a858d995493d7c Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 5 Nov 2021 13:57:11 -0400 Subject: [PATCH 4/4] Add a benchmark and property test Also fix a bug when the index is 0 length, found by the property test Also cleanup the existing test a little bit by using the '\x00' literal --- index.go | 9 ++++- index_test.go | 106 +++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 107 insertions(+), 8 deletions(-) diff --git a/index.go b/index.go index 8c27e46..7ff31ce 100644 --- a/index.go +++ b/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,6 +819,10 @@ 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] { diff --git a/index_test.go b/index_test.go index b3d77f6..efe3f53 100644 --- a/index_test.go +++ b/index_test.go @@ -5,10 +5,13 @@ import ( crand "crypto/rand" "encoding/binary" "fmt" + "math/rand" "reflect" "sort" "strings" "testing" + "testing/quick" + "time" ) type TestObject struct { @@ -1340,13 +1343,11 @@ func TestCompoundMultiIndex_FromObject(t *testing.T) { if !ok { t.Fatalf("should be ok") } - // this goes in between each index prefix to compare them correctly - nilb := string([]byte{0}) want := []string{ - "Foo1" + nilb + "Test" + nilb + "Qux" + nilb, - "Foo1" + nilb + "Test" + nilb + "Qux2" + nilb, - "Foo1" + nilb + "Test2" + nilb + "Qux" + nilb, - "Foo1" + nilb + "Test2" + nilb + "Qux2" + nilb, + "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 { @@ -1356,3 +1357,96 @@ func TestCompoundMultiIndex_FromObject(t *testing.T) { 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) + } + } +}