Fix index not found error not being propagated

Fix an issue where single width index not finding a given key returns
`0` offset instead of `index.ErrNotFound`.

Reflect the changes in blockstore to return appropriate IPFS blockstore
error.

Add tests that asserts error types both in index and blockstore
packages.

Remove redundant TODOs.

Relates to:
- https://github.com/ipld/go-car/pull/158

v2/blockstore/readonly.go

@@ -165,14 +165,15 @@ func (b *ReadOnly) Get(key cid.Cid) (blocks.Block, error) {
 	defer b.mu.RUnlock()
 
 	offset, err := b.idx.Get(key)
-	// TODO Improve error handling; not all errors mean NotFound.
 	if err != nil {
-		return nil, blockstore.ErrNotFound
+		if err == index.ErrNotFound {
+			err = blockstore.ErrNotFound
+		}
+		return nil, err
 	}
 	entry, data, err := b.readBlock(int64(offset))
 	if err != nil {
-		// TODO Improve error handling; not all errors mean NotFound.
-		return nil, blockstore.ErrNotFound
+		return nil, err
 	}
 	if !bytes.Equal(key.Hash(), entry.Hash()) {
 		return nil, blockstore.ErrNotFound

v2/blockstore/readonly_test.go

@@ -2,6 +2,8 @@ package blockstore
 
 import (
 	"context"
+	blockstore "github.com/ipfs/go-ipfs-blockstore"
+	"github.com/ipfs/go-merkledag"
 	"io"
 	"os"
 	"testing"
@@ -16,6 +18,17 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestReadOnlyGetReturnsBlockstoreNotFoundWhenCidDoesNotExist(t *testing.T) {
+	subject, err := OpenReadOnly("../testdata/sample-v1.car")
+	require.NoError(t, err)
+	nonExistingKey := merkledag.NewRawNode([]byte("lobstermuncher")).Block.Cid()
+
+	// Assert blockstore API returns blockstore.ErrNotFound
+	gotBlock, err := subject.Get(nonExistingKey)
+	require.Equal(t, blockstore.ErrNotFound, err)
+	require.Nil(t, gotBlock)
+}
+
 func TestReadOnly(t *testing.T) {
 	tests := []struct {
 		name       string

v2/blockstore/readwrite_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/multiformats/go-multihash"
 	"github.com/stretchr/testify/require"
 
+	ipfsblockstore "github.com/ipfs/go-ipfs-blockstore"
 	"github.com/ipld/go-car/v2/blockstore"
 
 	blocks "github.com/ipfs/go-block-format"
@@ -34,6 +35,19 @@ var (
 	anotherTestBlockWithCidV0 = blocks.NewBlock([]byte("barreleye"))
 )
 
+func TestReadWriteGetReturnsBlockstoreNotFoundWhenCidDoesNotExist(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "readwrite-err-not-found.car")
+	subject, err := blockstore.OpenReadWrite(path, []cid.Cid{})
+	t.Cleanup(func() { subject.Close() })
+	require.NoError(t, err)
+	nonExistingKey := merkledag.NewRawNode([]byte("undadasea")).Block.Cid()
+
+	// Assert blockstore API returns blockstore.ErrNotFound
+	gotBlock, err := subject.Get(nonExistingKey)
+	require.Equal(t, ipfsblockstore.ErrNotFound, err)
+	require.Nil(t, gotBlock)
+}
+
 func TestBlockstore(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
 	defer cancel()

v2/index/indexsorted.go

@@ -82,20 +82,20 @@ func (s *singleWidthIndex) Get(c cid.Cid) (uint64, error) {
 	if err != nil {
 		return 0, err
 	}
-	return s.get(d.Digest), nil
+	return s.get(d.Digest)
 }
 
-func (s *singleWidthIndex) get(d []byte) uint64 {
+func (s *singleWidthIndex) get(d []byte) (uint64, error) {
 	idx := sort.Search(int(s.len), func(i int) bool {
 		return s.Less(i, d)
 	})
 	if uint64(idx) == s.len {
-		return 0
+		return 0, ErrNotFound
 	}
 	if !bytes.Equal(d[:], s.index[idx*int(s.width):(idx+1)*int(s.width)-8]) {
-		return 0
+		return 0, ErrNotFound
 	}
-	return binary.LittleEndian.Uint64(s.index[(idx+1)*int(s.width)-8 : (idx+1)*int(s.width)])
+	return binary.LittleEndian.Uint64(s.index[(idx+1)*int(s.width)-8 : (idx+1)*int(s.width)]), nil
 }
 
 func (s *singleWidthIndex) Load(items []Record) error {
@@ -121,7 +121,7 @@ func (m *multiWidthIndex) Get(c cid.Cid) (uint64, error) {
 		return 0, err
 	}
 	if s, ok := (*m)[uint32(len(d.Digest)+8)]; ok {
-		return s.get(d.Digest), nil
+		return s.get(d.Digest)
 	}
 	return 0, ErrNotFound
 }

v2/index/indexsorted_test.go

+package index
+
+import (
+	"github.com/ipfs/go-merkledag"
+	"github.com/multiformats/go-multicodec"
+	"github.com/stretchr/testify/require"
+	"testing"
+)
+
+func TestSortedIndexCodec(t *testing.T) {
+	require.Equal(t, multicodec.CarIndexSorted, newSorted().Codec())
+}
+
+func TestSortedIndex_GetReturnsNotFoundWhenCidDoesNotExist(t *testing.T) {
+	nonExistingKey := merkledag.NewRawNode([]byte("lobstermuncher")).Block.Cid()
+	tests := []struct {
+		name    string
+		subject Index
+	}{
+		{
+			"SingleSorted",
+			newSingleSorted(),
+		},
+		{
+			"Sorted",
+			newSorted(),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotOffset, err := tt.subject.Get(nonExistingKey)
+			require.Equal(t, ErrNotFound, err)
+			require.Equal(t, uint64(0), gotOffset)
+		})
+	}
+}