Skip to content

Commit a39a5f9

Browse files
committed
Fix Delete and Prepend methods to update index
Fixes the bson.Document.Delete and bson.Document.Prepend methods to properly update the index when being called. Fixes bson.Document.Prepend to set the index for the newly prepended element to 0 instead of the last element. GODRIVER-430 Change-Id: I9852541b5b7c39ee9f32f3359a91cb7612edb6e5
1 parent 9f4e27d commit a39a5f9

File tree

2 files changed

+80
-18
lines changed

2 files changed

+80
-18
lines changed

bson/document.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -217,16 +217,19 @@ func (d *Document) Prepend(elems ...*Element) *Document {
217217
}
218218
remaining--
219219
d.elems[idx] = elem
220+
for idx := range d.index {
221+
d.index[idx]++
222+
}
220223
i := sort.Search(len(d.index), func(i int) bool {
221224
return bytes.Compare(
222225
d.keyFromIndex(i), elem.value.data[elem.value.start+1:elem.value.offset]) >= 0
223226
})
224227
if i < len(d.index) {
225228
d.index = append(d.index, 0)
226229
copy(d.index[i+1:], d.index[i:])
227-
d.index[i] = uint32(len(d.elems) - 1)
230+
d.index[i] = 0
228231
} else {
229-
d.index = append(d.index, uint32(len(d.elems)-1))
232+
d.index = append(d.index, 0)
230233
}
231234
}
232235
return d
@@ -367,9 +370,6 @@ func (d *Document) LookupElementErr(key ...string) (*Element, error) {
367370
// returned. If the key does not exist, then nil is returned and the delete is
368371
// a no-op. The same is true if something along the depth tree does not exist
369372
// or is not a traversable type.
370-
//
371-
// TODO(skriptble): This method differs from Lookup when it comes to errors.
372-
// Should this method return errors to be consistent?
373373
func (d *Document) Delete(key ...string) *Element {
374374
if d == nil {
375375
panic(ErrNilDocument)
@@ -389,6 +389,11 @@ func (d *Document) Delete(key ...string) *Element {
389389
if len(key) == 1 {
390390
d.index = append(d.index[:i], d.index[i+1:]...)
391391
d.elems = append(d.elems[:keyIndex], d.elems[keyIndex+1:]...)
392+
for j := range d.index {
393+
if d.index[j] > keyIndex {
394+
d.index[j]--
395+
}
396+
}
392397
return elem
393398
}
394399
switch elem.value.Type() {

bson/document_test.go

Lines changed: 70 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,54 @@ func TestDocument(t *testing.T) {
203203
}()
204204
}
205205
})
206+
t.Run("Update Index Properly", func(t *testing.T) {
207+
a, b, c, d, e := EC.Null("a"), EC.Null("b"), EC.Null("c"), EC.Null("d"), EC.Null("e")
208+
testCases := []struct {
209+
name string
210+
elems [][]*Element
211+
index [][]uint32
212+
}{
213+
{
214+
"two",
215+
[][]*Element{{a}, {b}},
216+
[][]uint32{{0}, {1, 0}},
217+
},
218+
{
219+
"three",
220+
[][]*Element{{a}, {b}, {c}},
221+
[][]uint32{{0}, {1, 0}, {2, 1, 0}},
222+
},
223+
{
224+
"four",
225+
[][]*Element{{a}, {d}, {b}, {c}},
226+
[][]uint32{{0}, {1, 0}, {2, 0, 1}, {3, 1, 0, 2}},
227+
},
228+
{
229+
"five",
230+
[][]*Element{{a}, {d}, {e}, {b}, {c}},
231+
[][]uint32{{0}, {1, 0}, {2, 1, 0}, {3, 0, 2, 1}, {4, 1, 0, 3, 2}},
232+
},
233+
}
234+
235+
for _, tc := range testCases {
236+
t.Run(tc.name, func(t *testing.T) {
237+
d := NewDocument()
238+
for idx := range tc.elems {
239+
d.Prepend(tc.elems[idx]...)
240+
index := d.index
241+
for jdx := range tc.index[idx] {
242+
if tc.index[idx][jdx] != index[jdx] {
243+
t.Errorf(
244+
"Indexes do not match at %d: got %v; want %v",
245+
idx, index, tc.index[idx],
246+
)
247+
break
248+
}
249+
}
250+
}
251+
})
252+
}
253+
})
206254
testCases := []struct {
207255
name string
208256
elems [][]*Element
@@ -396,34 +444,43 @@ func TestDocument(t *testing.T) {
396444
t.Errorf("Delete should return nil element when deleting with empty key. got %#v; want %#v", got, want)
397445
}
398446
})
447+
d, c, b, a := EC.Null("d"), EC.Null("c"), EC.Null("b"), EC.Null("a")
399448
testCases := []struct {
400449
name string
401450
d *Document
402-
key []string
403-
want *Element
451+
keys [][]string
452+
want []*Element
404453
}{
405-
{"first", (&Document{}).Append(EC.Null("x")), []string{"x"},
406-
&Element{&Value{start: 0, offset: 3}},
454+
{"first", (&Document{}).Append(EC.Null("x")), [][]string{{"x"}},
455+
[]*Element{{&Value{start: 0, offset: 3}}},
407456
},
408457
{"depth-one", (&Document{}).Append(EC.SubDocumentFromElements("x", EC.Null("y"))),
409-
[]string{"x", "y"},
410-
&Element{&Value{start: 0, offset: 3}},
458+
[][]string{{"x", "y"}},
459+
[]*Element{{&Value{start: 0, offset: 3}}},
411460
},
412461
{"invalid-depth-traversal", (&Document{}).Append(EC.Null("x")),
413-
[]string{"x", "y"},
414-
nil,
462+
[][]string{{"x", "y"}},
463+
[]*Element{nil},
415464
},
416465
{"not-found", (&Document{}).Append(EC.Null("x")),
417-
[]string{"y"},
418-
nil,
466+
[][]string{{"y"}},
467+
[]*Element{nil},
468+
},
469+
{
470+
"delete twice",
471+
NewDocument(d, c, b, a),
472+
[][]string{{"d"}, {"c"}},
473+
[]*Element{d, c},
419474
},
420475
}
421476

422477
for _, tc := range testCases {
423478
t.Run(tc.name, func(t *testing.T) {
424-
got := tc.d.Delete(tc.key...)
425-
if !elementEqual(got, tc.want) {
426-
t.Errorf("Returned element does not match expected element. got %#v; want %#v", got, tc.want)
479+
for idx := range tc.keys {
480+
got := tc.d.Delete(tc.keys[idx]...)
481+
if !elementEqual(got, tc.want[idx]) {
482+
t.Errorf("Returned element does not match expected element. got %#v; want %#v", got, tc.want[idx])
483+
}
427484
}
428485
})
429486
}

0 commit comments

Comments
 (0)