Skip to content

Commit

Permalink
[FAB-5284] Remove error return value of ClassifyMsg
Browse files Browse the repository at this point in the history
This method used to take envelope, which may cause unmarshal
error. It now takes only channel header, so we could safely
remove the `error` return value, as it's always nil.

Change-Id: Id6bebb8dda5eca190a14d8cbfe7b3d92cccdde1d
Signed-off-by: Jay Guo <guojiannan1101@gmail.com>
  • Loading branch information
guoger committed Sep 7, 2017
1 parent a657db2 commit e2ab69c
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 38 deletions.
2 changes: 1 addition & 1 deletion orderer/common/broadcast/broadcast_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (ms *mockSupport) Configure(configUpdate *cb.Envelope, config *cb.Envelope,
return ms.Order(config, configSeq)
}

func (ms *mockSupport) ClassifyMsg(chdr *cb.ChannelHeader) (msgprocessor.Classification, error) {
func (ms *mockSupport) ClassifyMsg(chdr *cb.ChannelHeader) msgprocessor.Classification {
panic("UNIMPLMENTED")
}

Expand Down
2 changes: 1 addition & 1 deletion orderer/common/msgprocessor/msgprocessor.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ const (
// arrives through the Broadcast interface.
type Processor interface {
// ClassifyMsg inspects the message header to determine which type of processing is necessary
ClassifyMsg(chdr *cb.ChannelHeader) (Classification, error)
ClassifyMsg(chdr *cb.ChannelHeader) Classification

// ProcessNormalMsg will check the validity of a message based on the current configuration. It returns the current
// configuration sequence number and nil on success, or an error if the message is not valid
Expand Down
14 changes: 5 additions & 9 deletions orderer/common/msgprocessor/standardchannel.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,20 +58,16 @@ func CreateStandardChannelFilters(filterSupport channelconfig.Resources) *RuleSe
}

// ClassifyMsg inspects the message to determine which type of processing is necessary
func (s *StandardChannel) ClassifyMsg(chdr *cb.ChannelHeader) (Classification, error) {
func (s *StandardChannel) ClassifyMsg(chdr *cb.ChannelHeader) Classification {
switch chdr.Type {
case int32(cb.HeaderType_CONFIG_UPDATE):
return ConfigUpdateMsg, nil
return ConfigUpdateMsg
case int32(cb.HeaderType_ORDERER_TRANSACTION):
// XXX eventually, this should return an error, but for now to allow the old message flow, return ConfigUpdateMsg
return ConfigUpdateMsg, nil
// return 0, fmt.Errorf("Transactions of type ORDERER_TRANSACTION cannot be Broadcast")
return ConfigUpdateMsg
case int32(cb.HeaderType_CONFIG):
// XXX eventually, this should return an error, but for now to allow the old message flow, return ConfigUpdateMsg
return ConfigUpdateMsg, nil
// return 0, fmt.Errorf("Transactions of type CONFIG cannot be Broadcast")
return ConfigUpdateMsg
default:
return NormalMsg, nil
return NormalMsg
}
}

Expand Down
12 changes: 4 additions & 8 deletions orderer/common/msgprocessor/standardchannel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,24 +42,20 @@ func (ms *mockSystemChannelFilterSupport) ChainID() string {

func TestClassifyMsg(t *testing.T) {
t.Run("ConfigUpdate", func(t *testing.T) {
class, err := (&StandardChannel{}).ClassifyMsg(&cb.ChannelHeader{Type: int32(cb.HeaderType_CONFIG_UPDATE)})
class := (&StandardChannel{}).ClassifyMsg(&cb.ChannelHeader{Type: int32(cb.HeaderType_CONFIG_UPDATE)})
assert.Equal(t, class, ConfigUpdateMsg)
assert.Nil(t, err)
})
t.Run("OrdererTx", func(t *testing.T) {
class, err := (&StandardChannel{}).ClassifyMsg(&cb.ChannelHeader{Type: int32(cb.HeaderType_ORDERER_TRANSACTION)})
class := (&StandardChannel{}).ClassifyMsg(&cb.ChannelHeader{Type: int32(cb.HeaderType_ORDERER_TRANSACTION)})
assert.Equal(t, class, ConfigUpdateMsg)
assert.Nil(t, err)
})
t.Run("ConfigTx", func(t *testing.T) {
class, err := (&StandardChannel{}).ClassifyMsg(&cb.ChannelHeader{Type: int32(cb.HeaderType_CONFIG)})
class := (&StandardChannel{}).ClassifyMsg(&cb.ChannelHeader{Type: int32(cb.HeaderType_CONFIG)})
assert.Equal(t, class, ConfigUpdateMsg)
assert.Nil(t, err)
})
t.Run("EndorserTx", func(t *testing.T) {
class, err := (&StandardChannel{}).ClassifyMsg(&cb.ChannelHeader{Type: int32(cb.HeaderType_ENDORSER_TRANSACTION)})
class := (&StandardChannel{}).ClassifyMsg(&cb.ChannelHeader{Type: int32(cb.HeaderType_ENDORSER_TRANSACTION)})
assert.Equal(t, class, NormalMsg)
assert.Nil(t, err)
})
}

Expand Down
7 changes: 1 addition & 6 deletions orderer/common/multichannel/registrar.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,8 @@ func (r *Registrar) BroadcastChannelSupport(msg *cb.Envelope) (*cb.ChannelHeader
cs = r.systemChannel
}

class, err := cs.ClassifyMsg(chdr)
if err != nil {
return nil, false, nil, fmt.Errorf("could not classify message: %s", err)
}

isConfig := false
switch class {
switch cs.ClassifyMsg(chdr) {
case msgprocessor.ConfigUpdateMsg:
isConfig = true
default:
Expand Down
5 changes: 1 addition & 4 deletions orderer/common/multichannel/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,7 @@ func (mch *mockChain) Start() {
logger.Panicf("If a message has arrived to this point, it should already have had header inspected once: %s", err)
}

class, err := mch.support.ClassifyMsg(chdr)
if err != nil {
logger.Panicf("If a message has arrived to this point, it should already have been classified once: %s", err)
}
class := mch.support.ClassifyMsg(chdr)
switch class {
case msgprocessor.ConfigUpdateMsg:
batch := mch.support.BlockCutter().Cut()
Expand Down
5 changes: 1 addition & 4 deletions orderer/consensus/kafka/chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,10 +398,7 @@ func processRegular(regularMessage *ab.KafkaMessageRegular, support consensus.Co
logger.Panicf("If a message has arrived to this point, it should already have had its header inspected once")
}

class, err := support.ClassifyMsg(chdr)
if err != nil {
logger.Panicf("[channel: %s] If a message has arrived to this point, it should already have been classified once", support.ChainID())
}
class := support.ClassifyMsg(chdr)
switch class {
case msgprocessor.ConfigUpdateMsg:
_, err := support.ProcessNormalMsg(env)
Expand Down
7 changes: 2 additions & 5 deletions orderer/mocks/common/multichannel/multichannel.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@ type ConsenterSupport struct {
// ClassifyMsgVal is returned by ClassifyMsg
ClassifyMsgVal msgprocessor.Classification

// ClassifyMsgErr is the err returned by ClassifyMsg
ClassifyMsgErr error

// ConfigSeqVal is returned as the configSeq for Process*Msg
ConfigSeqVal uint64

Expand Down Expand Up @@ -120,8 +117,8 @@ func (mcs *ConsenterSupport) NewSignatureHeader() (*cb.SignatureHeader, error) {
}

// ClassifyMsg returns ClassifyMsgVal, ClassifyMsgErr
func (mcs *ConsenterSupport) ClassifyMsg(chdr *cb.ChannelHeader) (msgprocessor.Classification, error) {
return mcs.ClassifyMsgVal, mcs.ClassifyMsgErr
func (mcs *ConsenterSupport) ClassifyMsg(chdr *cb.ChannelHeader) msgprocessor.Classification {
return mcs.ClassifyMsgVal
}

// ProcessNormalMsg returns ConfigSeqVal, ProcessNormalMsgErr
Expand Down

0 comments on commit e2ab69c

Please sign in to comment.