Skip to content

Commit

Permalink
feat(collections): do not error when iterating empty collections (#17290
Browse files Browse the repository at this point in the history
)

Co-authored-by: unknown unknown <unknown@unknown>
  • Loading branch information
testinginprod and unknown unknown authored Aug 7, 2023
1 parent 60198f0 commit b61c72f
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 20 deletions.
4 changes: 4 additions & 0 deletions collections/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ Ref: https://keepachangelog.com/en/1.0.0/

* [#17024](https://github.com/cosmos/cosmos-sdk/pull/17024) - Introduces `Triple`, a composite key with three keys.

### API Breaking

* [#17290](https://github.com/cosmos/cosmos-sdk/pull/17290) - Collections iteration methods (Iterate, Walk) will not error when the collection is empty.

### Improvements

* [#17021](https://github.com/cosmos/cosmos-sdk/pull/17021) Make collections implement the `appmodule.HasGenesis` interface.
Expand Down
8 changes: 0 additions & 8 deletions collections/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package collections
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
)
Expand Down Expand Up @@ -40,13 +39,6 @@ func (m Map[K, V]) exportGenesis(ctx context.Context, writer io.Writer) error {

it, err := m.Iterate(ctx, nil)
if err != nil {
// if the iterator is invalid, the state can be empty
// and we can just return an empty array.
if errors.Is(err, ErrInvalidIterator) {
_, err = io.WriteString(writer, "]")
return err
}

return err
}
defer it.Close()
Expand Down
8 changes: 6 additions & 2 deletions collections/indexes/reverse_pair_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,12 @@ func TestReversePair(t *testing.T) {
// assert if we remove address1 atom balance, we can no longer find it in the index
err = indexedMap.Remove(ctx, collections.Join("address1", "atom"))
require.NoError(t, err)
_, err = indexedMap.Indexes.Denom.MatchExact(ctx, "atom")
require.ErrorIs(t, collections.ErrInvalidIterator, err)
iter, err = indexedMap.Indexes.Denom.MatchExact(ctx, "atom")
require.NoError(t, err)
defer iter.Close()
pks, err = iter.PrimaryKeys()
require.NoError(t, err)
require.Empty(t, pks)
}

func TestUncheckedReversePair(t *testing.T) {
Expand Down
7 changes: 4 additions & 3 deletions collections/iter.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package collections

import (
"bytes"
"context"
"errors"
"fmt"
Expand Down Expand Up @@ -161,6 +162,9 @@ func parseRangeInstruction[K any](prefix []byte, keyCodec codec.KeyCodec[K], r R
} else {
endBytes = nextBytesPrefixKey(prefix)
}
if bytes.Compare(startBytes, endBytes) == 1 {
return nil, nil, 0, ErrInvalidIterator
}
return startBytes, endBytes, order, nil
}

Expand Down Expand Up @@ -191,9 +195,6 @@ func newIterator[K, V any](ctx context.Context, start, end []byte, order Order,
if err != nil {
return Iterator[K, V]{}, err
}
if !iter.Valid() {
return Iterator[K, V]{}, ErrInvalidIterator
}

return Iterator[K, V]{
kc: m.kc,
Expand Down
11 changes: 6 additions & 5 deletions collections/map.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package collections

import (
"bytes"
"context"
"fmt"

Expand Down Expand Up @@ -149,8 +150,7 @@ func (m Map[K, V]) Walk(ctx context.Context, ranger Ranger[K], walkFunc func(key
}

// Clear clears the collection contained within the provided key range.
// A nil ranger equals to clearing the whole collection. In case the collection
// is empty no error will be returned.
// A nil ranger equals to clearing the whole collection.
// NOTE: this API needs to be used with care, considering that as of today
// cosmos-sdk stores the deletion records to be committed in a memory cache,
// clearing a lot of data might make the node go OOM.
Expand Down Expand Up @@ -216,6 +216,10 @@ func (m Map[K, V]) IterateRaw(ctx context.Context, start, end []byte, order Orde
prefixedEnd = append(m.prefix, end...)
}

if bytes.Compare(prefixedStart, prefixedEnd) == 1 {
return Iterator[K, V]{}, ErrInvalidIterator
}

s := m.sa(ctx)
var (
storeIter store.Iterator
Expand All @@ -233,9 +237,6 @@ func (m Map[K, V]) IterateRaw(ctx context.Context, start, end []byte, order Orde
return Iterator[K, V]{}, err
}

if !storeIter.Valid() {
return Iterator[K, V]{}, ErrInvalidIterator
}
return Iterator[K, V]{
kc: m.kc,
vc: m.vc,
Expand Down
16 changes: 14 additions & 2 deletions collections/map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package collections

import (
"context"
"fmt"
"testing"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -51,8 +52,10 @@ func TestMap_Clear(t *testing.T) {
ctx, m := makeTest()
err := m.Clear(ctx, nil)
require.NoError(t, err)
_, err = m.Iterate(ctx, nil)
require.ErrorIs(t, err, ErrInvalidIterator)
err = m.Walk(ctx, nil, func(key, value uint64) (bool, error) {
return false, fmt.Errorf("should never be called")
})
require.NoError(t, err)
})

t.Run("custom ranger", func(t *testing.T) {
Expand Down Expand Up @@ -104,6 +107,15 @@ func TestMap_IterateRaw(t *testing.T) {
keys, err = iter.Keys()
require.NoError(t, err)
require.Equal(t, []uint64{2, 1, 0}, keys)

// test invalid iter
_, err = m.IterateRaw(ctx, []byte{0x2, 0x0}, []byte{0x0, 0x0}, OrderAscending)
require.ErrorIs(t, err, ErrInvalidIterator)

// test on empty collection iterating does not error
require.NoError(t, m.Clear(ctx, nil))
_, err = m.IterateRaw(ctx, nil, nil, OrderAscending)
require.NoError(t, err)
}

func Test_encodeKey(t *testing.T) {
Expand Down

0 comments on commit b61c72f

Please sign in to comment.