[FAB-8851] Make options consistent across client pkgs

Change-Id: Ice87f4818aaafb7db1e23a872b4c9a500b82a438
Signed-off-by: Troy Ronda <troy@troyronda.com>

pkg/client/channel/api.go

@@ -9,20 +9,23 @@ package channel
 import (
 	"time"
 
+	"github.com/hyperledger/fabric-sdk-go/pkg/common/context"
 	"github.com/hyperledger/fabric-sdk-go/pkg/context/api/fab"
+	"github.com/hyperledger/fabric-sdk-go/pkg/core/config"
 	"github.com/hyperledger/fabric-sdk-go/pkg/errors/retry"
 	pb "github.com/hyperledger/fabric-sdk-go/third_party/github.com/hyperledger/fabric/protos/peer"
+	"github.com/pkg/errors"
 )
 
 // opts allows the user to specify more advanced options
-type opts struct {
+type requestOptions struct {
 	Targets []fab.Peer // targets
 	Timeout time.Duration
 	Retry   retry.Opts
 }
 
-//Option func for each Opts argument
-type Option func(opts *opts) error
+// RequestOption func for each Opts argument
+type RequestOption func(ctx context.Client, opts *requestOptions) error
 
 // Request contains the parameters to query and execute an invocation transaction
 type Request struct {
@@ -42,24 +45,51 @@ type Response struct {
 }
 
 //WithTimeout encapsulates time.Duration to Option
-func WithTimeout(timeout time.Duration) Option {
-	return func(o *opts) error {
+func WithTimeout(timeout time.Duration) RequestOption {
+	return func(ctx context.Client, o *requestOptions) error {
 		o.Timeout = timeout
 		return nil
 	}
 }
 
 //WithTargets encapsulates ProposalProcessors to Option
-func WithTargets(targets []fab.Peer) Option {
-	return func(o *opts) error {
+func WithTargets(targets ...fab.Peer) RequestOption {
+	return func(ctx context.Client, o *requestOptions) error {
 		o.Targets = targets
 		return nil
 	}
 }
 
+// WithTargetURLs allows overriding of the target peers for the request.
+// Targets are specified by URL, and the SDK will create the underlying peer
+// objects.
+func WithTargetURLs(urls ...string) RequestOption {
+	return func(ctx context.Client, opts *requestOptions) error {
+
+		var targets []fab.Peer
+
+		for _, url := range urls {
+
+			peerCfg, err := config.NetworkPeerConfigFromURL(ctx.Config(), url)
+			if err != nil {
+				return err
+			}
+
+			peer, err := ctx.InfraProvider().CreatePeerFromConfig(peerCfg)
+			if err != nil {
+				return errors.WithMessage(err, "creating peer from config failed")
+			}
+
+			targets = append(targets, peer)
+		}
+
+		return WithTargets(targets...)(ctx, opts)
+	}
+}
+
 // WithRetry option to configure retries
-func WithRetry(retryOpt retry.Opts) Option {
-	return func(o *opts) error {
+func WithRetry(retryOpt retry.Opts) RequestOption {
+	return func(ctx context.Client, o *requestOptions) error {
 		o.Retry = retryOpt
 		return nil
 	}

pkg/client/channel/api_test.go

@@ -0,0 +1,76 @@
+/*
+Copyright SecureKey Technologies Inc. All Rights Reserved.
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package channel
+
+import (
+	"testing"
+
+	"github.com/hyperledger/fabric-sdk-go/pkg/context/api/core"
+	fcmocks "github.com/hyperledger/fabric-sdk-go/pkg/fab/mocks"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestWithTargetURLsInvalid(t *testing.T) {
+	ctx := setupMockTestContext("test", "Org1MSP")
+	opt := WithTargetURLs("invalid")
+
+	mockConfig := &fcmocks.MockConfig{}
+
+	oConfig := &core.PeerConfig{
+		URL: "127.0.0.1:7050",
+	}
+
+	mockConfig.SetCustomPeerCfg(oConfig)
+	ctx.SetConfig(mockConfig)
+
+	opts := requestOptions{}
+	err := opt(ctx, &opts)
+	assert.NotNil(t, err, "Should have failed for invalid target peer")
+}
+
+func TestWithTargetURLsValid(t *testing.T) {
+	ctx := setupMockTestContext("test", "Org1MSP")
+	opt := WithTargetURLs("127.0.0.1:7050")
+
+	mockConfig := &fcmocks.MockConfig{}
+
+	pConfig1 := core.PeerConfig{
+		URL: "127.0.0.1:7050",
+	}
+
+	npConfig1 := core.NetworkPeer{
+		PeerConfig: pConfig1,
+		MspID:      "MYMSP",
+	}
+
+	pConfig2 := core.PeerConfig{
+		URL: "127.0.0.1:7051",
+	}
+
+	npConfig2 := core.NetworkPeer{
+		PeerConfig: pConfig2,
+		MspID:      "OTHERMSP",
+	}
+
+	mockConfig.SetCustomPeerCfg(&pConfig1)
+	mockConfig.SetCustomNetworkPeerCfg([]core.NetworkPeer{npConfig2, npConfig1})
+	ctx.SetConfig(mockConfig)
+
+	opts := requestOptions{}
+	err := opt(ctx, &opts)
+	assert.Nil(t, err, "Should have failed for invalid target peer")
+
+	assert.Equal(t, 1, len(opts.Targets), "should have one peer")
+	assert.Equal(t, pConfig1.URL, opts.Targets[0].URL(), "", "Wrong URL")
+	assert.Equal(t, npConfig1.MspID, opts.Targets[0].MSPID(), "", "Wrong MSP")
+}
+
+func setupMockTestContext(userName string, mspID string) *fcmocks.MockContext {
+	user := fcmocks.NewMockUserWithMSPID(userName, mspID)
+	ctx := fcmocks.NewMockContext(user)
+	return ctx
+}

pkg/client/channel/chclient.go

@@ -116,19 +116,19 @@ func New(channelProvider context.ChannelProvider, opts ...ClientOption) (*Client
 }
 
 // Query chaincode using request and optional options provided
-func (cc *Client) Query(request Request, options ...Option) (Response, error) {
-	return cc.InvokeHandler(invoke.NewQueryHandler(), request, cc.addDefaultTimeout(core.Query, options...)...)
+func (cc *Client) Query(request Request, options ...RequestOption) (Response, error) {
+	return cc.InvokeHandler(invoke.NewQueryHandler(), request, cc.addDefaultTimeout(cc.context, core.Query, options...)...)
 }
 
 // Execute prepares and executes transaction using request and optional options provided
-func (cc *Client) Execute(request Request, options ...Option) (Response, error) {
-	return cc.InvokeHandler(invoke.NewExecuteHandler(), request, cc.addDefaultTimeout(core.Execute, options...)...)
+func (cc *Client) Execute(request Request, options ...RequestOption) (Response, error) {
+	return cc.InvokeHandler(invoke.NewExecuteHandler(), request, cc.addDefaultTimeout(cc.context, core.Execute, options...)...)
 }
 
 //InvokeHandler invokes handler using request and options provided
-func (cc *Client) InvokeHandler(handler invoke.Handler, request Request, options ...Option) (Response, error) {
+func (cc *Client) InvokeHandler(handler invoke.Handler, request Request, options ...RequestOption) (Response, error) {
 	//Read execute tx options
-	txnOpts, err := cc.prepareOptsFromOptions(options...)
+	txnOpts, err := cc.prepareOptsFromOptions(cc.context, options...)
 	if err != nil {
 		return Response{}, err
 	}
@@ -159,7 +159,7 @@ func (cc *Client) InvokeHandler(handler invoke.Handler, request Request, options
 	}
 }
 
-func (cc *Client) resolveRetry(ctx *invoke.RequestContext, o opts) bool {
+func (cc *Client) resolveRetry(ctx *invoke.RequestContext, o requestOptions) bool {
 	errs, ok := ctx.Error.(multi.Errors)
 	if !ok {
 		errs = append(errs, ctx.Error)
@@ -181,7 +181,7 @@ func (cc *Client) resolveRetry(ctx *invoke.RequestContext, o opts) bool {
 }
 
 //prepareHandlerContexts prepares context objects for handlers
-func (cc *Client) prepareHandlerContexts(request Request, o opts) (*invoke.RequestContext, *invoke.ClientContext, error) {
+func (cc *Client) prepareHandlerContexts(request Request, o requestOptions) (*invoke.RequestContext, *invoke.ClientContext, error) {
 
 	if request.ChaincodeID == "" || request.Fcn == "" {
 		return nil, nil, errors.New("ChaincodeID and Fcn are required")
@@ -215,10 +215,10 @@ func (cc *Client) prepareHandlerContexts(request Request, o opts) (*invoke.Reque
 }
 
 //prepareOptsFromOptions Reads apitxn.Opts from Option array
-func (cc *Client) prepareOptsFromOptions(options ...Option) (opts, error) {
-	txnOpts := opts{}
+func (cc *Client) prepareOptsFromOptions(ctx context.Client, options ...RequestOption) (requestOptions, error) {
+	txnOpts := requestOptions{}
 	for _, option := range options {
-		err := option(&txnOpts)
+		err := option(ctx, &txnOpts)
 		if err != nil {
 			return txnOpts, errors.WithMessage(err, "Failed to read opts")
 		}
@@ -227,10 +227,10 @@ func (cc *Client) prepareOptsFromOptions(options ...Option) (opts, error) {
 }
 
 //addDefaultTimeout adds given default timeout if it is missing in options
-func (cc *Client) addDefaultTimeout(timeOutType core.TimeoutType, options ...Option) []Option {
-	txnOpts := opts{}
+func (cc *Client) addDefaultTimeout(ctx context.Client, timeOutType core.TimeoutType, options ...RequestOption) []RequestOption {
+	txnOpts := requestOptions{}
 	for _, option := range options {
-		option(&txnOpts)
+		option(ctx, &txnOpts)
 	}
 
 	if txnOpts.Timeout == 0 {

pkg/client/channel/chclient_test.go

@@ -174,7 +174,7 @@ func TestQueryWithOptTarget(t *testing.T) {
 	peers := []fab.Peer{testPeer}
 
 	response, err := chClient.Query(Request{ChaincodeID: "testCC", Fcn: "invoke",
-		Args: [][]byte{[]byte("query"), []byte("b")}}, WithTargets(peers))
+		Args: [][]byte{[]byte("query"), []byte("b")}}, WithTargets(peers...))
 	if err != nil {
 		t.Fatalf("Failed to invoke test cc: %s", err)
 	}

pkg/client/common/selection/dynamicselection/ccpolicyprovider.go

@@ -164,7 +164,7 @@ func (dp *ccPolicyProvider) queryChaincode(ccID string, ccFcn string, ccArgs [][
 			Args:        ccArgs,
 		}
 
-		resp, err := client.Query(request, channel.WithTargets([]fab.Peer{peer}))
+		resp, err := client.Query(request, channel.WithTargets(peer))
 		if err != nil {
 			queryErrors = append(queryErrors, err.Error())
 			continue

pkg/client/msp/msp.go

@@ -25,19 +25,19 @@ type MSP struct {
 	ctx     context.Client
 }
 
-// Option describes a functional parameter for the New constructor
-type Option func(*MSP) error
+// ClientOption describes a functional parameter for the New constructor
+type ClientOption func(*MSP) error
 
 // WithOrg option
-func WithOrg(orgName string) Option {
+func WithOrg(orgName string) ClientOption {
 	return func(msp *MSP) error {
 		msp.orgName = orgName
 		return nil
 	}
 }
 
 // New creates a new MSP instance
-func New(clientProvider context.ClientProvider, opts ...Option) (*MSP, error) {
+func New(clientProvider context.ClientProvider, opts ...ClientOption) (*MSP, error) {
 
 	ctx, err := clientProvider()
 	if err != nil {

test/integration/msp/custom_core_factory.go

@@ -14,27 +14,28 @@ import (
 
 // ========== MSP Provider Factory with custom crypto provider ============= //
 
-type customCoreFactory struct {
+// CustomCoreFactory is a custom factory for tests.
+type CustomCoreFactory struct {
 	defaultFactory    *defcore.ProviderFactory
 	customCryptoSuite core.CryptoSuite
 }
 
 // NewCustomCoreFactory creates a new instance of customCoreFactory
-func NewCustomCoreFactory(customCryptoSuite core.CryptoSuite) *customCoreFactory {
-	return &customCoreFactory{defaultFactory: defcore.NewProviderFactory(), customCryptoSuite: customCryptoSuite}
+func NewCustomCoreFactory(customCryptoSuite core.CryptoSuite) *CustomCoreFactory {
+	return &CustomCoreFactory{defaultFactory: defcore.NewProviderFactory(), customCryptoSuite: customCryptoSuite}
 }
 
 // CreateCryptoSuiteProvider creates custom crypto provider
-func (f *customCoreFactory) CreateCryptoSuiteProvider(config core.Config) (core.CryptoSuite, error) {
+func (f *CustomCoreFactory) CreateCryptoSuiteProvider(config core.Config) (core.CryptoSuite, error) {
 	return f.customCryptoSuite, nil
 }
 
 // CreateSigningManager creates SigningManager
-func (f *customCoreFactory) CreateSigningManager(cryptoProvider core.CryptoSuite, config core.Config) (core.SigningManager, error) {
+func (f *CustomCoreFactory) CreateSigningManager(cryptoProvider core.CryptoSuite, config core.Config) (core.SigningManager, error) {
 	return f.defaultFactory.CreateSigningManager(cryptoProvider, config)
 }
 
 // CreateInfraProvider creates InfraProvider
-func (f *customCoreFactory) CreateInfraProvider(config core.Config) (fab.InfraProvider, error) {
+func (f *CustomCoreFactory) CreateInfraProvider(config core.Config) (fab.InfraProvider, error) {
 	return f.defaultFactory.CreateInfraProvider(config)
 }

test/integration/msp/custom_msp_factory.go

@@ -14,22 +14,23 @@ import (
 
 // ========== MSP Provider Factory with custom user store ============= //
 
-type customMSPFactory struct {
+// CustomMSPFactory is a custom factory for tests.
+type CustomMSPFactory struct {
 	defaultFactory  *defmsp.ProviderFactory
 	customUserStore msp.UserStore
 }
 
 // NewCustomMSPFactory creates a custom MSPFactory
-func NewCustomMSPFactory(customUserStore msp.UserStore) *customMSPFactory {
-	return &customMSPFactory{defaultFactory: defmsp.NewProviderFactory(), customUserStore: customUserStore}
+func NewCustomMSPFactory(customUserStore msp.UserStore) *CustomMSPFactory {
+	return &CustomMSPFactory{defaultFactory: defmsp.NewProviderFactory(), customUserStore: customUserStore}
 }
 
 // CreateUserStore creates UserStore
-func (f *customMSPFactory) CreateUserStore(config core.Config) (msp.UserStore, error) {
+func (f *CustomMSPFactory) CreateUserStore(config core.Config) (msp.UserStore, error) {
 	return f.customUserStore, nil
 }
 
 // CreateProvider creates an MSP provider
-func (f *customMSPFactory) CreateProvider(config core.Config, cryptoProvider core.CryptoSuite, userStore msp.UserStore) (msp.Provider, error) {
+func (f *CustomMSPFactory) CreateProvider(config core.Config, cryptoProvider core.CryptoSuite, userStore msp.UserStore) (msp.Provider, error) {
 	return f.defaultFactory.CreateProvider(config, cryptoProvider, f.customUserStore)
 }

test/integration/orgs/multiple_orgs_test.go

@@ -229,7 +229,7 @@ func testWithOrg1(t *testing.T, sdk *fabsdk.FabricSDK) int {
 	}
 
 	// Org2 user moves funds on org2 peer
-	response, err = chClientOrg2User.Execute(channel.Request{ChaincodeID: "exampleCC", Fcn: "invoke", Args: integration.ExampleCCTxArgs()}, channel.WithTargets([]fab.Peer{orgTestPeer1}))
+	response, err = chClientOrg2User.Execute(channel.Request{ChaincodeID: "exampleCC", Fcn: "invoke", Args: integration.ExampleCCTxArgs()}, channel.WithTargets(orgTestPeer1))
 	if err != nil {
 		t.Fatalf("Failed to move funds: %s", err)
 	}
@@ -293,13 +293,13 @@ func testWithOrg1(t *testing.T, sdk *fabsdk.FabricSDK) int {
 	}
 
 	// Org2 user moves funds on org2 peer (cc policy fails since both Org1 and Org2 peers should participate)
-	response, err = chClientOrg2User.Execute(channel.Request{ChaincodeID: "exampleCC", Fcn: "invoke", Args: integration.ExampleCCTxArgs()}, channel.WithTargets([]fab.Peer{orgTestPeer1}))
+	response, err = chClientOrg2User.Execute(channel.Request{ChaincodeID: "exampleCC", Fcn: "invoke", Args: integration.ExampleCCTxArgs()}, channel.WithTargets(orgTestPeer1))
 	if err == nil {
 		t.Fatalf("Should have failed to move funds due to cc policy")
 	}
 
 	// Org2 user moves funds (cc policy ok since we have provided peers for both Orgs)
-	response, err = chClientOrg2User.Execute(channel.Request{ChaincodeID: "exampleCC", Fcn: "invoke", Args: integration.ExampleCCTxArgs()}, channel.WithTargets([]fab.Peer{orgTestPeer0, orgTestPeer1}))
+	response, err = chClientOrg2User.Execute(channel.Request{ChaincodeID: "exampleCC", Fcn: "invoke", Args: integration.ExampleCCTxArgs()}, channel.WithTargets(orgTestPeer0, orgTestPeer1))
 	if err != nil {
 		t.Fatalf("Failed to move funds: %s", err)
 	}
@@ -364,7 +364,7 @@ func verifyValue(t *testing.T, chClient *channel.Client, expected int) {
 	var valueInt int
 	for i := 0; i < pollRetries; i++ {
 		// Query final value on org1 peer
-		response, err := chClient.Query(channel.Request{ChaincodeID: "exampleCC", Fcn: "invoke", Args: integration.ExampleCCQueryArgs()}, channel.WithTargets([]fab.Peer{orgTestPeer0}))
+		response, err := chClient.Query(channel.Request{ChaincodeID: "exampleCC", Fcn: "invoke", Args: integration.ExampleCCQueryArgs()}, channel.WithTargets(orgTestPeer0))
 		if err != nil {
 			t.Fatalf("Failed to query funds after transaction: %s", err)
 		}

test/integration/utils.go

@@ -155,6 +155,7 @@ func HasPeerJoinedChannel(client *resmgmt.Client, target string, channel string)
 	return foundChannel, nil
 }
 
+// CleanupTestPath removes the contents of a state store.
 func CleanupTestPath(t *testing.T, storePath string) {
 	err := os.RemoveAll(storePath)
 	if err != nil {