Small NodeManager refactoring (#253)

Introduced `isNodeAvailable()`.
2017-09-11 16:07:35 +02:00 · 2017-09-11 16:07:35 +02:00 · 4fb0faa061
parent c85525e139
commit 4fb0faa061
3 changed files with 68 additions and 167 deletions
--- a/cmd/statusd/utils.go
+++ b/cmd/statusd/utils.go
@ -18,6 +18,7 @@ import (
 	gethparams "github.com/ethereum/go-ethereum/params"

 	"fmt"
+
 	"github.com/status-im/status-go/geth/common"
 	"github.com/status-im/status-go/geth/node"
 	"github.com/status-im/status-go/geth/params"
--- a/geth/node/manager.go
+++ b/geth/node/manager.go
@ -129,16 +129,20 @@ func (m *NodeManager) StopNode() (<-chan struct{}, error) {
 	m.Lock()
 	defer m.Unlock()

+	if err := m.isNodeAvailable(); err != nil {
+		return nil, err
+	}
+	if m.nodeStopped == nil {
+		return nil, ErrNoRunningNode
+	}
+
+	<-m.nodeStarted // make sure you operate on fully started node
+
 	return m.stopNode()
 }

 // stopNode stop Status node. Stopped node cannot be resumed.
 func (m *NodeManager) stopNode() (<-chan struct{}, error) {
-	if m.node == nil || m.nodeStarted == nil || m.nodeStopped == nil {
-		return nil, ErrNoRunningNode
-	}
-	<-m.nodeStarted // make sure you operate on fully started node
-
 	// now attempt to stop
 	if err := m.node.Stop(); err != nil {
 		return nil, err
@ -179,10 +183,10 @@ func (m *NodeManager) IsNodeRunning() bool {
 	m.RLock()
 	defer m.RUnlock()

-	// make sure that node is fully started
-	if m.node == nil || m.nodeStarted == nil {
+	if err := m.isNodeAvailable(); err != nil {
 		return false
 	}
+
 	<-m.nodeStarted

 	return true
@ -193,10 +197,10 @@ func (m *NodeManager) Node() (*node.Node, error) {
 	m.RLock()
 	defer m.RUnlock()

-	// make sure that node is fully started
-	if m.node == nil || m.nodeStarted == nil {
-		return nil, ErrNoRunningNode
+	if err := m.isNodeAvailable(); err != nil {
+		return nil, err
 	}
+
 	<-m.nodeStarted

 	return m.node, nil
@ -207,17 +211,17 @@ func (m *NodeManager) PopulateStaticPeers() error {
 	m.RLock()
 	defer m.RUnlock()

+	if err := m.isNodeAvailable(); err != nil {
+		return err
+	}
+
+	<-m.nodeStarted
+
 	return m.populateStaticPeers()
 }

 // populateStaticPeers connects current node with our publicly available LES/SHH/Swarm cluster
 func (m *NodeManager) populateStaticPeers() error {
-	// make sure that node is fully started
-	if m.node == nil || m.nodeStarted == nil {
-		return ErrNoRunningNode
-	}
-	<-m.nodeStarted
-
 	if !m.config.BootClusterConfig.Enabled {
 		log.Info("Boot cluster is disabled")
 		return nil
@ -240,17 +244,17 @@ func (m *NodeManager) AddPeer(url string) error {
 	m.RLock()
 	defer m.RUnlock()

+	if err := m.isNodeAvailable(); err != nil {
+		return err
+	}
+
+	<-m.nodeStarted
+
 	return m.addPeer(url)
 }

 // addPeer adds new static peer node
 func (m *NodeManager) addPeer(url string) error {
-	// make sure that node is fully started
-	if m.node == nil || m.nodeStarted == nil {
-		return ErrNoRunningNode
-	}
-	<-m.nodeStarted
-
 	server := m.node.Server()
 	if server == nil {
 		return ErrNoRunningNode
@ -272,18 +276,18 @@ func (m *NodeManager) ResetChainData() (<-chan struct{}, error) {
 	m.Lock()
 	defer m.Unlock()

+	if err := m.isNodeAvailable(); err != nil {
+		return nil, err
+	}
+
+	<-m.nodeStarted
+
 	return m.resetChainData()
 }

 // resetChainData remove chain data from data directory.
 // Node is stopped, and new node is started, with clean data directory.
 func (m *NodeManager) resetChainData() (<-chan struct{}, error) {
-	// make sure that node is fully started
-	if m.node == nil || m.nodeStarted == nil {
-		return nil, ErrNoRunningNode
-	}
-	<-m.nodeStarted
-
 	prevConfig := *m.config
 	nodeStopped, err := m.stopNode()
 	if err != nil {
@ -316,17 +320,17 @@ func (m *NodeManager) RestartNode() (<-chan struct{}, error) {
 	m.Lock()
 	defer m.Unlock()

+	if err := m.isNodeAvailable(); err != nil {
+		return nil, err
+	}
+
+	<-m.nodeStarted
+
 	return m.restartNode()
 }

 // restartNode restart running Status node, fails if node is not running
 func (m *NodeManager) restartNode() (<-chan struct{}, error) {
-	// make sure that node is fully started
-	if m.node == nil || m.nodeStarted == nil {
-		return nil, ErrNoRunningNode
-	}
-	<-m.nodeStarted
-
 	prevConfig := *m.config
 	nodeStopped, err := m.stopNode()
 	if err != nil {
@ -345,10 +349,10 @@ func (m *NodeManager) NodeConfig() (*params.NodeConfig, error) {
 	m.RLock()
 	defer m.RUnlock()

-	// make sure that node is fully started
-	if m.node == nil || m.nodeStarted == nil {
-		return nil, ErrNoRunningNode
+	if err := m.isNodeAvailable(); err != nil {
+		return nil, err
 	}
+
 	<-m.nodeStarted

 	return m.config, nil
@ -359,10 +363,10 @@ func (m *NodeManager) LightEthereumService() (*les.LightEthereum, error) {
 	m.RLock()
 	defer m.RUnlock()

-	// make sure that node is fully started
-	if m.node == nil || m.nodeStarted == nil {
-		return nil, ErrNoRunningNode
+	if err := m.isNodeAvailable(); err != nil {
+		return nil, err
 	}
+
 	<-m.nodeStarted

 	if m.lesService == nil {
@ -384,10 +388,10 @@ func (m *NodeManager) WhisperService() (*whisper.Whisper, error) {
 	m.RLock()
 	defer m.RUnlock()

-	// make sure that node is fully started
-	if m.node == nil || m.nodeStarted == nil {
-		return nil, ErrNoRunningNode
+	if err := m.isNodeAvailable(); err != nil {
+		return nil, err
 	}
+
 	<-m.nodeStarted

 	if m.whisperService == nil {
@ -409,10 +413,10 @@ func (m *NodeManager) AccountManager() (*accounts.Manager, error) {
 	m.RLock()
 	defer m.RUnlock()

-	// make sure that node is fully started
-	if m.node == nil || m.nodeStarted == nil {
-		return nil, ErrNoRunningNode
+	if err := m.isNodeAvailable(); err != nil {
+		return nil, err
 	}
+
 	<-m.nodeStarted

 	accountManager := m.node.AccountManager()
@ -428,10 +432,10 @@ func (m *NodeManager) AccountKeyStore() (*keystore.KeyStore, error) {
 	m.RLock()
 	defer m.RUnlock()

-	// make sure that node is fully started
-	if m.node == nil || m.nodeStarted == nil {
-		return nil, ErrNoRunningNode
+	if err := m.isNodeAvailable(); err != nil {
+		return nil, err
 	}
+
 	<-m.nodeStarted

 	accountManager := m.node.AccountManager()
@ -457,9 +461,8 @@ func (m *NodeManager) RPCLocalClient() (*rpc.Client, error) {
 	m.Lock()
 	defer m.Unlock()

-	// make sure that node is fully started
-	if m.node == nil || m.nodeStarted == nil {
-		return nil, ErrNoRunningNode
+	if err := m.isNodeAvailable(); err != nil {
+		return nil, err
 	}

 	<-m.nodeStarted
@ -517,10 +520,10 @@ func (m *NodeManager) RPCServer() (*rpc.Server, error) {
 	m.RLock()
 	defer m.RUnlock()

-	// make sure that node is fully started
-	if m.node == nil || m.nodeStarted == nil {
-		return nil, ErrNoRunningNode
+	if err := m.isNodeAvailable(); err != nil {
+		return nil, err
 	}
+
 	<-m.nodeStarted

 	if m.rpcServer == nil {
@ -551,3 +554,12 @@ func (m *NodeManager) initLog(config *params.NodeConfig) {
 		}
 	}
 }
+
+// isNodeAvailable check if we have a node running and make sure is fully started
+func (m *NodeManager) isNodeAvailable() error {
+	if m.nodeStarted == nil || m.node == nil {
+		return ErrNoRunningNode
+	}
+
+	return nil
+}
--- a/geth/node/manager_test.go
+++ b/geth/node/manager_test.go
@ -37,124 +37,12 @@ func (s *ManagerTestSuite) SetupTest() {
 func (s *ManagerTestSuite) TestReferences() {
 	s.Require().NotNil(s.NodeManager)

-	var nilNodeManager *node.NodeManager
-
 	// test for nil values of nodeManager
 	var noNodeTests = []struct {
 		name        string
 		initFn      func() (interface{}, error)
 		expectedErr error
 	}{
-		{
-			"null manager, StartNode()",
-			func() (interface{}, error) {
-				return nilNodeManager.StartNode(nil)
-			},
-			node.ErrInvalidNodeManager,
-		},
-		{
-			"null manager, StopNode()",
-			func() (interface{}, error) {
-				return nilNodeManager.StopNode()
-			},
-			node.ErrInvalidNodeManager,
-		},
-		{
-			"null manager, RestartNode()",
-			func() (interface{}, error) {
-				return nilNodeManager.RestartNode()
-			},
-			node.ErrInvalidNodeManager,
-		},
-		{
-			"null manager, ResetChainData()",
-			func() (interface{}, error) {
-				return nilNodeManager.ResetChainData()
-			},
-			node.ErrInvalidNodeManager,
-		},
-		{
-			"null manager, IsNodeRunning()",
-			func() (interface{}, error) {
-				result := nilNodeManager.IsNodeRunning()
-				var err error
-				if !result {
-					err = node.ErrInvalidNodeManager
-				}
-				return nil, err
-			},
-			node.ErrInvalidNodeManager,
-		},
-		{
-			"null manager, PopulateStaticPeers()",
-			func() (interface{}, error) {
-				return nil, nilNodeManager.PopulateStaticPeers()
-			},
-			node.ErrInvalidNodeManager,
-		},
-		{
-			"null manager, AddPeer()",
-			func() (interface{}, error) {
-				return nil, nilNodeManager.AddPeer("enode://da3bf389a031f33fb55c9f5f54fde8473912402d27fffaa50efd74c0d0515f3a61daf6d52151f2876b19c15828e6f670352bff432b5ec457652e74755e8c864f@51.15.62.116:30303")
-			},
-			node.ErrInvalidNodeManager,
-		},
-		{
-			"null manager, get NodeConfig",
-			func() (interface{}, error) {
-				return nilNodeManager.NodeConfig()
-			},
-			node.ErrInvalidNodeManager,
-		},
-		{
-			"null manager, get Node",
-			func() (interface{}, error) {
-				return nilNodeManager.Node()
-			},
-			node.ErrInvalidNodeManager,
-		},
-		{
-			"null manager, get LES",
-			func() (interface{}, error) {
-				return nilNodeManager.LightEthereumService()
-			},
-			node.ErrInvalidNodeManager,
-		},
-		{
-			"null manager, get Whisper",
-			func() (interface{}, error) {
-				return nilNodeManager.WhisperService()
-			},
-			node.ErrInvalidNodeManager,
-		},
-		{
-			"null manager, get AccountManager",
-			func() (interface{}, error) {
-				return nilNodeManager.AccountManager()
-			},
-			node.ErrInvalidNodeManager,
-		},
-		{
-			"null manager, get AccountKeyStore",
-			func() (interface{}, error) {
-				return nilNodeManager.AccountKeyStore()
-			},
-			node.ErrInvalidNodeManager,
-		},
-		{
-			"null manager, get RPC Client",
-			func() (interface{}, error) {
-				return nilNodeManager.RPCClient()
-			},
-			node.ErrInvalidNodeManager,
-		},
-		{
-			"null manager, get RPC Server",
-			func() (interface{}, error) {
-				return nilNodeManager.RPCServer()
-			},
-			node.ErrInvalidNodeManager,
-		},
 		{
 			"non-null manager, no running node, RestartNode()",
 			func() (interface{}, error) {