[FABG-724] Don't Query all the peers for selection

Change-Id: I17805a695f61e04d923cf75e7249d7e7524c54c7
Signed-off-by: Sandra Vrtikapa <[email protected]>

pkg/client/common/discovery/dynamicdiscovery/chservice.go

@@ -7,9 +7,8 @@ SPDX-License-Identifier: Apache-2.0
 package dynamicdiscovery
 
 import (
-	"math/rand"
-
 	discclient "github.com/hyperledger/fabric-sdk-go/internal/github.com/hyperledger/fabric/discovery/client"
+	"github.com/hyperledger/fabric-sdk-go/pkg/client/common/random"
 	coptions "github.com/hyperledger/fabric-sdk-go/pkg/common/options"
 	contextAPI "github.com/hyperledger/fabric-sdk-go/pkg/common/providers/context"
 	"github.com/hyperledger/fabric-sdk-go/pkg/common/providers/fab"
@@ -88,7 +87,7 @@ func (s *ChannelService) getTargets(ctx contextAPI.Client) ([]fab.PeerConfig, er
 	}
 
 	//pick number of peers given in channel policy
-	return pickRandomNPeerConfigs(chPeers, chConfig.Policies.QueryChannelConfig.QueryDiscovery), nil
+	return random.PickRandomNPeerConfigs(chPeers, chConfig.Policies.QueryChannelConfig.QueryDiscovery), nil
 }
 
 // evaluate validates the responses and returns the peers
@@ -141,16 +140,3 @@ type peerEndpoint struct {
 func (p *peerEndpoint) BlockHeight() uint64 {
 	return p.blockHeight
 }
-
-//pickRandomNPeerConfigs picks N random  unique peer configs from given channel peer list
-func pickRandomNPeerConfigs(chPeers []fab.ChannelPeer, n int) []fab.PeerConfig {
-
-	var result []fab.PeerConfig
-	for _, index := range rand.Perm(len(chPeers)) {
-		result = append(result, chPeers[index].PeerConfig)
-		if len(result) == n {
-			break
-		}
-	}
-	return result
-}

pkg/client/common/discovery/dynamicdiscovery/chservice_test.go

@@ -10,8 +10,6 @@ import (
 	"testing"
 	"time"
 
-	"fmt"
-
 	"github.com/hyperledger/fabric-sdk-go/pkg/client/common/discovery"
 	clientmocks "github.com/hyperledger/fabric-sdk-go/pkg/client/common/mocks"
 	contextAPI "github.com/hyperledger/fabric-sdk-go/pkg/common/providers/context"
@@ -223,127 +221,6 @@ func TestDiscoveryServiceWithNewOrgJoined(t *testing.T) {
 
 }
 
-func TestPickRandomNPeerConfigs(t *testing.T) {
-	counter := 20
-	allChPeers := createNChannelPeers(counter)
-
-	result := pickRandomNPeerConfigs(allChPeers, 4)
-	assert.NotNil(t, result)
-	assert.NotEmpty(t, result)
-	assert.Equal(t, 4, len(result))
-	verifyDuplicates(t, result)
-
-	result = pickRandomNPeerConfigs(allChPeers, 1)
-	assert.NotNil(t, result)
-	assert.NotEmpty(t, result)
-	assert.Equal(t, 1, len(result))
-	verifyDuplicates(t, result)
-
-	result = pickRandomNPeerConfigs(allChPeers, 19)
-	assert.NotNil(t, result)
-	assert.NotEmpty(t, result)
-	assert.Equal(t, 19, len(result))
-	verifyDuplicates(t, result)
-
-	result = pickRandomNPeerConfigs(allChPeers, 20)
-	assert.NotNil(t, result)
-	assert.NotEmpty(t, result)
-	assert.Equal(t, 20, len(result))
-	verifyDuplicates(t, result)
-
-	result = pickRandomNPeerConfigs(allChPeers, 21)
-	assert.NotNil(t, result)
-	assert.NotEmpty(t, result)
-	assert.Equal(t, 20, len(result))
-	verifyDuplicates(t, result)
-
-	result = pickRandomNPeerConfigs(allChPeers, 24)
-	assert.NotNil(t, result)
-	assert.NotEmpty(t, result)
-	assert.Equal(t, 20, len(result))
-	verifyDuplicates(t, result)
-
-	counter = 7
-	allChPeers = createNChannelPeers(counter)
-
-	result = pickRandomNPeerConfigs(allChPeers, 6)
-	assert.NotNil(t, result)
-	assert.NotEmpty(t, result)
-	assert.Equal(t, 6, len(result))
-	verifyDuplicates(t, result)
-
-	result = pickRandomNPeerConfigs(allChPeers, 7)
-	assert.NotNil(t, result)
-	assert.NotEmpty(t, result)
-	assert.Equal(t, 7, len(result))
-	verifyDuplicates(t, result)
-
-	result = pickRandomNPeerConfigs(allChPeers, 8)
-	assert.NotNil(t, result)
-	assert.NotEmpty(t, result)
-	assert.Equal(t, 7, len(result))
-	verifyDuplicates(t, result)
-
-	counter = 2
-	allChPeers = createNChannelPeers(counter)
-
-	result = pickRandomNPeerConfigs(allChPeers, 2)
-	assert.NotNil(t, result)
-	assert.NotEmpty(t, result)
-	assert.Equal(t, 2, len(result))
-	verifyDuplicates(t, result)
-
-	result = pickRandomNPeerConfigs(allChPeers, 24)
-	assert.NotNil(t, result)
-	assert.NotEmpty(t, result)
-	assert.Equal(t, 2, len(result))
-	verifyDuplicates(t, result)
-
-	counter = 1
-	allChPeers = createNChannelPeers(counter)
-
-	result = pickRandomNPeerConfigs(allChPeers, 1)
-	assert.NotNil(t, result)
-	assert.NotEmpty(t, result)
-	assert.Equal(t, 1, len(result))
-	verifyDuplicates(t, result)
-
-	result = pickRandomNPeerConfigs(allChPeers, 2)
-	assert.NotNil(t, result)
-	assert.NotEmpty(t, result)
-	assert.Equal(t, 1, len(result))
-	verifyDuplicates(t, result)
-
-	result = pickRandomNPeerConfigs(allChPeers, 24)
-	assert.NotNil(t, result)
-	assert.NotEmpty(t, result)
-	assert.Equal(t, 1, len(result))
-	verifyDuplicates(t, result)
-
-}
-
-func createNChannelPeers(n int) []pfab.ChannelPeer {
-	allChPeers := make([]pfab.ChannelPeer, n)
-	for i := 0; i < n; i++ {
-		allChPeers[i] = pfab.ChannelPeer{
-			NetworkPeer: pfab.NetworkPeer{
-				PeerConfig: pfab.PeerConfig{URL: fmt.Sprintf("URL-%d", i)},
-			},
-		}
-	}
-	return allChPeers
-}
-
-func verifyDuplicates(t *testing.T, chPeers []pfab.PeerConfig) {
-	seen := make(map[string]bool)
-	for _, v := range chPeers {
-		if seen[v.URL] {
-			t.Fatalf("found duplicate channel peer: %s", v.URL)
-		}
-		seen[v.URL] = true
-	}
-}
-
 type blockHeightFilter struct {
 	minBlockHeight uint64
 }

pkg/client/common/random/random.go

@@ -0,0 +1,26 @@
+/*
+Copyright SecureKey Technologies Inc. All Rights Reserved.
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package random
+
+import (
+	"math/rand"
+
+	"github.com/hyperledger/fabric-sdk-go/pkg/common/providers/fab"
+)
+
+//PickRandomNPeerConfigs picks N random  unique peer configs from given channel peer list
+func PickRandomNPeerConfigs(chPeers []fab.ChannelPeer, n int) []fab.PeerConfig {
+
+	var result []fab.PeerConfig
+	for _, index := range rand.Perm(len(chPeers)) {
+		result = append(result, chPeers[index].PeerConfig)
+		if len(result) == n {
+			break
+		}
+	}
+	return result
+}

pkg/client/common/random/random_test.go

@@ -0,0 +1,136 @@
+/*
+Copyright SecureKey Technologies Inc. All Rights Reserved.
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package random
+
+import (
+	"fmt"
+	"testing"
+
+	pfab "github.com/hyperledger/fabric-sdk-go/pkg/common/providers/fab"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestPickRandomNPeerConfigs(t *testing.T) {
+	counter := 20
+	allChPeers := createNChannelPeers(counter)
+
+	result := PickRandomNPeerConfigs(allChPeers, 4)
+	assert.NotNil(t, result)
+	assert.NotEmpty(t, result)
+	assert.Equal(t, 4, len(result))
+	verifyDuplicates(t, result)
+
+	result = PickRandomNPeerConfigs(allChPeers, 1)
+	assert.NotNil(t, result)
+	assert.NotEmpty(t, result)
+	assert.Equal(t, 1, len(result))
+	verifyDuplicates(t, result)
+
+	result = PickRandomNPeerConfigs(allChPeers, 19)
+	assert.NotNil(t, result)
+	assert.NotEmpty(t, result)
+	assert.Equal(t, 19, len(result))
+	verifyDuplicates(t, result)
+
+	result = PickRandomNPeerConfigs(allChPeers, 20)
+	assert.NotNil(t, result)
+	assert.NotEmpty(t, result)
+	assert.Equal(t, 20, len(result))
+	verifyDuplicates(t, result)
+
+	result = PickRandomNPeerConfigs(allChPeers, 21)
+	assert.NotNil(t, result)
+	assert.NotEmpty(t, result)
+	assert.Equal(t, 20, len(result))
+	verifyDuplicates(t, result)
+
+	result = PickRandomNPeerConfigs(allChPeers, 24)
+	assert.NotNil(t, result)
+	assert.NotEmpty(t, result)
+	assert.Equal(t, 20, len(result))
+	verifyDuplicates(t, result)
+
+	counter = 7
+	allChPeers = createNChannelPeers(counter)
+
+	result = PickRandomNPeerConfigs(allChPeers, 6)
+	assert.NotNil(t, result)
+	assert.NotEmpty(t, result)
+	assert.Equal(t, 6, len(result))
+	verifyDuplicates(t, result)
+
+	result = PickRandomNPeerConfigs(allChPeers, 7)
+	assert.NotNil(t, result)
+	assert.NotEmpty(t, result)
+	assert.Equal(t, 7, len(result))
+	verifyDuplicates(t, result)
+
+	result = PickRandomNPeerConfigs(allChPeers, 8)
+	assert.NotNil(t, result)
+	assert.NotEmpty(t, result)
+	assert.Equal(t, 7, len(result))
+	verifyDuplicates(t, result)
+
+	counter = 2
+	allChPeers = createNChannelPeers(counter)
+
+	result = PickRandomNPeerConfigs(allChPeers, 2)
+	assert.NotNil(t, result)
+	assert.NotEmpty(t, result)
+	assert.Equal(t, 2, len(result))
+	verifyDuplicates(t, result)
+
+	result = PickRandomNPeerConfigs(allChPeers, 24)
+	assert.NotNil(t, result)
+	assert.NotEmpty(t, result)
+	assert.Equal(t, 2, len(result))
+	verifyDuplicates(t, result)
+
+	counter = 1
+	allChPeers = createNChannelPeers(counter)
+
+	result = PickRandomNPeerConfigs(allChPeers, 1)
+	assert.NotNil(t, result)
+	assert.NotEmpty(t, result)
+	assert.Equal(t, 1, len(result))
+	verifyDuplicates(t, result)
+
+	result = PickRandomNPeerConfigs(allChPeers, 2)
+	assert.NotNil(t, result)
+	assert.NotEmpty(t, result)
+	assert.Equal(t, 1, len(result))
+	verifyDuplicates(t, result)
+
+	result = PickRandomNPeerConfigs(allChPeers, 24)
+	assert.NotNil(t, result)
+	assert.NotEmpty(t, result)
+	assert.Equal(t, 1, len(result))
+	verifyDuplicates(t, result)
+
+}
+
+func createNChannelPeers(n int) []pfab.ChannelPeer {
+	allChPeers := make([]pfab.ChannelPeer, n)
+	for i := 0; i < n; i++ {
+		allChPeers[i] = pfab.ChannelPeer{
+			NetworkPeer: pfab.NetworkPeer{
+				PeerConfig: pfab.PeerConfig{URL: fmt.Sprintf("URL-%d", i)},
+			},
+		}
+	}
+	return allChPeers
+}
+
+func verifyDuplicates(t *testing.T, chPeers []pfab.PeerConfig) {
+	seen := make(map[string]bool)
+	for _, v := range chPeers {
+		if seen[v.URL] {
+			t.Fatalf("found duplicate channel peer: %s", v.URL)
+		}
+		seen[v.URL] = true
+	}
+}

pkg/client/common/selection/fabricselection/fabricselection.go

@@ -15,6 +15,7 @@ import (
 
 	discclient "github.com/hyperledger/fabric-sdk-go/internal/github.com/hyperledger/fabric/discovery/client"
 	"github.com/hyperledger/fabric-sdk-go/internal/github.com/hyperledger/fabric/protos/discovery"
+	"github.com/hyperledger/fabric-sdk-go/pkg/client/common/random"
 	soptions "github.com/hyperledger/fabric-sdk-go/pkg/client/common/selection/options"
 	"github.com/hyperledger/fabric-sdk-go/pkg/common/errors/multi"
 	"github.com/hyperledger/fabric-sdk-go/pkg/common/errors/retry"
@@ -226,7 +227,7 @@ func (s *Service) query(req *discclient.Request, chaincodes []*fab.ChaincodeCall
 	responses, err := s.discClient.Send(reqCtx, req, targets...)
 	if err != nil {
 		if len(responses) == 0 {
-			return nil, errors.Wrapf(err, "error calling discover service send")
+			return nil, errors.Wrapf(err, "error calling discover service send for selection")
 		}
 		logger.Warnf("Received %d response(s) and one or more errors from discovery client: %s", len(responses), err)
 	}
@@ -270,17 +271,19 @@ func (s *Service) query(req *discclient.Request, chaincodes []*fab.ChaincodeCall
 }
 
 func (s *Service) getTargets(ctx contextAPI.Client) ([]fab.PeerConfig, error) {
-	// TODO: The number of peers to query should be retrieved from the channel policy.
-	// This will done in a future patch.
+
 	chpeers, ok := ctx.EndpointConfig().ChannelPeers(s.channelID)
 	if !ok {
 		return nil, errors.Errorf("failed to get peer configs for channel [%s]", s.channelID)
 	}
-	targets := make([]fab.PeerConfig, len(chpeers))
-	for i := 0; i < len(targets); i++ {
-		targets[i] = chpeers[i].NetworkPeer.PeerConfig
+
+	chConfig, ok := ctx.EndpointConfig().ChannelConfig(s.channelID)
+	if !ok {
+		return nil, errors.Errorf("failed to get channel endpoint config for channel [%s]", s.channelID)
 	}
-	return targets, nil
+
+	//pick number of peers based on channel policy
+	return random.PickRandomNPeerConfigs(chpeers, chConfig.Policies.QueryChannelConfig.QueryDiscovery), nil
 }
 
 func asChaincodeInterests(chaincodes []*fab.ChaincodeCall) *discovery.ChaincodeInterest {