kademlia: move marshalling to debugapi (#1516)

pkg/debugapi/topology.go

@@ -14,14 +14,9 @@ import (
 )
 
 func (s *Service) topologyHandler(w http.ResponseWriter, r *http.Request) {
-	ms, ok := s.topologyDriver.(json.Marshaler)
-	if !ok {
-		s.logger.Error("topology driver cast to json marshaler")
-		jsonhttp.InternalServerError(w, "topology json marshal interface error")
-		return
-	}
+	params := s.topologyDriver.Snapshot()
 
-	b, err := ms.MarshalJSON()
+	b, err := json.Marshal(params)
 	if err != nil {
 		s.logger.Errorf("topology marshal to json: %v", err)
 		jsonhttp.InternalServerError(w, err)

pkg/debugapi/topology_test.go

@@ -5,47 +5,20 @@
 package debugapi_test
 
 import (
-	"encoding/json"
-	"errors"
 	"net/http"
 	"testing"
 
-	"github.com/ethersphere/bee/pkg/jsonhttp"
 	"github.com/ethersphere/bee/pkg/jsonhttp/jsonhttptest"
-	topmock "github.com/ethersphere/bee/pkg/topology/mock"
 )
 
-type topologyResponse struct {
-	Topology string `json:"topology"`
-}
-
 func TestTopologyOK(t *testing.T) {
-	marshalFunc := func() ([]byte, error) {
-		return json.Marshal(topologyResponse{Topology: "abcd"})
-	}
-	testServer := newTestServer(t, testServerOptions{
-		TopologyOpts: []topmock.Option{topmock.WithMarshalJSONFunc(marshalFunc)},
-	})
+	testServer := newTestServer(t, testServerOptions{})
 
-	jsonhttptest.Request(t, testServer.Client, http.MethodGet, "/topology", http.StatusOK,
-		jsonhttptest.WithExpectedJSONResponse(topologyResponse{
-			Topology: "abcd",
-		}),
-	)
-}
+	var body []byte
+	opts := jsonhttptest.WithPutResponseBody(&body)
+	jsonhttptest.Request(t, testServer.Client, http.MethodGet, "/topology", http.StatusOK, opts)
 
-func TestTopologyError(t *testing.T) {
-	marshalFunc := func() ([]byte, error) {
-		return nil, errors.New("error")
+	if len(body) == 0 {
+		t.Error("empty response")
 	}
-	testServer := newTestServer(t, testServerOptions{
-		TopologyOpts: []topmock.Option{topmock.WithMarshalJSONFunc(marshalFunc)},
-	})
-
-	jsonhttptest.Request(t, testServer.Client, http.MethodGet, "/topology", http.StatusInternalServerError,
-		jsonhttptest.WithExpectedJSONResponse(jsonhttp.StatusResponse{
-			Message: "error",
-			Code:    http.StatusInternalServerError,
-		}),
-	)
 }

pkg/kademlia/kademlia.go

@@ -993,51 +993,10 @@ func (k *Kad) IsBalanced(bin uint8) bool {
 	return true
 }
 
-// MarshalJSON returns a JSON representation of Kademlia.
-func (k *Kad) MarshalJSON() ([]byte, error) {
-	return k.marshal(false)
-}
-
-func (k *Kad) marshal(indent bool) ([]byte, error) {
-	type binInfo struct {
-		BinPopulation     uint     `json:"population"`
-		BinConnected      uint     `json:"connected"`
-		DisconnectedPeers []string `json:"disconnectedPeers"`
-		ConnectedPeers    []string `json:"connectedPeers"`
-	}
-
-	type kadBins struct {
-		Bin0  binInfo `json:"bin_0"`
-		Bin1  binInfo `json:"bin_1"`
-		Bin2  binInfo `json:"bin_2"`
-		Bin3  binInfo `json:"bin_3"`
-		Bin4  binInfo `json:"bin_4"`
-		Bin5  binInfo `json:"bin_5"`
-		Bin6  binInfo `json:"bin_6"`
-		Bin7  binInfo `json:"bin_7"`
-		Bin8  binInfo `json:"bin_8"`
-		Bin9  binInfo `json:"bin_9"`
-		Bin10 binInfo `json:"bin_10"`
-		Bin11 binInfo `json:"bin_11"`
-		Bin12 binInfo `json:"bin_12"`
-		Bin13 binInfo `json:"bin_13"`
-		Bin14 binInfo `json:"bin_14"`
-		Bin15 binInfo `json:"bin_15"`
-	}
-
-	type kadParams struct {
-		Base           string    `json:"baseAddr"`       // base address string
-		Population     int       `json:"population"`     // known
-		Connected      int       `json:"connected"`      // connected count
-		Timestamp      time.Time `json:"timestamp"`      // now
-		NNLowWatermark int       `json:"nnLowWatermark"` // low watermark for depth calculation
-		Depth          uint8     `json:"depth"`          // current depth
-		Bins           kadBins   `json:"bins"`           // individual bin info
-	}
-
-	var infos []binInfo
+func (k *Kad) Snapshot() *topology.KadParams {
+	var infos []topology.BinInfo
 	for i := int(swarm.MaxPO); i >= 0; i-- {
-		infos = append(infos, binInfo{})
+		infos = append(infos, topology.BinInfo{})
 	}
 
 	_ = k.connectedPeers.EachBin(func(addr swarm.Address, po uint8) (bool, bool, error) {
@@ -1061,14 +1020,14 @@ func (k *Kad) marshal(indent bool) ([]byte, error) {
 		return false, false, nil
 	})
 
-	j := &kadParams{
+	return &topology.KadParams{
 		Base:           k.base.String(),
 		Population:     k.knownPeers.Length(),
 		Connected:      k.connectedPeers.Length(),
 		Timestamp:      time.Now(),
 		NNLowWatermark: nnLowWatermark,
 		Depth:          k.NeighborhoodDepth(),
-		Bins: kadBins{
+		Bins: topology.KadBins{
 			Bin0:  infos[0],
 			Bin1:  infos[1],
 			Bin2:  infos[2],
@@ -1087,15 +1046,12 @@ func (k *Kad) marshal(indent bool) ([]byte, error) {
 			Bin15: infos[15],
 		},
 	}
-	if indent {
-		return json.MarshalIndent(j, "", "  ")
-	}
-	return json.Marshal(j)
 }
 
 // String returns a string represenstation of Kademlia.
 func (k *Kad) String() string {
-	b, err := k.marshal(true)
+	j := k.Snapshot()
+	b, err := json.MarshalIndent(j, "", "  ")
 	if err != nil {
 		k.logger.Errorf("could not marshal kademlia into json: %v", err)
 		return ""

pkg/kademlia/kademlia_test.go

@@ -7,8 +7,10 @@ package kademlia_test
 import (
 	"context"
 	"errors"
+	"fmt"
 	"io/ioutil"
 	"math/rand"
+	"reflect"
 	"sync/atomic"
 	"testing"
 	"time"
@@ -914,8 +916,9 @@ func TestKademlia_SubscribePeersChange(t *testing.T) {
 	})
 }
 
-func TestMarshal(t *testing.T) {
-	_, kad, ab, _, signer := newTestKademlia(nil, nil, kademlia.Options{})
+func TestSnapshot(t *testing.T) {
+	var conns = new(int32)
+	sa, kad, ab, _, signer := newTestKademlia(conns, nil, kademlia.Options{})
 	if err := kad.Start(context.Background()); err != nil {
 		t.Fatal(err)
 	}
@@ -923,10 +926,31 @@ func TestMarshal(t *testing.T) {
 
 	a := test.RandomAddress()
 	addOne(t, signer, kad, ab, a)
-	_, err := kad.MarshalJSON()
-	if err != nil {
-		t.Fatal(err)
+
+	snap := kad.Snapshot()
+
+	waitConn(t, conns)
+
+	if snap.Connected != 0 {
+		t.Errorf("expected %d connected peers but got %d", 0, snap.Connected)
+	}
+	if snap.Population != 1 {
+		t.Errorf("expected population %d but got %d", 1, snap.Population)
 	}
+
+	po := swarm.Proximity(sa.Bytes(), a.Bytes())
+
+	if binP := getBinPopulation(&snap.Bins, po); binP != 1 {
+		t.Errorf("expected bin(%d) to have population %d but got %d", po, 1, snap.Population)
+	}
+}
+
+func getBinPopulation(bins *topology.KadBins, po uint8) uint64 {
+	rv := reflect.ValueOf(bins)
+	bin := fmt.Sprintf("Bin%d", po)
+	b0 := reflect.Indirect(rv).FieldByName(bin)
+	bp := b0.FieldByName("BinPopulation")
+	return bp.Uint()
 }
 
 func TestStart(t *testing.T) {

pkg/kademlia/mock/kademlia.go

@@ -174,6 +174,10 @@ func (m *Mock) Close() error {
 	panic("not implemented") // TODO: Implement
 }
 
+func (m *Mock) Snapshot() *topology.KadParams {
+	panic("not implemented") // TODO: Implement
+}
+
 type Option interface {
 	apply(*Mock)
 }

pkg/topology/mock/mock.go

@@ -158,8 +158,8 @@ func (d *mock) EachPeerRev(f topology.EachPeerFunc) (err error) {
 	return nil
 }
 
-func (d *mock) MarshalJSON() ([]byte, error) {
-	return d.marshalJSONFunc()
+func (d *mock) Snapshot() *topology.KadParams {
+	return new(topology.KadParams)
 }
 
 func (d *mock) Close() error {

pkg/topology/topology.go

@@ -10,6 +10,7 @@ import (
 	"context"
 	"errors"
 	"io"
+	"time"
 
 	"github.com/ethersphere/bee/pkg/swarm"
 )
@@ -27,6 +28,7 @@ type Driver interface {
 	NeighborhoodDepth() uint8
 	SubscribePeersChange() (c <-chan struct{}, unsubscribe func())
 	io.Closer
+	Snapshot() *KadParams
 }
 
 type PeerAdder interface {
@@ -51,3 +53,39 @@ type EachPeerer interface {
 
 // EachPeerFunc is a callback that is called with a peer and its PO
 type EachPeerFunc func(swarm.Address, uint8) (stop, jumpToNext bool, err error)
+
+type BinInfo struct {
+	BinPopulation     uint     `json:"population"`
+	BinConnected      uint     `json:"connected"`
+	DisconnectedPeers []string `json:"disconnectedPeers"`
+	ConnectedPeers    []string `json:"connectedPeers"`
+}
+
+type KadBins struct {
+	Bin0  BinInfo `json:"bin_0"`
+	Bin1  BinInfo `json:"bin_1"`
+	Bin2  BinInfo `json:"bin_2"`
+	Bin3  BinInfo `json:"bin_3"`
+	Bin4  BinInfo `json:"bin_4"`
+	Bin5  BinInfo `json:"bin_5"`
+	Bin6  BinInfo `json:"bin_6"`
+	Bin7  BinInfo `json:"bin_7"`
+	Bin8  BinInfo `json:"bin_8"`
+	Bin9  BinInfo `json:"bin_9"`
+	Bin10 BinInfo `json:"bin_10"`
+	Bin11 BinInfo `json:"bin_11"`
+	Bin12 BinInfo `json:"bin_12"`
+	Bin13 BinInfo `json:"bin_13"`
+	Bin14 BinInfo `json:"bin_14"`
+	Bin15 BinInfo `json:"bin_15"`
+}
+
+type KadParams struct {
+	Base           string    `json:"baseAddr"`       // base address string
+	Population     int       `json:"population"`     // known
+	Connected      int       `json:"connected"`      // connected count
+	Timestamp      time.Time `json:"timestamp"`      // now
+	NNLowWatermark int       `json:"nnLowWatermark"` // low watermark for depth calculation
+	Depth          uint8     `json:"depth"`          // current depth
+	Bins           KadBins   `json:"bins"`           // individual bin info
+}