From cab89d9ce09d40c57b8d3e854882d571620f88ca Mon Sep 17 00:00:00 2001 From: Marie Gauthier Date: Tue, 7 Sep 2021 17:36:23 +0200 Subject: [PATCH 1/2] Fix bytes key field max length (#518) Co-authored-by: Ru Horlick --- orm/primary_key.go | 8 ++++---- orm/primary_key_test.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/orm/primary_key.go b/orm/primary_key.go index 91e439dbb0..c960315924 100644 --- a/orm/primary_key.go +++ b/orm/primary_key.go @@ -40,7 +40,7 @@ type PrimaryKeyed interface { // integers are encoded using 4 or 8 byte big endian. // // IMPORTANT: []byte parts are encoded with a single byte length prefix, - // so cannot be longer than 256 bytes. + // so cannot be longer than 255 bytes. // // The `IndexKeyCodec` used with the `PrimaryKeyTable` may add certain // constraints to the byte representation as max length = 255 in @@ -97,11 +97,11 @@ func primaryKeyFieldBytes(field interface{}) []byte { } // Prefix the byte array with its length as 8 bytes. The function will panic -// if the bytes length is bigger than 256. +// if the bytes length is bigger than 255. func AddLengthPrefix(bytes []byte) []byte { byteLen := len(bytes) - if byteLen > 256 { - panic("Cannot create primary key with an []byte of length greater than 256 bytes. Try again with a smaller []byte.") + if byteLen > 255 { + panic("Cannot create primary key with an []byte of length greater than 255 bytes. Try again with a smaller []byte.") } prefixedBytes := make([]byte, 1+len(bytes)) diff --git a/orm/primary_key_test.go b/orm/primary_key_test.go index 9c7ad5eb90..ca28a91495 100644 --- a/orm/primary_key_test.go +++ b/orm/primary_key_test.go @@ -313,7 +313,7 @@ func TestAddLengthPrefix(t *testing.T) { } require.Panics(t, func() { - orm.AddLengthPrefix(make([]byte, 300)) + orm.AddLengthPrefix(make([]byte, 256)) }) } From 4e3666b34c905abdd573dabfc9614593d9861991 Mon Sep 17 00:00:00 2001 From: Ru Horlick Date: Tue, 7 Sep 2021 17:42:25 +0100 Subject: [PATCH 2/2] refactor: Address first batch of changes from orm audit (#490) Address first batch of updates from ORM internal API audit #490: Change Save to Update Refactor TableExportable interface to not expose private table structure Add Set function Change Create to check for non-existence Rename AfterSaveInterceptor to AfterSetInterceptor for consistency Closes #490 --- orm/auto_uint64.go | 61 +++++++++--- orm/genesis.go | 93 ++---------------- orm/genesis_test.go | 4 +- orm/go.mod | 2 +- orm/index.go | 4 +- orm/index_test.go | 6 +- orm/orm.go | 8 +- orm/orm_scenario_test.go | 14 +-- orm/primary_key.go | 51 ++++++---- orm/primary_key_property_test.go | 20 +++- orm/table.go | 155 +++++++++++++++++++----------- orm/table_test.go | 2 +- x/ecocredit/server/credit_type.go | 2 +- x/ecocredit/server/genesis.go | 13 ++- x/ecocredit/server/msg_server.go | 4 +- x/group/server/genesis.go | 21 ++-- x/group/server/msg_server.go | 20 ++-- 17 files changed, 255 insertions(+), 225 deletions(-) diff --git a/orm/auto_uint64.go b/orm/auto_uint64.go index 3aa8e99a1b..52be56ce06 100644 --- a/orm/auto_uint64.go +++ b/orm/auto_uint64.go @@ -3,6 +3,7 @@ package orm import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/errors" ) var _ Indexable = &AutoUInt64TableBuilder{} @@ -37,7 +38,6 @@ func (a AutoUInt64TableBuilder) Build() AutoUInt64Table { } } -var _ SequenceExportable = &AutoUInt64Table{} var _ TableExportable = &AutoUInt64Table{} // AutoUInt64Table is the table type which an auto incrementing ID. @@ -46,8 +46,11 @@ type AutoUInt64Table struct { seq Sequence } -// Create a new persistent object with an auto generated uint64 primary key. They key is returned. -// Create iterates though the registered callbacks and may add secondary index keys by them. +// Create a new persistent object with an auto generated uint64 primary key. The +// key is returned. +// +// Create iterates through the registered callbacks that may add secondary index +// keys. func (a AutoUInt64Table) Create(ctx HasKVStore, obj codec.ProtoMarshaler) (uint64, error) { autoIncID := a.seq.NextVal(ctx) err := a.table.Create(ctx, EncodeSequence(autoIncID), obj) @@ -57,20 +60,32 @@ func (a AutoUInt64Table) Create(ctx HasKVStore, obj codec.ProtoMarshaler) (uint6 return autoIncID, nil } -// Save updates the given object under the rowID key. It expects the key to exists already -// and fails with an `ErrNotFound` otherwise. Any caller must therefore make sure that this contract -// is fulfilled. Parameters must not be nil. +// Update updates the given object under the rowID key. It expects the key to +// exists already and fails with an `ErrNotFound` otherwise. Any caller must +// therefore make sure that this contract is fulfilled. Parameters must not be +// nil. // -// Save iterates though the registered callbacks and may add or remove secondary index keys by them. -func (a AutoUInt64Table) Save(ctx HasKVStore, rowID uint64, newValue codec.ProtoMarshaler) error { - return a.table.Save(ctx, EncodeSequence(rowID), newValue) +// Update iterates through the registered callbacks that may add or remove +// secondary index keys. +func (a AutoUInt64Table) Update(ctx HasKVStore, rowID uint64, newValue codec.ProtoMarshaler) error { + return a.table.Update(ctx, EncodeSequence(rowID), newValue) } -// Delete removes the object under the rowID key. It expects the key to exists already -// and fails with a `ErrNotFound` otherwise. Any caller must therefore make sure that this contract -// is fulfilled. +// Set persists the given object under the rowID key. It does not check if the +// key already exists and overwrites the value if it does. // -// Delete iterates though the registered callbacks and removes secondary index keys by them. +// Set iterates through the registered callbacks that may add secondary index +// keys. +func (a AutoUInt64Table) Set(ctx HasKVStore, rowID uint64, newValue codec.ProtoMarshaler) error { + return a.table.Set(ctx, EncodeSequence(rowID), newValue) +} + +// Delete removes the object under the rowID key. It expects the key to exists +// already and fails with a `ErrNotFound` otherwise. Any caller must therefore +// make sure that this contract is fulfilled. +// +// Delete iterates through the registered callbacks that remove secondary index +// keys. func (a AutoUInt64Table) Delete(ctx HasKVStore, rowID uint64) error { return a.table.Delete(ctx, EncodeSequence(rowID)) } @@ -128,7 +143,21 @@ func (a AutoUInt64Table) Sequence() Sequence { return a.seq } -// Table satisfies the TableExportable interface and must not be used otherwise. -func (a AutoUInt64Table) Table() table { - return a.table +// Export stores all the values in the table in the passed ModelSlicePtr and +// returns the current value of the associated sequence. +func (a AutoUInt64Table) Export(ctx HasKVStore, dest ModelSlicePtr) (uint64, error) { + _, err := a.table.Export(ctx, dest) + if err != nil { + return 0, err + } + return a.seq.CurVal(ctx), nil +} + +// Import clears the table and initializes it from the given data interface{}. +// data should be a slice of structs that implement PrimaryKeyed. +func (a AutoUInt64Table) Import(ctx HasKVStore, data interface{}, seqValue uint64) error { + if err := a.seq.InitVal(ctx, seqValue); err != nil { + return errors.Wrap(err, "sequence") + } + return a.table.Import(ctx, data, seqValue) } diff --git a/orm/genesis.go b/orm/genesis.go index dc9afd49b8..64c07f5609 100644 --- a/orm/genesis.go +++ b/orm/genesis.go @@ -1,88 +1,15 @@ package orm -import ( - "reflect" - - "github.com/cosmos/cosmos-sdk/store/prefix" - "github.com/cosmos/cosmos-sdk/types/errors" -) - // TableExportable type TableExportable interface { - // Table returns the table to export - Table() table -} - -// SequenceExportable -type SequenceExportable interface { - // Sequence returns the sequence to export - Sequence() Sequence -} - -// ExportTableData iterates over the given table entries and stores them at the passed ModelSlicePtr. -// When the given table implements the `SequenceExportable` interface then it's current value -// is returned as well or otherwise defaults to 0. -func ExportTableData(ctx HasKVStore, t TableExportable, dest ModelSlicePtr) (uint64, error) { - it, err := t.Table().PrefixScan(ctx, nil, nil) - if err != nil { - return 0, errors.Wrap(err, "table PrefixScan failure when exporting table data") - } - _, err = ReadAll(it, dest) - if err != nil { - return 0, err - } - var seqValue uint64 - if st, ok := t.(SequenceExportable); ok { - seqValue = st.Sequence().CurVal(ctx) - } - return seqValue, err -} - -// ImportTableData initializes a table and attaches indexers from the given data interface{}. -// data should be a slice of structs that implement PrimaryKeyed (eg []*GroupInfo). -// The seqValue is optional and only used with tables that implement the `SequenceExportable` interface. -func ImportTableData(ctx HasKVStore, t TableExportable, data interface{}, seqValue uint64) error { - table := t.Table() - if err := clearAllInTable(ctx, table); err != nil { - return errors.Wrap(err, "clear old entries") - } - - if st, ok := t.(SequenceExportable); ok { - if err := st.Sequence().InitVal(ctx, seqValue); err != nil { - return errors.Wrap(err, "sequence") - } - } - - // Provided data must be a slice - modelSlice := reflect.ValueOf(data) - if modelSlice.Kind() != reflect.Slice { - return errors.Wrap(ErrArgument, "data must be a slice") - } - - // Create table entries - for i := 0; i < modelSlice.Len(); i++ { - obj, ok := modelSlice.Index(i).Interface().(PrimaryKeyed) - if !ok { - return errors.Wrapf(ErrArgument, "unsupported type :%s", reflect.TypeOf(data).Elem().Elem()) - } - err := table.Create(ctx, PrimaryKey(obj), obj) - if err != nil { - return err - } - } - - return nil -} - -// clearAllInTable deletes all entries in a table with delete interceptors called -func clearAllInTable(ctx HasKVStore, table table) error { - store := prefix.NewStore(ctx.KVStore(table.storeKey), []byte{table.prefix}) - it := store.Iterator(nil, nil) - defer it.Close() - for ; it.Valid(); it.Next() { - if err := table.Delete(ctx, it.Key()); err != nil { - return err - } - } - return nil + // Export stores all the values in the table in the passed + // ModelSlicePtr. If the table has an associated sequence, then its + // current value is returned, otherwise 0 is returned by default. + Export(HasKVStore, ModelSlicePtr) (uint64, error) + + // Import clears the table and initializes it from the given data + // interface{}. data should be a slice of structs that implement + // PrimaryKeyed (eg []*GroupInfo). The seqValue is optional and only + // used with tables that have an associated sequence. + Import(HasKVStore, interface{}, uint64) error } diff --git a/orm/genesis_test.go b/orm/genesis_test.go index a097842c36..7104d1d4e3 100644 --- a/orm/genesis_test.go +++ b/orm/genesis_test.go @@ -34,7 +34,7 @@ func TestImportExportTableData(t *testing.T) { }, } - err = orm.ImportTableData(ctx, table, groups, 2) + err = table.Import(ctx, groups, 2) require.NoError(t, err) for _, g := range groups { @@ -46,7 +46,7 @@ func TestImportExportTableData(t *testing.T) { } var exported []*testdata.GroupInfo - seq, err := orm.ExportTableData(ctx, table, &exported) + seq, err := table.Export(ctx, &exported) require.NoError(t, err) require.Equal(t, seq, uint64(2)) diff --git a/orm/go.mod b/orm/go.mod index 2dae2f8db3..12cf4c5244 100644 --- a/orm/go.mod +++ b/orm/go.mod @@ -7,7 +7,7 @@ require ( github.com/gogo/protobuf v1.3.3 github.com/stretchr/testify v1.7.0 github.com/tendermint/tm-db v0.6.4 - pgregory.net/rapid v0.4.7 // indirect + pgregory.net/rapid v0.4.7 ) replace github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alpha.regen.1 diff --git a/orm/index.go b/orm/index.go index 915c698d1b..c0eb86d5ac 100644 --- a/orm/index.go +++ b/orm/index.go @@ -58,7 +58,7 @@ func newIndex(builder Indexable, prefix byte, indexer *Indexer) (MultiKeyIndex, indexer: indexer, indexKeyCodec: codec, } - builder.AddAfterSaveInterceptor(idx.onSave) + builder.AddAfterSetInterceptor(idx.onSet) builder.AddAfterDeleteInterceptor(idx.onDelete) return idx, nil } @@ -135,7 +135,7 @@ func (i MultiKeyIndex) ReversePrefixScan(ctx HasKVStore, start []byte, end []byt return indexIterator{ctx: ctx, it: it, rowGetter: i.rowGetter, keyCodec: i.indexKeyCodec}, nil } -func (i MultiKeyIndex) onSave(ctx HasKVStore, rowID RowID, newValue, oldValue codec.ProtoMarshaler) error { +func (i MultiKeyIndex) onSet(ctx HasKVStore, rowID RowID, newValue, oldValue codec.ProtoMarshaler) error { store := prefix.NewStore(ctx.KVStore(i.storeKey), []byte{i.prefix}) if oldValue == nil { return i.indexer.OnCreate(store, rowID, newValue) diff --git a/orm/index_test.go b/orm/index_test.go index 12ff9f2f18..19b313beea 100644 --- a/orm/index_test.go +++ b/orm/index_test.go @@ -27,7 +27,7 @@ func (b *nilCodecBuilder) RowGetter() orm.RowGetter { return func(a orm.HasKVStore, b orm.RowID, c codec.ProtoMarshaler) error { return nil } } func (b *nilCodecBuilder) IndexKeyCodec() orm.IndexKeyCodec { return nil } -func (b *nilCodecBuilder) AddAfterSaveInterceptor(orm.AfterSaveInterceptor) {} +func (b *nilCodecBuilder) AddAfterSetInterceptor(orm.AfterSetInterceptor) {} func (b *nilCodecBuilder) AddAfterDeleteInterceptor(orm.AfterDeleteInterceptor) {} type nilStoreKeyBuilder struct{} @@ -39,7 +39,7 @@ func (b *nilStoreKeyBuilder) RowGetter() orm.RowGetter { func (b *nilStoreKeyBuilder) IndexKeyCodec() orm.IndexKeyCodec { return orm.Max255DynamicLengthIndexKeyCodec{} } -func (b *nilStoreKeyBuilder) AddAfterSaveInterceptor(orm.AfterSaveInterceptor) {} +func (b *nilStoreKeyBuilder) AddAfterSetInterceptor(orm.AfterSetInterceptor) {} func (b *nilStoreKeyBuilder) AddAfterDeleteInterceptor(orm.AfterDeleteInterceptor) {} type nilRowGetterBuilder struct{} @@ -53,7 +53,7 @@ func (b *nilRowGetterBuilder) RowGetter() orm.RowGetter { func (b *nilRowGetterBuilder) IndexKeyCodec() orm.IndexKeyCodec { return orm.Max255DynamicLengthIndexKeyCodec{} } -func (b *nilRowGetterBuilder) AddAfterSaveInterceptor(orm.AfterSaveInterceptor) {} +func (b *nilRowGetterBuilder) AddAfterSetInterceptor(orm.AfterSetInterceptor) {} func (b *nilRowGetterBuilder) AddAfterDeleteInterceptor(orm.AfterDeleteInterceptor) {} func TestNewIndex(t *testing.T) { diff --git a/orm/orm.go b/orm/orm.go index 1797d9228e..65581d363e 100644 --- a/orm/orm.go +++ b/orm/orm.go @@ -40,7 +40,7 @@ func (r RowID) Bytes() []byte { return r } -// Validateable is an interface that Persistent types can implement and is called on any orm save or update operation. +// Validateable is an interface that Persistent types can implement and is called on any orm set operation. type Validateable interface { // ValidateBasic is a sanity check on the data. Any error returned prevents create or updates. ValidateBasic() error @@ -131,12 +131,12 @@ type Indexable interface { StoreKey() sdk.StoreKey RowGetter() RowGetter IndexKeyCodec() IndexKeyCodec - AddAfterSaveInterceptor(interceptor AfterSaveInterceptor) + AddAfterSetInterceptor(interceptor AfterSetInterceptor) AddAfterDeleteInterceptor(interceptor AfterDeleteInterceptor) } -// AfterSaveInterceptor defines a callback function to be called on Create + Update. -type AfterSaveInterceptor func(ctx HasKVStore, rowID RowID, newValue, oldValue codec.ProtoMarshaler) error +// AfterSetInterceptor defines a callback function to be called on Create + Update. +type AfterSetInterceptor func(ctx HasKVStore, rowID RowID, newValue, oldValue codec.ProtoMarshaler) error // AfterDeleteInterceptor defines a callback function to be called on Delete operations. type AfterDeleteInterceptor func(ctx HasKVStore, rowID RowID, value codec.ProtoMarshaler) error diff --git a/orm/orm_scenario_test.go b/orm/orm_scenario_test.go index a0de59a9d0..469419fe0c 100644 --- a/orm/orm_scenario_test.go +++ b/orm/orm_scenario_test.go @@ -65,7 +65,7 @@ func TestKeeperEndToEndWithAutoUInt64Table(t *testing.T) { // when updated g.Admin = []byte("new-admin-address") - err = k.groupTable.Save(ctx, rowID, &g) + err = k.groupTable.Update(ctx, rowID, &g) require.NoError(t, err) // then indexes are updated, too @@ -152,7 +152,7 @@ func TestKeeperEndToEndWithPrimaryKeyTable(t *testing.T) { Weight: m.Weight, } // then it should fail as the primary key is immutable - err = k.groupMemberTable.Save(ctx, updatedMember) + err = k.groupMemberTable.Update(ctx, updatedMember) require.Error(t, err) // and when entity updated with non primary key attribute modified @@ -162,7 +162,7 @@ func TestKeeperEndToEndWithPrimaryKeyTable(t *testing.T) { Weight: 99, } // then it should not fail - err = k.groupMemberTable.Save(ctx, updatedMember) + err = k.groupMemberTable.Update(ctx, updatedMember) require.NoError(t, err) // and when entity deleted @@ -298,13 +298,13 @@ func TestExportImportStateAutoUInt64Table(t *testing.T) { require.Equal(t, uint64(i), groupRowID) } var groups []*testdata.GroupInfo - seqVal, err := orm.ExportTableData(ctx, k.groupTable, &groups) + seqVal, err := k.groupTable.Export(ctx, &groups) require.NoError(t, err) // when a new db seeded ctx = orm.NewMockContext() - err = orm.ImportTableData(ctx, k.groupTable, groups, seqVal) + err = k.groupTable.Import(ctx, groups, seqVal) require.NoError(t, err) // then all data is set again @@ -354,13 +354,13 @@ func TestExportImportStatePrimaryKeyTable(t *testing.T) { testRecords[i-1] = g } var groupMembers []*testdata.GroupMember - _, err := orm.ExportTableData(ctx, k.groupMemberTable, &groupMembers) + _, err := k.groupMemberTable.Export(ctx, &groupMembers) require.NoError(t, err) // when a new db seeded ctx = orm.NewMockContext() - err = orm.ImportTableData(ctx, k.groupMemberTable, groupMembers, 0) + err = k.groupMemberTable.Import(ctx, groupMembers, 0) require.NoError(t, err) // then all data is set again diff --git a/orm/primary_key.go b/orm/primary_key.go index c960315924..de6023e7b6 100644 --- a/orm/primary_key.go +++ b/orm/primary_key.go @@ -127,29 +127,40 @@ type PrimaryKeyTable struct { // Create persists the given object under their primary key. It checks if the // key already exists and may return an `ErrUniqueConstraint`. -// Create iterates though the registered callbacks and may add secondary index keys by them. +// +// Create iterates through the registered callbacks that may add secondary +// index keys. func (a PrimaryKeyTable) Create(ctx HasKVStore, obj PrimaryKeyed) error { rowID := PrimaryKey(obj) - if a.table.Has(ctx, rowID) { - return ErrUniqueConstraint - } return a.table.Create(ctx, rowID, obj) } -// Save updates the given object under the primary key. It expects the key to exists already -// and fails with an `ErrNotFound` otherwise. Any caller must therefore make sure that this contract -// is fulfilled. Parameters must not be nil. +// Update updates the given object under the primary key. It expects the key to +// exists already and fails with an `ErrNotFound` otherwise. Any caller must +// therefore make sure that this contract is fulfilled. Parameters must not be +// nil. // -// Save iterates though the registered callbacks and may add or remove secondary index keys by them. -func (a PrimaryKeyTable) Save(ctx HasKVStore, newValue PrimaryKeyed) error { - return a.table.Save(ctx, PrimaryKey(newValue), newValue) +// Update iterates through the registered callbacks that may add or remove +// secondary index keys. +func (a PrimaryKeyTable) Update(ctx HasKVStore, newValue PrimaryKeyed) error { + return a.table.Update(ctx, PrimaryKey(newValue), newValue) } -// Delete removes the object. It expects the primary key to exists already -// and fails with a `ErrNotFound` otherwise. Any caller must therefore make sure that this contract -// is fulfilled. +// Set persists the given object under the rowID key. It does not check if the +// key already exists and overwrites the value if it does. // -// Delete iterates though the registered callbacks and removes secondary index keys by them. +// Set iterates through the registered callbacks that may add secondary index +// keys. +func (a PrimaryKeyTable) Set(ctx HasKVStore, newValue PrimaryKeyed) error { + return a.table.Set(ctx, PrimaryKey(newValue), newValue) +} + +// Delete removes the object. It expects the primary key to exists already and +// fails with a `ErrNotFound` otherwise. Any caller must therefore make sure +// that this contract is fulfilled. +// +// Delete iterates through the registered callbacks that remove secondary index +// keys. func (a PrimaryKeyTable) Delete(ctx HasKVStore, obj PrimaryKeyed) error { return a.table.Delete(ctx, PrimaryKey(obj)) } @@ -206,7 +217,13 @@ func (a PrimaryKeyTable) ReversePrefixScan(ctx HasKVStore, start, end []byte) (I return a.table.ReversePrefixScan(ctx, start, end) } -// Table satisfies the TableExportable interface and must not be used otherwise. -func (a PrimaryKeyTable) Table() table { - return a.table +// Export stores all the values in the table in the passed ModelSlicePtr. +func (a PrimaryKeyTable) Export(ctx HasKVStore, dest ModelSlicePtr) (uint64, error) { + return a.table.Export(ctx, dest) +} + +// Import clears the table and initializes it from the given data interface{}. +// data should be a slice of structs that implement PrimaryKeyed. +func (a PrimaryKeyTable) Import(ctx HasKVStore, data interface{}, seqValue uint64) error { + return a.table.Import(ctx, data, seqValue) } diff --git a/orm/primary_key_property_test.go b/orm/primary_key_property_test.go index 02ca9c897b..22289b2b2c 100644 --- a/orm/primary_key_property_test.go +++ b/orm/primary_key_property_test.go @@ -116,9 +116,9 @@ func (m *primaryKeyMachine) Create(t *rapid.T) { } } -// Save is one of the model commands. It updates the value at a given primary +// Update is one of the model commands. It updates the value at a given primary // key and fails if that primary key doesn't already exist in the table. -func (m *primaryKeyMachine) Save(t *rapid.T) { +func (m *primaryKeyMachine) Update(t *rapid.T) { gm := m.genGroupMember().Draw(t, "gm").(*testdata.GroupMember) // We can only really change the weight here, because Group and Member @@ -126,8 +126,8 @@ func (m *primaryKeyMachine) Save(t *rapid.T) { newWeight := rapid.Uint64().Draw(t, "newWeight").(uint64) gm.Weight = newWeight - // Perform the real Save - err := m.table.Save(m.ctx, gm) + // Perform the real Update + err := m.table.Update(m.ctx, gm) if m.state[string(orm.PrimaryKey(gm))] == nil { // If there's no value in the model, we expect an error @@ -141,6 +141,18 @@ func (m *primaryKeyMachine) Save(t *rapid.T) { } } +// Set is one of the model commands. It sets the value at a key in the table +// whether it exists or not. +func (m *primaryKeyMachine) Set(t *rapid.T) { + g := genGroupMember.Draw(t, "g").(*testdata.GroupMember) + pk := string(orm.PrimaryKey(g)) + + err := m.table.Set(m.ctx, g) + + require.NoError(t, err) + m.state[pk] = g +} + // Delete is one of the model commands. It removes the object with the given // primary key from the table and returns an error if that primary key doesn't // already exist in the table. diff --git a/orm/table.go b/orm/table.go index 78fd8b5693..dd1d64c3cb 100644 --- a/orm/table.go +++ b/orm/table.go @@ -18,7 +18,7 @@ type tableBuilder struct { prefixData byte storeKey sdk.StoreKey indexKeyCodec IndexKeyCodec - afterSave []AfterSaveInterceptor + afterSet []AfterSetInterceptor afterDelete []AfterDeleteInterceptor cdc codec.Codec } @@ -72,15 +72,15 @@ func (a tableBuilder) Build() table { model: a.model, prefix: a.prefixData, storeKey: a.storeKey, - afterSave: a.afterSave, + afterSet: a.afterSet, afterDelete: a.afterDelete, cdc: a.cdc, } } -// AddAfterSaveInterceptor can be used to register a callback function that is executed after an object is created and/or updated. -func (a *tableBuilder) AddAfterSaveInterceptor(interceptor AfterSaveInterceptor) { - a.afterSave = append(a.afterSave, interceptor) +// AddAfterSetInterceptor can be used to register a callback function that is executed after an object is created and/or updated. +func (a *tableBuilder) AddAfterSetInterceptor(interceptor AfterSetInterceptor) { + a.afterSet = append(a.afterSet, interceptor) } // AddAfterDeleteInterceptor can be used to register a callback function that is executed after an object is deleted. @@ -104,58 +104,48 @@ type table struct { model reflect.Type prefix byte storeKey sdk.StoreKey - afterSave []AfterSaveInterceptor + afterSet []AfterSetInterceptor afterDelete []AfterDeleteInterceptor cdc codec.Codec } -// Create persists the given object under the rowID key. It does not check if the -// key already exists. Any caller must either make sure that this contract is fulfilled -// by providing a universal unique ID or sequence that is guaranteed to not exist yet or -// by checking the state via `Has` function before. +// Create persists the given object under the rowID key, returning an +// ErrUniqueConstraint if a value already exists at that key. // -// Create iterates though the registered callbacks and may add secondary index keys by them. +// Create iterates through the registered callbacks that may add secondary index +// keys. func (a table) Create(ctx HasKVStore, rowID RowID, obj codec.ProtoMarshaler) error { - if err := a.assertValue(rowID, obj); err != nil { - return err + if a.Has(ctx, rowID) { + return ErrUniqueConstraint } - store := prefix.NewStore(ctx.KVStore(a.storeKey), []byte{a.prefix}) - return a.save(ctx, store, rowID, obj, nil) -} -// assertValue performs common assertions for key and value to be stored using ORM. -func (a table) assertValue(key RowID, obj codec.ProtoMarshaler) error { - if len(key) == 0 { - return ErrEmptyKey - } - if err := assertCorrectType(a.model, obj); err != nil { - return err - } - return assertValid(obj) + return a.Set(ctx, rowID, obj) } -// save stores the K/V in the store -func (a table) save(ctx HasKVStore, store prefix.Store, key RowID, obj, oldObj codec.ProtoMarshaler) error { - val, err := a.cdc.Marshal(obj) - if err != nil { - return errors.Wrapf(err, "failed to serialize %T", val) +// Update updates the given object under the rowID key. It expects the key to +// exists already and fails with an `ErrNotFound` otherwise. Any caller must +// therefore make sure that this contract is fulfilled. Parameters must not be +// nil. +// +// Update iterates through the registered callbacks that may add or remove +// secondary index keys. +func (a table) Update(ctx HasKVStore, rowID RowID, newValue codec.ProtoMarshaler) error { + if !a.Has(ctx, rowID) { + return ErrNotFound } - store.Set(key, val) - for i, itc := range a.afterSave { - if err := itc(ctx, key, obj, oldObj); err != nil { - return errors.Wrapf(err, "interceptor %d failed", i) - } - } - return nil + return a.Set(ctx, rowID, newValue) } -// Save updates the given object under the rowID key. It expects the key to exists already -// and fails with an `ErrNotFound` otherwise. Any caller must therefore make sure that this contract -// is fulfilled. Parameters must not be nil. +// Set persists the given object under the rowID key. It does not check if the +// key already exists and overwrites the value if it does. // -// Save iterates though the registered callbacks and may add or remove secondary index keys by them. -func (a table) Save(ctx HasKVStore, rowID RowID, newValue codec.ProtoMarshaler) error { +// Set iterates through the registered callbacks that may add secondary index +// keys. +func (a table) Set(ctx HasKVStore, rowID RowID, newValue codec.ProtoMarshaler) error { + if len(rowID) == 0 { + return ErrEmptyKey + } if err := assertCorrectType(a.model, newValue); err != nil { return err } @@ -164,12 +154,25 @@ func (a table) Save(ctx HasKVStore, rowID RowID, newValue codec.ProtoMarshaler) } store := prefix.NewStore(ctx.KVStore(a.storeKey), []byte{a.prefix}) - var oldValue = reflect.New(a.model).Interface().(codec.ProtoMarshaler) - if err := a.GetOne(ctx, rowID, oldValue); err != nil { - return errors.Wrap(err, "load old value") + var oldValue codec.ProtoMarshaler + if a.Has(ctx, rowID) { + oldValue = reflect.New(a.model).Interface().(codec.ProtoMarshaler) + a.GetOne(ctx, rowID, oldValue) + } + + newValueEncoded, err := a.cdc.Marshal(newValue) + if err != nil { + return errors.Wrapf(err, "failed to serialize %T", newValue) } - return a.save(ctx, store, rowID, newValue, oldValue) + + store.Set(rowID, newValueEncoded) + for i, itc := range a.afterSet { + if err := itc(ctx, rowID, newValue, oldValue); err != nil { + return errors.Wrapf(err, "interceptor %d failed", i) + } + } + return nil } func assertValid(obj codec.ProtoMarshaler) error { @@ -181,11 +184,12 @@ func assertValid(obj codec.ProtoMarshaler) error { return nil } -// Delete removes the object under the rowID key. It expects the key to exists already -// and fails with a `ErrNotFound` otherwise. Any caller must therefore make sure that this contract -// is fulfilled. +// Delete removes the object under the rowID key. It expects the key to exists +// already and fails with a `ErrNotFound` otherwise. Any caller must therefore +// make sure that this contract is fulfilled. // -// Delete iterates though the registered callbacks and removes secondary index keys by them. +// Delete iterates through the registered callbacks that remove secondary index +// keys. func (a table) Delete(ctx HasKVStore, rowID RowID) error { store := prefix.NewStore(ctx.KVStore(a.storeKey), []byte{a.prefix}) @@ -203,8 +207,8 @@ func (a table) Delete(ctx HasKVStore, rowID RowID) error { return nil } -// Has checks if a key exists. Returns false when the key is empty or nil because -// we don't allow creation of values without a key. +// Has checks if a key exists. Returns false when the key is empty or nil +// because we don't allow creation of values without a key. func (a table) Has(ctx HasKVStore, key RowID) bool { if len(key) == 0 { return false @@ -275,8 +279,51 @@ func (a table) ReversePrefixScan(ctx HasKVStore, start, end RowID) (Iterator, er }, nil } -func (a table) Table() table { - return a +// Export stores all the values in the table in the passed ModelSlicePtr. +func (a table) Export(ctx HasKVStore, dest ModelSlicePtr) (uint64, error) { + it, err := a.PrefixScan(ctx, nil, nil) + if err != nil { + return 0, errors.Wrap(err, "table Export failure when exporting table data") + } + _, err = ReadAll(it, dest) + if err != nil { + return 0, err + } + return 0, nil +} + +// Import clears the table and initializes it from the given data interface{}. +// data should be a slice of structs that implement PrimaryKeyed. +func (a table) Import(ctx HasKVStore, data interface{}, _ uint64) error { + // Clear all data + store := prefix.NewStore(ctx.KVStore(a.storeKey), []byte{a.prefix}) + it := store.Iterator(nil, nil) + defer it.Close() + for ; it.Valid(); it.Next() { + if err := a.Delete(ctx, it.Key()); err != nil { + return err + } + } + + // Provided data must be a slice + modelSlice := reflect.ValueOf(data) + if modelSlice.Kind() != reflect.Slice { + return errors.Wrap(ErrArgument, "data must be a slice") + } + + // Import values from slice + for i := 0; i < modelSlice.Len(); i++ { + obj, ok := modelSlice.Index(i).Interface().(PrimaryKeyed) + if !ok { + return errors.Wrapf(ErrArgument, "unsupported type :%s", reflect.TypeOf(data).Elem().Elem()) + } + err := a.Create(ctx, PrimaryKey(obj), obj) + if err != nil { + return err + } + } + + return nil } // typeSafeIterator is initialized with a type safe RowGetter only. diff --git a/orm/table_test.go b/orm/table_test.go index 047c9bcf64..e52a70bd6f 100644 --- a/orm/table_test.go +++ b/orm/table_test.go @@ -186,7 +186,7 @@ func TestUpdate(t *testing.T) { require.NoError(t, err) // when - err = myTable.Save(ctx, []byte("my-id"), spec.src) + err = myTable.Update(ctx, []byte("my-id"), spec.src) require.True(t, spec.expErr.Is(err), "got ", err) // then diff --git a/x/ecocredit/server/credit_type.go b/x/ecocredit/server/credit_type.go index 3b7e42f710..d904385550 100644 --- a/x/ecocredit/server/credit_type.go +++ b/x/ecocredit/server/credit_type.go @@ -42,7 +42,7 @@ func (s serverImpl) getCreditTypeSeqNextVal(ctx sdk.Context, creditType ecocredi case nil: // Increment the sequence number creditTypeSeq.SeqNumber++ - err = s.creditTypeSeqTable.Save(ctx, &creditTypeSeq) + err = s.creditTypeSeqTable.Update(ctx, &creditTypeSeq) if err != nil { return 0, err } diff --git a/x/ecocredit/server/genesis.go b/x/ecocredit/server/genesis.go index 3532658b6e..0778c355a3 100644 --- a/x/ecocredit/server/genesis.go +++ b/x/ecocredit/server/genesis.go @@ -8,7 +8,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/pkg/errors" - "github.com/regen-network/regen-ledger/orm" "github.com/regen-network/regen-ledger/types/math" abci "github.com/tendermint/tendermint/abci/types" @@ -24,15 +23,15 @@ func (s serverImpl) InitGenesis(ctx types.Context, cdc codec.Codec, data json.Ra s.paramSpace.SetParamSet(ctx.Context, &genesisState.Params) - if err := orm.ImportTableData(ctx, s.creditTypeSeqTable, genesisState.Sequences, 0); err != nil { + if err := s.creditTypeSeqTable.Import(ctx, genesisState.Sequences, 0); err != nil { return nil, errors.Wrap(err, "sequences") } - if err := orm.ImportTableData(ctx, s.classInfoTable, genesisState.ClassInfo, 0); err != nil { + if err := s.classInfoTable.Import(ctx, genesisState.ClassInfo, 0); err != nil { return nil, errors.Wrap(err, "class-info") } - if err := orm.ImportTableData(ctx, s.batchInfoTable, genesisState.BatchInfo, 0); err != nil { + if err := s.batchInfoTable.Import(ctx, genesisState.BatchInfo, 0); err != nil { return nil, errors.Wrap(err, "batch-info") } @@ -139,17 +138,17 @@ func (s serverImpl) ExportGenesis(ctx types.Context, cdc codec.Codec) (json.RawM store := ctx.KVStore(s.storeKey) var classInfo []*ecocredit.ClassInfo - if _, err := orm.ExportTableData(ctx, s.classInfoTable, &classInfo); err != nil { + if _, err := s.classInfoTable.Export(ctx, &classInfo); err != nil { return nil, errors.Wrap(err, "class-info") } var batchInfo []*ecocredit.BatchInfo - if _, err := orm.ExportTableData(ctx, s.batchInfoTable, &batchInfo); err != nil { + if _, err := s.batchInfoTable.Export(ctx, &batchInfo); err != nil { return nil, errors.Wrap(err, "batch-info") } var sequences []*ecocredit.CreditTypeSeq - if _, err := orm.ExportTableData(ctx, s.creditTypeSeqTable, &sequences); err != nil { + if _, err := s.creditTypeSeqTable.Export(ctx, &sequences); err != nil { return nil, errors.Wrap(err, "batch-info") } diff --git a/x/ecocredit/server/msg_server.go b/x/ecocredit/server/msg_server.go index 0d29b80964..ef529c4728 100644 --- a/x/ecocredit/server/msg_server.go +++ b/x/ecocredit/server/msg_server.go @@ -429,7 +429,7 @@ func (s serverImpl) Cancel(goCtx context.Context, req *ecocredit.MsgCancel) (*ec } batchInfo.AmountCancelled = amountCancelled.String() - if err = s.batchInfoTable.Save(ctx, &batchInfo); err != nil { + if err = s.batchInfoTable.Update(ctx, &batchInfo); err != nil { return nil, err } @@ -455,7 +455,7 @@ func (s serverImpl) nextBatchInClass(ctx types.Context, classInfo *ecocredit.Cla // Update the ClassInfo classInfo.NumBatches = nextVal - err := s.classInfoTable.Save(ctx, classInfo) + err := s.classInfoTable.Update(ctx, classInfo) if err != nil { return 0, err } diff --git a/x/group/server/genesis.go b/x/group/server/genesis.go index 7c125a8eeb..8eb9ca142a 100644 --- a/x/group/server/genesis.go +++ b/x/group/server/genesis.go @@ -7,7 +7,6 @@ import ( "github.com/cosmos/cosmos-sdk/types/errors" abci "github.com/tendermint/tendermint/abci/types" - "github.com/regen-network/regen-ledger/orm" "github.com/regen-network/regen-ledger/types" "github.com/regen-network/regen-ledger/x/group" ) @@ -16,26 +15,26 @@ func (s serverImpl) InitGenesis(ctx types.Context, cdc codec.Codec, data json.Ra var genesisState group.GenesisState cdc.MustUnmarshalJSON(data, &genesisState) - if err := orm.ImportTableData(ctx, s.groupTable, genesisState.Groups, genesisState.GroupSeq); err != nil { + if err := s.groupTable.Import(ctx, genesisState.Groups, genesisState.GroupSeq); err != nil { return nil, errors.Wrap(err, "groups") } - if err := orm.ImportTableData(ctx, s.groupMemberTable, genesisState.GroupMembers, 0); err != nil { + if err := s.groupMemberTable.Import(ctx, genesisState.GroupMembers, 0); err != nil { return nil, errors.Wrap(err, "group members") } - if err := orm.ImportTableData(ctx, s.groupAccountTable, genesisState.GroupAccounts, 0); err != nil { + if err := s.groupAccountTable.Import(ctx, genesisState.GroupAccounts, 0); err != nil { return nil, errors.Wrap(err, "group accounts") } if err := s.groupAccountSeq.InitVal(ctx, genesisState.GroupAccountSeq); err != nil { return nil, errors.Wrap(err, "group account seq") } - if err := orm.ImportTableData(ctx, s.proposalTable, genesisState.Proposals, genesisState.ProposalSeq); err != nil { + if err := s.proposalTable.Import(ctx, genesisState.Proposals, genesisState.ProposalSeq); err != nil { return nil, errors.Wrap(err, "proposals") } - if err := orm.ImportTableData(ctx, s.voteTable, genesisState.Votes, 0); err != nil { + if err := s.voteTable.Import(ctx, genesisState.Votes, 0); err != nil { return nil, errors.Wrap(err, "votes") } @@ -46,7 +45,7 @@ func (s serverImpl) ExportGenesis(ctx types.Context, cdc codec.Codec) (json.RawM genesisState := group.NewGenesisState() var groups []*group.GroupInfo - groupSeq, err := orm.ExportTableData(ctx, s.groupTable, &groups) + groupSeq, err := s.groupTable.Export(ctx, &groups) if err != nil { return nil, errors.Wrap(err, "groups") } @@ -54,14 +53,14 @@ func (s serverImpl) ExportGenesis(ctx types.Context, cdc codec.Codec) (json.RawM genesisState.GroupSeq = groupSeq var groupMembers []*group.GroupMember - _, err = orm.ExportTableData(ctx, s.groupMemberTable, &groupMembers) + _, err = s.groupMemberTable.Export(ctx, &groupMembers) if err != nil { return nil, errors.Wrap(err, "group members") } genesisState.GroupMembers = groupMembers var groupAccounts []*group.GroupAccountInfo - _, err = orm.ExportTableData(ctx, s.groupAccountTable, &groupAccounts) + _, err = s.groupAccountTable.Export(ctx, &groupAccounts) if err != nil { return nil, errors.Wrap(err, "group accounts") } @@ -69,7 +68,7 @@ func (s serverImpl) ExportGenesis(ctx types.Context, cdc codec.Codec) (json.RawM genesisState.GroupAccountSeq = s.groupAccountSeq.CurVal(ctx) var proposals []*group.Proposal - proposalSeq, err := orm.ExportTableData(ctx, s.proposalTable, &proposals) + proposalSeq, err := s.proposalTable.Export(ctx, &proposals) if err != nil { return nil, errors.Wrap(err, "proposals") } @@ -77,7 +76,7 @@ func (s serverImpl) ExportGenesis(ctx types.Context, cdc codec.Codec) (json.RawM genesisState.ProposalSeq = proposalSeq var votes []*group.Vote - _, err = orm.ExportTableData(ctx, s.voteTable, &votes) + _, err = s.voteTable.Export(ctx, &votes) if err != nil { return nil, errors.Wrap(err, "votes") } diff --git a/x/group/server/msg_server.go b/x/group/server/msg_server.go index 87206a70af..3f7cf73e30 100644 --- a/x/group/server/msg_server.go +++ b/x/group/server/msg_server.go @@ -164,8 +164,8 @@ func (s serverImpl) UpdateGroupMembers(goCtx context.Context, req *group.MsgUpda if err != nil { return err } - // Save updated group member in the groupMemberTable. - if err := s.groupMemberTable.Save(ctx, &groupMember); err != nil { + // Update updated group member in the groupMemberTable. + if err := s.groupMemberTable.Update(ctx, &groupMember); err != nil { return sdkerrors.Wrap(err, "add member") } // else handle create. @@ -181,7 +181,7 @@ func (s serverImpl) UpdateGroupMembers(goCtx context.Context, req *group.MsgUpda // Update group in the groupTable. g.TotalWeight = totalWeight.String() g.Version++ - return s.groupTable.Save(ctx, g.GroupId, g) + return s.groupTable.Update(ctx, g.GroupId, g) } err := s.doUpdateGroup(ctx, req, action, "members updated") @@ -198,7 +198,7 @@ func (s serverImpl) UpdateGroupAdmin(goCtx context.Context, req *group.MsgUpdate g.Admin = req.NewAdmin g.Version++ - return s.groupTable.Save(ctx, g.GroupId, g) + return s.groupTable.Update(ctx, g.GroupId, g) } err := s.doUpdateGroup(ctx, req, action, "admin updated") @@ -214,7 +214,7 @@ func (s serverImpl) UpdateGroupMetadata(goCtx context.Context, req *group.MsgUpd action := func(g *group.GroupInfo) error { g.Metadata = req.Metadata g.Version++ - return s.groupTable.Save(ctx, g.GroupId, g) + return s.groupTable.Update(ctx, g.GroupId, g) } if err := assertMetadataLength(req.Metadata, "group metadata"); err != nil { @@ -318,7 +318,7 @@ func (s serverImpl) UpdateGroupAccountAdmin(goCtx context.Context, req *group.Ms action := func(groupAccount *group.GroupAccountInfo) error { groupAccount.Admin = req.NewAdmin groupAccount.Version++ - return s.groupAccountTable.Save(ctx, groupAccount) + return s.groupAccountTable.Update(ctx, groupAccount) } err := s.doUpdateGroupAccount(ctx, req.Address, req.Admin, action, "group account admin updated") @@ -340,7 +340,7 @@ func (s serverImpl) UpdateGroupAccountDecisionPolicy(goCtx context.Context, req } groupAccount.Version++ - return s.groupAccountTable.Save(ctx, groupAccount) + return s.groupAccountTable.Update(ctx, groupAccount) } err := s.doUpdateGroupAccount(ctx, req.Address, req.Admin, action, "group account decision policy updated") @@ -358,7 +358,7 @@ func (s serverImpl) UpdateGroupAccountMetadata(goCtx context.Context, req *group action := func(groupAccount *group.GroupAccountInfo) error { groupAccount.Metadata = metadata groupAccount.Version++ - return s.groupAccountTable.Save(ctx, groupAccount) + return s.groupAccountTable.Update(ctx, groupAccount) } if err := assertMetadataLength(metadata, "group account metadata"); err != nil { @@ -579,7 +579,7 @@ func (s serverImpl) Vote(goCtx context.Context, req *group.MsgVote) (*group.MsgV return nil, err } - if err = s.proposalTable.Save(ctx, id, &proposal); err != nil { + if err = s.proposalTable.Update(ctx, id, &proposal); err != nil { return nil, err } @@ -646,7 +646,7 @@ func (s serverImpl) Exec(goCtx context.Context, req *group.MsgExec) (*group.MsgE } storeUpdates := func() (*group.MsgExecResponse, error) { - if err := s.proposalTable.Save(ctx, id, &proposal); err != nil { + if err := s.proposalTable.Update(ctx, id, &proposal); err != nil { return nil, err } return &group.MsgExecResponse{}, nil