From 60757942212ea8ece3131eca69dd4de973ad0235 Mon Sep 17 00:00:00 2001 From: Troy Ronda Date: Tue, 13 Mar 2018 20:28:16 -0400 Subject: [PATCH] [FAB-8851] Make options consistent across client pkgs Change-Id: Ice87f4818aaafb7db1e23a872b4c9a500b82a438 Signed-off-by: Troy Ronda --- pkg/client/channel/api.go | 48 +++++++++--- pkg/client/channel/api_test.go | 76 +++++++++++++++++++ pkg/client/channel/chclient.go | 28 +++---- pkg/client/channel/chclient_test.go | 2 +- .../dynamicselection/ccpolicyprovider.go | 2 +- pkg/client/msp/msp.go | 8 +- test/integration/msp/custom_core_factory.go | 13 ++-- test/integration/msp/custom_msp_factory.go | 11 +-- test/integration/orgs/multiple_orgs_test.go | 8 +- test/integration/utils.go | 1 + 10 files changed, 153 insertions(+), 44 deletions(-) create mode 100644 pkg/client/channel/api_test.go diff --git a/pkg/client/channel/api.go b/pkg/client/channel/api.go index 205b0498ad..20117cbcc4 100644 --- a/pkg/client/channel/api.go +++ b/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 } diff --git a/pkg/client/channel/api_test.go b/pkg/client/channel/api_test.go new file mode 100644 index 0000000000..b7acb8d152 --- /dev/null +++ b/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 +} diff --git a/pkg/client/channel/chclient.go b/pkg/client/channel/chclient.go index cd9eb02aad..de86c4da70 100644 --- a/pkg/client/channel/chclient.go +++ b/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 { diff --git a/pkg/client/channel/chclient_test.go b/pkg/client/channel/chclient_test.go index c602dcdac3..5d8788d0d3 100644 --- a/pkg/client/channel/chclient_test.go +++ b/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) } diff --git a/pkg/client/common/selection/dynamicselection/ccpolicyprovider.go b/pkg/client/common/selection/dynamicselection/ccpolicyprovider.go index 1729e9f941..7381fe822e 100644 --- a/pkg/client/common/selection/dynamicselection/ccpolicyprovider.go +++ b/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 diff --git a/pkg/client/msp/msp.go b/pkg/client/msp/msp.go index db3e51f44c..cf55b9159f 100644 --- a/pkg/client/msp/msp.go +++ b/pkg/client/msp/msp.go @@ -25,11 +25,11 @@ 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 @@ -37,7 +37,7 @@ func WithOrg(orgName string) Option { } // 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 { diff --git a/test/integration/msp/custom_core_factory.go b/test/integration/msp/custom_core_factory.go index c56e417d6d..36da74f20f 100644 --- a/test/integration/msp/custom_core_factory.go +++ b/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) } diff --git a/test/integration/msp/custom_msp_factory.go b/test/integration/msp/custom_msp_factory.go index bb65c4b7c9..e263c2f0b0 100644 --- a/test/integration/msp/custom_msp_factory.go +++ b/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) } diff --git a/test/integration/orgs/multiple_orgs_test.go b/test/integration/orgs/multiple_orgs_test.go index e76e19a202..a5f408fa0b 100644 --- a/test/integration/orgs/multiple_orgs_test.go +++ b/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) } diff --git a/test/integration/utils.go b/test/integration/utils.go index 81107100a1..74feb17b33 100644 --- a/test/integration/utils.go +++ b/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 {