Skip to content

Commit 2c8e308

Browse files
dmhillydmhilly
committed
Fixed bugs in gridfs implementation
GODRIVER-724 Change-Id: I5afcd5b7dc810f1696a7ba1f258f3a80c4d0284c
1 parent 4f3b0a8 commit 2c8e308

File tree

6 files changed

+46
-179
lines changed

6 files changed

+46
-179
lines changed

‎data/gridfs/download.json

Lines changed: 0 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -355,97 +355,6 @@
355355
"error": "ChunkIsMissing"
356356
}
357357
},
358-
{
359-
"description": "Download when an intermediate chunk is the wrong size",
360-
"arrange": {
361-
"data": [
362-
{
363-
"update": "fs.chunks",
364-
"updates": [
365-
{
366-
"q": {
367-
"files_id": {
368-
"$oid": "000000000000000000000005"
369-
},
370-
"n": 1
371-
},
372-
"u": {
373-
"$set": {
374-
"data": {
375-
"$hex": "556677"
376-
}
377-
}
378-
}
379-
},
380-
{
381-
"q": {
382-
"files_id": {
383-
"$oid": "000000000000000000000005"
384-
},
385-
"n": 2
386-
},
387-
"u": {
388-
"$set": {
389-
"data": {
390-
"$hex": "8899aa"
391-
}
392-
}
393-
}
394-
}
395-
]
396-
}
397-
]
398-
},
399-
"act": {
400-
"operation": "download",
401-
"arguments": {
402-
"id": {
403-
"$oid": "000000000000000000000005"
404-
}
405-
}
406-
},
407-
"assert": {
408-
"error": "ChunkIsWrongSize"
409-
}
410-
},
411-
{
412-
"description": "Download when final chunk is the wrong size",
413-
"arrange": {
414-
"data": [
415-
{
416-
"update": "fs.chunks",
417-
"updates": [
418-
{
419-
"q": {
420-
"files_id": {
421-
"$oid": "000000000000000000000005"
422-
},
423-
"n": 2
424-
},
425-
"u": {
426-
"$set": {
427-
"data": {
428-
"$hex": "99"
429-
}
430-
}
431-
}
432-
}
433-
]
434-
}
435-
]
436-
},
437-
"act": {
438-
"operation": "download",
439-
"arguments": {
440-
"id": {
441-
"$oid": "000000000000000000000005"
442-
}
443-
}
444-
},
445-
"assert": {
446-
"error": "ChunkIsWrongSize"
447-
}
448-
},
449358
{
450359
"description": "Download legacy file with no name",
451360
"act": {

‎data/gridfs/download.yml

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -152,35 +152,6 @@ tests:
152152
id: { "$oid" : "000000000000000000000005" }
153153
assert:
154154
error: "ChunkIsMissing"
155-
-
156-
description: "Download when an intermediate chunk is the wrong size"
157-
arrange:
158-
data:
159-
-
160-
{ update : "fs.chunks", updates : [
161-
{ q : { files_id : { "$oid" : "000000000000000000000005" }, n : 1 }, u : { $set : { data : { $hex : "556677" } } } },
162-
{ q : { files_id : { "$oid" : "000000000000000000000005" }, n : 2 }, u : { $set : { data : { $hex : "8899aa" } } } }
163-
] }
164-
act:
165-
operation: download
166-
arguments:
167-
id: { "$oid" : "000000000000000000000005" }
168-
assert:
169-
error: "ChunkIsWrongSize"
170-
-
171-
description: "Download when final chunk is the wrong size"
172-
arrange:
173-
data:
174-
-
175-
{ update : "fs.chunks", updates : [
176-
{ q : { files_id : { "$oid" : "000000000000000000000005" }, n : 2 }, u : { $set : { data : { $hex : "99" } } } }
177-
] }
178-
act:
179-
operation: download
180-
arguments:
181-
id: { "$oid" : "000000000000000000000005" }
182-
assert:
183-
error: "ChunkIsWrongSize"
184155
-
185156
description: "Download legacy file with no name"
186157
act:

‎mongo/gridfs/bucket.go

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -458,31 +458,34 @@ func (b *Bucket) createIndexes(ctx context.Context) error {
458458

459459
docRes := cloned.FindOne(ctx, bsonx.Doc{}, options.FindOne().SetProjection(bsonx.Doc{{"_id", bsonx.Int32(1)}}))
460460

461-
err = docRes.Err()
462-
if err == mongo.ErrNoDocuments {
463-
filesIv := b.filesColl.Indexes()
464-
chunksIv := b.chunksColl.Indexes()
465-
466-
filesModel := mongo.IndexModel{
467-
Keys: bson.D{
468-
{"filename", int32(1)},
469-
{"uploadDate", int32(1)},
470-
},
471-
}
461+
_, err = docRes.DecodeBytes()
462+
if err != mongo.ErrNoDocuments {
463+
// nil, or error that occured during the FindOne operation
464+
return err
465+
}
472466

473-
chunksModel := mongo.IndexModel{
474-
Keys: bson.D{
475-
{"files_id", int32(1)},
476-
{"n", int32(1)},
477-
},
478-
}
467+
filesIv := b.filesColl.Indexes()
468+
chunksIv := b.chunksColl.Indexes()
479469

480-
if err = createIndexIfNotExists(ctx, filesIv, filesModel); err != nil {
481-
return err
482-
}
483-
if err = createIndexIfNotExists(ctx, chunksIv, chunksModel); err != nil {
484-
return err
485-
}
470+
filesModel := mongo.IndexModel{
471+
Keys: bson.D{
472+
{"filename", int32(1)},
473+
{"uploadDate", int32(1)},
474+
},
475+
}
476+
477+
chunksModel := mongo.IndexModel{
478+
Keys: bson.D{
479+
{"files_id", int32(1)},
480+
{"n", int32(1)},
481+
},
482+
}
483+
484+
if err = createIndexIfNotExists(ctx, filesIv, filesModel); err != nil {
485+
return err
486+
}
487+
if err = createIndexIfNotExists(ctx, chunksIv, chunksModel); err != nil {
488+
return err
486489
}
487490

488491
return nil

‎mongo/gridfs/download_stream.go

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ type DownloadStream struct {
3636
closed bool
3737
buffer []byte // store up to 1 chunk if the user provided buffer isn't big enough
3838
bufferStart int
39+
bufferEnd int
3940
expectedChunk int32 // index of next expected chunk
4041
readDeadline time.Time
4142
fileLen int64
@@ -93,21 +94,21 @@ func (ds *DownloadStream) Read(p []byte) (int, error) {
9394
var err error
9495

9596
for bytesCopied < len(p) {
96-
if ds.bufferStart == 0 {
97-
// buffer empty
97+
if ds.bufferStart >= ds.bufferEnd {
98+
// Buffer is empty and can load in data from new chunk.
9899
err = ds.fillBuffer(ctx)
99100
if err != nil {
100101
if err == errNoMoreChunks {
101102
return bytesCopied, nil
102103
}
103-
104104
return bytesCopied, err
105105
}
106106
}
107107

108-
copied := copy(p, ds.buffer[ds.bufferStart:])
108+
copied := copy(p[bytesCopied:], ds.buffer[ds.bufferStart:ds.bufferEnd])
109+
109110
bytesCopied += copied
110-
ds.bufferStart = (ds.bufferStart + copied) % int(ds.chunkSize)
111+
ds.bufferStart += copied
111112
}
112113

113114
return len(p), nil
@@ -187,22 +188,10 @@ func (ds *DownloadStream) fillBuffer(ctx context.Context) error {
187188
}
188189

189190
_, dataBytes := data.Binary()
191+
copied := copy(ds.buffer, dataBytes)
190192

191-
bytesLen := int32(len(dataBytes))
192-
if ds.expectedChunk == ds.numChunks {
193-
// final chunk can be fewer than ds.chunkSize bytes
194-
bytesDownloaded := ds.chunkSize * (ds.expectedChunk - 1)
195-
bytesRemaining := ds.fileLen - int64(bytesDownloaded)
196-
197-
if int64(bytesLen) != bytesRemaining {
198-
return ErrWrongSize
199-
}
200-
} else if bytesLen != ds.chunkSize {
201-
// all intermediate chunks must have size ds.chunkSize
202-
return ErrWrongSize
203-
}
204-
205-
copy(ds.buffer, dataBytes)
206193
ds.bufferStart = 0
194+
ds.bufferEnd = copied
195+
207196
return nil
208197
}

‎mongo/gridfs/upload_stream.go

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -106,24 +106,22 @@ func (us *UploadStream) Write(p []byte) (int, error) {
106106

107107
origLen := len(p)
108108
for {
109-
if us.bufferIndex == UploadBufferSize {
110-
err := us.uploadChunks(ctx)
111-
if err != nil {
112-
return 0, err
113-
}
114-
us.bufferIndex = 0
115-
}
116-
117109
if len(p) == 0 {
118110
break
119111
}
120112

121113
n := copy(us.buffer[us.bufferIndex:], p) // copy as much as possible
122114
p = p[n:]
123115
us.bufferIndex += n
124-
break
125-
}
126116

117+
if us.bufferIndex == UploadBufferSize {
118+
err := us.uploadChunks(ctx)
119+
if err != nil {
120+
return 0, err
121+
}
122+
us.bufferIndex = 0
123+
}
124+
}
127125
return origLen, nil
128126
}
129127

@@ -151,16 +149,15 @@ func (us *UploadStream) uploadChunks(ctx context.Context) error {
151149
numChunks := math.Ceil(float64(us.bufferIndex) / float64(us.chunkSize))
152150

153151
docs := make([]interface{}, int(numChunks))
154-
152+
begChunkIndex := us.chunkIndex
155153
for i := 0; i < us.bufferIndex; i += int(us.chunkSize) {
156154
var chunkData []byte
157155
if us.bufferIndex-i < int(us.chunkSize) {
158156
chunkData = us.buffer[i:us.bufferIndex]
159157
} else {
160158
chunkData = us.buffer[i : i+int(us.chunkSize)]
161159
}
162-
163-
docs[us.chunkIndex] = bsonx.Doc{
160+
docs[us.chunkIndex-begChunkIndex] = bsonx.Doc{
164161
{"_id", bsonx.ObjectID(primitive.NewObjectID())},
165162
{"files_id", bsonx.ObjectID(us.FileID)},
166163
{"n", bsonx.Int32(int32(us.chunkIndex))},

‎mongo/options/gridfsoptions.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,7 @@ type UploadOptions struct {
109109

110110
// GridFSUpload creates a new *UploadOptions
111111
func GridFSUpload() *UploadOptions {
112-
return &UploadOptions{
113-
ChunkSizeBytes: &DefaultChunkSize,
114-
}
112+
return &UploadOptions{}
115113
}
116114

117115
// SetChunkSizeBytes sets the chunk size in bytes for the upload. Defaults to 255KB if not set.

0 commit comments

Comments
 (0)