[FAB-2321] Cleanup channel config

https://jira.hyperledger.org/browse/FAB-2321

The channel config values handling has been moved around a lot, this is
the first in a series of CRs designed to cleanup the style of the config
packages.

Change-Id: I6456516fe182e1e37bcd9585ebff64f314941a63
Signed-off-by: Jason Yellick <jyellick@us.ibm.com>

common/configtx/api/api.go

@@ -17,7 +17,8 @@ limitations under the License.
 package api
 
 import (
-	configvalues "github.com/hyperledger/fabric/common/configvalues/api"
+	configvalues "github.com/hyperledger/fabric/common/configvalues"
+	configvalueschannel "github.com/hyperledger/fabric/common/configvalues/channel"
 	"github.com/hyperledger/fabric/common/policies"
 	"github.com/hyperledger/fabric/msp"
 	cb "github.com/hyperledger/fabric/protos/common"
@@ -51,7 +52,7 @@ type Resources interface {
 	PolicyManager() policies.Manager
 
 	// ChannelConfig returns the ChannelConfig for the chain
-	ChannelConfig() configvalues.Channel
+	ChannelConfig() configvalueschannel.ConfigReader
 
 	// OrdererConfig returns the configtxorderer.SharedConfig for the channel
 	OrdererConfig() configvalues.Orderer

common/configtx/config.go

@@ -20,7 +20,7 @@ import (
 	"fmt"
 
 	"github.com/hyperledger/fabric/common/configtx/api"
-	configvaluesapi "github.com/hyperledger/fabric/common/configvalues/api"
+	configvaluesapi "github.com/hyperledger/fabric/common/configvalues"
 	"github.com/hyperledger/fabric/common/policies"
 	cb "github.com/hyperledger/fabric/protos/common"
 )

common/configtx/initializer.go

@@ -21,8 +21,8 @@ import (
 
 	"github.com/hyperledger/fabric/common/cauthdsl"
 	"github.com/hyperledger/fabric/common/configtx/api"
-	configvaluesapi "github.com/hyperledger/fabric/common/configvalues/api"
-	configtxchannel "github.com/hyperledger/fabric/common/configvalues/channel"
+	configvaluesapi "github.com/hyperledger/fabric/common/configvalues"
+	configvalueschannel "github.com/hyperledger/fabric/common/configvalues/channel"
 	configtxapplication "github.com/hyperledger/fabric/common/configvalues/channel/application"
 	configtxorderer "github.com/hyperledger/fabric/common/configvalues/channel/orderer"
 	configtxmsp "github.com/hyperledger/fabric/common/configvalues/msp"
@@ -33,7 +33,7 @@ import (
 
 type resources struct {
 	policyManager     *policies.ManagerImpl
-	channelConfig     *configtxchannel.SharedConfigImpl
+	channelConfig     *configvalueschannel.Config
 	ordererConfig     *configtxorderer.ManagerImpl
 	applicationConfig *configtxapplication.SharedConfigImpl
 	mspConfigHandler  *configtxmsp.MSPConfigHandler
@@ -45,7 +45,7 @@ func (r *resources) PolicyManager() policies.Manager {
 }
 
 // ChannelConfig returns the api.ChannelConfig for the chain
-func (r *resources) ChannelConfig() configvaluesapi.Channel {
+func (r *resources) ChannelConfig() configvalueschannel.ConfigReader {
 	return r.channelConfig
 }
 
@@ -85,15 +85,15 @@ func newResources() *resources {
 
 	return &resources{
 		policyManager:     policies.NewManagerImpl(RootGroupKey, policyProviderMap),
-		channelConfig:     configtxchannel.NewSharedConfigImpl(ordererConfig, applicationConfig),
+		channelConfig:     configvalueschannel.NewConfig(ordererConfig, applicationConfig),
 		ordererConfig:     ordererConfig,
 		applicationConfig: applicationConfig,
 		mspConfigHandler:  mspConfigHandler,
 	}
 }
 
 type valueProposerRoot struct {
-	channelConfig    *configtxchannel.SharedConfigImpl
+	channelConfig    *configvalueschannel.Config
 	mspConfigHandler *configtxmsp.MSPConfigHandler
 }
 

common/configvalues/api/api.go → common/configvalues/api.go

@@ -24,20 +24,6 @@ import (
 	pb "github.com/hyperledger/fabric/protos/peer"
 )
 
-// Channel stores the common channel config
-type Channel interface {
-	// HashingAlgorithm returns the default algorithm to be used when hashing
-	// such as computing block hashes, and CreationPolicy digests
-	HashingAlgorithm() func(input []byte) []byte
-
-	// BlockDataHashingStructureWidth returns the width to use when constructing the
-	// Merkle tree to compute the BlockData hash
-	BlockDataHashingStructureWidth() uint32
-
-	// OrdererAddresses returns the list of valid orderer addresses to connect to to invoke Broadcast/Deliver
-	OrdererAddresses() []string
-}
-
 // Org stores the common organizational config
 type Org interface {
 	// Name returns the name this org is referred to in config

common/configvalues/channel/application/organization.go

@@ -19,7 +19,7 @@ package application
 import (
 	"fmt"
 
-	"github.com/hyperledger/fabric/common/configvalues/api"
+	"github.com/hyperledger/fabric/common/configvalues"
 	"github.com/hyperledger/fabric/common/configvalues/channel/common/organization"
 	mspconfig "github.com/hyperledger/fabric/common/configvalues/msp"
 	cb "github.com/hyperledger/fabric/protos/common"

common/configvalues/channel/application/organization_test.go

@@ -19,7 +19,7 @@ package application
 import (
 	"testing"
 
-	"github.com/hyperledger/fabric/common/configvalues/api"
+	api "github.com/hyperledger/fabric/common/configvalues"
 	cb "github.com/hyperledger/fabric/protos/common"
 	pb "github.com/hyperledger/fabric/protos/peer"
 

common/configvalues/channel/application/sharedconfig.go

@@ -17,7 +17,7 @@ limitations under the License.
 package application
 
 import (
-	"github.com/hyperledger/fabric/common/configvalues/api"
+	api "github.com/hyperledger/fabric/common/configvalues"
 	"github.com/hyperledger/fabric/common/configvalues/channel/common/organization"
 	"github.com/hyperledger/fabric/common/configvalues/msp"
 	cb "github.com/hyperledger/fabric/protos/common"

common/configvalues/channel/application/sharedconfig_test.go

@@ -19,7 +19,7 @@ package application
 import (
 	"testing"
 
-	"github.com/hyperledger/fabric/common/configvalues/api"
+	api "github.com/hyperledger/fabric/common/configvalues"
 
 	logging "github.com/op/go-logging"
 )

common/configvalues/channel/common/organization/organization.go

@@ -19,7 +19,7 @@ package organization
 import (
 	"fmt"
 
-	"github.com/hyperledger/fabric/common/configvalues/api"
+	"github.com/hyperledger/fabric/common/configvalues"
 	mspconfig "github.com/hyperledger/fabric/common/configvalues/msp"
 	"github.com/hyperledger/fabric/msp"
 	cb "github.com/hyperledger/fabric/protos/common"

common/configvalues/channel/common/organization/organization_test.go

@@ -19,7 +19,7 @@ package organization
 import (
 	"testing"
 
-	"github.com/hyperledger/fabric/common/configvalues/api"
+	api "github.com/hyperledger/fabric/common/configvalues"
 
 	logging "github.com/op/go-logging"
 )

common/configvalues/channel/config.go

@@ -20,7 +20,7 @@ import (
 	"fmt"
 	"math"
 
-	"github.com/hyperledger/fabric/common/configvalues/api"
+	api "github.com/hyperledger/fabric/common/configvalues"
 	"github.com/hyperledger/fabric/common/configvalues/channel/application"
 	"github.com/hyperledger/fabric/common/configvalues/channel/orderer"
 	"github.com/hyperledger/fabric/common/util"
@@ -45,7 +45,7 @@ var Schema = &cb.ConfigGroupSchema{
 	},
 }
 
-// Chain config keys
+// Channel config keys
 const (
 	// HashingAlgorithmKey is the cb.ConfigItem type key name for the HashingAlgorithm message
 	HashingAlgorithmKey = "HashingAlgorithm"
@@ -63,87 +63,100 @@ const (
 	SHA3Shake256 = "SHAKE256"
 )
 
-var logger = logging.MustGetLogger("configtx/handlers/chainconfig")
+var logger = logging.MustGetLogger("configvalues/channel")
 
-type chainConfig struct {
+type ConfigReader interface {
+	// HashingAlgorithm returns the default algorithm to be used when hashing
+	// such as computing block hashes, and CreationPolicy digests
+	HashingAlgorithm() func(input []byte) []byte
+
+	// BlockDataHashingStructureWidth returns the width to use when constructing the
+	// Merkle tree to compute the BlockData hash
+	BlockDataHashingStructureWidth() uint32
+
+	// OrdererAddresses returns the list of valid orderer addresses to connect to to invoke Broadcast/Deliver
+	OrdererAddresses() []string
+}
+
+type values struct {
 	hashingAlgorithm               func(input []byte) []byte
 	blockDataHashingStructureWidth uint32
 	ordererAddresses               []string
 }
 
 // SharedConfigImpl is an implementation of Manager and configtx.ConfigHandler
 // In general, it should only be referenced as an Impl for the configtx.Manager
-type SharedConfigImpl struct {
-	pendingConfig *chainConfig
-	config        *chainConfig
+type Config struct {
+	pending *values
+	current *values
 
 	ordererConfig     *orderer.ManagerImpl
 	applicationConfig *application.SharedConfigImpl
 }
 
 // NewSharedConfigImpl creates a new SharedConfigImpl with the given CryptoHelper
-func NewSharedConfigImpl(ordererConfig *orderer.ManagerImpl, applicationConfig *application.SharedConfigImpl) *SharedConfigImpl {
-	return &SharedConfigImpl{
-		config:            &chainConfig{},
+func NewConfig(ordererConfig *orderer.ManagerImpl, applicationConfig *application.SharedConfigImpl) *Config {
+	return &Config{
+		current:           &values{},
 		ordererConfig:     ordererConfig,
 		applicationConfig: applicationConfig,
 	}
 }
 
 // HashingAlgorithm returns a function pointer to the chain hashing algorihtm
-func (pm *SharedConfigImpl) HashingAlgorithm() func(input []byte) []byte {
-	return pm.config.hashingAlgorithm
+func (c *Config) HashingAlgorithm() func(input []byte) []byte {
+	return c.current.hashingAlgorithm
 }
 
 // BlockDataHashingStructure returns the width to use when forming the block data hashing structure
-func (pm *SharedConfigImpl) BlockDataHashingStructureWidth() uint32 {
-	return pm.config.blockDataHashingStructureWidth
+func (c *Config) BlockDataHashingStructureWidth() uint32 {
+	return c.current.blockDataHashingStructureWidth
 }
 
 // OrdererAddresses returns the list of valid orderer addresses to connect to to invoke Broadcast/Deliver
-func (pm *SharedConfigImpl) OrdererAddresses() []string {
-	return pm.config.ordererAddresses
+func (c *Config) OrdererAddresses() []string {
+	return c.current.ordererAddresses
 }
 
 // BeginValueProposals is used to start a new config proposal
-func (pm *SharedConfigImpl) BeginValueProposals(groups []string) ([]api.ValueProposer, error) {
+func (c *Config) BeginValueProposals(groups []string) ([]api.ValueProposer, error) {
 	handlers := make([]api.ValueProposer, len(groups))
 
 	for i, group := range groups {
 		switch group {
 		case application.GroupKey:
-			handlers[i] = pm.applicationConfig
+			handlers[i] = c.applicationConfig
 		case orderer.GroupKey:
-			handlers[i] = pm.ordererConfig
+			handlers[i] = c.ordererConfig
 		default:
 			return nil, fmt.Errorf("Disallowed channel group: %s", group)
 		}
 	}
 
-	if pm.pendingConfig != nil {
+	if c.pending != nil {
 		logger.Panicf("Programming error, cannot call begin in the middle of a proposal")
 	}
 
-	pm.pendingConfig = &chainConfig{}
+	c.pending = &values{}
 	return handlers, nil
 }
 
 // RollbackProposals is used to abandon a new config proposal
-func (pm *SharedConfigImpl) RollbackProposals() {
-	pm.pendingConfig = nil
+func (c *Config) RollbackProposals() {
+	c.pending = nil
 }
 
 // CommitProposals is used to commit a new config proposal
-func (pm *SharedConfigImpl) CommitProposals() {
-	if pm.pendingConfig == nil {
+func (c *Config) CommitProposals() {
+	if c.pending == nil {
 		logger.Panicf("Programming error, cannot call commit without an existing proposal")
 	}
-	pm.config = pm.pendingConfig
-	pm.pendingConfig = nil
+	c.current = c.pending
+	c.pending = nil
 }
 
 // ProposeValue is used to add new config to the config proposal
-func (pm *SharedConfigImpl) ProposeValue(key string, configValue *cb.ConfigValue) error {
+func (c *Config) ProposeValue(key string, configValue *cb.ConfigValue) error {
 	switch key {
 	case HashingAlgorithmKey:
 		hashingAlgorithm := &cb.HashingAlgorithm{}
@@ -152,7 +165,7 @@ func (pm *SharedConfigImpl) ProposeValue(key string, configValue *cb.ConfigValue
 		}
 		switch hashingAlgorithm.Name {
 		case SHA3Shake256:
-			pm.pendingConfig.hashingAlgorithm = util.ComputeCryptoHash
+			c.pending.hashingAlgorithm = util.ComputeCryptoHash
 		default:
 			return fmt.Errorf("Unknown hashing algorithm type: %s", hashingAlgorithm.Name)
 		}
@@ -166,13 +179,13 @@ func (pm *SharedConfigImpl) ProposeValue(key string, configValue *cb.ConfigValue
 			return fmt.Errorf("BlockDataHashStructure width only supported at MaxUint32 in this version")
 		}
 
-		pm.pendingConfig.blockDataHashingStructureWidth = blockDataHashingStructure.Width
+		c.pending.blockDataHashingStructureWidth = blockDataHashingStructure.Width
 	case OrdererAddressesKey:
 		ordererAddresses := &cb.OrdererAddresses{}
 		if err := proto.Unmarshal(configValue.Value, ordererAddresses); err != nil {
 			return fmt.Errorf("Unmarshaling error for HashingAlgorithm: %s", err)
 		}
-		pm.pendingConfig.ordererAddresses = ordererAddresses.Addresses
+		c.pending.ordererAddresses = ordererAddresses.Addresses
 	default:
 		logger.Warningf("Uknown Chain config item with key %s", key)
 	}

common/configvalues/channel/config_test.go

@@ -20,7 +20,6 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/hyperledger/fabric/common/configvalues/api"
 	cb "github.com/hyperledger/fabric/protos/common"
 
 	logging "github.com/op/go-logging"
@@ -44,7 +43,7 @@ func makeInvalidConfigValue() *cb.ConfigValue {
 }
 
 func TestInterface(t *testing.T) {
-	_ = api.Channel(NewSharedConfigImpl(nil, nil))
+	_ = ConfigReader(NewConfig(nil, nil))
 }
 
 func TestDoubleBegin(t *testing.T) {
@@ -54,7 +53,7 @@ func TestDoubleBegin(t *testing.T) {
 		}
 	}()
 
-	m := NewSharedConfigImpl(nil, nil)
+	m := NewConfig(nil, nil)
 	m.BeginValueProposals(nil)
 	m.BeginValueProposals(nil)
 }
@@ -66,15 +65,15 @@ func TestCommitWithoutBegin(t *testing.T) {
 		}
 	}()
 
-	m := NewSharedConfigImpl(nil, nil)
+	m := NewConfig(nil, nil)
 	m.CommitProposals()
 }
 
 func TestRollback(t *testing.T) {
-	m := NewSharedConfigImpl(nil, nil)
-	m.pendingConfig = &chainConfig{}
+	m := NewConfig(nil, nil)
+	m.pending = &values{}
 	m.RollbackProposals()
-	if m.pendingConfig != nil {
+	if m.pending != nil {
 		t.Fatalf("Should have cleared pending config on rollback")
 	}
 }
@@ -84,7 +83,7 @@ func TestHashingAlgorithm(t *testing.T) {
 	invalidAlgorithm := TemplateHashingAlgorithm("MD5")
 	validAlgorithm := DefaultHashingAlgorithm()
 
-	m := NewSharedConfigImpl(nil, nil)
+	m := NewConfig(nil, nil)
 	m.BeginValueProposals(nil)
 
 	err := m.ProposeValue(HashingAlgorithmKey, invalidMessage)
@@ -114,7 +113,7 @@ func TestBlockDataHashingStructure(t *testing.T) {
 	invalidWidth := TemplateBlockDataHashingStructure(0)
 	validWidth := DefaultBlockDataHashingStructure()
 
-	m := NewSharedConfigImpl(nil, nil)
+	m := NewConfig(nil, nil)
 	m.BeginValueProposals(nil)
 
 	err := m.ProposeValue(BlockDataHashingStructureKey, invalidMessage)
@@ -142,7 +141,7 @@ func TestBlockDataHashingStructure(t *testing.T) {
 func TestOrdererAddresses(t *testing.T) {
 	invalidMessage := makeInvalidConfigValue()
 	validMessage := DefaultOrdererAddresses()
-	m := NewSharedConfigImpl(nil, nil)
+	m := NewConfig(nil, nil)
 	m.BeginValueProposals(nil)
 
 	err := m.ProposeValue(OrdererAddressesKey, invalidMessage)