Skip to content

Commit

Permalink
[FAB-14534] add type checking in tests
Browse files Browse the repository at this point in the history
By declaring unnamed variables of the expected interface, we can ensure
we've implemented all of the required methods of the intended protolator
interface. This activity did catch one messing method on
DynamicConsortiumConfigValue.

Also cleaned up copy pasta in godoc for the protolator interfaces and
switched from assert.Equal to gomega MatchJSON to improve the data
reported during test failures and reduce the likelihood of a test
failure due to changing the order of fields in the JSON.

Change-Id: I404e7cfb05963f50a3053c11e80ea085e3561b00
Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com>
  • Loading branch information
sykesm committed Mar 8, 2019
1 parent ba63949 commit 47f22ea
Show file tree
Hide file tree
Showing 10 changed files with 189 additions and 31 deletions.
32 changes: 11 additions & 21 deletions common/tools/protolator/api.go
Original file line number Diff line number Diff line change
@@ -1,17 +1,7 @@
/*
Copyright IBM Corp. 2017 All Rights Reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
SPDX-License-Identifier: Apache-2.0
*/

package protolator
Expand Down Expand Up @@ -63,21 +53,21 @@ type StaticallyOpaqueFieldProto interface {
// StaticallyOpaqueMapFieldProto should be implemented by protos which have maps to bytes fields
// which are the marshaled value of a fixed type
type StaticallyOpaqueMapFieldProto interface {
// StaticallyOpaqueFields returns the field names which contain opaque data
// StaticallyOpaqueMapFields returns the field names which contain opaque data
StaticallyOpaqueMapFields() []string

// StaticallyOpaqueFieldProto returns a newly allocated proto message of the correct
// StaticallyOpaqueMapFieldProto returns a newly allocated proto message of the correct
// type for the field name.
StaticallyOpaqueMapFieldProto(name string, key string) (proto.Message, error)
}

// StaticallyOpaqueSliceFieldProto should be implemented by protos which have maps to bytes fields
// which are the marshaled value of a fixed type
type StaticallyOpaqueSliceFieldProto interface {
// StaticallyOpaqueFields returns the field names which contain opaque data
// StaticallyOpaqueSliceFields returns the field names which contain opaque data
StaticallyOpaqueSliceFields() []string

// StaticallyOpaqueFieldProto returns a newly allocated proto message of the correct
// StaticallyOpaqueSliceFieldProto returns a newly allocated proto message of the correct
// type for the field name.
StaticallyOpaqueSliceFieldProto(name string, index int) (proto.Message, error)
}
Expand All @@ -96,18 +86,18 @@ type VariablyOpaqueFieldProto interface {
// VariablyOpaqueMapFieldProto should be implemented by protos which have maps to bytes fields
// which are the marshaled value of a a message type determined by the other contents of the proto
type VariablyOpaqueMapFieldProto interface {
// VariablyOpaqueFields returns the field names which contain opaque data
// VariablyOpaqueMapFields returns the field names which contain opaque data
VariablyOpaqueMapFields() []string

// VariablyOpaqueFieldProto returns a newly allocated proto message of the correct
// VariablyOpaqueMapFieldProto returns a newly allocated proto message of the correct
// type for the field name.
VariablyOpaqueMapFieldProto(name string, key string) (proto.Message, error)
}

// VariablyOpaqueSliceFieldProto should be implemented by protos which have maps to bytes fields
// which are the marshaled value of a a message type determined by the other contents of the proto
type VariablyOpaqueSliceFieldProto interface {
// VariablyOpaqueFields returns the field names which contain opaque data
// VariablyOpaqueSliceFields returns the field names which contain opaque data
VariablyOpaqueSliceFields() []string

// VariablyOpaqueFieldProto returns a newly allocated proto message of the correct
Expand All @@ -129,7 +119,7 @@ type DynamicFieldProto interface {
// DynamicMapFieldProto should be implemented by protos which have maps to messages whose attributes
// (such as their opaque types) cannot be determined until runtime
type DynamicMapFieldProto interface {
// DynamicFields returns the field names which are dynamic
// DynamicMapFields returns the field names which are dynamic
DynamicMapFields() []string

// DynamicMapFieldProto returns a newly allocated dynamic message, decorating an underlying
Expand All @@ -140,15 +130,15 @@ type DynamicMapFieldProto interface {
// DynamicSliceFieldProto should be implemented by protos which have slices of messages whose attributes
// (such as their opaque types) cannot be determined until runtime
type DynamicSliceFieldProto interface {
// DynamicFields returns the field names which are dynamic
// DynamicSliceFields returns the field names which are dynamic
DynamicSliceFields() []string

// DynamicSliceFieldProto returns a newly allocated dynamic message, decorating an underlying
// proto message with the runtime determined function
DynamicSliceFieldProto(name string, index int, underlying proto.Message) (proto.Message, error)
}

// DecoreatedProto should be implemented by the dynamic wrappers applied by the Dynamic*FieldProto interfaces
// DecoratedProto should be implemented by the dynamic wrappers applied by the Dynamic*FieldProto interfaces
// This is necessary for the proto system to unmarshal, because it discovers proto message type by reflection
// (Rather than by interface definition as it probably should ( https://github.com/golang/protobuf/issues/291 )
type DecoratedProto interface {
Expand Down
16 changes: 8 additions & 8 deletions common/tools/protolator/integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/hyperledger/fabric/protos/msp"
pb "github.com/hyperledger/fabric/protos/peer"
"github.com/hyperledger/fabric/protoutil"
. "github.com/onsi/gomega"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -234,11 +235,9 @@ func TestChannelCreationPolicy(t *testing.T) {
}

func TestStaticMarshal(t *testing.T) {
/*
To generate artifacts:
configtxgen -channelID test -outputBlock block.pb -profile SampleSingleMSPSolo
configtxgen -inspectBlock block.pb > block.json
*/
// To generate artifacts:
// configtxgen -channelID test -outputBlock block.pb -profile SampleSingleMSPSolo
// configtxgen -inspectBlock block.pb > block.json

blockBin, err := ioutil.ReadFile("testdata/block.pb")
require.NoError(t, err)
Expand All @@ -250,8 +249,9 @@ func TestStaticMarshal(t *testing.T) {
jsonBin, err := ioutil.ReadFile("testdata/block.json")
require.NoError(t, err)

var buffer bytes.Buffer
require.NoError(t, protolator.DeepMarshalJSON(&buffer, block))
buf := &bytes.Buffer{}
require.NoError(t, protolator.DeepMarshalJSON(buf, block))

assert.Equal(t, jsonBin, buffer.Bytes())
gt := NewGomegaWithT(t)
gt.Expect(buf).To(MatchJSON(jsonBin))
}
8 changes: 7 additions & 1 deletion common/tools/protolator/integration/testdata/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,13 @@
"values": {
"ChannelCreationPolicy": {
"mod_policy": "/Channel/Orderer/Admins",
"value": "CAMSCAoGQWRtaW5z",
"value": {
"type": 3,
"value": {
"rule": "ANY",
"sub_policy": "Admins"
}
},
"version": "0"
}
},
Expand Down
2 changes: 1 addition & 1 deletion common/tools/protolator/protoext/commonext/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func (sh *SignatureHeader) StaticallyOpaqueFields() []string {

func (sh *SignatureHeader) StaticallyOpaqueFieldProto(name string) (proto.Message, error) {
switch name {
case sh.StaticallyOpaqueFields()[0]: // channel_header
case sh.StaticallyOpaqueFields()[0]: // creator
return &msp.SerializedIdentity{}, nil
default:
return nil, fmt.Errorf("unknown header field: %s", name)
Expand Down
50 changes: 50 additions & 0 deletions common/tools/protolator/protoext/commonext/commonext_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
Copyright IBM Corp. All Rights Reserved.
SPDX-License-Identifier: Apache-2.0
*/

package commonext_test

import (
"github.com/hyperledger/fabric/common/tools/protolator"
"github.com/hyperledger/fabric/common/tools/protolator/protoext/commonext"
)

// ensure structs implement expected interfaces
var (
_ protolator.StaticallyOpaqueFieldProto = &commonext.Envelope{}
_ protolator.DecoratedProto = &commonext.Envelope{}
_ protolator.VariablyOpaqueFieldProto = &commonext.Payload{}
_ protolator.DecoratedProto = &commonext.Payload{}
_ protolator.StaticallyOpaqueFieldProto = &commonext.Header{}
_ protolator.DecoratedProto = &commonext.Header{}
_ protolator.StaticallyOpaqueFieldProto = &commonext.SignatureHeader{}
_ protolator.DecoratedProto = &commonext.SignatureHeader{}
_ protolator.StaticallyOpaqueSliceFieldProto = &commonext.BlockData{}
_ protolator.DecoratedProto = &commonext.BlockData{}

_ protolator.StaticallyOpaqueFieldProto = &commonext.ConfigUpdateEnvelope{}
_ protolator.DecoratedProto = &commonext.ConfigUpdateEnvelope{}
_ protolator.StaticallyOpaqueFieldProto = &commonext.ConfigSignature{}
_ protolator.DecoratedProto = &commonext.ConfigSignature{}
_ protolator.DynamicFieldProto = &commonext.Config{}
_ protolator.DecoratedProto = &commonext.Config{}
_ protolator.StaticallyOpaqueMapFieldProto = &commonext.ConfigUpdate{}
_ protolator.DecoratedProto = &commonext.ConfigUpdate{}

_ protolator.DynamicMapFieldProto = &commonext.DynamicChannelGroup{}
_ protolator.DecoratedProto = &commonext.DynamicChannelGroup{}
_ protolator.StaticallyOpaqueFieldProto = &commonext.DynamicChannelConfigValue{}
_ protolator.DecoratedProto = &commonext.DynamicChannelConfigValue{}
_ protolator.DynamicMapFieldProto = &commonext.DynamicConsortiumsGroup{}
_ protolator.DecoratedProto = &commonext.DynamicConsortiumsGroup{}
_ protolator.DynamicMapFieldProto = &commonext.DynamicConsortiumGroup{}
_ protolator.DecoratedProto = &commonext.DynamicConsortiumGroup{}
_ protolator.VariablyOpaqueFieldProto = &commonext.DynamicConsortiumConfigValue{}
_ protolator.DecoratedProto = &commonext.DynamicConsortiumConfigValue{}
_ protolator.DynamicMapFieldProto = &commonext.DynamicConsortiumOrgGroup{}
_ protolator.DecoratedProto = &commonext.DynamicConsortiumOrgGroup{}
_ protolator.StaticallyOpaqueFieldProto = &commonext.DynamicConsortiumOrgConfigValue{}
_ protolator.DecoratedProto = &commonext.DynamicConsortiumOrgConfigValue{}
)
4 changes: 4 additions & 0 deletions common/tools/protolator/protoext/commonext/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,10 @@ func (dccv *DynamicConsortiumConfigValue) Underlying() proto.Message {
return dccv.ConfigValue
}

func (dccv *DynamicConsortiumConfigValue) VariablyOpaqueFields() []string {
return []string{"value"}
}

func (dccv *DynamicConsortiumConfigValue) VariablyOpaqueFieldProto(name string) (proto.Message, error) {
if name != "value" {
return nil, fmt.Errorf("not a marshaled field: %s", name)
Expand Down
22 changes: 22 additions & 0 deletions common/tools/protolator/protoext/ledger/rwsetext/rwsetext_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
Copyright IBM Corp. All Rights Reserved.
SPDX-License-Identifier: Apache-2.0
*/

package rwsetext_test

import (
"github.com/hyperledger/fabric/common/tools/protolator"
"github.com/hyperledger/fabric/common/tools/protolator/protoext/ledger/rwsetext"
)

// ensure structs implement expected interfaces
var (
_ protolator.DynamicSliceFieldProto = &rwsetext.TxReadWriteSet{}
_ protolator.DecoratedProto = &rwsetext.TxReadWriteSet{}
_ protolator.StaticallyOpaqueFieldProto = &rwsetext.DynamicNsReadWriteSet{}
_ protolator.DecoratedProto = &rwsetext.DynamicNsReadWriteSet{}
_ protolator.StaticallyOpaqueFieldProto = &rwsetext.DynamicCollectionHashedReadWriteSet{}
_ protolator.DecoratedProto = &rwsetext.DynamicCollectionHashedReadWriteSet{}
)
21 changes: 21 additions & 0 deletions common/tools/protolator/protoext/mspext/mspext_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
Copyright IBM Corp. All Rights Reserved.
SPDX-License-Identifier: Apache-2.0
*/

package mspext_test

import (
"github.com/hyperledger/fabric/common/tools/protolator"
"github.com/hyperledger/fabric/common/tools/protolator/protoext/mspext"
)

// ensure structs implement expected interfaces
var (
_ protolator.VariablyOpaqueFieldProto = &mspext.MSPConfig{}
_ protolator.DecoratedProto = &mspext.MSPConfig{}

_ protolator.VariablyOpaqueFieldProto = &mspext.MSPPrincipal{}
_ protolator.DecoratedProto = &mspext.MSPPrincipal{}
)
26 changes: 26 additions & 0 deletions common/tools/protolator/protoext/ordererext/ordererext_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
Copyright IBM Corp. All Rights Reserved.
SPDX-License-Identifier: Apache-2.0
*/

package ordererext_test

import (
"github.com/hyperledger/fabric/common/tools/protolator"
"github.com/hyperledger/fabric/common/tools/protolator/protoext/ordererext"
)

// ensure structs implement expected interfaces
var (
_ protolator.DynamicMapFieldProto = &ordererext.DynamicOrdererGroup{}
_ protolator.DecoratedProto = &ordererext.DynamicOrdererGroup{}
_ protolator.VariablyOpaqueFieldProto = &ordererext.ConsensusType{}
_ protolator.DecoratedProto = &ordererext.ConsensusType{}
_ protolator.DynamicMapFieldProto = &ordererext.DynamicOrdererOrgGroup{}
_ protolator.DecoratedProto = &ordererext.DynamicOrdererOrgGroup{}
_ protolator.StaticallyOpaqueFieldProto = &ordererext.DynamicOrdererConfigValue{}
_ protolator.DecoratedProto = &ordererext.DynamicOrdererConfigValue{}
_ protolator.StaticallyOpaqueFieldProto = &ordererext.DynamicOrdererOrgConfigValue{}
_ protolator.DecoratedProto = &ordererext.DynamicOrdererOrgConfigValue{}
)
39 changes: 39 additions & 0 deletions common/tools/protolator/protoext/peerext/peerext_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
Copyright IBM Corp. All Rights Reserved.
SPDX-License-Identifier: Apache-2.0
*/

package peerext_test

import (
"github.com/hyperledger/fabric/common/tools/protolator"
"github.com/hyperledger/fabric/common/tools/protolator/protoext/peerext"
)

// ensure structs implement expected interfaces
var (
_ protolator.DynamicMapFieldProto = &peerext.DynamicApplicationGroup{}
_ protolator.DecoratedProto = &peerext.DynamicApplicationGroup{}
_ protolator.DynamicMapFieldProto = &peerext.DynamicApplicationOrgGroup{}
_ protolator.DecoratedProto = &peerext.DynamicApplicationOrgGroup{}
_ protolator.StaticallyOpaqueFieldProto = &peerext.DynamicApplicationConfigValue{}
_ protolator.DecoratedProto = &peerext.DynamicApplicationConfigValue{}
_ protolator.StaticallyOpaqueFieldProto = &peerext.DynamicApplicationOrgConfigValue{}
_ protolator.DecoratedProto = &peerext.DynamicApplicationOrgConfigValue{}

_ protolator.StaticallyOpaqueFieldProto = &peerext.ChaincodeProposalPayload{}
_ protolator.DecoratedProto = &peerext.ChaincodeProposalPayload{}
_ protolator.StaticallyOpaqueFieldProto = &peerext.ChaincodeAction{}
_ protolator.DecoratedProto = &peerext.ChaincodeAction{}

_ protolator.StaticallyOpaqueFieldProto = &peerext.ProposalResponsePayload{}
_ protolator.DecoratedProto = &peerext.ProposalResponsePayload{}

_ protolator.StaticallyOpaqueFieldProto = &peerext.TransactionAction{}
_ protolator.DecoratedProto = &peerext.TransactionAction{}
_ protolator.StaticallyOpaqueFieldProto = &peerext.ChaincodeActionPayload{}
_ protolator.DecoratedProto = &peerext.ChaincodeActionPayload{}
_ protolator.StaticallyOpaqueFieldProto = &peerext.ChaincodeEndorsedAction{}
_ protolator.DecoratedProto = &peerext.ChaincodeEndorsedAction{}
)

0 comments on commit 47f22ea

Please sign in to comment.