Skip to content

Commit

Permalink
[FAB-11863] Remove redudant return value in GetChain
Browse files Browse the repository at this point in the history
Change-Id: I1f52050868f9594e5fc4f624f076cfbbf137c07c
Signed-off-by: Hui Kang <kangh@us.ibm.com>
  • Loading branch information
Hui Kang committed Oct 10, 2018
1 parent 3564cac commit 2e1118b
Show file tree
Hide file tree
Showing 12 changed files with 46 additions and 61 deletions.
6 changes: 3 additions & 3 deletions common/deliver/deliver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions common/deliver/deliver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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() {
Expand Down
21 changes: 8 additions & 13 deletions common/deliver/mock/chain_manager.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions core/peer/deliverevents_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions core/peer/peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions core/peer/peer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
11 changes: 5 additions & 6 deletions orderer/common/multichannel/registrar.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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 {
Expand Down
17 changes: 8 additions & 9 deletions orderer/common/multichannel/registrar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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++ {
Expand Down Expand Up @@ -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() {
Expand All @@ -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")
}

Expand Down
6 changes: 3 additions & 3 deletions orderer/common/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions orderer/consensus/etcdraft/consenter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions orderer/consensus/etcdraft/consenter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 3 additions & 9 deletions orderer/consensus/etcdraft/mocks/chain_getter.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 2e1118b

Please sign in to comment.