Skip to content

Commit d947d92

Browse files
author
iwysiu
authored
GODRIVER-1469 ensure gridfs index checking supports indexes created i… (#341)
1 parent 6f5e216 commit d947d92

File tree

2 files changed

+197
-24
lines changed

2 files changed

+197
-24
lines changed

‎mongo/gridfs/bucket.go

Lines changed: 58 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,8 @@ package gridfs // import "go.mongodb.org/mongo-driver/mongo/gridfs"
99
import (
1010
"bytes"
1111
"context"
12-
13-
"io"
14-
1512
"errors"
16-
13+
"io"
1714
"time"
1815

1916
"go.mongodb.org/mongo-driver/bson"
@@ -455,8 +452,50 @@ func (b *Bucket) findChunks(ctx context.Context, fileID interface{}) (*mongo.Cur
455452
return chunksCursor, nil
456453
}
457454

455+
// returns true if the 2 index documents are equal
456+
func numericalIndexDocsEqual(expected, actual bsoncore.Document) (bool, error) {
457+
if bytes.Equal(expected, actual) {
458+
return true, nil
459+
}
460+
461+
actualElems, err := actual.Elements()
462+
if err != nil {
463+
return false, err
464+
}
465+
expectedElems, err := expected.Elements()
466+
if err != nil {
467+
return false, err
468+
}
469+
470+
if len(actualElems) != len(expectedElems) {
471+
return false, nil
472+
}
473+
474+
for idx, expectedElem := range expectedElems {
475+
actualElem := actualElems[idx]
476+
if actualElem.Key() != expectedElem.Key() {
477+
return false, nil
478+
}
479+
480+
actualVal := actualElem.Value()
481+
expectedVal := expectedElem.Value()
482+
actualInt, actualOK := actualVal.AsInt64OK()
483+
expectedInt, expectedOK := expectedVal.AsInt64OK()
484+
485+
//GridFS indexes always have numeric values
486+
if !actualOK || !expectedOK {
487+
return false, nil
488+
}
489+
490+
if actualInt != expectedInt {
491+
return false, nil
492+
}
493+
}
494+
return true, nil
495+
}
496+
458497
// Create an index if it doesn't already exist
459-
func createIndexIfNotExists(ctx context.Context, iv mongo.IndexView, model mongo.IndexModel) error {
498+
func createNumericalIndexIfNotExists(ctx context.Context, iv mongo.IndexView, model mongo.IndexModel) error {
460499
c, err := iv.List(ctx)
461500
if err != nil {
462501
return err
@@ -465,33 +504,31 @@ func createIndexIfNotExists(ctx context.Context, iv mongo.IndexView, model mongo
465504
_ = c.Close(ctx)
466505
}()
467506

468-
var found bool
507+
modelKeysBytes, err := bson.Marshal(model.Keys)
508+
if err != nil {
509+
return err
510+
}
511+
modelKeysDoc := bsoncore.Document(modelKeysBytes)
512+
469513
for c.Next(ctx) {
470514
keyElem, err := c.Current.LookupErr("key")
471515
if err != nil {
472516
return err
473517
}
474518

475519
keyElemDoc := keyElem.Document()
476-
modelKeysDoc, err := bson.Marshal(model.Keys)
477-
if err != nil {
478-
return err
479-
}
480520

481-
if bytes.Equal(modelKeysDoc, keyElemDoc) {
482-
found = true
483-
break
484-
}
485-
}
486-
487-
if !found {
488-
_, err = iv.CreateOne(ctx, model)
521+
found, err := numericalIndexDocsEqual(modelKeysDoc, bsoncore.Document(keyElemDoc))
489522
if err != nil {
490523
return err
491524
}
525+
if found {
526+
return nil
527+
}
492528
}
493529

494-
return nil
530+
_, err = iv.CreateOne(ctx, model)
531+
return err
495532
}
496533

497534
// create indexes on the files and chunks collection if needed
@@ -528,10 +565,10 @@ func (b *Bucket) createIndexes(ctx context.Context) error {
528565
Options: options.Index().SetUnique(true),
529566
}
530567

531-
if err = createIndexIfNotExists(ctx, filesIv, filesModel); err != nil {
568+
if err = createNumericalIndexIfNotExists(ctx, filesIv, filesModel); err != nil {
532569
return err
533570
}
534-
if err = createIndexIfNotExists(ctx, chunksIv, chunksModel); err != nil {
571+
if err = createNumericalIndexIfNotExists(ctx, chunksIv, chunksModel); err != nil {
535572
return err
536573
}
537574

‎mongo/integration/gridfs_test.go

Lines changed: 139 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,14 @@ import (
1414
"testing"
1515
"time"
1616

17+
"go.mongodb.org/mongo-driver/bson"
18+
"go.mongodb.org/mongo-driver/event"
1719
"go.mongodb.org/mongo-driver/internal/testutil/assert"
1820
"go.mongodb.org/mongo-driver/internal/testutil/israce"
1921
"go.mongodb.org/mongo-driver/mongo"
2022
"go.mongodb.org/mongo-driver/mongo/gridfs"
2123
"go.mongodb.org/mongo-driver/mongo/integration/mtest"
2224
"go.mongodb.org/mongo-driver/mongo/options"
23-
"go.mongodb.org/mongo-driver/x/bsonx"
2425
)
2526

2627
func TestGridFS(x *testing.T) {
@@ -45,6 +46,141 @@ func TestGridFS(x *testing.T) {
4546
findIndex(findCtx, mt, mt.DB.Collection("fs.files"), false, "key", "filename")
4647
findIndex(findCtx, mt, mt.DB.Collection("fs.chunks"), true, "key", "files_id")
4748
})
49+
// should not create a new index if index is numerically the same
50+
mt.Run("equivalent indexes", func(mt *mtest.T) {
51+
tests := []struct {
52+
name string
53+
filesIndex bson.D
54+
chunksIndex bson.D
55+
newIndexes bool
56+
}{
57+
{
58+
"numerically equal",
59+
bson.D{
60+
{"key", bson.D{{"filename", float64(1.0)}, {"uploadDate", float64(1.0)}}},
61+
{"name", "filename_1_uploadDate_1"},
62+
},
63+
bson.D{
64+
{"key", bson.D{{"files_id", float64(1.0)}, {"n", float64(1.0)}}},
65+
{"name", "files_id_1_n_1"},
66+
{"unique", true},
67+
},
68+
false,
69+
},
70+
{
71+
"numerically inequal",
72+
bson.D{
73+
{"key", bson.D{{"filename", float64(-1.0)}, {"uploadDate", float64(1.0)}}},
74+
{"name", "filename_-1_uploadDate_1"},
75+
},
76+
bson.D{
77+
{"key", bson.D{{"files_id", float64(1.0)}, {"n", float64(-1.0)}}},
78+
{"name", "files_id_1_n_-1"},
79+
{"unique", true},
80+
},
81+
true,
82+
},
83+
}
84+
for _, test := range tests {
85+
mt.Run(test.name, func(mt *mtest.T) {
86+
mt.Run("OpenUploadStream", func(mt *mtest.T) {
87+
// add indexes with floats to collections manually
88+
res := mt.DB.RunCommand(context.Background(),
89+
bson.D{
90+
{"createIndexes", "fs.files"},
91+
{"indexes", bson.A{
92+
test.filesIndex,
93+
}},
94+
},
95+
)
96+
assert.Nil(mt, res.Err(), "createIndexes error: %v", res.Err())
97+
98+
res = mt.DB.RunCommand(context.Background(),
99+
bson.D{
100+
{"createIndexes", "fs.chunks"},
101+
{"indexes", bson.A{
102+
test.chunksIndex,
103+
}},
104+
},
105+
)
106+
assert.Nil(mt, res.Err(), "createIndexes error: %v", res.Err())
107+
108+
mt.ClearEvents()
109+
110+
bucket, err := gridfs.NewBucket(mt.DB)
111+
assert.Nil(mt, err, "NewBucket error: %v", err)
112+
defer func() {
113+
_ = bucket.Drop()
114+
}()
115+
116+
_, err = bucket.OpenUploadStream("filename")
117+
assert.Nil(mt, err, "OpenUploadStream error: %v", err)
118+
119+
mt.FilterStartedEvents(func(evt *event.CommandStartedEvent) bool {
120+
return evt.CommandName == "createIndexes"
121+
})
122+
evt := mt.GetStartedEvent()
123+
if test.newIndexes {
124+
if evt == nil {
125+
mt.Fatalf("expected createIndexes events but got none")
126+
}
127+
} else {
128+
if evt != nil {
129+
mt.Fatalf("expected no createIndexes events but got %v", evt.Command)
130+
}
131+
}
132+
})
133+
mt.Run("UploadFromStream", func(mt *mtest.T) {
134+
// add indexes with floats to collections manually
135+
res := mt.DB.RunCommand(context.Background(),
136+
bson.D{
137+
{"createIndexes", "fs.files"},
138+
{"indexes", bson.A{
139+
test.filesIndex,
140+
}},
141+
},
142+
)
143+
assert.Nil(mt, res.Err(), "createIndexes error: %v", res.Err())
144+
145+
res = mt.DB.RunCommand(context.Background(),
146+
bson.D{
147+
{"createIndexes", "fs.chunks"},
148+
{"indexes", bson.A{
149+
test.chunksIndex,
150+
}},
151+
},
152+
)
153+
assert.Nil(mt, res.Err(), "createIndexes error: %v", res.Err())
154+
155+
mt.ClearEvents()
156+
var fileContent []byte
157+
bucket, err := gridfs.NewBucket(mt.DB)
158+
assert.Nil(mt, err, "NewBucket error: %v", err)
159+
defer func() {
160+
_ = bucket.Drop()
161+
}()
162+
163+
_, err = bucket.UploadFromStream("filename", bytes.NewBuffer(fileContent))
164+
assert.Nil(mt, err, "UploadFromStream error: %v", err)
165+
166+
mt.FilterStartedEvents(func(evt *event.CommandStartedEvent) bool {
167+
return evt.CommandName == "createIndexes"
168+
})
169+
evt := mt.GetStartedEvent()
170+
if test.newIndexes {
171+
if evt == nil {
172+
mt.Fatalf("expected createIndexes events but got none")
173+
}
174+
} else {
175+
if evt != nil {
176+
mt.Fatalf("expected no createIndexes events but got %v", evt.Command)
177+
}
178+
}
179+
})
180+
})
181+
}
182+
})
183+
48184
mt.RunOpts("round trip", mtest.NewOptions().MaxServerVersion("3.6"), func(mt *mtest.T) {
49185
skipRoundTripTest(mt)
50186
oneK := 1024
@@ -135,10 +271,10 @@ func skipRoundTripTest(mt *mtest.T) {
135271
return
136272
}
137273

138-
var serverStatus bsonx.Doc
274+
var serverStatus bson.Raw
139275
err := mt.DB.RunCommand(
140276
context.Background(),
141-
bsonx.Doc{{"serverStatus", bsonx.Int32(1)}},
277+
bson.D{{"serverStatus", 1}},
142278
).Decode(&serverStatus)
143279
assert.Nil(mt, err, "serverStatus error %v", err)
144280

0 commit comments

Comments
 (0)