diff --git a/common/deliver/deliver.go b/common/deliver/deliver.go index 54cb481c0a57b5df2fde7a8850f6384b16a6c5ca..5d6229ce7a49958b88743aca31ffafb03ce42991 100644 --- a/common/deliver/deliver.go +++ b/common/deliver/deliver.go @@ -31,7 +31,7 @@ var logger = flogging.MustGetLogger("common/deliver") // ChainManager provides a way for the Handler to look up the Chain. type ChainManager interface { - GetChain(chainID string) (Chain, bool) + GetChain(chainID string) Chain } //go:generate counterfeiter -o mock/chain.go -fake-name Chain . Chain @@ -183,8 +183,8 @@ func (h *Handler) deliverBlocks(ctx context.Context, srv *Server, envelope *cb.E return srv.SendStatusResponse(cb.Status_BAD_REQUEST) } - chain, ok := h.ChainManager.GetChain(chdr.ChannelId) - if !ok { + chain := h.ChainManager.GetChain(chdr.ChannelId) + if chain == nil { // Note, we log this at DEBUG because SDKs will poll waiting for channels to be created // So we would expect our log to be somewhat flooded with these logger.Debugf("Rejecting deliver for %s because channel %s not found", addr, chdr.ChannelId) diff --git a/common/deliver/deliver_test.go b/common/deliver/deliver_test.go index d63bb624497837078c4162961b66b71d32b41142..414c8d95eea229f47771ba10c1b7ea5f8de0f696 100644 --- a/common/deliver/deliver_test.go +++ b/common/deliver/deliver_test.go @@ -120,7 +120,7 @@ var _ = Describe("Deliver", func() { fakeChain.ReaderReturns(fakeBlockReader) fakeChainManager = &mock.ChainManager{} - fakeChainManager.GetChainReturns(fakeChain, true) + fakeChainManager.GetChainReturns(fakeChain) fakePolicyChecker = &mock.PolicyChecker{} fakeReceiver = &mock.Receiver{} @@ -453,7 +453,7 @@ var _ = Describe("Deliver", func() { Context("when the channel is not found", func() { BeforeEach(func() { - fakeChainManager.GetChainReturns(nil, false) + fakeChainManager.GetChainReturns(nil) }) It("sends status not found", func() { diff --git a/common/deliver/mock/chain_manager.go b/common/deliver/mock/chain_manager.go index 7debb59e7ff1d35dd47467951b7b3b55ba4d8edf..750a1f0dcacfb701328f29f25bb0d7eee58487fc 100644 --- a/common/deliver/mock/chain_manager.go +++ b/common/deliver/mock/chain_manager.go @@ -8,24 +8,22 @@ import ( ) type ChainManager struct { - GetChainStub func(chainID string) (deliver.Chain, bool) + GetChainStub func(chainID string) deliver.Chain getChainMutex sync.RWMutex getChainArgsForCall []struct { chainID string } getChainReturns struct { result1 deliver.Chain - result2 bool } getChainReturnsOnCall map[int]struct { result1 deliver.Chain - result2 bool } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } -func (fake *ChainManager) GetChain(chainID string) (deliver.Chain, bool) { +func (fake *ChainManager) GetChain(chainID string) deliver.Chain { fake.getChainMutex.Lock() ret, specificReturn := fake.getChainReturnsOnCall[len(fake.getChainArgsForCall)] fake.getChainArgsForCall = append(fake.getChainArgsForCall, struct { @@ -37,9 +35,9 @@ func (fake *ChainManager) GetChain(chainID string) (deliver.Chain, bool) { return fake.GetChainStub(chainID) } if specificReturn { - return ret.result1, ret.result2 + return ret.result1 } - return fake.getChainReturns.result1, fake.getChainReturns.result2 + return fake.getChainReturns.result1 } func (fake *ChainManager) GetChainCallCount() int { @@ -54,26 +52,23 @@ func (fake *ChainManager) GetChainArgsForCall(i int) string { return fake.getChainArgsForCall[i].chainID } -func (fake *ChainManager) GetChainReturns(result1 deliver.Chain, result2 bool) { +func (fake *ChainManager) GetChainReturns(result1 deliver.Chain) { fake.GetChainStub = nil fake.getChainReturns = struct { result1 deliver.Chain - result2 bool - }{result1, result2} + }{result1} } -func (fake *ChainManager) GetChainReturnsOnCall(i int, result1 deliver.Chain, result2 bool) { +func (fake *ChainManager) GetChainReturnsOnCall(i int, result1 deliver.Chain) { fake.GetChainStub = nil if fake.getChainReturnsOnCall == nil { fake.getChainReturnsOnCall = make(map[int]struct { result1 deliver.Chain - result2 bool }) } fake.getChainReturnsOnCall[i] = struct { result1 deliver.Chain - result2 bool - }{result1, result2} + }{result1} } func (fake *ChainManager) Invocations() map[string][][]interface{} { diff --git a/core/peer/deliverevents_test.go b/core/peer/deliverevents_test.go index cf20dfdf9641e190c60eef4b371f2006bb1e06ae..5efd9fe9b54ad1229fac923caf475b5d4698fed9 100644 --- a/core/peer/deliverevents_test.go +++ b/core/peer/deliverevents_test.go @@ -98,9 +98,9 @@ type mockChainManager struct { mock.Mock } -func (m *mockChainManager) GetChain(chainID string) (deliver.Chain, bool) { +func (m *mockChainManager) GetChain(chainID string) deliver.Chain { args := m.Called(chainID) - return args.Get(0).(deliver.Chain), args.Get(1).(bool) + return args.Get(0).(deliver.Chain) } // mockDeliverServer mock implementation of the Deliver_DeliverServer diff --git a/core/peer/peer.go b/core/peer/peer.go index e2e28c8f87f8a436db9efb857d8e33cf3c3615fe..76092a8fdf49cb834fd605ac8ab1ae340c73def9 100644 --- a/core/peer/peer.go +++ b/core/peer/peer.go @@ -713,12 +713,12 @@ func (*collectionSupport) GetIdentityDeserializer(chainID string) msp.IdentityDe type DeliverChainManager struct { } -func (DeliverChainManager) GetChain(chainID string) (deliver.Chain, bool) { +func (DeliverChainManager) GetChain(chainID string) deliver.Chain { channel, ok := chains.list[chainID] if !ok { - return nil, ok + return nil } - return channel.cs, ok + return channel.cs } // fileLedgerBlockStore implements the interface expected by diff --git a/core/peer/peer_test.go b/core/peer/peer_test.go index fc38b960930fa93c88339ec939d30b02d66d0f4a..cadd4b9d08ea18c2185ec35608bede2cc8109651 100644 --- a/core/peer/peer_test.go +++ b/core/peer/peer_test.go @@ -208,12 +208,10 @@ func TestDeliverSupportManager(t *testing.T) { MockInitialize() manager := &DeliverChainManager{} - chainSupport, ok := manager.GetChain("fake") + chainSupport := manager.GetChain("fake") assert.Nil(t, chainSupport, "chain support should be nil") - assert.False(t, ok, "Should not find fake channel") MockCreateChain("testchain") - chainSupport, ok = manager.GetChain("testchain") + chainSupport = manager.GetChain("testchain") assert.NotNil(t, chainSupport, "chain support should not be nil") - assert.True(t, ok, "Should find testchain channel") } diff --git a/orderer/common/multichannel/registrar.go b/orderer/common/multichannel/registrar.go index 85a686c824c9a019e8e5494910337eae88b87c24..98f155502c6753424365b29ed73af4f81856f34f 100644 --- a/orderer/common/multichannel/registrar.go +++ b/orderer/common/multichannel/registrar.go @@ -206,8 +206,8 @@ func (r *Registrar) BroadcastChannelSupport(msg *cb.Envelope) (*cb.ChannelHeader return nil, false, nil, fmt.Errorf("could not determine channel ID: %s", err) } - cs, ok := r.GetChain(chdr.ChannelId) - if !ok { + cs := r.GetChain(chdr.ChannelId) + if cs == nil { cs = r.systemChannel } @@ -223,13 +223,12 @@ func (r *Registrar) BroadcastChannelSupport(msg *cb.Envelope) (*cb.ChannelHeader return chdr, isConfig, cs, nil } -// GetChain retrieves the chain support for a chain (and whether it exists) -func (r *Registrar) GetChain(chainID string) (*ChainSupport, bool) { +// GetChain retrieves the chain support for a chain if it exists +func (r *Registrar) GetChain(chainID string) *ChainSupport { r.lock.RLock() defer r.lock.RUnlock() - cs, ok := r.chains[chainID] - return cs, ok + return r.chains[chainID] } func (r *Registrar) newLedgerResources(configTx *cb.Envelope) *ledgerResources { diff --git a/orderer/common/multichannel/registrar_test.go b/orderer/common/multichannel/registrar_test.go index a660e8342c37e565d820d9dc7ae62a8865fbbeb7..0e2812c49a8510d90d63761dfb06966c30745ff5 100644 --- a/orderer/common/multichannel/registrar_test.go +++ b/orderer/common/multichannel/registrar_test.go @@ -144,11 +144,11 @@ func TestManagerImpl(t *testing.T) { manager := NewRegistrar(lf, mockCrypto()) manager.Initialize(consenters) - _, ok := manager.GetChain("Fake") - assert.False(t, ok, "Should not have found a chain that was not created") + chainSupport := manager.GetChain("Fake") + assert.Nilf(t, chainSupport, "Should not have found a chain that was not created") - chainSupport, ok := manager.GetChain(genesisconfig.TestChainID) - assert.True(t, ok, "Should have gotten chain which was initialized by ramledger") + chainSupport = manager.GetChain(genesisconfig.TestChainID) + assert.NotNilf(t, chainSupport, "Should have gotten chain which was initialized by ramledger") messages := make([]*cb.Envelope, conf.Orderer.BatchSize.MaxMessageCount) for i := 0; i < int(conf.Orderer.BatchSize.MaxMessageCount); i++ { @@ -198,8 +198,8 @@ func TestNewChain(t *testing.T) { wrapped := wrapConfigTx(ingressTx) - chainSupport, ok := manager.GetChain(manager.SystemChannelID()) - assert.True(t, ok, "Could not find system channel") + chainSupport := manager.GetChain(manager.SystemChannelID()) + assert.NotNilf(t, chainSupport, "Could not find system channel") chainSupport.Configure(wrapped, 0) func() { @@ -216,9 +216,8 @@ func TestNewChain(t *testing.T) { assert.True(t, proto.Equal(wrapped, utils.UnmarshalEnvelopeOrPanic(block.Data.Data[0])), "Orderer config block contains wrong transaction") }() - chainSupport, ok = manager.GetChain(newChainID) - - if !ok { + chainSupport = manager.GetChain(newChainID) + if chainSupport == nil { t.Fatalf("Should have gotten new chain which was created") } diff --git a/orderer/common/server/server.go b/orderer/common/server/server.go index 94d913ed9407d67a31c66c777ae4b12d4bda2c1a..6e74e19c982eae3a93578ccb1ca106165e2e5334 100644 --- a/orderer/common/server/server.go +++ b/orderer/common/server/server.go @@ -38,7 +38,7 @@ type deliverSupport struct { *multichannel.Registrar } -func (ds deliverSupport) GetChain(chainID string) (deliver.Chain, bool) { +func (ds deliverSupport) GetChain(chainID string) deliver.Chain { return ds.Registrar.GetChain(chainID) } @@ -159,8 +159,8 @@ func (s *server) Deliver(srv ab.AtomicBroadcast_DeliverServer) error { }() policyChecker := func(env *cb.Envelope, channelID string) error { - chain, ok := s.GetChain(channelID) - if !ok { + chain := s.GetChain(channelID) + if chain == nil { return errors.Errorf("channel %s not found", channelID) } sf := msgprocessor.NewSigFilter(policies.ChannelReaders, chain) diff --git a/orderer/consensus/etcdraft/consenter.go b/orderer/consensus/etcdraft/consenter.go index 1dd0e6052c184b3b6a80758a11e1f2c7597fe660..7a69f7774004de2a9324f7344326ed74c34e552c 100644 --- a/orderer/consensus/etcdraft/consenter.go +++ b/orderer/consensus/etcdraft/consenter.go @@ -33,7 +33,7 @@ type ChainGetter interface { // GetChain obtains the ChainSupport for the given channel. // Returns nil, false when the ChainSupport for the given channel // isn't found. - GetChain(chainID string) (*multichannel.ChainSupport, bool) + GetChain(chainID string) *multichannel.ChainSupport } // Consenter implements etddraft consenter @@ -61,8 +61,8 @@ func (c *Consenter) TargetChannel(message proto.Message) string { // ReceiverByChain returns the MessageReceiver for the given channelID or nil // if not found. func (c *Consenter) ReceiverByChain(channelID string) MessageReceiver { - cs, exists := c.Chains.GetChain(channelID) - if !exists { + cs := c.Chains.GetChain(channelID) + if cs == nil { return nil } if cs.Chain == nil { diff --git a/orderer/consensus/etcdraft/consenter_test.go b/orderer/consensus/etcdraft/consenter_test.go index 2f85c863011ce17e8370e49ee24ea553735e7203..2b56fc76df92b0b4993ee4d9fc6fcf659a6a7bf5 100644 --- a/orderer/consensus/etcdraft/consenter_test.go +++ b/orderer/consensus/etcdraft/consenter_test.go @@ -58,12 +58,12 @@ var _ = Describe("Consenter", func() { Chain: chainInstance, } BeforeEach(func() { - chainGetter.On("GetChain", "mychannel").Return(cs, true) - chainGetter.On("GetChain", "badChainObject").Return(&multichannel.ChainSupport{}, true) - chainGetter.On("GetChain", "notmychannel").Return(nil, false) + chainGetter.On("GetChain", "mychannel").Return(cs) + chainGetter.On("GetChain", "badChainObject").Return(&multichannel.ChainSupport{}) + chainGetter.On("GetChain", "notmychannel").Return(nil) chainGetter.On("GetChain", "notraftchain").Return(&multichannel.ChainSupport{ Chain: &multichannel.ChainSupport{}, - }, true) + }) }) It("calls the chain getter and returns the reference when it is found", func() { consenter := newConsenter(chainGetter) diff --git a/orderer/consensus/etcdraft/mocks/chain_getter.go b/orderer/consensus/etcdraft/mocks/chain_getter.go index 974ed7e93dcb960a7dc50659a407036df31bf822..d49e11e0e515de5c880629fc0242cc06562eae45 100644 --- a/orderer/consensus/etcdraft/mocks/chain_getter.go +++ b/orderer/consensus/etcdraft/mocks/chain_getter.go @@ -1,4 +1,5 @@ // Code generated by mockery v1.0.0. DO NOT EDIT. + package mocks import mock "github.com/stretchr/testify/mock" @@ -10,7 +11,7 @@ type ChainGetter struct { } // GetChain provides a mock function with given fields: chainID -func (_m *ChainGetter) GetChain(chainID string) (*multichannel.ChainSupport, bool) { +func (_m *ChainGetter) GetChain(chainID string) *multichannel.ChainSupport { ret := _m.Called(chainID) var r0 *multichannel.ChainSupport @@ -22,12 +23,5 @@ func (_m *ChainGetter) GetChain(chainID string) (*multichannel.ChainSupport, boo } } - var r1 bool - if rf, ok := ret.Get(1).(func(string) bool); ok { - r1 = rf(chainID) - } else { - r1 = ret.Get(1).(bool) - } - - return r0, r1 + return r0 }