Skip to content

Commit

Permalink
[FAB-7830] Refactor client: delay error propagation
Browse files Browse the repository at this point in the history 
This change adjusts fabsdk.NewClient to delay
execution in order to avoid erros on the call.
This allows for a cleaner client API.

Change-Id: Ib0455c7139ee3ab634a5723af724aba5c7d337bf
Signed-off-by: Troy Ronda <troy@troyronda.com>
  • Loading branch information
@troyronda
troyronda committed Jan 20, 2018
1 parent 199cc9d commit 94ac20c
Show file tree
Hide file tree
Showing 11 changed files with 96 additions and 201 deletions.
13 changes: 3 additions & 10 deletions def/fabapi/deprecated_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,14 @@ func TestNewSDK(t *testing.T) {
t.Fatalf("Error initializing SDK: %s", err)
}

c1, err := sdk.NewClient(fabsdk.WithUser("User1"))
if err != nil {
t.Fatalf("Failed to create client: %s", err)
}

// Default channel client (uses organisation from client configuration)
_, err = c1.Channel("mychannel")
_, err = sdk.NewClient(fabsdk.WithUser("User1")).Channel("mychannel")
if err != nil {
t.Fatalf("Failed to create new channel client: %s", err)
}

c2, err := sdk.NewClient(fabsdk.WithUser("User1"), fabsdk.WithOrg("Org2"))
if err != nil {
t.Fatalf("Failed to create client: %s", err)
}
// keep a common client context for the next two tests
c2 := sdk.NewClient(fabsdk.WithUser("User1"), fabsdk.WithOrg("Org2"))

// Test configuration failure for channel client (mychannel does't have event source configured for Org2)
_, err = c2.Channel("mychannel")
Expand Down
103 changes: 51 additions & 52 deletions pkg/fabsdk/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@ import (

// Client represents the fabric transaction clients
type Client struct {
opts *clientOptions
identity apifabclient.User
providers apisdk.SDK
clientFactory apisdk.SessionClientFactory
provider clientProvider
}

// ClientOption configures the clients created by the SDK.
Expand All @@ -33,6 +30,15 @@ type clientOptions struct {
targetFilter resmgmt.TargetFilter
}

type clientProvider func() (*clientContext, error)

type clientContext struct {
opts *clientOptions
identity apifabclient.User
providers apisdk.SDK
clientFactory apisdk.SessionClientFactory
}

// WithOrg uses the configuration and users from the named organization.
func WithOrg(name string) ClientOption {
return func(opts *clientOptions) error {
Expand All @@ -59,24 +65,32 @@ func withConfig(configProvider apiconfig.Config) ClientOption {
}

// NewClient allows creation of transactions using the supplied identity as the credential.
func (sdk *FabricSDK) NewClient(identityOpt IdentityOption, opts ...ClientOption) (*Client, error) {
o, err := newClientOptions(sdk.ConfigProvider(), opts)
if err != nil {
return nil, errors.WithMessage(err, "unable to retrieve configuration from SDK")
}
func (sdk *FabricSDK) NewClient(identityOpt IdentityOption, opts ...ClientOption) *Client {
// delay execution of the following logic to avoid error return from this function.
// this is done to allow a cleaner API - i.e., client, err := sdk.NewClient(args).<Desired Interface>(extra args)
provider := func() (*clientContext, error) {
o, err := newClientOptions(sdk.ConfigProvider(), opts)
if err != nil {
return nil, errors.WithMessage(err, "unable to retrieve configuration from SDK")
}

identity, err := sdk.newIdentity(o.orgID, identityOpt)
if err != nil {
return nil, errors.WithMessage(err, "unable to create client context")
}
identity, err := sdk.newIdentity(o.orgID, identityOpt)
if err != nil {
return nil, errors.WithMessage(err, "unable to create client context")
}

cc := clientContext{
opts: o,
identity: identity,
providers: sdk,
clientFactory: sdk.opts.Session,
}
return &cc, nil
}
client := Client{
opts: o,
identity: identity,
providers: sdk,
clientFactory: sdk.opts.Session,
provider: provider,
}
return &client, nil
return &client
}

func newClientOptions(config apiconfig.Config, options []ClientOption) (*clientOptions, error) {
Expand Down Expand Up @@ -107,8 +121,13 @@ func newClientOptions(config apiconfig.Config, options []ClientOption) (*clientO

// ChannelMgmt returns a client API for managing channels
func (c *Client) ChannelMgmt() (chmgmt.ChannelMgmtClient, error) {
session := newSession(c.identity)
client, err := c.clientFactory.NewChannelMgmtClient(c.providers, session, c.opts.configProvider)
p, err := c.provider()
if err != nil {
return nil, errors.WithMessage(err, "unable to get client provider context")
}

session := newSession(p.identity)
client, err := p.clientFactory.NewChannelMgmtClient(p.providers, session, p.opts.configProvider)
if err != nil {
return nil, errors.WithMessage(err, "failed to create new channel management client")
}
Expand All @@ -118,52 +137,32 @@ func (c *Client) ChannelMgmt() (chmgmt.ChannelMgmtClient, error) {

// ResourceMgmt returns a client API for managing system resources
func (c *Client) ResourceMgmt() (resmgmt.ResourceMgmtClient, error) {
session := newSession(c.identity)
client, err := c.clientFactory.NewResourceMgmtClient(c.providers, session, c.opts.configProvider, c.opts.targetFilter)
p, err := c.provider()
if err != nil {
return nil, errors.WithMessage(err, "failed to created new resource management client")
return nil, errors.WithMessage(err, "unable to get client provider context")
}

return client, nil
}

// Channel returns a client API for transacting on a channel
func (c *Client) Channel(id string) (apitxn.ChannelClient, error) {
session := newSession(c.identity)
client, err := c.clientFactory.NewChannelClient(c.providers, session, c.opts.configProvider, id)
session := newSession(p.identity)
client, err := p.clientFactory.NewResourceMgmtClient(p.providers, session, p.opts.configProvider, p.opts.targetFilter)
if err != nil {
return nil, errors.WithMessage(err, "failed to created new resource management client")
}

return client, nil
}

// NewClientChannelMgmt returns a new client for managing channels
func (sdk *FabricSDK) NewClientChannelMgmt(identity IdentityOption, opts ...ClientOption) (chmgmt.ChannelMgmtClient, error) {
c, err := sdk.NewClient(identity, opts...)
if err != nil {
return nil, errors.WithMessage(err, "error creating client from SDK")
}

return c.ChannelMgmt()
}

// NewClientResourceMgmt returns a new client for managing system resources
func (sdk *FabricSDK) NewClientResourceMgmt(identity IdentityOption, opts ...ClientOption) (resmgmt.ResourceMgmtClient, error) {
c, err := sdk.NewClient(identity, opts...)
// Channel returns a client API for transacting on a channel
func (c *Client) Channel(id string) (apitxn.ChannelClient, error) {
p, err := c.provider()
if err != nil {
return nil, errors.WithMessage(err, "error creating client from SDK")
return nil, errors.WithMessage(err, "unable to get client provider context")
}

return c.ResourceMgmt()
}

// NewClientChannel returns a new client for a channel
func (sdk *FabricSDK) NewClientChannel(identity IdentityOption, channelID string, opts ...ClientOption) (apitxn.ChannelClient, error) {
c, err := sdk.NewClient(identity, opts...)
session := newSession(p.identity)
client, err := p.clientFactory.NewChannelClient(p.providers, session, p.opts.configProvider, id)
if err != nil {
return nil, errors.WithMessage(err, "error creating client from SDK")
return nil, errors.WithMessage(err, "failed to created new resource management client")
}

return c.Channel(channelID)
return client, nil
}
52 changes: 7 additions & 45 deletions pkg/fabsdk/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func TestNewGoodClientOpt(t *testing.T) {
t.Fatalf("Expected no error from New, but got %v", err)
}

_, err = sdk.NewClient(WithUser(clientValidUser), goodClientOpt())
_, err = sdk.NewClient(WithUser(clientValidUser), goodClientOpt()).ChannelMgmt()
if err != nil {
t.Fatalf("Expected no error from Client, but got %v", err)
}
Expand All @@ -45,7 +45,7 @@ func TestFromConfigGoodClientOpt(t *testing.T) {
t.Fatalf("Expected no error from New, but got %v", err)
}

_, err = sdk.NewClient(WithUser(clientValidUser), goodClientOpt())
_, err = sdk.NewClient(WithUser(clientValidUser), goodClientOpt()).ChannelMgmt()
if err != nil {
t.Fatalf("Expected no error from Client, but got %v", err)
}
Expand All @@ -63,7 +63,7 @@ func TestNewBadClientOpt(t *testing.T) {
t.Fatalf("Expected no error from New, but got %v", err)
}

_, err = sdk.NewClient(WithUser(clientValidUser), badClientOpt())
_, err = sdk.NewClient(WithUser(clientValidUser), badClientOpt()).ChannelMgmt()
if err == nil {
t.Fatal("Expected error from Client")
}
Expand All @@ -81,7 +81,7 @@ func TestClient(t *testing.T) {
t.Fatalf("Expected no error from New, but got %v", err)
}

_, err = sdk.NewClient(WithUser(clientValidUser))
_, err = sdk.NewClient(WithUser(clientValidUser)).ChannelMgmt()
if err != nil {
t.Fatalf("Expected no error from Client, but got %v", err)
}
Expand All @@ -93,12 +93,12 @@ func TestWithOrg(t *testing.T) {
t.Fatalf("Expected no error from New, but got %v", err)
}

_, err = sdk.NewClient(WithUser("notarealuser"), WithOrg(clientValidExtraOrg))
_, err = sdk.NewClient(WithUser("notarealuser"), WithOrg(clientValidExtraOrg)).ChannelMgmt()
if err == nil {
t.Fatal("Expected error from Client")
}

_, err = sdk.NewClient(WithUser(clientValidExtraUser), WithOrg(clientValidExtraOrg))
_, err = sdk.NewClient(WithUser(clientValidExtraUser), WithOrg(clientValidExtraOrg)).ChannelMgmt()
if err != nil {
t.Fatalf("Expected no error from Client, but got %v", err)
}
Expand Down Expand Up @@ -143,50 +143,12 @@ func TestNoIdentity(t *testing.T) {
t.Fatalf("Expected no error from New, but got %v", err)
}

_, err = sdk.NewClient(noopIdentityOpt(), goodClientOpt())
_, err = sdk.NewClient(noopIdentityOpt(), goodClientOpt()).ChannelMgmt()
if err == nil {
t.Fatal("Expected error from Client")
}
}

func TestNewChannelMgmtClient(t *testing.T) {
sdk, err := New(configImpl.FromFile("../../test/fixtures/config/config_test.yaml"))
if err != nil {
t.Fatalf("Error initializing SDK: %s", err)
}

// Test configuration failure for channel management client (invalid user/default organisation)
_, err = sdk.NewClientChannelMgmt(WithUser("Invalid"))
if err == nil {
t.Fatalf("Should have failed to create channel client due to invalid user")
}

// Test valid configuration for channel management client
_, err = sdk.NewClientChannelMgmt(WithUser(clientValidAdmin))
if err != nil {
t.Fatalf("Failed to create new channel client: %s", err)
}
}

func TestNewResourceMgmtClient(t *testing.T) {
sdk, err := New(configImpl.FromFile("../../test/fixtures/config/config_test.yaml"))
if err != nil {
t.Fatalf("Error initializing SDK: %s", err)
}

// Test configuration failure for resource management client (invalid user/default organisation)
_, err = sdk.NewClientResourceMgmt(WithUser("Invalid"))
if err == nil {
t.Fatalf("Should have failed to create resource management client due to invalid user")
}

// Test valid configuration for resource management client
_, err = sdk.NewClientResourceMgmt(WithUser(clientValidAdmin))
if err != nil {
t.Fatalf("Failed to create new resource management client: %s", err)
}
}

func noopIdentityOpt() IdentityOption {
return func(o *identityOptions, sdk *FabricSDK, orgName string) error {
return nil
Expand Down
37 changes: 6 additions & 31 deletions pkg/fabsdk/deprecated.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/hyperledger/fabric-sdk-go/api/apitxn"
chmgmt "github.com/hyperledger/fabric-sdk-go/api/apitxn/chmgmtclient"
resmgmt "github.com/hyperledger/fabric-sdk-go/api/apitxn/resmgmtclient"
"github.com/hyperledger/fabric-sdk-go/pkg/errors"
apisdk "github.com/hyperledger/fabric-sdk-go/pkg/fabsdk/api"
)

Expand Down Expand Up @@ -49,11 +48,7 @@ func (sdk *FabricSDK) NewChannelMgmtClientWithOpts(userName string, opt *Channel
o = append(o, withConfig(opt.ConfigProvider))
}

c, err := sdk.NewClient(WithUser(userName), o...)
if err != nil {
return nil, errors.WithMessage(err, "error creating client from SDK")
}

c := sdk.NewClient(WithUser(userName), o...)
return c.ChannelMgmt()
}

Expand All @@ -71,11 +66,7 @@ func (sdk *FabricSDK) NewResourceMgmtClientWithOpts(userName string, opt *Resour
o = append(o, withConfig(opt.ConfigProvider))
}

c, err := sdk.NewClient(WithUser(userName), o...)
if err != nil {
return nil, errors.WithMessage(err, "error creating client from SDK")
}

c := sdk.NewClient(WithUser(userName), o...)
return c.ResourceMgmt()
}

Expand All @@ -90,44 +81,28 @@ func (sdk *FabricSDK) NewChannelClientWithOpts(channelID string, userName string
o = append(o, withConfig(opt.ConfigProvider))
}

c, err := sdk.NewClient(WithUser(userName), o...)
if err != nil {
return nil, errors.WithMessage(err, "error creating client from SDK")
}

c := sdk.NewClient(WithUser(userName), o...)
return c.Channel(channelID)
}

// NewChannelMgmtClient returns a new client for managing channels
// This function is deprecated.
func (sdk *FabricSDK) NewChannelMgmtClient(userName string, opts ...ClientOption) (chmgmt.ChannelMgmtClient, error) {
c, err := sdk.NewClient(WithUser(userName), opts...)
if err != nil {
return nil, errors.WithMessage(err, "error creating client from SDK")
}

c := sdk.NewClient(WithUser(userName), opts...)
return c.ChannelMgmt()
}

// NewResourceMgmtClient returns a new client for managing system resources
// This function is deprecated.
func (sdk *FabricSDK) NewResourceMgmtClient(userName string, opts ...ClientOption) (resmgmt.ResourceMgmtClient, error) {
c, err := sdk.NewClient(WithUser(userName), opts...)
if err != nil {
return nil, errors.WithMessage(err, "error creating client from SDK")
}

c := sdk.NewClient(WithUser(userName), opts...)
return c.ResourceMgmt()
}

// NewChannelClient returns a new client for a channel
// This function is deprecated.
func (sdk *FabricSDK) NewChannelClient(channelID string, userName string, opts ...ClientOption) (apitxn.ChannelClient, error) {
c, err := sdk.NewClient(WithUser(userName), opts...)
if err != nil {
return nil, errors.WithMessage(err, "error creating client from SDK")
}

c := sdk.NewClient(WithUser(userName), opts...)
return c.Channel(channelID)
}

Expand Down
Loading

0 comments on commit 94ac20c

Please sign in to comment.