From 08df4e33b431207e6a92c8ceeec6fa11aa48a5ee Mon Sep 17 00:00:00 2001 From: YACOVM Date: Sat, 29 Apr 2017 21:31:30 +0300 Subject: [PATCH] [FAB-3520] Eventer doesn't trigger subsequent updates The eventer applies the config on an object that is returned from an invocation of config.Organizations(), which returns the same reference of map in each config update. The config.Organizations() is a map from org name to config.ApplicationOrg which has the anchor peers. Due to the fact that the references are the same, the ce.lastConfig.orgMap is the same as the new orgMap of a new config update, and this makes the logic think that there has not been any update of anchor peers at every real update (of anchor peers). I fixed the bug by cloning the map, and also fixed the test in eventer_test.go(line 98) to fail when the code of the eventer.go is before the fix. Change-Id: Iede772975216a88d00badfc9d7092f92580737a8 Signed-off-by: Yacov Manevich --- gossip/service/eventer.go | 36 +++++++++++++-- gossip/service/eventer_test.go | 65 +++++++++------------------ gossip/service/gossip_service_test.go | 5 ++- 3 files changed, 57 insertions(+), 49 deletions(-) diff --git a/gossip/service/eventer.go b/gossip/service/eventer.go index 2df961907ca..403d42db05c 100644 --- a/gossip/service/eventer.go +++ b/gossip/service/eventer.go @@ -68,8 +68,8 @@ func newConfigEventer(receiver configEventReceiver) *configEventer { // Note, that a changing sequence number is ignored as changing configuration func (ce *configEventer) ProcessConfigUpdate(config Config) { logger.Debugf("Processing new config for channel %s", config.ChainID()) - - if ce.lastConfig != nil && reflect.DeepEqual(ce.lastConfig.orgMap, config.Organizations()) { + orgMap := cloneOrgConfig(config.Organizations()) + if ce.lastConfig != nil && reflect.DeepEqual(ce.lastConfig.orgMap, orgMap) { logger.Debugf("Ignoring new config for channel %s because it contained no anchor peer updates", config.ChainID()) return } @@ -80,7 +80,7 @@ func (ce *configEventer) ProcessConfigUpdate(config Config) { } newConfig := &configStore{ - orgMap: config.Organizations(), + orgMap: orgMap, anchorPeers: newAnchorPeers, } ce.lastConfig = newConfig @@ -88,3 +88,33 @@ func (ce *configEventer) ProcessConfigUpdate(config Config) { logger.Debugf("Calling out because config was updated for channel %s", config.ChainID()) ce.receiver.configUpdated(config) } + +func cloneOrgConfig(src map[string]config.ApplicationOrg) map[string]config.ApplicationOrg { + clone := make(map[string]config.ApplicationOrg) + for k, v := range src { + clone[k] = &appGrp{ + name: v.Name(), + mspID: v.MSPID(), + anchorPeers: v.AnchorPeers(), + } + } + return clone +} + +type appGrp struct { + name string + mspID string + anchorPeers []*peer.AnchorPeer +} + +func (ag *appGrp) Name() string { + return ag.name +} + +func (ag *appGrp) MSPID() string { + return ag.mspID +} + +func (ag *appGrp) AnchorPeers() []*peer.AnchorPeer { + return ag.anchorPeers +} diff --git a/gossip/service/eventer_test.go b/gossip/service/eventer_test.go index 57e1a9db489..de5f8c5c936 100644 --- a/gossip/service/eventer_test.go +++ b/gossip/service/eventer_test.go @@ -27,24 +27,10 @@ import ( const testChainID = "foo" -type applicationOrgs []*peer.AnchorPeer - func init() { util.SetupTestLogging() } -func (ao applicationOrgs) AnchorPeers() []*peer.AnchorPeer { - return ao -} - -func (ao applicationOrgs) MSPID() string { - return "ORG1" -} - -func (ao applicationOrgs) Name() string { - panic("Unimplimented") -} - type mockReceiver struct { orgs map[string]config.ApplicationOrg sequence uint64 @@ -76,11 +62,9 @@ func TestInitialUpdate(t *testing.T) { mc := &mockConfig{ sequence: 7, orgs: map[string]config.ApplicationOrg{ - testOrgID: applicationOrgs([]*peer.AnchorPeer{ - &peer.AnchorPeer{ - Port: 9, - }, - }), + testOrgID: &appGrp{ + anchorPeers: []*peer.AnchorPeer{{Port: 9}}, + }, }, } @@ -95,15 +79,14 @@ func TestInitialUpdate(t *testing.T) { } func TestSecondUpdate(t *testing.T) { + appGrps := map[string]config.ApplicationOrg{ + testOrgID: &appGrp{ + anchorPeers: []*peer.AnchorPeer{{Port: 9}}, + }, + } mc := &mockConfig{ sequence: 7, - orgs: map[string]config.ApplicationOrg{ - testOrgID: applicationOrgs([]*peer.AnchorPeer{ - &peer.AnchorPeer{ - Port: 9, - }, - }), - }, + orgs: appGrps, } mr := &mockReceiver{} @@ -112,18 +95,14 @@ func TestSecondUpdate(t *testing.T) { ce.ProcessConfigUpdate(mc) mc.sequence = 8 - mc.orgs = map[string]config.ApplicationOrg{ - testOrgID: applicationOrgs([]*peer.AnchorPeer{ - &peer.AnchorPeer{ - Port: 10, - }, - }), + appGrps[testOrgID] = &appGrp{ + anchorPeers: []*peer.AnchorPeer{{Port: 10}}, } ce.ProcessConfigUpdate(mc) if !reflect.DeepEqual(mc, (*mockConfig)(mr)) { - t.Fatalf("Should have updated config on initial update but did not") + t.Fatal("Should have updated config on initial update but did not") } } @@ -131,11 +110,9 @@ func TestSecondSameUpdate(t *testing.T) { mc := &mockConfig{ sequence: 7, orgs: map[string]config.ApplicationOrg{ - testOrgID: applicationOrgs([]*peer.AnchorPeer{ - &peer.AnchorPeer{ - Port: 9, - }, - }), + testOrgID: &appGrp{ + anchorPeers: []*peer.AnchorPeer{{Port: 9}}, + }, }, } @@ -148,11 +125,11 @@ func TestSecondSameUpdate(t *testing.T) { ce.ProcessConfigUpdate(mc) if mr.sequence != 0 { - t.Errorf("Should not have updated sequence when reprocessing same config") + t.Error("Should not have updated sequence when reprocessing same config") } if mr.orgs != nil { - t.Errorf("Should not have updated anchor peers when reprocessing same config") + t.Error("Should not have updated anchor peers when reprocessing same config") } } @@ -160,11 +137,9 @@ func TestUpdatedSeqOnly(t *testing.T) { mc := &mockConfig{ sequence: 7, orgs: map[string]config.ApplicationOrg{ - testOrgID: applicationOrgs([]*peer.AnchorPeer{ - &peer.AnchorPeer{ - Port: 9, - }, - }), + testOrgID: &appGrp{ + anchorPeers: []*peer.AnchorPeer{{Port: 9}}, + }, }, } diff --git a/gossip/service/gossip_service_test.go b/gossip/service/gossip_service_test.go index 4b5b18e03ac..798346358cc 100644 --- a/gossip/service/gossip_service_test.go +++ b/gossip/service/gossip_service_test.go @@ -740,7 +740,10 @@ func TestChannelConfig(t *testing.T) { mc := &mockConfig{ sequence: 1, orgs: map[string]config.ApplicationOrg{ - testOrgID: applicationOrgs([]*peer.AnchorPeer{}), + string(orgInChannelA): &appGrp{ + mspID: string(orgInChannelA), + anchorPeers: []*peer.AnchorPeer{}, + }, }, } gService.JoinChan(jcm, gossipCommon.ChainID("A"))