[FAB-3723] Unit tests and cleanup for core/peer pkg

Added missing test coverage for config.go,removed
unused code and cleaned up a bit to aid in
eventually removing test code from peer.go.

Increased # lines covered from 51% to 78%

Change-Id: I0249d3c9edf6f764b2c1b66dc4af3af1048430a4
Signed-off-by: Gari Singh <gari.r.singh@gmail.com>

core/peer/config.go

@@ -53,12 +53,6 @@ var peerEndpointError error
 
 // Cached values of commonly used configuration constants.
 
-// Note: There is some kind of circular import issue that prevents us from
-// importing the "core" package into the "peer" package. The
-// 'peer.SecurityEnabled' bit is a duplicate of the 'core.SecurityEnabled'
-// bit.
-var securityEnabled bool
-
 // CacheConfiguration computes and caches commonly-used constants and
 // computed constants as package variables. Routines which were previously
 // global have been embedded here to preserve the original abstraction.
@@ -92,16 +86,12 @@ func CacheConfiguration() (err error) {
 	}
 
 	localAddress, localAddressError = getLocalAddress()
-	peerEndpoint, peerEndpointError = getPeerEndpoint()
-
-	securityEnabled = true
+	peerEndpoint, _ = getPeerEndpoint()
 
 	configurationCached = true
 
 	if localAddressError != nil {
 		return localAddressError
-	} else if peerEndpointError != nil {
-		return peerEndpointError
 	}
 	return
 }
@@ -131,14 +121,6 @@ func GetPeerEndpoint() (*pb.PeerEndpoint, error) {
 	return peerEndpoint, peerEndpointError
 }
 
-// SecurityEnabled returns the securityEnabled property from cached configuration
-func SecurityEnabled() bool {
-	if !configurationCached {
-		cacheConfiguration()
-	}
-	return securityEnabled
-}
-
 // GetSecureConfig returns the secure server configuration for the peer
 func GetSecureConfig() (comm.SecureServerConfig, error) {
 	secureConfig := comm.SecureServerConfig{

core/peer/config_test.go

@@ -0,0 +1,125 @@
+/*
+Copyright IBM Corp. 2017 All Rights Reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+		 http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+package peer
+
+import (
+	"net"
+	"testing"
+
+	"github.com/spf13/viper"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCacheConfigurationNegative(t *testing.T) {
+
+	// set a bad peer.address
+	viper.Set("peer.addressAutoDetect", true)
+	viper.Set("peer.address", "testing.com")
+	cacheConfiguration()
+	err := CacheConfiguration()
+	assert.Error(t, err, "Expected error for bad configuration")
+}
+
+func TestConfiguration(t *testing.T) {
+
+	var ips []string
+	// get the interface addresses
+	if addresses, err := net.InterfaceAddrs(); err == nil {
+		for _, address := range addresses {
+			// eliminate loopback interfaces
+			if ip, ok := address.(*net.IPNet); ok && !ip.IP.IsLoopback() {
+				ips = append(ips, ip.IP.String()+":7051")
+				t.Logf("found interface address [%s]", ip.IP.String())
+			}
+		}
+	} else {
+		t.Fatal("Failed to get interface addresses")
+	}
+
+	var tests = []struct {
+		name             string
+		settings         map[string]interface{}
+		validAddresses   []string
+		invalidAddresses []string
+	}{
+		{
+			name: "test1",
+			settings: map[string]interface{}{
+				"peer.addressAutoDetect": false,
+				"peer.address":           "testing.com:7051",
+				"peer.id":                "testPeer",
+			},
+			validAddresses:   []string{"testing.com:7051"},
+			invalidAddresses: ips,
+		},
+		{
+			name: "test2",
+			settings: map[string]interface{}{
+				"peer.addressAutoDetect": true,
+				"peer.address":           "testing.com:7051",
+				"peer.id":                "testPeer",
+			},
+			validAddresses:   ips,
+			invalidAddresses: []string{"testing.com:7051"},
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			for k, v := range test.settings {
+				viper.Set(k, v)
+			}
+			// reset the cache
+			configurationCached = false
+			// GetLocalAddress
+			address, err := GetLocalAddress()
+			assert.NoError(t, err, "GetLocalAddress returned unexpected error")
+			assert.Contains(t, test.validAddresses, address,
+				"GetLocalAddress returned unexpected address")
+			assert.NotContains(t, test.invalidAddresses, address,
+				"GetLocalAddress returned invalid address")
+			// reset the cache
+			configurationCached = false
+			// GetPeerEndpoint
+			pe, err := GetPeerEndpoint()
+			assert.NoError(t, err, "GetPeerEndpoint returned unexpected error")
+			assert.Equal(t, test.settings["peer.id"], pe.Id.Name,
+				"GetPeerEndpoint returned the wrong peer ID")
+			assert.Equal(t, address, pe.Address,
+				"GetPeerEndpoint returned the wrong peer address")
+
+			// now check with cached configuration
+			err = CacheConfiguration()
+			assert.NoError(t, err, "CacheConfiguration should not have returned an err")
+			// check functions again
+			// GetLocalAddress
+			address, err = GetLocalAddress()
+			assert.NoError(t, err, "GetLocalAddress should not have returned error")
+			assert.Contains(t, test.validAddresses, address,
+				"GetLocalAddress returned unexpected address")
+			assert.NotContains(t, test.invalidAddresses, address,
+				"GetLocalAddress returned invalid address")
+			// GetPeerEndpoint
+			pe, err = GetPeerEndpoint()
+			assert.NoError(t, err, "GetPeerEndpoint returned unexpected error")
+			assert.Equal(t, test.settings["peer.id"], pe.Id.Name,
+				"GetPeerEndpoint returned the wrong peer ID")
+			assert.Equal(t, address, pe.Address,
+				"GetPeerEndpoint returned the wrong peer address")
+		})
+	}
+}

core/peer/peer.go

@@ -267,8 +267,12 @@ func CreateChainFromBlock(cb *common.Block) error {
 func MockCreateChain(cid string) error {
 	var ledger ledger.PeerLedger
 	var err error
-	if ledger, err = createLedger(cid); err != nil {
-		return err
+
+	if ledger = GetLedger(cid); ledger == nil {
+		gb, _ := configtxtest.MakeGenesisBlock(cid)
+		if ledger, err = ledgermgmt.CreateLedger(gb); err != nil {
+			return err
+		}
 	}
 
 	// Here we need to mock also the policy manager
@@ -485,17 +489,6 @@ func SetCurrConfigBlock(block *common.Block, cid string) error {
 	return fmt.Errorf("Chain %s doesn't exist on the peer", cid)
 }
 
-// createLedger function is used only for the testing (see function 'MockCreateChain').
-// TODO - this function should not be in this file which contains production code
-func createLedger(cid string) (ledger.PeerLedger, error) {
-	var ledger ledger.PeerLedger
-	if ledger = GetLedger(cid); ledger != nil {
-		return ledger, nil
-	}
-	gb, _ := configtxtest.MakeGenesisBlock(cid)
-	return ledgermgmt.CreateLedger(gb)
-}
-
 // NewPeerClientConnection Returns a new grpc.ClientConn to the configured local PEER.
 func NewPeerClientConnection() (*grpc.ClientConn, error) {
 	return NewPeerClientConnectionWithAddress(viper.GetString("peer.address"))

core/peer/peer_test.go

@@ -20,11 +20,13 @@ import (
 	"fmt"
 	"net"
 	"os"
+	"path/filepath"
 	"testing"
 
 	configtxtest "github.com/hyperledger/fabric/common/configtx/test"
 	"github.com/hyperledger/fabric/common/localmsp"
 	mscc "github.com/hyperledger/fabric/common/mocks/scc"
+	"github.com/hyperledger/fabric/core/comm"
 	ccp "github.com/hyperledger/fabric/core/common/ccprovider"
 	"github.com/hyperledger/fabric/core/common/sysccprovider"
 	"github.com/hyperledger/fabric/core/deliverservice"
@@ -68,6 +70,56 @@ func (*mockDeliveryClientFactory) Service(g service.GossipService, endpoints []s
 	return &mockDeliveryClient{}, nil
 }
 
+func TestCreatePeerServer(t *testing.T) {
+
+	server, err := CreatePeerServer(":4050", comm.SecureServerConfig{})
+	assert.NoError(t, err, "CreatePeerServer returned unexpected error")
+	assert.Equal(t, "[::]:4050", server.Address(),
+		"CreatePeerServer returned the wrong address")
+	server.Stop()
+
+	_, err = CreatePeerServer("", comm.SecureServerConfig{})
+	assert.Error(t, err, "expected CreatePeerServer to return error with missing address")
+
+}
+
+func TestGetSecureConfig(t *testing.T) {
+
+	// good config without TLS
+	viper.Set("peer.tls.enabled", false)
+	sc, _ := GetSecureConfig()
+	assert.Equal(t, false, sc.UseTLS, "SecureConfig.UseTLS should be false")
+
+	// good config with TLS
+	viper.Set("peer.tls.enabled", true)
+	viper.Set("peer.tls.cert.file", filepath.Join("testdata", "Org1-server1-cert.pem"))
+	viper.Set("peer.tls.key.file", filepath.Join("testdata", "Org1-server1-key.pem"))
+	viper.Set("peer.tls.rootcert.file", filepath.Join("testdata", "Org1-cert.pem"))
+	sc, _ = GetSecureConfig()
+	assert.Equal(t, true, sc.UseTLS, "SecureConfig.UseTLS should be true")
+
+	// bad config with TLS
+	viper.Set("peer.tls.rootcert.file", filepath.Join("testdata", "Org11-cert.pem"))
+	_, err := GetSecureConfig()
+	assert.Error(t, err, "GetSecureConfig should return error with bad root cert path")
+	viper.Set("peer.tls.cert.file", filepath.Join("testdata", "Org11-cert.pem"))
+	_, err = GetSecureConfig()
+	assert.Error(t, err, "GetSecureConfig should return error with bad tls cert path")
+
+	// disable TLS for remaining tests
+	viper.Set("peer.tls.enabled", false)
+
+}
+
+func TestInitChain(t *testing.T) {
+
+	chainId := "testChain"
+	chainInitializer = func(cid string) {
+		assert.Equal(t, chainId, cid, "chainInitializer received unexpected cid")
+	}
+	InitChain(chainId)
+}
+
 func TestInitialize(t *testing.T) {
 	viper.Set("peer.fileSystemPath", "/var/hyperledger/test/")
 
@@ -121,6 +173,9 @@ func TestCreateChainFromBlock(t *testing.T) {
 		t.Fatalf("failed to get correct ledger")
 	}
 
+	// Config block from ledger
+	block, err = getCurrConfigBlockFromLedger(ledger)
+
 	// Bad ledger
 	ledger = GetLedger("BogusChain")
 	if ledger != nil {
@@ -139,6 +194,26 @@ func TestCreateChainFromBlock(t *testing.T) {
 		t.Fatalf("got a bogus block")
 	}
 
+	// Correct PolicyManager
+	pmgr := GetPolicyManager(testChainID)
+	if pmgr == nil {
+		t.Fatal("failed to get PolicyManager")
+	}
+
+	// Bad PolicyManager
+	pmgr = GetPolicyManager("BogusChain")
+	if pmgr != nil {
+		t.Fatal("got a bogus PolicyManager")
+	}
+
+	// PolicyManagerGetter
+	pmg := NewChannelPolicyManagerGetter()
+	assert.NotNil(t, pmg, "PolicyManagerGetter should not be nil")
+
+	pmgr, ok := pmg.Manager(testChainID)
+	assert.NotNil(t, pmgr, "PolicyManager should not be nil")
+	assert.Equal(t, true, ok, "expected Manage() to return true")
+
 	// Chaos monkey test
 	Initialize(nil)