blockservice: use functional option pattern for construction and add …

…Allowlist tests

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

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.

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)
+}