[TASK] Remove nodes without links from graph.json (#22)

* [TASK] [models] graph.json should only save nodes with links

* [TASK] [models] graph.json should only save nodes with links and there main mac (with tests)

models/graph.go

@@ -34,6 +34,7 @@ type GraphLink struct {
 // GraphBuilder a temporaty struct during fill the graph from the node neighbours
 type graphBuilder struct {
 	macToID map[string]string      // mapping from MAC address to node id
+	idToMac map[string]string      // mapping from node id to one MAC address
 	links   map[string]*GraphLink  // mapping from $idA-$idB to existing link
 	vpn     map[string]interface{} // IDs/addresses of VPN servers
 }
@@ -42,11 +43,14 @@ type graphBuilder struct {
 func (nodes *Nodes) BuildGraph() *Graph {
 	builder := &graphBuilder{
 		macToID: make(map[string]string),
+		idToMac: make(map[string]string),
 		links:   make(map[string]*GraphLink),
 		vpn:     make(map[string]interface{}),
 	}
 
+	nodes.RLock()
 	builder.readNodes(nodes.List)
+	nodes.RUnlock()
 
 	graph := &Graph{Version: 1}
 	graph.Batadv.Directed = false
@@ -63,6 +67,10 @@ func (builder *graphBuilder) readNodes(nodes map[string]*Node) {
 				builder.vpn[sourceID] = nil
 			}
 
+			if len(nodeinfo.Network.Mac) > 0 {
+				builder.macToID[sourceID] = nodeinfo.Network.Mac
+			}
+
 			// Batman neighbours
 			for _, batinterface := range nodeinfo.Network.Mesh {
 				interfaces := batinterface.Interfaces
@@ -107,34 +115,49 @@ func (builder *graphBuilder) readNodes(nodes map[string]*Node) {
 	}
 }
 
-func (builder *graphBuilder) extract() ([]*GraphNode, []*GraphLink) {
-	links := make([]*GraphLink, len(builder.links))
-	nodes := make([]*GraphNode, len(builder.macToID))
-	idToIndex := make(map[string]int)
+type graphNodeCache struct {
+	idToMac   map[string]string
+	idToIndex map[string]int
+	count     int
+	Nodes     []*GraphNode
+}
 
-	// collect nodes and create mapping to index
-	i := 0
-	for mac, nodeID := range builder.macToID {
-		nodes[i] = &GraphNode{
-			ID:     mac,
+func newGraphNodeCache(idToMac map[string]string) *graphNodeCache {
+	return &graphNodeCache{
+		idToMac:   idToMac,
+		idToIndex: make(map[string]int),
+	}
+}
+
+func (gn *graphNodeCache) getIndex(nodeID string) int {
+	index, ok := gn.idToIndex[nodeID]
+	if !ok {
+		node := &GraphNode{
+			ID:     gn.idToMac[nodeID],
 			NodeID: nodeID,
 		}
-		idToIndex[nodeID] = i
-		i++
+		gn.Nodes = append(gn.Nodes, node)
+		gn.idToIndex[nodeID] = gn.count
+		index = gn.count
+		gn.count++
 	}
+	return index
+}
+
+func (builder *graphBuilder) extract() ([]*GraphNode, []*GraphLink) {
+	links := make([]*GraphLink, len(builder.links))
+	cache := newGraphNodeCache(builder.idToMac)
 
 	// collect links
-	i = 0
+	i := 0
 	for key, link := range builder.links {
 		pos := strings.IndexByte(key, '-')
-
-		link.Source = idToIndex[key[:pos]]
-		link.Target = idToIndex[key[pos+1:]]
+		link.Source = cache.getIndex(key[:pos])
+		link.Target = cache.getIndex(key[pos+1:])
 		links[i] = link
 		i++
 	}
-
-	return nodes, links
+	return cache.Nodes, links
 }
 
 func (builder *graphBuilder) isVPN(ids ...string) bool {

models/graph_test.go

@@ -17,15 +17,16 @@ type TestNode struct {
 
 func TestGenerateGraph(t *testing.T) {
 	assert := assert.New(t)
-	nodes := testGetNodesByFile("node1.json", "node2.json", "node3.json")
+	nodes := testGetNodesByFile("node1.json", "node2.json", "node3.json", "node4.json")
 
 	graph := nodes.BuildGraph()
 	assert.NotNil(graph)
 	assert.Equal(1, graph.Version, "Wrong Version")
 	assert.NotNil(graph.Batadv, "no Batadv")
 	assert.Equal(false, graph.Batadv.Directed, "directed batadv")
-	assert.Equal(3, len(graph.Batadv.Nodes), "wrong Nodes count")
-	assert.Equal(2, len(graph.Batadv.Links), "wrong Links count")
+	assert.Len(graph.Batadv.Links, 3, "wrong Links count")
+	assert.Equal(4, testNodesCountWithLinks(graph.Batadv.Links), "wrong unneed nodes in graph")
+	assert.Len(graph.Batadv.Nodes, 4, "wrong Nodes count")
 	// TODO more tests required
 }
 
@@ -65,3 +66,12 @@ func testfile(name string, obj interface{}) {
 		panic(err)
 	}
 }
+
+func testNodesCountWithLinks(links []*GraphLink) int {
+	indexMap := make(map[int]bool)
+	for _, l := range links {
+		indexMap[l.Source] = true
+		indexMap[l.Target] = true
+	}
+	return len(indexMap)
+}

models/nodes_test.go

@@ -33,7 +33,7 @@ func TestExpire(t *testing.T) {
 	nodes.expire()
 
 	// one expired?
-	assert.Equal(2, len(nodes.List))
+	assert.Len(nodes.List, 2)
 	assert.Nil(nodes.List["expire"])
 
 	// one offline?
@@ -58,13 +58,13 @@ func TestLoadAndSave(t *testing.T) {
 	save(nodes, tmpfile.Name())
 	os.Remove(tmpfile.Name())
 
-	assert.Equal(1, len(nodes.List))
+	assert.Len(nodes.List, 1)
 }
 
 func TestUpdateNodes(t *testing.T) {
 	assert := assert.New(t)
 	nodes := &Nodes{List: make(map[string]*Node)}
-	assert.Equal(0, len(nodes.List))
+	assert.Len(nodes.List, 0)
 
 	res := &data.ResponseData{
 		Neighbours: &data.Neighbours{},
@@ -73,5 +73,5 @@ func TestUpdateNodes(t *testing.T) {
 	}
 	nodes.Update("abcdef012345", res)
 
-	assert.Equal(1, len(nodes.List))
+	assert.Len(nodes.List, 1)
 }

models/stats_test.go

@@ -17,12 +17,12 @@ func TestGlobalStats(t *testing.T) {
 	assert.EqualValues(25, stats.Clients)
 
 	// check models
-	assert.EqualValues(2, len(stats.Models))
+	assert.Len(stats.Models, 2)
 	assert.EqualValues(2, stats.Models["TP-Link 841"])
 	assert.EqualValues(1, stats.Models["Xeon Multi-Core"])
 
 	// check firmwares
-	assert.EqualValues(1, len(stats.Firmwares))
+	assert.Len(stats.Firmwares, 1)
 	assert.EqualValues(1, stats.Firmwares["2016.1.6+entenhausen1"])
 
 	fields := stats.Fields()
@@ -35,13 +35,13 @@ func TestNodesV1(t *testing.T) {
 	nodes := createTestNodes().GetNodesV1()
 
 	assert := assert.New(t)
-	assert.Equal(2, len(nodes.List))
+	assert.Len(nodes.List, 2)
 }
 func TestNodesV2(t *testing.T) {
 	nodes := createTestNodes().GetNodesV2()
 
 	assert := assert.New(t)
-	assert.Equal(2, len(nodes.List))
+	assert.Len(nodes.List, 2)
 }
 
 func createTestNodes() *Nodes {

models/testdata/node4.json

@@ -0,0 +1,25 @@
+{
+	"nodeinfo":{
+		"node_id":"node4.json",
+		"network":{
+			"mesh":{
+				"bat0":{
+					"interfaces":{
+						"wireless":["unneed","unneed2"],
+						"tunnel":["d"]
+					}
+				}
+			}
+		}
+	},
+	"neighbours":{
+		"batadv":{
+			"d":{
+					"neighbours":{
+						"a":{"tq":200,"lastseen":0.42}
+					}
+				}
+		}
+	}
+
+}