Skip to content

Commit

Permalink
[FIXED] DeleteRange deletes one message too much (#6111)
Browse files Browse the repository at this point in the history
When a `DeleteRange` was used to mark deletes, when looping over it with
`dr.Range(..)` it would delete one message too much.

Compared with a `DeleteSlice` the `DeleteRange` also didn't return the
correct `last` for `dr.State()`. Also being +1 what it should be.

Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
  • Loading branch information
derekcollison authored Nov 12, 2024
2 parents 1418887 + c663cbe commit dd0c986
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 2 deletions.
8 changes: 6 additions & 2 deletions server/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,12 +295,16 @@ type DeleteRange struct {
}

func (dr *DeleteRange) State() (first, last, num uint64) {
return dr.First, dr.First + dr.Num, dr.Num
deletesAfterFirst := dr.Num
if deletesAfterFirst > 0 {
deletesAfterFirst--
}
return dr.First, dr.First + deletesAfterFirst, dr.Num
}

// Range will range over all the deleted sequences represented by this block.
func (dr *DeleteRange) Range(f func(uint64) bool) {
for seq := dr.First; seq <= dr.First+dr.Num; seq++ {
for seq := dr.First; seq < dr.First+dr.Num; seq++ {
if !f(seq) {
return
}
Expand Down
32 changes: 32 additions & 0 deletions server/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,35 @@ func TestStoreMsgLoadNextMsgMulti(t *testing.T) {
},
)
}

func TestStoreDeleteSlice(t *testing.T) {
ds := DeleteSlice{2}
var deletes []uint64
ds.Range(func(seq uint64) bool {
deletes = append(deletes, seq)
return true
})
require_Len(t, len(deletes), 1)
require_Equal(t, deletes[0], 2)

first, last, num := ds.State()
require_Equal(t, first, 2)
require_Equal(t, last, 2)
require_Equal(t, num, 1)
}

func TestStoreDeleteRange(t *testing.T) {
dr := DeleteRange{First: 2, Num: 1}
var deletes []uint64
dr.Range(func(seq uint64) bool {
deletes = append(deletes, seq)
return true
})
require_Len(t, len(deletes), 1)
require_Equal(t, deletes[0], 2)

first, last, num := dr.State()
require_Equal(t, first, 2)
require_Equal(t, last, 2)
require_Equal(t, num, 1)
}

0 comments on commit dd0c986

Please sign in to comment.