From 5020b6610c469739fc02f3cebd2ff97f6ae5a319 Mon Sep 17 00:00:00 2001 From: Rod Vagg Date: Wed, 15 Feb 2023 18:03:13 +1100 Subject: [PATCH 1/4] feat: add ErrNotFound * Intended to replace github.com/ipfs/go-ipld-format#ErrNotFound * A new IsNotFound() that uses feature detection rather than type checking so it's compatible with old and new forms. Ref: https://github.com/ipld/go-car/pull/363 Ref: https://github.com/ipld/go-ipld-prime/pull/493 --- storage/memstore/memstore.go | 9 +++--- storage/notfound.go | 60 ++++++++++++++++++++++++++++++++++++ storage/notfound_test.go | 58 ++++++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 storage/notfound.go create mode 100644 storage/notfound_test.go diff --git a/storage/memstore/memstore.go b/storage/memstore/memstore.go index 1b1e66d4..b484e834 100644 --- a/storage/memstore/memstore.go +++ b/storage/memstore/memstore.go @@ -3,8 +3,9 @@ package memstore import ( "bytes" "context" - "fmt" "io" + + "github.com/ipld/go-ipld-prime/storage" ) // Store is a simple in-memory storage. @@ -56,7 +57,7 @@ func (store *Store) Get(ctx context.Context, key string) ([]byte, error) { store.beInitialized() content, exists := store.Bag[key] if !exists { - return nil, fmt.Errorf("404") // FIXME this needs a standard error type + return nil, storage.ErrNotFound{Key: key} } cpy := make([]byte, len(content)) copy(cpy, content) @@ -82,7 +83,7 @@ func (store *Store) Put(ctx context.Context, key string, content []byte) error { func (store *Store) GetStream(ctx context.Context, key string) (io.ReadCloser, error) { content, exists := store.Bag[key] if !exists { - return nil, fmt.Errorf("404") // FIXME this needs a standard error type + return nil, storage.ErrNotFound{Key: key} } return noopCloser{bytes.NewReader(content)}, nil } @@ -91,7 +92,7 @@ func (store *Store) GetStream(ctx context.Context, key string) (io.ReadCloser, e func (store *Store) Peek(ctx context.Context, key string) ([]byte, io.Closer, error) { content, exists := store.Bag[key] if !exists { - return nil, nil, fmt.Errorf("404") // FIXME this needs a standard error type + return nil, nil, storage.ErrNotFound{Key: key} } return content, noopCloser{nil}, nil } diff --git a/storage/notfound.go b/storage/notfound.go new file mode 100644 index 00000000..f6779a9c --- /dev/null +++ b/storage/notfound.go @@ -0,0 +1,60 @@ +package storage + +import "github.com/ipfs/go-cid" + +// compatible with the go-ipld-format ErrNotFound, match against +// interface{NotFound() bool} +// this could go into go-ipld-prime, but for now we'll just exercise the +// feature-test pattern + +// ErrNotFound is a 404, but for block storage systems. It is returned when +// a block is not found. The Key is typically the binary form of a CID +// (CID#KeyString()). +// +// ErrNotFound implements `interface{NotFound() bool}`, which makes it roughly +// compatible with the legacy github.com/ipfs/go-ipld-format#ErrNotFound. +// The IsNotFound() function here will test for this and therefore be compatible +// with this ErrNotFound, and the legacy ErrNotFound. The same is not true for +// the legacy github.com/ipfs/go-ipld-format#IsNotFound. +type ErrNotFound struct { + Key string +} + +// NewErrNotFound is a convenience factory that creates a new ErrNotFound error +// from a CID. +func NewErrNotFound(c cid.Cid) ErrNotFound { + return ErrNotFound{Key: c.KeyString()} +} + +func (e ErrNotFound) Error() string { + if c, err := cid.Cast([]byte(e.Key)); err == nil && c != cid.Undef { + return "ipld: could not find " + c.String() + } + return "ipld: could not find " + e.Key +} + +// NotFound always returns true, and is used to feature-test for ErrNotFound +// errors. +func (e ErrNotFound) NotFound() bool { + return true +} + +// Is allows errors.Is to work with this error type. +func (e ErrNotFound) Is(err error) bool { + switch err.(type) { + case ErrNotFound: + return true + default: + return false + } +} + +// IsNotFound returns true if the error is a ErrNotFound. As it uses a +// feature-test, it is also compatible with the legacy +// github.com/ipfs/go-ipld-format#ErrNotFound. +func IsNotFound(err error) bool { + if nf, ok := err.(interface{ NotFound() bool }); ok { + return nf.NotFound() + } + return false +} diff --git a/storage/notfound_test.go b/storage/notfound_test.go new file mode 100644 index 00000000..32ba046b --- /dev/null +++ b/storage/notfound_test.go @@ -0,0 +1,58 @@ +package storage_test + +import ( + "testing" + + "github.com/ipfs/go-cid" + "github.com/ipld/go-ipld-prime/storage" +) + +func TestNotFound(t *testing.T) { + nf := storage.ErrNotFound{Key: "foo"} + if !storage.IsNotFound(nf) { + t.Fatal("expected ErrNotFound to be a NotFound error") + } + if nf.Error() != "ipld: could not find foo" { + t.Fatal("unexpected error message") + } + + nf = storage.NewErrNotFound(cid.MustParse("bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi")) + if !storage.IsNotFound(nf) { + t.Fatal("expected ErrNotFound to be a NotFound error") + } + if nf.Error() != "ipld: could not find bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi" { + t.Fatal("unexpected error message") + } + + wnf := &weirdNotFoundError{} + if !storage.IsNotFound(wnf) { + t.Fatal("expected weirdNotFoundError to be a NotFound error") + } + + // a weirder case, this one implements `NotFound()` but it returns false, so + // this shouldn't be a NotFound error + wnnf := &weirdNotNotFoundError{} + if storage.IsNotFound(wnnf) { + t.Fatal("expected weirdNotNotFoundError to NOT be a NotFound error") + } +} + +type weirdNotFoundError struct{} + +func (weirdNotFoundError) NotFound() bool { + return true +} + +func (weirdNotFoundError) Error() string { + return "weird not found error" +} + +type weirdNotNotFoundError struct{} + +func (weirdNotNotFoundError) NotFound() bool { + return false +} + +func (weirdNotNotFoundError) Error() string { + return "weird not NOT found error" +} From 74bd630dd35be7934a2d6f86109b16e5154a118e Mon Sep 17 00:00:00 2001 From: Rod Vagg Date: Fri, 17 Feb 2023 13:58:14 +1100 Subject: [PATCH 2/4] feat: Is() performs interface check, IsNotFound() unwraps --- storage/notfound.go | 64 +++++++++++++++++++++++++++++----------- storage/notfound_test.go | 55 +++++++++++++++++++++++++++++++--- 2 files changed, 98 insertions(+), 21 deletions(-) diff --git a/storage/notfound.go b/storage/notfound.go index f6779a9c..0795a5a6 100644 --- a/storage/notfound.go +++ b/storage/notfound.go @@ -1,11 +1,10 @@ package storage -import "github.com/ipfs/go-cid" +import ( + "errors" -// compatible with the go-ipld-format ErrNotFound, match against -// interface{NotFound() bool} -// this could go into go-ipld-prime, but for now we'll just exercise the -// feature-test pattern + "github.com/ipfs/go-cid" +) // ErrNotFound is a 404, but for block storage systems. It is returned when // a block is not found. The Key is typically the binary form of a CID @@ -16,6 +15,13 @@ import "github.com/ipfs/go-cid" // The IsNotFound() function here will test for this and therefore be compatible // with this ErrNotFound, and the legacy ErrNotFound. The same is not true for // the legacy github.com/ipfs/go-ipld-format#IsNotFound. +// +// errors.Is() should be preferred as the standard Go way to test for errors; +// however due to the move of the legacy ErrNotFound to this package, it may +// not report correctly where older block storage packages emit the legacy +// ErrNotFound. The IsNotFound() function provides a maximally compatible +// matching function that should be able to determine whether an ErrNotFound, +// either new or legacy, exists within a wrapped error chain. type ErrNotFound struct { Key string } @@ -39,22 +45,46 @@ func (e ErrNotFound) NotFound() bool { return true } -// Is allows errors.Is to work with this error type. +// Is allows errors.Is to work with this error type. It is compatible with the +// legacy github.com/ipfs/go-ipld-format#ErrNotFound, and other related error +// types as it uses a feature-test on the NotFound() method. +// +// It is important to note that because errors.Is() performs a reverse match, +// whereby the Is() of the error being checked is called on the target, +// the legacy ErrNotFound#Is will perform a strict type match, which will fail +// where the original error is of the legacy type. Where compatibility is +// required across multiple block storage systems that may return legacy error +// types, use the IsNotFound() function instead. func (e ErrNotFound) Is(err error) bool { - switch err.(type) { - case ErrNotFound: - return true - default: - return false + if v, ok := err.(interface{ NotFound() bool }); ok && v.NotFound() { + return v.NotFound() } + return false } -// IsNotFound returns true if the error is a ErrNotFound. As it uses a -// feature-test, it is also compatible with the legacy -// github.com/ipfs/go-ipld-format#ErrNotFound. +var enf = ErrNotFound{} + +// IsNotFound returns true if the error is a ErrNotFound, or compatible with an +// ErrNotFound, or wraps such an error. Compatibility is determined by the +// type implementing the NotFound() method which returns true. +// It is compatible with the legacy github.com/ipfs/go-ipld-format#ErrNotFound, +// and other related error types. +// +// This is NOT the same as errors.Is(err, storage.ErrNotFound{}) which relies on +// the Is() of the original err rather than the target. IsNotFound() uses the +// Is() of storage.ErrNotFound to perform the check. The difference being that +// the err being checked doesn't need to have a feature-testing Is() method for +// this to succeed, it only needs to have a NotFound() method that returns true. +// +// Prefer this method for maximal compatibility, including wrapped errors, that +// implement the minimal interface{ NotFound() true }. func IsNotFound(err error) bool { - if nf, ok := err.(interface{ NotFound() bool }); ok { - return nf.NotFound() + for { + if enf.Is(err) { + return true + } + if err = errors.Unwrap(err); err == nil { + return false + } } - return false } diff --git a/storage/notfound_test.go b/storage/notfound_test.go index 32ba046b..56d207d2 100644 --- a/storage/notfound_test.go +++ b/storage/notfound_test.go @@ -1,6 +1,8 @@ package storage_test import ( + "errors" + "fmt" "testing" "github.com/ipfs/go-cid" @@ -12,6 +14,9 @@ func TestNotFound(t *testing.T) { if !storage.IsNotFound(nf) { t.Fatal("expected ErrNotFound to be a NotFound error") } + if !errors.Is(nf, storage.ErrNotFound{}) { + t.Fatal("expected ErrNotFound to be a NotFound error") + } if nf.Error() != "ipld: could not find foo" { t.Fatal("unexpected error message") } @@ -20,19 +25,47 @@ func TestNotFound(t *testing.T) { if !storage.IsNotFound(nf) { t.Fatal("expected ErrNotFound to be a NotFound error") } + if !errors.Is(nf, storage.ErrNotFound{}) { + t.Fatal("expected ErrNotFound to be a NotFound error") + } if nf.Error() != "ipld: could not find bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi" { t.Fatal("unexpected error message") } - wnf := &weirdNotFoundError{} + wrappedNf := fmt.Errorf("wrapped outer: %w", fmt.Errorf("wrapped inner: %w", nf)) + if !storage.IsNotFound(wrappedNf) { + t.Fatal("expected wrapped ErrNotFound to be a NotFound error") + } + if !errors.Is(wrappedNf, storage.ErrNotFound{}) { + t.Fatal("expected wrapped ErrNotFound to be a NotFound error") + } + + fmt.Println("WeirdNotFoundErr") + wnf := weirdNotFoundError{} if !storage.IsNotFound(wnf) { t.Fatal("expected weirdNotFoundError to be a NotFound error") } + if !errors.Is(wnf, storage.ErrNotFound{}) { + t.Fatal("expected weirdNotFoundError to be a NotFound error") + } - // a weirder case, this one implements `NotFound()` but it returns false, so - // this shouldn't be a NotFound error - wnnf := &weirdNotNotFoundError{} + // a weirder case, this one implements `NotFound()` but it returns false; but + // it also implements the same Is() that will claim it's a not-found, so + // it should work one way around, but not the other, when it's being asked + // whether an error is or not + wnnf := weirdNotNotFoundError{} if storage.IsNotFound(wnnf) { + // this shouldn't be true because we test NotFound()==true + t.Fatal("expected weirdNotNotFoundError to NOT be a NotFound error") + } + if !errors.Is(wnnf, storage.ErrNotFound{}) { + // this should be true, because weirdNotNotFoundError.Is() performs the + // check on storage.ErrNotFound{}.NotFound() which does return true. + t.Fatal("expected weirdNotNotFoundError to be a NotFound error") + } + if errors.Is(nf, weirdNotNotFoundError{}) { + // switch them around and we get the same result as storage.IsNotFound, but + // won't work with wrapped weirdNotNotFoundError errors. t.Fatal("expected weirdNotNotFoundError to NOT be a NotFound error") } } @@ -43,6 +76,13 @@ func (weirdNotFoundError) NotFound() bool { return true } +func (weirdNotFoundError) Is(err error) bool { + if v, ok := err.(interface{ NotFound() bool }); ok && v.NotFound() { + return v.NotFound() + } + return false +} + func (weirdNotFoundError) Error() string { return "weird not found error" } @@ -53,6 +93,13 @@ func (weirdNotNotFoundError) NotFound() bool { return false } +func (weirdNotNotFoundError) Is(err error) bool { + if v, ok := err.(interface{ NotFound() bool }); ok && v.NotFound() { + return v.NotFound() + } + return false +} + func (weirdNotNotFoundError) Error() string { return "weird not NOT found error" } From db67fbde90fceaad3698b007b4ea5fb3e3ee0227 Mon Sep 17 00:00:00 2001 From: Rod Vagg Date: Tue, 28 Mar 2023 15:28:52 +1100 Subject: [PATCH 3/4] fix: go mod tidy --- storage/benchmarks/go.mod | 13 +++++++++++++ storage/benchmarks/go.sum | 28 ++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/storage/benchmarks/go.mod b/storage/benchmarks/go.mod index f292f7f9..5b89c565 100644 --- a/storage/benchmarks/go.mod +++ b/storage/benchmarks/go.mod @@ -16,12 +16,25 @@ require ( github.com/alexbrainman/goissue34681 v0.0.0-20191006012335-3fc7a47baff5 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/google/uuid v1.3.0 // indirect + github.com/ipfs/go-cid v0.4.0 // indirect github.com/ipfs/go-datastore v0.6.0 // indirect github.com/ipfs/go-log v1.0.3 // indirect github.com/ipfs/go-log/v2 v2.0.3 // indirect github.com/jbenet/goprocess v0.1.4 // indirect + github.com/klauspost/cpuid/v2 v2.0.9 // indirect + github.com/minio/sha256-simd v1.0.0 // indirect + github.com/mr-tron/base58 v1.2.0 // indirect + github.com/multiformats/go-base32 v0.0.3 // indirect + github.com/multiformats/go-base36 v0.1.0 // indirect + github.com/multiformats/go-multibase v0.0.3 // indirect + github.com/multiformats/go-multihash v0.2.1 // indirect + github.com/multiformats/go-varint v0.0.6 // indirect github.com/opentracing/opentracing-go v1.1.0 // indirect + github.com/spaolacci/murmur3 v1.1.0 // indirect go.uber.org/atomic v1.6.0 // indirect go.uber.org/multierr v1.5.0 // indirect go.uber.org/zap v1.10.0 // indirect + golang.org/x/crypto v0.1.0 // indirect + golang.org/x/sys v0.1.0 // indirect + lukechampine.com/blake3 v1.1.6 // indirect ) diff --git a/storage/benchmarks/go.sum b/storage/benchmarks/go.sum index 731e4d49..1d74bdfc 100644 --- a/storage/benchmarks/go.sum +++ b/storage/benchmarks/go.sum @@ -12,6 +12,8 @@ github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm4 github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I= github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/ipfs/go-cid v0.4.0 h1:a4pdZq0sx6ZSxbCizebnKiMCx/xI/aBBFlB73IgH4rA= +github.com/ipfs/go-cid v0.4.0/go.mod h1:uQHwDeX4c6CtyrFwdqyhpNcxVewur1M7l7fNU7LKwZk= github.com/ipfs/go-datastore v0.5.0/go.mod h1:9zhEApYMTl17C8YDp7JmU7sQZi2/wqiYh73hakZ90Bk= github.com/ipfs/go-datastore v0.6.0 h1:JKyz+Gvz1QEZw0LsX1IBn+JFCJQH4SJVFtM4uWU0Myk= github.com/ipfs/go-datastore v0.6.0/go.mod h1:rt5M3nNbSO/8q1t4LNkLyUwRs8HupMeN/8O4Vn9YAT8= @@ -30,12 +32,30 @@ github.com/jbenet/goprocess v0.1.4/go.mod h1:5yspPrukOVuOLORacaBi858NqyClJPQxYZl github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= +github.com/klauspost/cpuid/v2 v2.0.4/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= +github.com/klauspost/cpuid/v2 v2.0.9 h1:lgaqFMSdTdQYdZ04uHyN2d/eKdOMyi2YLSvlQIBFYa4= +github.com/klauspost/cpuid/v2 v2.0.9/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/minio/sha256-simd v1.0.0 h1:v1ta+49hkWZyvaKwrQB8elexRqm6Y0aMLjCNsrYxo6g= +github.com/minio/sha256-simd v1.0.0/go.mod h1:OuYzVNI5vcoYIAmbIvHPl3N3jUzVedXbKy5RFepssQM= +github.com/mr-tron/base58 v1.1.0/go.mod h1:xcD2VGqlgYjBdcBLw+TuYLr8afG+Hj8g2eTVqeSzSU8= +github.com/mr-tron/base58 v1.2.0 h1:T/HDJBh4ZCPbU39/+c3rRvE0uKBQlU27+QI8LJ4t64o= +github.com/mr-tron/base58 v1.2.0/go.mod h1:BinMc/sQntlIE1frQmRFPUoPA1Zkr8VRgBdjWI2mNwc= +github.com/multiformats/go-base32 v0.0.3 h1:tw5+NhuwaOjJCC5Pp82QuXbrmLzWg7uxlMFp8Nq/kkI= +github.com/multiformats/go-base32 v0.0.3/go.mod h1:pLiuGC8y0QR3Ue4Zug5UzK9LjgbkL8NSQj0zQ5Nz/AA= +github.com/multiformats/go-base36 v0.1.0 h1:JR6TyF7JjGd3m6FbLU2cOxhC0Li8z8dLNGQ89tUg4F4= +github.com/multiformats/go-base36 v0.1.0/go.mod h1:kFGE83c6s80PklsHO9sRn2NCoffoRdUUOENyW/Vv6sM= +github.com/multiformats/go-multibase v0.0.3 h1:l/B6bJDQjvQ5G52jw4QGSYeOTZoAwIO77RblWplfIqk= +github.com/multiformats/go-multibase v0.0.3/go.mod h1:5+1R4eQrT3PkYZ24C3W2Ue2tPwIdYQD509ZjSb5y9Oc= +github.com/multiformats/go-multihash v0.2.1 h1:aem8ZT0VA2nCHHk7bPJ1BjUbHNciqZC/d16Vve9l108= +github.com/multiformats/go-multihash v0.2.1/go.mod h1:WxoMcYG85AZVQUyRyo9s4wULvW5qrI9vb2Lt6evduFc= +github.com/multiformats/go-varint v0.0.6 h1:gk85QWKxh3TazbLxED/NlDVv8+q+ReFJk7Y2W/KhfNY= +github.com/multiformats/go-varint v0.0.6/go.mod h1:3Ls8CIEsrijN6+B7PbrXRPxHRPuXSrVKRY101jdMZYE= github.com/opentracing/opentracing-go v1.1.0 h1:pWlfV3Bxv7k65HYwkikxat0+s3pV4bsqf19k25Ur8rU= github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o= github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= @@ -44,6 +64,8 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8= +github.com/spaolacci/murmur3 v1.1.0 h1:7c1g84S4BPRrfL5Xrdp6fOJ206sU9y293DDHaoy0bLI= +github.com/spaolacci/murmur3 v1.1.0/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= @@ -64,6 +86,8 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.1.0 h1:MDRAIl0xIo9Io2xV565hzXHw3zVseKrJKodhohM5CjU= +golang.org/x/crypto v0.1.0/go.mod h1:RecgLatLF4+eUMCP1PoPZQb+cVrJcOPbHkTkbkB9sbw= golang.org/x/lint v0.0.0-20190930215403-16217165b5de h1:5hukYrvBGR8/eNkX5mdUezrA6JiaEZDtJb9Ei+1LlBs= golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/mod v0.0.0-20190513183733-4bf6d317e70e/go.mod h1:mXi4GBBbnImb6dmsKGUJ2LatrhH/nqhxcFungHvyanc= @@ -81,6 +105,8 @@ golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U= +golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/tools v0.0.0-20180221164845-07fd8470d635/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= @@ -107,3 +133,5 @@ gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= honnef.co/go/tools v0.0.1-2019.2.3 h1:3JgtbtFHMiCmsznwGVTUWbgGov+pVqnlf1dEJTNAXeM= honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg= +lukechampine.com/blake3 v1.1.6 h1:H3cROdztr7RCfoaTpGZFQsrqvweFLrqS73j7L7cmR5c= +lukechampine.com/blake3 v1.1.6/go.mod h1:tkKEOtDkNtklkXtLNEOGNq5tcV90tJiA1vAA12R78LA= From 3142e1304e55928444c33887c3e85b12063a9e7d Mon Sep 17 00:00:00 2001 From: Rod Vagg Date: Thu, 8 Jun 2023 14:53:40 +1000 Subject: [PATCH 4/4] fix: change property of ErrNotFound - s/Key/Cid ABI compatible with github.com/ipfs/go-ipld-format#ErrNotFound for smoother upgrade path --- storage/benchmarks/go.mod | 4 ++-- storage/benchmarks/go.sum | 8 ++++---- storage/memstore/memstore.go | 6 +++--- storage/notfound.go | 25 ++++++++++++++++++------- storage/notfound_test.go | 4 ++-- 5 files changed, 29 insertions(+), 18 deletions(-) diff --git a/storage/benchmarks/go.mod b/storage/benchmarks/go.mod index 5b89c565..0de635d7 100644 --- a/storage/benchmarks/go.mod +++ b/storage/benchmarks/go.mod @@ -16,7 +16,7 @@ require ( github.com/alexbrainman/goissue34681 v0.0.0-20191006012335-3fc7a47baff5 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/google/uuid v1.3.0 // indirect - github.com/ipfs/go-cid v0.4.0 // indirect + github.com/ipfs/go-cid v0.4.1 // indirect github.com/ipfs/go-datastore v0.6.0 // indirect github.com/ipfs/go-log v1.0.3 // indirect github.com/ipfs/go-log/v2 v2.0.3 // indirect @@ -27,7 +27,7 @@ require ( github.com/multiformats/go-base32 v0.0.3 // indirect github.com/multiformats/go-base36 v0.1.0 // indirect github.com/multiformats/go-multibase v0.0.3 // indirect - github.com/multiformats/go-multihash v0.2.1 // indirect + github.com/multiformats/go-multihash v0.2.2 // indirect github.com/multiformats/go-varint v0.0.6 // indirect github.com/opentracing/opentracing-go v1.1.0 // indirect github.com/spaolacci/murmur3 v1.1.0 // indirect diff --git a/storage/benchmarks/go.sum b/storage/benchmarks/go.sum index 1d74bdfc..cccfb65e 100644 --- a/storage/benchmarks/go.sum +++ b/storage/benchmarks/go.sum @@ -12,8 +12,8 @@ github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm4 github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I= github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= -github.com/ipfs/go-cid v0.4.0 h1:a4pdZq0sx6ZSxbCizebnKiMCx/xI/aBBFlB73IgH4rA= -github.com/ipfs/go-cid v0.4.0/go.mod h1:uQHwDeX4c6CtyrFwdqyhpNcxVewur1M7l7fNU7LKwZk= +github.com/ipfs/go-cid v0.4.1 h1:A/T3qGvxi4kpKWWcPC/PgbvDA2bjVLO7n4UeVwnbs/s= +github.com/ipfs/go-cid v0.4.1/go.mod h1:uQHwDeX4c6CtyrFwdqyhpNcxVewur1M7l7fNU7LKwZk= github.com/ipfs/go-datastore v0.5.0/go.mod h1:9zhEApYMTl17C8YDp7JmU7sQZi2/wqiYh73hakZ90Bk= github.com/ipfs/go-datastore v0.6.0 h1:JKyz+Gvz1QEZw0LsX1IBn+JFCJQH4SJVFtM4uWU0Myk= github.com/ipfs/go-datastore v0.6.0/go.mod h1:rt5M3nNbSO/8q1t4LNkLyUwRs8HupMeN/8O4Vn9YAT8= @@ -52,8 +52,8 @@ github.com/multiformats/go-base36 v0.1.0 h1:JR6TyF7JjGd3m6FbLU2cOxhC0Li8z8dLNGQ8 github.com/multiformats/go-base36 v0.1.0/go.mod h1:kFGE83c6s80PklsHO9sRn2NCoffoRdUUOENyW/Vv6sM= github.com/multiformats/go-multibase v0.0.3 h1:l/B6bJDQjvQ5G52jw4QGSYeOTZoAwIO77RblWplfIqk= github.com/multiformats/go-multibase v0.0.3/go.mod h1:5+1R4eQrT3PkYZ24C3W2Ue2tPwIdYQD509ZjSb5y9Oc= -github.com/multiformats/go-multihash v0.2.1 h1:aem8ZT0VA2nCHHk7bPJ1BjUbHNciqZC/d16Vve9l108= -github.com/multiformats/go-multihash v0.2.1/go.mod h1:WxoMcYG85AZVQUyRyo9s4wULvW5qrI9vb2Lt6evduFc= +github.com/multiformats/go-multihash v0.2.2 h1:Uu7LWs/PmWby1gkj1S1DXx3zyd3aVabA4FiMKn/2tAc= +github.com/multiformats/go-multihash v0.2.2/go.mod h1:dXgKXCXjBzdscBLk9JkjINiEsCKRVch90MdaGiKsvSM= github.com/multiformats/go-varint v0.0.6 h1:gk85QWKxh3TazbLxED/NlDVv8+q+ReFJk7Y2W/KhfNY= github.com/multiformats/go-varint v0.0.6/go.mod h1:3Ls8CIEsrijN6+B7PbrXRPxHRPuXSrVKRY101jdMZYE= github.com/opentracing/opentracing-go v1.1.0 h1:pWlfV3Bxv7k65HYwkikxat0+s3pV4bsqf19k25Ur8rU= diff --git a/storage/memstore/memstore.go b/storage/memstore/memstore.go index b484e834..7a0369e0 100644 --- a/storage/memstore/memstore.go +++ b/storage/memstore/memstore.go @@ -57,7 +57,7 @@ func (store *Store) Get(ctx context.Context, key string) ([]byte, error) { store.beInitialized() content, exists := store.Bag[key] if !exists { - return nil, storage.ErrNotFound{Key: key} + return nil, storage.NewErrNotFoundForKey(key) } cpy := make([]byte, len(content)) copy(cpy, content) @@ -83,7 +83,7 @@ func (store *Store) Put(ctx context.Context, key string, content []byte) error { func (store *Store) GetStream(ctx context.Context, key string) (io.ReadCloser, error) { content, exists := store.Bag[key] if !exists { - return nil, storage.ErrNotFound{Key: key} + return nil, storage.NewErrNotFoundForKey(key) } return noopCloser{bytes.NewReader(content)}, nil } @@ -92,7 +92,7 @@ func (store *Store) GetStream(ctx context.Context, key string) (io.ReadCloser, e func (store *Store) Peek(ctx context.Context, key string) ([]byte, io.Closer, error) { content, exists := store.Bag[key] if !exists { - return nil, nil, storage.ErrNotFound{Key: key} + return nil, nil, storage.NewErrNotFoundForKey(key) } return content, noopCloser{nil}, nil } diff --git a/storage/notfound.go b/storage/notfound.go index 0795a5a6..8496d856 100644 --- a/storage/notfound.go +++ b/storage/notfound.go @@ -7,8 +7,9 @@ import ( ) // ErrNotFound is a 404, but for block storage systems. It is returned when -// a block is not found. The Key is typically the binary form of a CID -// (CID#KeyString()). +// a block is not found. The Cid property may be cid.Undef if the NotFound error +// was not created with a specific CID (e.g. when using a non-CID key in a +// storage Get operation). // // ErrNotFound implements `interface{NotFound() bool}`, which makes it roughly // compatible with the legacy github.com/ipfs/go-ipld-format#ErrNotFound. @@ -23,20 +24,30 @@ import ( // matching function that should be able to determine whether an ErrNotFound, // either new or legacy, exists within a wrapped error chain. type ErrNotFound struct { - Key string + Cid cid.Cid } // NewErrNotFound is a convenience factory that creates a new ErrNotFound error // from a CID. func NewErrNotFound(c cid.Cid) ErrNotFound { - return ErrNotFound{Key: c.KeyString()} + return ErrNotFound{Cid: c} +} + +// NewErrNotFound is a convenience factory that creates a new ErrNotFound error +// from a key. If the key is a CID#KeyString(), then it will be cast to a CID, +// otherwise the Cid of the ErrNotFound will be cid.Undef. +func NewErrNotFoundForKey(key string) ErrNotFound { + if c, err := cid.Cast([]byte(key)); err == nil { + return ErrNotFound{Cid: c} + } + return ErrNotFound{Cid: cid.Undef} } func (e ErrNotFound) Error() string { - if c, err := cid.Cast([]byte(e.Key)); err == nil && c != cid.Undef { - return "ipld: could not find " + c.String() + if e.Cid == cid.Undef { + return "ipld: could not find node" } - return "ipld: could not find " + e.Key + return "ipld: could not find " + e.Cid.String() } // NotFound always returns true, and is used to feature-test for ErrNotFound diff --git a/storage/notfound_test.go b/storage/notfound_test.go index 56d207d2..f8a223a5 100644 --- a/storage/notfound_test.go +++ b/storage/notfound_test.go @@ -10,14 +10,14 @@ import ( ) func TestNotFound(t *testing.T) { - nf := storage.ErrNotFound{Key: "foo"} + nf := storage.ErrNotFound{} if !storage.IsNotFound(nf) { t.Fatal("expected ErrNotFound to be a NotFound error") } if !errors.Is(nf, storage.ErrNotFound{}) { t.Fatal("expected ErrNotFound to be a NotFound error") } - if nf.Error() != "ipld: could not find foo" { + if nf.Error() != "ipld: could not find node" { t.Fatal("unexpected error message") }