feat(orm)!: return ormerrors.NotFound for Get methods in codegen (#11113)

## Description

I was talking with @technicallyty and we agreed that returning an error value when an entity is not found is the common golang idiom. What do you think @fdymylja ?

You can see how this looks in practice in `module_test.go`. I'm not actually sure this makes the code more succinct or the orm any easier to use. But maybe in certain cases yes.



---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
This commit is contained in:
Aaron Craelius 2022-02-04 11:07:36 -05:00 committed by GitHub
parent a7fb1a1d51
commit 81105764f9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 78 additions and 43 deletions

View File

@ -4,7 +4,7 @@ go 1.17
require (
github.com/cosmos/cosmos-proto v1.0.0-alpha7
github.com/cosmos/cosmos-sdk/api v0.1.0-alpha3
github.com/cosmos/cosmos-sdk/api v0.1.0-alpha4
github.com/cosmos/cosmos-sdk/errors v1.0.0-beta.2
github.com/iancoleman/strcase v0.2.0
github.com/stretchr/testify v1.7.0
@ -41,7 +41,7 @@ require (
golang.org/x/text v0.3.6 // indirect
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
google.golang.org/genproto v0.0.0-20211223182754-3ac035c7e7cb // indirect
google.golang.org/grpc v1.43.0 // indirect
google.golang.org/grpc v1.44.0 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect
)

View File

@ -23,11 +23,10 @@ github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1/go.mod h1:eXthEFrGJvWH
github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8NzMklzPG4d5KIOhIy30Tk=
github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk=
github.com/cosmos/cosmos-proto v1.0.0-alpha6/go.mod h1:msdDWOvfStHLG+Z2y2SJ0dcqimZ2vc8M1MPnZ4jOF7U=
github.com/cosmos/cosmos-proto v1.0.0-alpha7 h1:yqYUOHF2jopwZh4dVQp3xgqwftE5/2hkrwIV6vkUbO0=
github.com/cosmos/cosmos-proto v1.0.0-alpha7/go.mod h1:dosO4pSAbJF8zWCzCoTWP7nNsjcvSUBQmniFxDg5daw=
github.com/cosmos/cosmos-sdk/api v0.1.0-alpha3 h1:tqpedvX/1UgQyX3W2hlW6Xg801FyojYT/NOHbW0oG+s=
github.com/cosmos/cosmos-sdk/api v0.1.0-alpha3/go.mod h1:Ht15guGn9F8b0lv8NkjXE9/asAvVUOt2n4gvQ4q5MyU=
github.com/cosmos/cosmos-sdk/api v0.1.0-alpha4 h1:z2si9sQNUTj2jw+24SujuUxcoNS3TVga/fdYsS4rJII=
github.com/cosmos/cosmos-sdk/api v0.1.0-alpha4/go.mod h1:gZu6sOu2vl4Fd7I+BjDSx2bxndwPgFLGfOegek3SQQo=
github.com/cosmos/cosmos-sdk/errors v1.0.0-beta.2 h1:bBglNlra8ZHb4dmbEE8V85ihLA+DkriSm7tcx6x/JWo=
github.com/cosmos/cosmos-sdk/errors v1.0.0-beta.2/go.mod h1:Gi7pzVRnvZ1N16JAXpLADzng0ePoE7YeEHaULSFB2Ts=
github.com/cpuguy83/go-md2man v1.0.10/go.mod h1:SmD6nW6nTyfqj6ABTjUi3V3JVMnlJmwcJI5acqYI6dE=
@ -250,8 +249,8 @@ google.golang.org/grpc v1.33.1/go.mod h1:fr5YgcSWrqhRRxogOsw7RzIpsmvOZ6IcH4kBYTp
google.golang.org/grpc v1.36.0/go.mod h1:qjiiYl8FncCW8feJPdyg3v6XW24KsRHe+dy9BAGRRjU=
google.golang.org/grpc v1.40.0/go.mod h1:ogyxbiOoUXAkP+4+xa6PZSE9DZgIHtSpzjDTB9KAK34=
google.golang.org/grpc v1.42.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU=
google.golang.org/grpc v1.43.0 h1:Eeu7bZtDZ2DpRCsLhUlcrLnvYaMK1Gz86a+hMVvELmM=
google.golang.org/grpc v1.43.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU=
google.golang.org/grpc v1.44.0 h1:weqSxi/TMs1SqFRMHCtBgXRs8k3X39QIDEZ0pRcttUg=
google.golang.org/grpc v1.44.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU=
google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8=
google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0=
google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM=
@ -263,7 +262,6 @@ google.golang.org/protobuf v1.23.1-0.20200526195155-81db48ad09cc/go.mod h1:EGpAD
google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c=
google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw=
google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
google.golang.org/protobuf v1.27.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
google.golang.org/protobuf v1.27.1 h1:SnqbnDw1V7RiZcXPx5MEeqPv2s79L9i7BJUlG/+RurQ=
google.golang.org/protobuf v1.27.1/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

View File

@ -23,6 +23,8 @@ type tableGen struct {
ormTable ormtable.Table
}
const notFoundDocs = " returns nil and an error which responds true to ormerrors.IsNotFound() if the record was not found."
func newTableGen(fileGen fileGen, msg *protogen.Message, table *ormv1alpha1.TableDescriptor) (*tableGen, error) {
t := &tableGen{fileGen: fileGen, msg: msg, table: table, fields: map[protoreflect.Name]*protogen.Field{}}
t.primaryKeyFields = fieldnames.CommaSeparatedFieldNames(table.PrimaryKey.Fields)
@ -64,6 +66,7 @@ func (t tableGen) genStoreInterface() {
t.P("Save(ctx ", contextPkg.Ident("Context"), ", ", t.param(t.msg.GoIdent.GoName), " *", t.QualifiedGoIdent(t.msg.GoIdent), ") error")
t.P("Delete(ctx ", contextPkg.Ident("Context"), ", ", t.param(t.msg.GoIdent.GoName), " *", t.QualifiedGoIdent(t.msg.GoIdent), ") error")
t.P("Has(ctx ", contextPkg.Ident("Context"), ", ", t.fieldsArgs(t.primaryKeyFields.Names()), ") (found bool, err error)")
t.P("// Get", notFoundDocs)
t.P("Get(ctx ", contextPkg.Ident("Context"), ", ", t.fieldsArgs(t.primaryKeyFields.Names()), ") (*", t.QualifiedGoIdent(t.msg.GoIdent), ", error)")
for _, idx := range t.uniqueIndexes {
@ -80,7 +83,7 @@ func (t tableGen) genStoreInterface() {
}
// returns the has and get (in that order) function signature for unique indexes.
func (t tableGen) uniqueIndexSig(idxFields string) (string, string) {
func (t tableGen) uniqueIndexSig(idxFields string) (string, string, string) {
fieldsSlc := strings.Split(idxFields, ",")
camelFields := t.fieldsToCamelCase(idxFields)
@ -90,12 +93,13 @@ func (t tableGen) uniqueIndexSig(idxFields string) (string, string) {
hasFuncSig := fmt.Sprintf("%s (ctx context.Context, %s) (found bool, err error)", hasFuncName, args)
getFuncSig := fmt.Sprintf("%s (ctx context.Context, %s) (*%s, error)", getFuncName, args, t.msg.GoIdent.GoName)
return hasFuncSig, getFuncSig
return hasFuncSig, getFuncSig, getFuncName
}
func (t tableGen) genUniqueIndexSig(idx *ormv1alpha1.SecondaryIndexDescriptor) {
hasSig, getSig := t.uniqueIndexSig(idx.Fields)
hasSig, getSig, getFuncName := t.uniqueIndexSig(idx.Fields)
t.P(hasSig)
t.P("// ", getFuncName, notFoundDocs)
t.P(getSig)
}
@ -191,16 +195,19 @@ func (t tableGen) genStoreImpl() {
t.P(receiver, "Get(ctx ", contextPkg.Ident("Context"), ", ", t.fieldsArgs(t.primaryKeyFields.Names()), ") (*", varTypeName, ", error) {")
t.P("var ", varName, " ", varTypeName)
t.P("found, err := ", receiverVar, ".table.PrimaryKey().Get(ctx, &", varName, ", ", t.primaryKeyFields.String(), ")")
t.P("if !found {")
t.P("if err != nil {")
t.P("return nil, err")
t.P("}")
t.P("return &", varName, ", err")
t.P("if !found {")
t.P("return nil, ", ormErrPkg.Ident("NotFound"))
t.P("}")
t.P("return &", varName, ", nil")
t.P("}")
t.P()
for _, idx := range t.uniqueIndexes {
fields := strings.Split(idx.Fields, ",")
hasName, getName := t.uniqueIndexSig(idx.Fields)
hasName, getName, _ := t.uniqueIndexSig(idx.Fields)
// has
t.P("func (", receiverVar, " ", t.messageStoreReceiverName(t.msg), ") ", hasName, "{")
@ -224,9 +231,12 @@ func (t tableGen) genStoreImpl() {
t.P(field, ",")
}
t.P(")")
t.P("if !found {")
t.P("if err != nil {")
t.P("return nil, err")
t.P("}")
t.P("if !found {")
t.P("return nil, ", ormErrPkg.Ident("NotFound"))
t.P("}")
t.P("return &", varName, ", nil")
t.P("}")
t.P()

View File

@ -16,6 +16,7 @@ type BalanceStore interface {
Save(ctx context.Context, balance *Balance) error
Delete(ctx context.Context, balance *Balance) error
Has(ctx context.Context, address string, denom string) (found bool, err error)
// Get returns nil and an error which responds true to ormerrors.IsNotFound() if the record was not found.
Get(ctx context.Context, address string, denom string) (*Balance, error)
List(ctx context.Context, prefixKey BalanceIndexKey, opts ...ormlist.Option) (BalanceIterator, error)
ListRange(ctx context.Context, from, to BalanceIndexKey, opts ...ormlist.Option) (BalanceIterator, error)
@ -102,10 +103,13 @@ func (this balanceStore) Has(ctx context.Context, address string, denom string)
func (this balanceStore) Get(ctx context.Context, address string, denom string) (*Balance, error) {
var balance Balance
found, err := this.table.PrimaryKey().Get(ctx, &balance, address, denom)
if !found {
if err != nil {
return nil, err
}
return &balance, err
if !found {
return nil, ormerrors.NotFound
}
return &balance, nil
}
func (this balanceStore) List(ctx context.Context, prefixKey BalanceIndexKey, opts ...ormlist.Option) (BalanceIterator, error) {
@ -144,6 +148,7 @@ type SupplyStore interface {
Save(ctx context.Context, supply *Supply) error
Delete(ctx context.Context, supply *Supply) error
Has(ctx context.Context, denom string) (found bool, err error)
// Get returns nil and an error which responds true to ormerrors.IsNotFound() if the record was not found.
Get(ctx context.Context, denom string) (*Supply, error)
List(ctx context.Context, prefixKey SupplyIndexKey, opts ...ormlist.Option) (SupplyIterator, error)
ListRange(ctx context.Context, from, to SupplyIndexKey, opts ...ormlist.Option) (SupplyIterator, error)
@ -212,10 +217,13 @@ func (this supplyStore) Has(ctx context.Context, denom string) (found bool, err
func (this supplyStore) Get(ctx context.Context, denom string) (*Supply, error) {
var supply Supply
found, err := this.table.PrimaryKey().Get(ctx, &supply, denom)
if !found {
if err != nil {
return nil, err
}
return &supply, err
if !found {
return nil, ormerrors.NotFound
}
return &supply, nil
}
func (this supplyStore) List(ctx context.Context, prefixKey SupplyIndexKey, opts ...ormlist.Option) (SupplyIterator, error) {

View File

@ -16,8 +16,10 @@ type ExampleTableStore interface {
Save(ctx context.Context, exampleTable *ExampleTable) error
Delete(ctx context.Context, exampleTable *ExampleTable) error
Has(ctx context.Context, u32 uint32, i64 int64, str string) (found bool, err error)
// Get returns nil and an error which responds true to ormerrors.IsNotFound() if the record was not found.
Get(ctx context.Context, u32 uint32, i64 int64, str string) (*ExampleTable, error)
HasByU64Str(ctx context.Context, u64 uint64, str string) (found bool, err error)
// GetByU64Str returns nil and an error which responds true to ormerrors.IsNotFound() if the record was not found.
GetByU64Str(ctx context.Context, u64 uint64, str string) (*ExampleTable, error)
List(ctx context.Context, prefixKey ExampleTableIndexKey, opts ...ormlist.Option) (ExampleTableIterator, error)
ListRange(ctx context.Context, from, to ExampleTableIndexKey, opts ...ormlist.Option) (ExampleTableIterator, error)
@ -150,10 +152,13 @@ func (this exampleTableStore) Has(ctx context.Context, u32 uint32, i64 int64, st
func (this exampleTableStore) Get(ctx context.Context, u32 uint32, i64 int64, str string) (*ExampleTable, error) {
var exampleTable ExampleTable
found, err := this.table.PrimaryKey().Get(ctx, &exampleTable, u32, i64, str)
if !found {
if err != nil {
return nil, err
}
return &exampleTable, err
if !found {
return nil, ormerrors.NotFound
}
return &exampleTable, nil
}
func (this exampleTableStore) HasByU64Str(ctx context.Context, u64 uint64, str string) (found bool, err error) {
@ -169,9 +174,12 @@ func (this exampleTableStore) GetByU64Str(ctx context.Context, u64 uint64, str s
u64,
str,
)
if !found {
if err != nil {
return nil, err
}
if !found {
return nil, ormerrors.NotFound
}
return &exampleTable, nil
}
@ -212,8 +220,10 @@ type ExampleAutoIncrementTableStore interface {
Save(ctx context.Context, exampleAutoIncrementTable *ExampleAutoIncrementTable) error
Delete(ctx context.Context, exampleAutoIncrementTable *ExampleAutoIncrementTable) error
Has(ctx context.Context, id uint64) (found bool, err error)
// Get returns nil and an error which responds true to ormerrors.IsNotFound() if the record was not found.
Get(ctx context.Context, id uint64) (*ExampleAutoIncrementTable, error)
HasByX(ctx context.Context, x string) (found bool, err error)
// GetByX returns nil and an error which responds true to ormerrors.IsNotFound() if the record was not found.
GetByX(ctx context.Context, x string) (*ExampleAutoIncrementTable, error)
List(ctx context.Context, prefixKey ExampleAutoIncrementTableIndexKey, opts ...ormlist.Option) (ExampleAutoIncrementTableIterator, error)
ListRange(ctx context.Context, from, to ExampleAutoIncrementTableIndexKey, opts ...ormlist.Option) (ExampleAutoIncrementTableIterator, error)
@ -299,10 +309,13 @@ func (this exampleAutoIncrementTableStore) Has(ctx context.Context, id uint64) (
func (this exampleAutoIncrementTableStore) Get(ctx context.Context, id uint64) (*ExampleAutoIncrementTable, error) {
var exampleAutoIncrementTable ExampleAutoIncrementTable
found, err := this.table.PrimaryKey().Get(ctx, &exampleAutoIncrementTable, id)
if !found {
if err != nil {
return nil, err
}
return &exampleAutoIncrementTable, err
if !found {
return nil, ormerrors.NotFound
}
return &exampleAutoIncrementTable, nil
}
func (this exampleAutoIncrementTableStore) HasByX(ctx context.Context, x string) (found bool, err error) {
@ -316,9 +329,12 @@ func (this exampleAutoIncrementTableStore) GetByX(ctx context.Context, x string)
found, err := this.table.GetIndexByID(1).(ormtable.UniqueIndex).Get(ctx, &exampleAutoIncrementTable,
x,
)
if !found {
if err != nil {
return nil, err
}
if !found {
return nil, ormerrors.NotFound
}
return &exampleAutoIncrementTable, nil
}

View File

@ -8,12 +8,6 @@ import (
"strings"
"testing"
"github.com/cosmos/cosmos-sdk/orm/types/ormerrors"
"github.com/cosmos/cosmos-sdk/orm/testing/ormtest"
"github.com/cosmos/cosmos-sdk/orm/types/ormjson"
"google.golang.org/protobuf/reflect/protoreflect"
"gotest.tools/v3/assert"
"gotest.tools/v3/golden"
@ -22,6 +16,9 @@ import (
"github.com/cosmos/cosmos-sdk/orm/internal/testpb"
"github.com/cosmos/cosmos-sdk/orm/model/ormdb"
"github.com/cosmos/cosmos-sdk/orm/model/ormtable"
"github.com/cosmos/cosmos-sdk/orm/testing/ormtest"
"github.com/cosmos/cosmos-sdk/orm/types/ormerrors"
"github.com/cosmos/cosmos-sdk/orm/types/ormjson"
)
// These tests use a simulated bank keeper. Addresses and balances use
@ -48,7 +45,7 @@ func (k keeper) Send(ctx context.Context, from, to, denom string, amount uint64)
func (k keeper) Mint(ctx context.Context, acct, denom string, amount uint64) error {
supply, err := k.store.SupplyStore().Get(ctx, denom)
if err != nil {
if err != nil && !ormerrors.IsNotFound(err) {
return err
}
@ -73,10 +70,6 @@ func (k keeper) Burn(ctx context.Context, acct, denom string, amount uint64) err
return err
}
if supply == nil {
return fmt.Errorf("no supply for %s", denom)
}
if amount > supply.Amount {
return fmt.Errorf("insufficient supply")
}
@ -97,7 +90,11 @@ func (k keeper) Burn(ctx context.Context, acct, denom string, amount uint64) err
func (k keeper) Balance(ctx context.Context, acct, denom string) (uint64, error) {
balance, err := k.store.BalanceStore().Get(ctx, acct, denom)
if balance == nil {
if err != nil {
if ormerrors.IsNotFound(err) {
return 0, nil
}
return 0, err
}
return balance.Amount, err
@ -106,6 +103,10 @@ func (k keeper) Balance(ctx context.Context, acct, denom string) (uint64, error)
func (k keeper) Supply(ctx context.Context, denom string) (uint64, error) {
supply, err := k.store.SupplyStore().Get(ctx, denom)
if supply == nil {
if ormerrors.IsNotFound(err) {
return 0, nil
}
return 0, err
}
return supply.Amount, err
@ -113,7 +114,7 @@ func (k keeper) Supply(ctx context.Context, denom string) (uint64, error) {
func (k keeper) addBalance(ctx context.Context, acct, denom string, amount uint64) error {
balance, err := k.store.BalanceStore().Get(ctx, acct, denom)
if err != nil {
if err != nil && !ormerrors.IsNotFound(err) {
return err
}
@ -137,10 +138,6 @@ func (k keeper) safeSubBalance(ctx context.Context, acct, denom string, amount u
return err
}
if balance == nil {
return fmt.Errorf("acct %x has no balance for %s", acct, denom)
}
if amount > balance.Amount {
return fmt.Errorf("insufficient funds")
}

View File

@ -4,6 +4,11 @@ import "github.com/cosmos/cosmos-sdk/errors"
var codespace = "orm"
// IsNotFound returns true if the error indicates that the record was not found.
func IsNotFound(err error) bool {
return errors.IsOf(err, NotFound)
}
var (
InvalidTableId = errors.New(codespace, 1, "invalid or missing table or single id, need a non-zero value")
MissingPrimaryKey = errors.New(codespace, 2, "table is missing primary key")
@ -32,5 +37,6 @@ var (
InvalidTableDefinition = errors.New(codespace, 25, "invalid table definition")
InvalidFileDescriptorID = errors.New(codespace, 26, "invalid file descriptor ID")
TableNotFound = errors.New(codespace, 27, "table not found")
JSONValidationError = errors.New(codespace, 23, "invalid JSON")
JSONValidationError = errors.New(codespace, 28, "invalid JSON")
NotFound = errors.New(codespace, 29, "not found")
)