diff --git a/nodeconf/service.go b/nodeconf/service.go index b42b7b0c..a625c1dd 100644 --- a/nodeconf/service.go +++ b/nodeconf/service.go @@ -9,6 +9,7 @@ import ( "github.com/anyproto/go-chash" "go.uber.org/zap" + "fmt" commonaccount "github.com/anyproto/any-sync/accountservice" "github.com/anyproto/any-sync/app" "github.com/anyproto/any-sync/app/logger" @@ -63,41 +64,59 @@ type service struct { networkProtoVersionChecker NetworkProtoVersionChecker } -func mergedTreeNodes(configNodes []Node, lastStoredNodes []Node) (mergedAddrs []string) { - treeNodes := make(map[string]int) - mustRewriteNods := false +// Merges nodes in app config coordinator nodes with nodes from file (lastStored). +// This is important to avoid the situation when locally stored configuration +// has obsolete coordinator nodes so client can't fetch up-to-date connection info +// (i.e. treeNodes) +func mergeCoordinatorAddrs(appConfig Configuration, lastStored Configuration) (mustRewriteLocalConfig bool) { + mustRewriteLocalConfig = false - for _, node := range lastStoredNodes { + appNodesByPeer := make(map[string]*Node) + for i, node := range appConfig.Nodes { if node.HasType(NodeTypeCoordinator) { - for _, addr := range node.Addresses { - treeNodes[addr] = 1 - } + appNodesByPeer[node.PeerId] = &appConfig.Nodes[i] } } - for _, node := range configNodes { + storedNodesByPeer := make(map[string]*Node) + for i, node := range lastStored.Nodes { if node.HasType(NodeTypeCoordinator) { - for _, addr := range node.Addresses { - if _, found := treeNodes[addr]; !found { - mustRewriteNods = true + storedNodesByPeer[node.PeerId] = &lastStored.Nodes[i] + } + } + for appPeerId, appNode := range appNodesByPeer { + if storedNode, found := storedNodesByPeer[appPeerId]; found { + // merge addresses: add missing from app config to stored + storedAddrs := make(map[string]bool) + for _, addr := range storedNode.Addresses { + storedAddrs[addr] = true + } + + for _, appAddr := range appNode.Addresses { + // assumming appNode.Addresses has no duplicates + if _, found := storedAddrs[appAddr]; !found { + + mustRewriteLocalConfig = true + storedNode.Addresses = append(storedNode.Addresses, appAddr) + fmt.Printf("%#v\n", storedNode) } - treeNodes[addr] += 1 } + + } else { + // append a whole node to stored config + mustRewriteLocalConfig = true + newNode := Node{} + newNode.Addresses = make([]string, len(appNode.Addresses)) + copy(newNode.Addresses, appNode.Addresses) + lastStored.Nodes = append(lastStored.Nodes, newNode) } } + fmt.Printf("%#v\n", lastStored.Nodes[0]) - if mustRewriteNods { - mergedAddrs = make([]string, 0) - for addr, _ := range treeNodes { - mergedAddrs = append(mergedAddrs, addr) - } - - return - } - - return nil + return } + func (s *service) Init(a *app.App) (err error) { s.config = a.MustComponent("config").(ConfigGetter).GetNodeConf() s.accountId = a.MustComponent(commonaccount.CName).(commonaccount.Service).Account().PeerId @@ -109,14 +128,7 @@ func (s *service) Init(a *app.App) (err error) { err = nil } - if mergedAddrs := mergedTreeNodes(s.config.Nodes, lastStored.Nodes); mergedAddrs != nil { - for _, node := range lastStored.Nodes { - if node.HasType(NodeTypeCoordinator) { - node.Addresses = mergedAddrs - break // can we break here? node structure in yaml allows to have multiple sections with the same nodeType - } - } - } + mergeCoordinatorAddrs(s.config, lastStored) var updatePeriodSec = 600 if confUpd, ok := a.MustComponent("config").(ConfigUpdateGetter); ok && confUpd.GetNodeConfUpdateInterval() > 0 { diff --git a/nodeconf/service_test.go b/nodeconf/service_test.go index 35f5f5d8..8bcec9a2 100644 --- a/nodeconf/service_test.go +++ b/nodeconf/service_test.go @@ -217,3 +217,79 @@ func newTestConf() *testConf { }, } } + +func TestService_mergeCoordinatorAddrs(t *testing.T) { + confApp := Configuration{ + Id: "test", + NetworkId: "testNetwork", + Nodes: []Node{ + { + PeerId: "12D3KooWKLCajM89S8unbt3tgGbRLgmiWnFZT3adn9A5pQciBSLa", + Addresses: []string{"127.0.0.1:4830", "192.168.1.1:8833"}, + Types: []NodeType{NodeTypeCoordinator}, + }, + { + PeerId: "12D3KooWKnXTtbveMDUFfeSqR5dt9a4JW66tZQXG7C7PdDh3vqGu", + Addresses: []string{"127.0.0.1:4730"}, + Types: []NodeType{NodeTypeTree}, + }, + { + PeerId: "12D3KooWKgVN2kW8xw5Uvm2sLUnkeUNQYAvcWvF58maTzev7FjPi", + Addresses: []string{"127.0.0.1:4731"}, + Types: []NodeType{NodeTypeTree}, + }, + { + PeerId: "12D3KooWCUPYuMnQhu9yREJgQyjcz8zWY83rZGmDLwb9YR6QkbZX", + Addresses: []string{"127.0.0.1:4732"}, + Types: []NodeType{NodeTypeTree}, + }, + { + PeerId: "12D3KooWQxiZ5a7vcy4DTJa8Gy1eVUmwb5ojN4SrJC9Rjxzigw6C", + Addresses: []string{"127.0.0.1:4733"}, + Types: []NodeType{NodeTypeFile}, + }, + }, + CreationTime: time.Now(), + } + + confStored := Configuration{ + Id: "test", + NetworkId: "testNetwork", + Nodes: []Node{ + { + PeerId: "12D3KooWKLCajM89S8unbt3tgGbRLgmiWnFZT3adn9A5pQciBSLa", + Addresses: []string{"127.0.0.1:4830"}, + Types: []NodeType{NodeTypeCoordinator}, + }, + { + PeerId: "12D3KooWKnXTtbveMDUFfeSqR5dt9a4JW66tZQXG7C7PdDh3vqGu", + Addresses: []string{"127.0.0.1:4730"}, + Types: []NodeType{NodeTypeTree}, + }, + { + PeerId: "12D3KooWKgVN2kW8xw5Uvm2sLUnkeUNQYAvcWvF58maTzev7FjPi", + Addresses: []string{"127.0.0.1:4731"}, + Types: []NodeType{NodeTypeTree}, + }, + { + PeerId: "12D3KooWCUPYuMnQhu9yREJgQyjcz8zWY83rZGmDLwb9YR6QkbZX", + Addresses: []string{"127.0.0.1:4732"}, + Types: []NodeType{NodeTypeTree}, + }, + { + PeerId: "12D3KooWQxiZ5a7vcy4DTJa8Gy1eVUmwb5ojN4SrJC9Rjxzigw6C", + Addresses: []string{"127.0.0.1:4733"}, + Types: []NodeType{NodeTypeFile}, + }, + }, + CreationTime: time.Now(), + } + + t.Run("coorditor nodes are merged", func(t *testing.T) { + mergeCoordinatorAddrs(confApp, confStored) + assert.Equal(t, 2, len(confStored.Nodes[0].Addresses)) + assert.Equal(t, "127.0.0.1:4830", confStored.Nodes[0].Addresses[0]) + assert.Equal(t, "192.168.1.1:8833", confStored.Nodes[0].Addresses[1]) + }) + +}