Merge "[FAB-16560] Fix review comments"

common/cauthdsl/cauthdsl_test.go

@@ -67,7 +67,7 @@ func (id *mockIdentity) Serialize() ([]byte, error) {
 	return id.idBytes, nil
 }
 
-func toSignedData(idBytesSlice [][]byte, deserializer msp.IdentityDeserializer) ([]msp.Identity, []bool) {
+func toIdentities(idBytesSlice [][]byte, deserializer msp.IdentityDeserializer) ([]msp.Identity, []bool) {
 	identities := make([]msp.Identity, len(idBytesSlice))
 	for i, idBytes := range idBytesSlice {
 		id, _ := deserializer.DeserializeIdentity(idBytes)
@@ -105,10 +105,10 @@ func TestSimpleSignature(t *testing.T) {
 		t.Fatalf("Could not create a new SignaturePolicyEvaluator using the given policy, crypto-helper: %s", err)
 	}
 
-	if !spe(toSignedData([][]byte{signers[0]}, &mockDeserializer{})) {
+	if !spe(toIdentities([][]byte{signers[0]}, &mockDeserializer{})) {
 		t.Errorf("Expected authentication to succeed with valid signatures")
 	}
-	if spe(toSignedData([][]byte{signers[1]}, &mockDeserializer{})) {
+	if spe(toIdentities([][]byte{signers[1]}, &mockDeserializer{})) {
 		t.Errorf("Expected authentication to fail because signers[1] is not authorized in the policy, despite his valid signature")
 	}
 }
@@ -121,10 +121,10 @@ func TestMultipleSignature(t *testing.T) {
 		t.Fatalf("Could not create a new SignaturePolicyEvaluator using the given policy, crypto-helper: %s", err)
 	}
 
-	if !spe(toSignedData(signers, &mockDeserializer{})) {
+	if !spe(toIdentities(signers, &mockDeserializer{})) {
 		t.Errorf("Expected authentication to succeed with  valid signatures")
 	}
-	if spe(toSignedData([][]byte{signers[0], signers[0]}, &mockDeserializer{})) {
+	if spe(toIdentities([][]byte{signers[0], signers[0]}, &mockDeserializer{})) {
 		t.Errorf("Expected authentication to fail because although there were two valid signatures, one was duplicated")
 	}
 }
@@ -137,16 +137,16 @@ func TestComplexNestedSignature(t *testing.T) {
 		t.Fatalf("Could not create a new SignaturePolicyEvaluator using the given policy, crypto-helper: %s", err)
 	}
 
-	if !spe(toSignedData(append(signers, [][]byte{[]byte("signer0")}...), &mockDeserializer{})) {
+	if !spe(toIdentities(append(signers, [][]byte{[]byte("signer0")}...), &mockDeserializer{})) {
 		t.Errorf("Expected authentication to succeed with valid signatures")
 	}
-	if !spe(toSignedData([][]byte{[]byte("signer0"), []byte("signer0"), []byte("signer0")}, &mockDeserializer{})) {
+	if !spe(toIdentities([][]byte{[]byte("signer0"), []byte("signer0"), []byte("signer0")}, &mockDeserializer{})) {
 		t.Errorf("Expected authentication to succeed with valid signatures")
 	}
-	if spe(toSignedData(signers, &mockDeserializer{})) {
+	if spe(toIdentities(signers, &mockDeserializer{})) {
 		t.Errorf("Expected authentication to fail with too few signatures")
 	}
-	if spe(toSignedData(append(signers, [][]byte{[]byte("signer1")}...), &mockDeserializer{})) {
+	if spe(toIdentities(append(signers, [][]byte{[]byte("signer1")}...), &mockDeserializer{})) {
 		t.Errorf("Expected authentication failure as there was a signature from signer[0] missing")
 	}
 }

common/cauthdsl/policy.go

@@ -7,14 +7,14 @@ SPDX-License-Identifier: Apache-2.0
 package cauthdsl
 
 import (
-	"errors"
 	"fmt"
 
 	"github.com/golang/protobuf/proto"
 	cb "github.com/hyperledger/fabric-protos-go/common"
 	"github.com/hyperledger/fabric/common/policies"
 	"github.com/hyperledger/fabric/msp"
 	"github.com/hyperledger/fabric/protoutil"
+	"github.com/pkg/errors"
 )
 
 type provider struct {
@@ -86,7 +86,7 @@ type policy struct {
 // 2) the signing identities satisfy the policy
 func (p *policy) EvaluateSignedData(signatureSet []*protoutil.SignedData) error {
 	if p == nil {
-		return fmt.Errorf("No such policy")
+		return errors.New("no such policy")
 	}
 
 	ids := policies.SignatureSetToValidIdentities(signatureSet, p.deserializer)

common/cauthdsl/policy_test.go

@@ -116,7 +116,7 @@ func TestNewPolicyErrorCase(t *testing.T) {
 
 	var pol4 *policy
 	err4 := pol4.EvaluateSignedData([]*protoutil.SignedData{})
-	assert.EqualError(t, err4, "No such policy")
+	assert.EqualError(t, err4, "no such policy")
 }
 
 func TestEnvelopeBasedPolicyProvider(t *testing.T) {

common/policies/implicitmeta.go

@@ -109,21 +109,24 @@ func (imp *ImplicitMetaPolicy) EvaluateIdentities(identities []msp.Identity) err
 	remaining := imp.Threshold
 
 	defer func() {
-		if remaining != 0 {
-			// This log message may be large and expensive to construct, so worth checking the log level
-			if logger.IsEnabledFor(zapcore.DebugLevel) {
-				var b bytes.Buffer
-				b.WriteString(fmt.Sprintf("Evaluation Failed: Only %d policies were satisfied, but needed %d of [ ", imp.Threshold-remaining, imp.Threshold))
-				for m := range imp.managers {
-					b.WriteString(m)
-					b.WriteString("/")
-					b.WriteString(imp.SubPolicyName)
-					b.WriteString(" ")
-				}
-				b.WriteString("]")
-				logger.Debugf(b.String())
-			}
+		// This log message may be large and expensive to construct, so worth checking the log level
+		if remaining == 0 {
+			return
+		}
+		if !logger.IsEnabledFor(zapcore.DebugLevel) {
+			return
+		}
+
+		var b bytes.Buffer
+		b.WriteString(fmt.Sprintf("Evaluation Failed: Only %d policies were satisfied, but needed %d of [ ", imp.Threshold-remaining, imp.Threshold))
+		for m := range imp.managers {
+			b.WriteString(m)
+			b.WriteString("/")
+			b.WriteString(imp.SubPolicyName)
+			b.WriteString(" ")
 		}
+		b.WriteString("]")
+		logger.Debugf(b.String())
 	}()
 
 	for _, policy := range imp.SubPolicies {