Addressbook get - not found (#319)

* addressbook get not found

pkg/addressbook/addressbook.go

@@ -5,6 +5,7 @@
 package addressbook
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 
@@ -17,6 +18,8 @@ const keyPrefix = "addressbook_entry_"
 
 var _ Interface = (*store)(nil)
 
+var ErrNotFound = errors.New("addressbook: not found")
+
 type Interface interface {
 	GetPutter
 	Remover
@@ -30,7 +33,7 @@ type GetPutter interface {
 }
 
 type Getter interface {
-	Get(overlay swarm.Address) (addr bzz.Address, err error)
+	Get(overlay swarm.Address) (addr *bzz.Address, err error)
 }
 
 type Putter interface {
@@ -51,12 +54,16 @@ func New(storer storage.StateStorer) Interface {
 	}
 }
 
-func (s *store) Get(overlay swarm.Address) (bzz.Address, error) {
+func (s *store) Get(overlay swarm.Address) (*bzz.Address, error) {
 	key := keyPrefix + overlay.String()
-	v := bzz.Address{}
+	v := &bzz.Address{}
 	err := s.store.Get(key, &v)
 	if err != nil {
-		return bzz.Address{}, err
+		if err == storage.ErrNotFound {
+			return nil, ErrNotFound
+		}
+
+		return nil, err
 	}
 	return v, nil
 }

pkg/addressbook/addressbook_test.go

@@ -22,7 +22,6 @@ func TestInMem(t *testing.T) {
 	run(t, func(t *testing.T) addressbook.Interface {
 		store := mock.NewStateStore()
 		book := addressbook.New(store)
-
 		return book
 	})
 }
@@ -56,13 +55,17 @@ func run(t *testing.T, f bookFunc) {
 		t.Fatal(err)
 	}
 
-	_, err = store.Get(addr2)
-	if err == nil {
-		t.Fatal("value found in store but should not have been")
+	if !bzzAddr.Equal(v) {
+		t.Fatalf("expectted: %s, want %s", v, multiaddr)
+	}
+
+	notFound, err := store.Get(addr2)
+	if err != addressbook.ErrNotFound {
+		t.Fatal(err)
 	}
 
-	if !bzzAddr.Equal(&v) {
-		t.Fatalf("value retrieved from store not equal to original stored address: %v, want %v", v, multiaddr)
+	if notFound != nil {
+		t.Fatalf("expected nil got %s", v)
 	}
 
 	overlays, err := store.Overlays()

pkg/debugapi/peer_test.go

@@ -17,7 +17,6 @@ import (
 	"github.com/ethersphere/bee/pkg/jsonhttp/jsonhttptest"
 	"github.com/ethersphere/bee/pkg/p2p"
 	"github.com/ethersphere/bee/pkg/p2p/mock"
-	"github.com/ethersphere/bee/pkg/storage"
 	"github.com/ethersphere/bee/pkg/swarm"
 	topmock "github.com/ethersphere/bee/pkg/topology/mock"
 	ma "github.com/multiformats/go-multiaddr"
@@ -59,7 +58,10 @@ func TestConnect(t *testing.T) {
 		})
 
 		bzzAddr, err := testServer.Addressbook.Get(overlay)
-		if err != nil && errors.Is(err, storage.ErrNotFound) && !bzzAddress.Equal(&bzzAddr) {
+		if err != nil {
+			t.Fatal(err)
+		}
+		if !bzzAddress.Equal(bzzAddr) {
 			t.Fatalf("found wrong underlay.  expected: %+v, found: %+v", bzzAddress, bzzAddr)
 		}
 	})
@@ -99,7 +101,10 @@ func TestConnect(t *testing.T) {
 		})
 
 		bzzAddr, err := testServer.Addressbook.Get(overlay)
-		if err != nil && errors.Is(err, storage.ErrNotFound) && !bzzAddress.Equal(&bzzAddr) {
+		if err != nil {
+			t.Fatal(err)
+		}
+		if !bzzAddress.Equal(bzzAddr) {
 			t.Fatalf("found wrong underlay.  expected: %+v, found: %+v", bzzAddress, bzzAddr)
 		}
 

pkg/hive/hive.go

@@ -6,7 +6,6 @@ package hive
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"time"
 
@@ -16,7 +15,6 @@ import (
 	"github.com/ethersphere/bee/pkg/logging"
 	"github.com/ethersphere/bee/pkg/p2p"
 	"github.com/ethersphere/bee/pkg/p2p/protobuf"
-	"github.com/ethersphere/bee/pkg/storage"
 	"github.com/ethersphere/bee/pkg/swarm"
 )
 
@@ -97,10 +95,11 @@ func (s *Service) sendPeers(ctx context.Context, peer swarm.Address, peers []swa
 	for _, p := range peers {
 		addr, err := s.addressBook.Get(p)
 		if err != nil {
-			if errors.Is(err, storage.ErrNotFound) {
-				s.logger.Debugf("Peer not found %s", p)
+			if err == addressbook.ErrNotFound {
+				s.logger.Debugf("hive broadcast peers: peer not found in the addressbook. Skipping peer %s", p)
 				continue
 			}
+
 			return err
 		}
 

pkg/kademlia/kademlia.go

@@ -16,7 +16,6 @@ import (
 	"github.com/ethersphere/bee/pkg/kademlia/pslice"
 	"github.com/ethersphere/bee/pkg/logging"
 	"github.com/ethersphere/bee/pkg/p2p"
-	"github.com/ethersphere/bee/pkg/storage"
 	"github.com/ethersphere/bee/pkg/swarm"
 	"github.com/ethersphere/bee/pkg/topology"
 	ma "github.com/multiformats/go-multiaddr"
@@ -134,17 +133,16 @@ func (k *Kad) manage() {
 
 				bzzAddr, err := k.addressBook.Get(peer)
 				if err != nil {
-					// either a peer is not known in the address book, in which case it
-					// should be removed, or that some severe I/O problem is at hand
-
-					if errors.Is(err, storage.ErrNotFound) {
+					if err == addressbook.ErrNotFound {
 						k.logger.Errorf("failed to get address book entry for peer: %s", peer.String())
 						peerToRemove = peer
 						return false, false, errMissingAddressBookEntry
 					}
-
+					// either a peer is not known in the address book, in which case it
+					// should be removed, or that some severe I/O problem is at hand
 					return false, false, err
 				}
+
 				k.logger.Debugf("kademlia dialing to peer %s", peer.String())
 
 				err = k.connect(ctx, peer, bzzAddr.Underlay, po)

pkg/kademlia/kademlia_test.go

@@ -377,7 +377,7 @@ func TestAddressBookPrune(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if !nonConnPeer.Equal(&p) {
+	if !nonConnPeer.Equal(p) {
 		t.Fatalf("expected %+v, got %+v", nonConnPeer, p)
 	}
 
@@ -392,7 +392,7 @@ func TestAddressBookPrune(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if !nonConnPeer.Equal(&p) {
+	if !nonConnPeer.Equal(p) {
 		t.Fatalf("expected %+v, got %+v", nonConnPeer, p)
 	}
 
@@ -407,7 +407,7 @@ func TestAddressBookPrune(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if !nonConnPeer.Equal(&p) {
+	if !nonConnPeer.Equal(p) {
 		t.Fatalf("expected %+v, got %+v", nonConnPeer, p)
 	}
 
@@ -417,13 +417,9 @@ func TestAddressBookPrune(t *testing.T) {
 	waitCounter(t, &conns, 1)
 	waitCounter(t, &failedConns, 1)
 
-	p, err = ab.Get(nonConnPeer.Overlay)
-	if err == nil {
-		t.Fatal("expected not found error")
-	}
-
-	if nonConnPeer.Equal(&p) {
-		t.Fatal("peer found in addressbook")
+	_, err = ab.Get(nonConnPeer.Overlay)
+	if err != addressbook.ErrNotFound {
+		t.Fatal(err)
 	}
 }
 

pkg/topology/full/full.go

@@ -17,7 +17,6 @@ import (
 	"github.com/ethersphere/bee/pkg/discovery"
 	"github.com/ethersphere/bee/pkg/logging"
 	"github.com/ethersphere/bee/pkg/p2p"
-	"github.com/ethersphere/bee/pkg/storage"
 	"github.com/ethersphere/bee/pkg/swarm"
 	"github.com/ethersphere/bee/pkg/topology"
 )
@@ -71,7 +70,7 @@ func (d *driver) AddPeer(ctx context.Context, addr swarm.Address) error {
 	connectedPeers := d.p2pService.Peers()
 	bzzAddress, err := d.addressBook.Get(addr)
 	if err != nil {
-		if errors.Is(err, storage.ErrNotFound) {
+		if err == addressbook.ErrNotFound {
 			return topology.ErrNotFound
 		}
 		return err