From dd32d670248f35c680c588c72c6f141ab05dd58c Mon Sep 17 00:00:00 2001 From: Jorropo Date: Wed, 16 Aug 2023 05:57:26 +0200 Subject: [PATCH] blockservice: use functional option pattern for construction and add Allowlist tests --- CHANGELOG.md | 3 ++ blockservice/blockservice.go | 50 +++++++++++++++++-------------- blockservice/blockservice_test.go | 47 +++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 04021016c..8c8369a13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,9 @@ The following emojis are used to highlight certain changes: ### Changed +* 🛠 `blockservice.New` now accepts a variadic of func options following the [Functional + Options pattern](https://www.sohamkamani.com/golang/options-pattern/). + ### Removed ### Fixed diff --git a/blockservice/blockservice.go b/blockservice/blockservice.go index cdcac33cc..37c4b35da 100644 --- a/blockservice/blockservice.go +++ b/blockservice/blockservice.go @@ -80,43 +80,49 @@ type blockService struct { checkFirst bool } -// New creates a BlockService with given datastore instance. -func New(bs blockstore.Blockstore, exchange exchange.Interface) BlockService { - if exchange == nil { - logger.Debug("blockservice running in local (offline) mode.") +type Option func(*blockService) + +// WriteThrough disable cache checks for writes and make them go straight to +// the blockstore. +func WriteThrough() Option { + return func(bs *blockService) { + bs.checkFirst = false } +} - return &blockService{ - allowlist: verifcid.DefaultAllowlist, - blockstore: bs, - exchange: exchange, - checkFirst: true, +// WithAllowlist sets a custom [verifcid.Allowlist] which will be used +func WithAllowlist(allowlist verifcid.Allowlist) Option { + return func(bs *blockService) { + bs.allowlist = allowlist } } -// NewWriteThrough creates a BlockService that guarantees writes will go -// through to the blockstore and are not skipped by cache checks. -func NewWriteThrough(bs blockstore.Blockstore, exchange exchange.Interface) BlockService { +// New creates a BlockService with given datastore instance. +func New(bs blockstore.Blockstore, exchange exchange.Interface, opts ...Option) BlockService { if exchange == nil { logger.Debug("blockservice running in local (offline) mode.") } - return &blockService{ + service := &blockService{ allowlist: verifcid.DefaultAllowlist, blockstore: bs, exchange: exchange, - checkFirst: false, + checkFirst: true, } -} -// NewWithAllowList creates a BlockService with customized multihash Allowlist. -func NewWithAllowList(bs blockstore.Blockstore, exchange exchange.Interface, allowlist verifcid.Allowlist) BlockService { - return &blockService{ - allowlist: allowlist, - blockstore: bs, - exchange: exchange, - checkFirst: true, + for _, opt := range opts { + opt(service) } + + return service +} + +// NewWriteThrough creates a BlockService that guarantees writes will go +// through to the blockstore and are not skipped by cache checks. +// +// Deprecated: Use [New] with the [WriteThrough] option. +func NewWriteThrough(bs blockstore.Blockstore, exchange exchange.Interface) BlockService { + return New(bs, exchange, WriteThrough()) } // Blockstore returns the blockstore behind this blockservice. diff --git a/blockservice/blockservice_test.go b/blockservice/blockservice_test.go index 14396c8a1..e36058040 100644 --- a/blockservice/blockservice_test.go +++ b/blockservice/blockservice_test.go @@ -7,15 +7,20 @@ import ( blockstore "github.com/ipfs/boxo/blockstore" exchange "github.com/ipfs/boxo/exchange" offline "github.com/ipfs/boxo/exchange/offline" + "github.com/ipfs/boxo/verifcid" blocks "github.com/ipfs/go-block-format" cid "github.com/ipfs/go-cid" ds "github.com/ipfs/go-datastore" dssync "github.com/ipfs/go-datastore/sync" butil "github.com/ipfs/go-ipfs-blocksutil" ipld "github.com/ipfs/go-ipld-format" + "github.com/multiformats/go-multihash" + "github.com/stretchr/testify/assert" ) func TestWriteThroughWorks(t *testing.T) { + t.Parallel() + bstore := &PutCountingBlockstore{ blockstore.NewBlockstore(dssync.MutexWrap(ds.NewMapDatastore())), 0, @@ -46,6 +51,8 @@ func TestWriteThroughWorks(t *testing.T) { } func TestExchangeWrite(t *testing.T) { + t.Parallel() + bstore := &PutCountingBlockstore{ blockstore.NewBlockstore(dssync.MutexWrap(ds.NewMapDatastore())), 0, @@ -117,6 +124,8 @@ func TestExchangeWrite(t *testing.T) { } func TestLazySessionInitialization(t *testing.T) { + t.Parallel() + ctx := context.Background() ctx, cancel := context.WithCancel(ctx) defer cancel() @@ -215,6 +224,8 @@ func (fe *fakeSessionExchange) NewSession(ctx context.Context) exchange.Fetcher } func TestNilExchange(t *testing.T) { + t.Parallel() + ctx := context.Background() ctx, cancel := context.WithCancel(ctx) defer cancel() @@ -241,3 +252,39 @@ func TestNilExchange(t *testing.T) { t.Fatal("got the wrong block") } } + +func TestAllowlist(t *testing.T) { + t.Parallel() + a := assert.New(t) + + ctx := context.Background() + ctx, cancel := context.WithCancel(ctx) + defer cancel() + + bgen := butil.NewBlockGenerator() + block := bgen.Next() + + data := []byte("this is some blake3 block") + mh, err := multihash.Sum(data, multihash.BLAKE3, -1) + a.NoError(err) + blake3 := cid.NewCidV1(cid.Raw, mh) + + bs := blockstore.NewBlockstore(dssync.MutexWrap(ds.NewMapDatastore())) + a.NoError(bs.Put(ctx, block)) + b, err := blocks.NewBlockWithCid(data, blake3) + a.NoError(err) + a.NoError(bs.Put(ctx, b)) + + check := func(getBlock func(context.Context, cid.Cid) (blocks.Block, error)) { + _, err := getBlock(ctx, block.Cid()) + a.Error(err) + a.ErrorIs(err, verifcid.ErrPossiblyInsecureHashFunction) + + _, err = getBlock(ctx, blake3) + a.NoError(err) + } + + blockservice := New(bs, nil, WithAllowlist(verifcid.NewAllowlist(map[uint64]bool{multihash.BLAKE3: true}))) + check(blockservice.GetBlock) + check(NewSession(ctx, blockservice).GetBlock) +}