[FAB-8725] JoinChannel should obey WithOrdererID

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

Author: troyronda

pkg/client/resmgmt/resmgmt.go

@@ -168,7 +168,7 @@ func (rc *Client) JoinChannel(channelID string, options ...RequestOption) error
 		return errors.New("must provide channel ID")
 	}
 
-	opts, err := rc.prepareResmgmtOpts(options...)
+	opts, err := rc.prepareRequestOpts(options...)
 	if err != nil {
 		return errors.WithMessage(err, "failed to get opts for JoinChannel")
 	}
@@ -182,17 +182,12 @@ func (rc *Client) JoinChannel(channelID string, options ...RequestOption) error
 		return errors.New("No targets available")
 	}
 
-	// TODO: should the code to get orderers from sdk config be part of channel service?
-	oConfig, err := rc.ctx.Config().ChannelOrderers(channelID)
+	ordererCfg, err := rc.ordererConfig(&opts, channelID)
 	if err != nil {
-		return errors.WithMessage(err, "failed to load orderer config")
-	}
-	if len(oConfig) == 0 {
-		return errors.Errorf("no orderers are configured for channel %s", channelID)
+		return errors.WithMessage(err, "failed to find orderer config")
 	}
 
-	// TODO: handle more than the first orderer.
-	orderer, err := rc.ctx.InfraProvider().CreateOrdererFromConfig(&oConfig[0])
+	orderer, err := rc.ctx.InfraProvider().CreateOrdererFromConfig(ordererCfg)
 	if err != nil {
 		return errors.WithMessage(err, "failed to create orderers from config")
 	}
@@ -306,7 +301,7 @@ func (rc *Client) InstallCC(req InstallCCRequest, options ...RequestOption) ([]I
 		return nil, err
 	}
 
-	opts, err := rc.prepareResmgmtOpts(options...)
+	opts, err := rc.prepareRequestOpts(options...)
 	if err != nil {
 		return nil, errors.WithMessage(err, "failed to get opts for InstallCC")
 	}
@@ -450,7 +445,7 @@ func (rc *Client) sendCCProposal(ccProposalType chaincodeProposalType, channelID
 		return err
 	}
 
-	opts, err := rc.prepareResmgmtOpts(options...)
+	opts, err := rc.prepareRequestOpts(options...)
 	if err != nil {
 		return errors.WithMessage(err, "failed to get opts for cc proposal")
 	}
@@ -555,18 +550,6 @@ func checkRequiredCCProposalParams(channelID string, req InstantiateCCRequest) e
 	return nil
 }
 
-//prepareResmgmtOpts Reads Opts from Option array
-func (rc *Client) prepareResmgmtOpts(options ...RequestOption) (Opts, error) {
-	resmgmtOpts := Opts{}
-	for _, option := range options {
-		err := option(&resmgmtOpts)
-		if err != nil {
-			return resmgmtOpts, errors.WithMessage(err, "Failed to read resource management opts")
-		}
-	}
-	return resmgmtOpts, nil
-}
-
 func createAndSendTransaction(sender fab.Sender, request fab.TransactionRequest) (*fab.TransactionResponse, error) {
 
 	tx, err := sender.CreateTransaction(request)
@@ -648,18 +631,9 @@ func (rc *Client) SaveChannel(req SaveChannelRequest, options ...RequestOption)
 		configSignatures = append(configSignatures, configSignature)
 	}
 
-	// Figure out orderer configuration
-	var ordererCfg *core.OrdererConfig
-	if opts.OrdererID != "" {
-		ordererCfg, err = rc.ctx.Config().OrdererConfig(opts.OrdererID)
-	} else {
-		// Default is random orderer from configuration
-		ordererCfg, err = rc.ctx.Config().RandomOrdererConfig()
-	}
-
-	// Check if retrieving orderer configuration went ok
-	if err != nil || ordererCfg == nil {
-		return errors.Errorf("failed to retrieve orderer config: %s", err)
+	ordererCfg, err := rc.ordererConfig(&opts, req.ChannelID)
+	if err != nil {
+		return errors.WithMessage(err, "failed to find orderer config")
 	}
 
 	orderer, err := orderer.New(rc.ctx.Config(), orderer.FromOrdererConfig(ordererCfg))
@@ -692,32 +666,9 @@ func (rc *Client) QueryConfigFromOrderer(channelID string, options ...RequestOpt
 		return nil, err
 	}
 
-	chCfg, err := rc.ctx.Config().ChannelConfig(channelID)
+	ordererCfg, err := rc.ordererConfig(&opts, channelID)
 	if err != nil {
-		return nil, err
-	}
-
-	var ordererCfg *core.OrdererConfig
-
-	// Figure out orderer configuration (first try opts, then random channel orderer, then random orderer )
-	if opts.OrdererID != "" {
-
-		ordererCfg, err = rc.ctx.Config().OrdererConfig(opts.OrdererID)
-
-	} else if chCfg != nil && len(chCfg.Orderers) > 0 {
-
-		// random channel orderer
-		randomNumber := rand.Intn(len(chCfg.Orderers))
-		ordererCfg, err = rc.ctx.Config().OrdererConfig(chCfg.Orderers[randomNumber])
-
-	} else {
-		// random orderer from configuration
-		ordererCfg, err = rc.ctx.Config().RandomOrdererConfig()
-	}
-
-	// Check if retrieving orderer configuration went ok
-	if err != nil || ordererCfg == nil {
-		return nil, errors.Errorf("failed to retrieve orderer config: %s", err)
+		return nil, errors.WithMessage(err, "failed to find orderer config")
 	}
 
 	orderer, err := orderer.New(rc.ctx.Config(), orderer.FromOrdererConfig(ordererCfg))
@@ -734,6 +685,38 @@ func (rc *Client) QueryConfigFromOrderer(channelID string, options ...RequestOpt
 
 }
 
+func (rc *Client) ordererConfig(opts *Opts, channelID string) (*core.OrdererConfig, error) {
+	if opts.OrdererID != "" {
+		ordererCfg, err := rc.ctx.Config().OrdererConfig(opts.OrdererID)
+		if err != nil {
+			return nil, errors.WithMessage(err, "orderer not found")
+		}
+		if ordererCfg == nil {
+			return nil, errors.New("orderer not found")
+		}
+		return ordererCfg, nil
+	}
+
+	orderers, err := rc.ctx.Config().ChannelOrderers(channelID)
+
+	// TODO: Not sure that we should fallback to global orderers section.
+	// For now - not doing so.
+	//if err != nil || len(orderers) == 0 {
+	//	orderers, err = rc.ctx.Config().OrderersConfig()
+	//}
+
+	if err != nil {
+		return nil, errors.WithMessage(err, "orderers lookup failed")
+	}
+	if len(orderers) == 0 {
+		return nil, errors.New("no orderers found")
+	}
+
+	// random channel orderer
+	randomNumber := rand.Intn(len(orderers))
+	return &orderers[randomNumber], nil
+}
+
 // prepareRequestOpts prepares request options
 func (rc *Client) prepareRequestOpts(options ...RequestOption) (Opts, error) {
 	opts := Opts{}

pkg/client/resmgmt/resmgmt_test.go

@@ -14,24 +14,23 @@ import (
 	"testing"
 	"time"
 
+	"github.com/golang/protobuf/proto"
+	"github.com/pkg/errors"
+	"github.com/stretchr/testify/assert"
 	"google.golang.org/grpc"
 
+	txnmocks "github.com/hyperledger/fabric-sdk-go/pkg/client/common/mocks"
 	"github.com/hyperledger/fabric-sdk-go/pkg/common/context"
+	contextImpl "github.com/hyperledger/fabric-sdk-go/pkg/context"
 	"github.com/hyperledger/fabric-sdk-go/pkg/context/api/core"
 	"github.com/hyperledger/fabric-sdk-go/pkg/context/api/fab"
+	"github.com/hyperledger/fabric-sdk-go/pkg/core/config"
+	fcmocks "github.com/hyperledger/fabric-sdk-go/pkg/fab/mocks"
 	"github.com/hyperledger/fabric-sdk-go/pkg/fab/peer"
 	"github.com/hyperledger/fabric-sdk-go/pkg/fab/resource/api"
+	"github.com/hyperledger/fabric-sdk-go/pkg/fabsdk/provider/fabpvdr"
 	"github.com/hyperledger/fabric-sdk-go/third_party/github.com/hyperledger/fabric/common/cauthdsl"
 	"github.com/hyperledger/fabric-sdk-go/third_party/github.com/hyperledger/fabric/protos/common"
-	"github.com/pkg/errors"
-
-	txnmocks "github.com/hyperledger/fabric-sdk-go/pkg/client/common/mocks"
-	"github.com/hyperledger/fabric-sdk-go/pkg/core/config"
-
-	"github.com/golang/protobuf/proto"
-	contextImpl "github.com/hyperledger/fabric-sdk-go/pkg/context"
-	fcmocks "github.com/hyperledger/fabric-sdk-go/pkg/fab/mocks"
-	"github.com/hyperledger/fabric-sdk-go/pkg/fabsdk/provider/fabpvdr"
 	pb "github.com/hyperledger/fabric-sdk-go/third_party/github.com/hyperledger/fabric/protos/peer"
 )
 
@@ -166,12 +165,6 @@ func TestJoinChannelRequiredParameters(t *testing.T) {
 		t.Fatalf("Should have failed for empty channel name")
 	}
 
-	// Test error when creating channel from configuration
-	err = rc.JoinChannel("error")
-	if err == nil {
-		t.Fatalf("Should have failed with generated error in NewChannel")
-	}
-
 	// Setup test client with different msp (default targets cannot be calculated)
 	ctx := setupTestContext("test", "otherMSP")
 	config := getNetworkConfig(t)
@@ -278,6 +271,40 @@ func TestJoinChannelDiscoveryError(t *testing.T) {
 
 }
 
+func TestOrdererConfigFail(t *testing.T) {
+
+	ctx := setupTestContext("test", "Org1MSP")
+
+	// No channel orderer, no global orderer
+	noOrdererConfig, err := config.FromFile("./testdata/noorderer_test.yaml")()
+	assert.Nil(t, err)
+
+	ctx.SetConfig(noOrdererConfig)
+	rc := setupResMgmtClient(ctx, nil, t)
+
+	opts := Opts{}
+	orderer, err := rc.ordererConfig(&opts, "mychannel")
+	assert.Nil(t, orderer)
+	assert.NotNil(t, err, "should fail since no orderer has been configured")
+}
+
+/*
+func TestOrdererConfigFromOpts(t *testing.T) {
+	ctx := setupTestContext("test", "Org1MSP")
+
+	// No channel orderer, no global orderer
+	noOrdererConfig, err := config.FromFile("./testdata/ccproposal_test.yaml")()
+	assert.Nil(t, err)
+
+	ctx.SetConfig(noOrdererConfig)
+	rc := setupResMgmtClient(ctx, nil, t)
+
+	opts := Opts{}
+	orderer, err := rc.ordererConfig(&opts, "mychannel")
+	assert.Nil(t, orderer)
+	assert.NotNil(t, err, "should fail since no orderer has been configured")
+}*/
+
 func TestJoinChannelNoOrdererConfig(t *testing.T) {
 
 	ctx := setupTestContext("test", "Org1MSP")
@@ -291,9 +318,7 @@ func TestJoinChannelNoOrdererConfig(t *testing.T) {
 	rc := setupResMgmtClient(ctx, nil, t)
 
 	err = rc.JoinChannel("mychannel")
-	if err == nil {
-		t.Fatalf("Should have failed to join channel since no orderer has been configured")
-	}
+	assert.NotNil(t, err, "Should have failed to join channel since no orderer has been configured")
 
 	// Misconfigured channel orderer
 	invalidChOrdererConfig, err := config.FromFile("./testdata/invalidchorderer_test.yaml")()
@@ -1432,6 +1457,17 @@ func TestSaveChannelWithOpts(t *testing.T) {
 	}
 }
 
+func TestJoinChannelWithInvalidOpts(t *testing.T) {
+
+	cc := setupDefaultResMgmtClient(t)
+	opts := WithOrdererID("Invalid")
+	err := cc.JoinChannel("mychannel", opts)
+	if err == nil {
+		t.Fatal("Should have failed for invalid orderer ID")
+	}
+
+}
+
 func TestSaveChannelWithMultipleSigningIdenities(t *testing.T) {
 	grpcServer := grpc.NewServer()
 	defer grpcServer.Stop()
@@ -1447,6 +1483,7 @@ func TestSaveChannelWithMultipleSigningIdenities(t *testing.T) {
 		GRPCOptions: grpcOpts,
 	}
 	mockConfig.SetCustomRandomOrdererCfg(oConfig)
+	mockConfig.SetCustomOrdererCfg(oConfig)
 	ctx.SetConfig(mockConfig)
 
 	cc := setupResMgmtClient(ctx, nil, t)

pkg/fab/mocks/mockconfig.go

@@ -245,11 +245,9 @@ func (c *MockConfig) ChannelPeers(name string) ([]config.ChannelPeer, error) {
 
 // ChannelOrderers returns a list of channel orderers
 func (c *MockConfig) ChannelOrderers(name string) ([]config.OrdererConfig, error) {
-	oConfig := config.OrdererConfig{
-		URL: "example.com",
-	}
+	oConfig, err := c.OrdererConfig("")
 
-	return []config.OrdererConfig{oConfig}, nil
+	return []config.OrdererConfig{*oConfig}, err
 }
 
 // NetworkPeers returns the mock network peers configuration