Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small NodeManager refactoring #253

Merged
merged 11 commits into from
Sep 11, 2017
Merged

Small NodeManager refactoring #253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a517078
Added new error
MarinX Aug 14, 2017
aebef5a
Added tests for node check (online/offline)
MarinX Aug 14, 2017
c99521d
Changes to PR by request
MarinX Aug 15, 2017
f04b813
Initial space remove
MarinX Aug 15, 2017
cd45a30
Add private method for checking if node is available
MarinX Aug 28, 2017
ae7204e
Removed blank space
MarinX Aug 28, 2017
d0e0ac0
Small fixes
MarinX Aug 28, 2017
e1f78f7
fix locks and tests
adambabik Aug 30, 2017
f5c5062
add and move conditions in StopNode
adambabik Aug 31, 2017
65b7fcb
remove code testing offline
adambabik Sep 11, 2017
e093f1a
remove unnecessary tests
adambabik Sep 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/statusd/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
122 changes: 67 additions & 55 deletions geth/node/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
112 changes: 0 additions & 112 deletions geth/node/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down