diff --git a/v2/blockstore/readonly.go b/v2/blockstore/readonly.go index dbf7bf61..2df9a794 100644 --- a/v2/blockstore/readonly.go +++ b/v2/blockstore/readonly.go @@ -24,7 +24,10 @@ import ( var _ blockstore.Blockstore = (*ReadOnly)(nil) -var errZeroLengthSection = fmt.Errorf("zero-length section not allowed by default; see WithZeroLengthSectionAsEOF option") +var ( + errZeroLengthSection = fmt.Errorf("zero-length carv2 section not allowed by default; see WithZeroLengthSectionAsEOF option") + errReadOnly = fmt.Errorf("called write method on a read-only carv2 blockstore") +) type ( // ReadOnly provides a read-only CAR Block Store. @@ -176,7 +179,7 @@ func (b *ReadOnly) readBlock(idx int64) (cid.Cid, []byte, error) { // DeleteBlock is unsupported and always panics. func (b *ReadOnly) DeleteBlock(_ cid.Cid) error { - panic("called write method on a read-only blockstore") + return errReadOnly } // Has indicates if the store contains a block that corresponds to the given key. @@ -303,12 +306,12 @@ func (b *ReadOnly) GetSize(key cid.Cid) (int, error) { // Put is not supported and always returns an error. func (b *ReadOnly) Put(blocks.Block) error { - panic("called write method on a read-only blockstore") + return errReadOnly } // PutMany is not supported and always returns an error. func (b *ReadOnly) PutMany([]blocks.Block) error { - panic("called write method on a read-only blockstore") + return errReadOnly } // WithAsyncErrorHandler returns a context with async error handling set to the given errHandler. diff --git a/v2/blockstore/readonly_test.go b/v2/blockstore/readonly_test.go index ecc30e1d..3fd16c8c 100644 --- a/v2/blockstore/readonly_test.go +++ b/v2/blockstore/readonly_test.go @@ -102,9 +102,9 @@ func TestReadOnly(t *testing.T) { require.Equal(t, wantBlock, gotBlock) // Assert write operations panic - require.Panics(t, func() { subject.Put(wantBlock) }) - require.Panics(t, func() { subject.PutMany([]blocks.Block{wantBlock}) }) - require.Panics(t, func() { subject.DeleteBlock(key) }) + require.Error(t, subject.Put(wantBlock)) + require.Error(t, subject.PutMany([]blocks.Block{wantBlock})) + require.Error(t, subject.DeleteBlock(key)) } ctx, cancel := context.WithTimeout(context.Background(), time.Second*2) diff --git a/v2/blockstore/readwrite.go b/v2/blockstore/readwrite.go index ba87ee60..1192e749 100644 --- a/v2/blockstore/readwrite.go +++ b/v2/blockstore/readwrite.go @@ -23,6 +23,8 @@ import ( var _ blockstore.Blockstore = (*ReadWrite)(nil) +var errFinalized = fmt.Errorf("cannot use a read-write carv2 blockstore after finalizing") + // ReadWrite implements a blockstore that stores blocks in CARv2 format. // Blocks put into the blockstore can be read back once they are successfully written. // This implementation is preferable for a write-heavy workload. @@ -284,22 +286,22 @@ func (b *ReadWrite) unfinalize() error { return err } -func (b *ReadWrite) panicIfFinalized() { - if b.header.DataSize != 0 { - panic("must not use a read-write blockstore after finalizing") - } +func (b *ReadWrite) finalized() bool { + return b.header.DataSize != 0 } // Put puts a given block to the underlying datastore func (b *ReadWrite) Put(blk blocks.Block) error { - // PutMany already calls panicIfFinalized. + // PutMany already checks b.finalized. return b.PutMany([]blocks.Block{blk}) } // PutMany puts a slice of blocks at the same time using batching // capabilities of the underlying datastore whenever possible. func (b *ReadWrite) PutMany(blks []blocks.Block) error { - b.panicIfFinalized() + if b.finalized() { + return errFinalized + } b.mu.Lock() defer b.mu.Unlock() @@ -362,31 +364,33 @@ func (b *ReadWrite) Finalize() error { } func (b *ReadWrite) AllKeysChan(ctx context.Context) (<-chan cid.Cid, error) { - b.panicIfFinalized() + if b.finalized() { + return nil, errFinalized + } return b.ReadOnly.AllKeysChan(ctx) } func (b *ReadWrite) Has(key cid.Cid) (bool, error) { - b.panicIfFinalized() + if b.finalized() { + return false, errFinalized + } return b.ReadOnly.Has(key) } func (b *ReadWrite) Get(key cid.Cid) (blocks.Block, error) { - b.panicIfFinalized() + if b.finalized() { + return nil, errFinalized + } return b.ReadOnly.Get(key) } func (b *ReadWrite) GetSize(key cid.Cid) (int, error) { - b.panicIfFinalized() + if b.finalized() { + return 0, errFinalized + } return b.ReadOnly.GetSize(key) } - -func (b *ReadWrite) HashOnRead(h bool) { - b.panicIfFinalized() - - b.ReadOnly.HashOnRead(h) -} diff --git a/v2/blockstore/readwrite_test.go b/v2/blockstore/readwrite_test.go index f3016e25..77442de0 100644 --- a/v2/blockstore/readwrite_test.go +++ b/v2/blockstore/readwrite_test.go @@ -494,18 +494,24 @@ func TestReadWritePanicsOnlyWhenFinalized(t *testing.T) { require.True(t, has) subject.HashOnRead(true) - // Delete should always panic regardless of finalize - require.Panics(t, func() { subject.DeleteBlock(oneTestBlockCid) }) + // Delete should always error regardless of finalize + require.Error(t, subject.DeleteBlock(oneTestBlockCid)) require.NoError(t, subject.Finalize()) - require.Panics(t, func() { subject.Get(oneTestBlockCid) }) - require.Panics(t, func() { subject.GetSize(anotherTestBlockCid) }) - require.Panics(t, func() { subject.Has(anotherTestBlockCid) }) - require.Panics(t, func() { subject.HashOnRead(true) }) - require.Panics(t, func() { subject.Put(oneTestBlockWithCidV1) }) - require.Panics(t, func() { subject.PutMany([]blocks.Block{anotherTestBlockWithCidV0}) }) - require.Panics(t, func() { subject.AllKeysChan(context.Background()) }) - require.Panics(t, func() { subject.DeleteBlock(oneTestBlockCid) }) + require.Error(t, subject.Finalize()) + + _, err = subject.Get(oneTestBlockCid) + require.Error(t, err) + _, err = subject.GetSize(anotherTestBlockCid) + require.Error(t, err) + _, err = subject.Has(anotherTestBlockCid) + require.Error(t, err) + + require.Error(t, subject.Put(oneTestBlockWithCidV1)) + require.Error(t, subject.PutMany([]blocks.Block{anotherTestBlockWithCidV0})) + _, err = subject.AllKeysChan(context.Background()) + require.Error(t, err) + require.Error(t, subject.DeleteBlock(oneTestBlockCid)) } func TestReadWriteWithPaddingWorksAsExpected(t *testing.T) {