From bc2923b965441ba140f2ba1b996d0acce0453995 Mon Sep 17 00:00:00 2001 From: Jason Yellick Date: Mon, 20 Feb 2017 15:22:40 -0500 Subject: [PATCH] [FAB-2391] Create common config Proposer https://jira.hyperledger.org/browse/FAB-2391 Today, each config handler implements the Propose/Commit/Rollback API on its own, which has led to a lot of duplicated code. This also means that each config handler must handle its own proto unmarshaling and error handling which makes inspecting the config from a human perspective cumbersome. This is the first step in mapping the config to/from a human readable form. Change-Id: I1dfd595b1e8be2a63ef82cb0b91616a932d4e83b Signed-off-by: Jason Yellick --- common/configvalues/api.go | 4 +- .../channel/application/organization.go | 2 +- .../common/organization/organization.go | 2 +- common/configvalues/root/proposer.go | 138 +++++++++++++++ common/configvalues/root/proposer_test.go | 162 ++++++++++++++++++ 5 files changed, 305 insertions(+), 3 deletions(-) create mode 100644 common/configvalues/root/proposer.go create mode 100644 common/configvalues/root/proposer_test.go diff --git a/common/configvalues/api.go b/common/configvalues/api.go index 3d64f139d6b..c9648a16784 100644 --- a/common/configvalues/api.go +++ b/common/configvalues/api.go @@ -14,7 +14,9 @@ See the License for the specific language governing permissions and limitations under the License. */ -package api +// Note, the directory is still configvalues, but this is stuttery and config +// is a more accurate and better name, TODO, update directory +package config import ( "time" diff --git a/common/configvalues/channel/application/organization.go b/common/configvalues/channel/application/organization.go index 78fa530e23c..2f6e6574b87 100644 --- a/common/configvalues/channel/application/organization.go +++ b/common/configvalues/channel/application/organization.go @@ -63,7 +63,7 @@ func (oc *ApplicationOrgConfig) AnchorPeers() []*pb.AnchorPeer { } // BeginValueProposals is used to start a new config proposal -func (oc *ApplicationOrgConfig) BeginValueProposals(groups []string) ([]api.ValueProposer, error) { +func (oc *ApplicationOrgConfig) BeginValueProposals(groups []string) ([]config.ValueProposer, error) { logger.Debugf("Beginning a possible new org config") if len(groups) != 0 { return nil, fmt.Errorf("ApplicationGroup does not support subgroups") diff --git a/common/configvalues/channel/common/organization/organization.go b/common/configvalues/channel/common/organization/organization.go index eb0150ae65a..f4863532d26 100644 --- a/common/configvalues/channel/common/organization/organization.go +++ b/common/configvalues/channel/common/organization/organization.go @@ -62,7 +62,7 @@ func NewOrgConfig(id string, mspConfig *mspconfig.MSPConfigHandler) *OrgConfig { } // BeginValueProposals is used to start a new config proposal -func (oc *OrgConfig) BeginValueProposals(groups []string) ([]api.ValueProposer, error) { +func (oc *OrgConfig) BeginValueProposals(groups []string) ([]config.ValueProposer, error) { logger.Debugf("Beginning a possible new org config") if len(groups) != 0 { return nil, fmt.Errorf("Orgs do not support sub-groups") diff --git a/common/configvalues/root/proposer.go b/common/configvalues/root/proposer.go new file mode 100644 index 00000000000..684575343aa --- /dev/null +++ b/common/configvalues/root/proposer.go @@ -0,0 +1,138 @@ +/* +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 config + +import ( + "fmt" + + api "github.com/hyperledger/fabric/common/configvalues" + cb "github.com/hyperledger/fabric/protos/common" + + "github.com/golang/protobuf/proto" + logging "github.com/op/go-logging" +) + +var logger = logging.MustGetLogger("common/config") + +// Values defines a mechanism to supply messages to unamrshal from config +// and a mechanism to validate the results +type Values interface { + // ProtoMsg behaves like a map lookup for key + ProtoMsg(key string) (proto.Message, bool) + + // Validate should ensure that the values set into the proto messages are correct + Validate() error + + // Commit should call back into the Value handler to update the config + Commit() +} + +// Handler +type Handler interface { + Allocate() Values + NewGroup(name string) (api.ValueProposer, error) +} + +type config struct { + allocated Values + groups map[string]api.ValueProposer +} + +type Proposer struct { + vh Handler + current *config + pending *config +} + +func NewProposer(vh Handler) *Proposer { + return &Proposer{ + vh: vh, + } +} + +// BeginValueProposals called when a config proposal is begun +func (p *Proposer) BeginValueProposals(groups []string) ([]api.ValueProposer, error) { + if p.pending != nil { + logger.Panicf("Duplicated BeginValueProposals without Rollback or Commit") + } + + result := make([]api.ValueProposer, len(groups)) + + p.pending = &config{ + allocated: p.vh.Allocate(), + groups: make(map[string]api.ValueProposer), + } + + for i, groupName := range groups { + var group api.ValueProposer + var ok bool + + if p.current == nil { + ok = false + } else { + group, ok = p.current.groups[groupName] + } + + if !ok { + var err error + group, err = p.vh.NewGroup(groupName) + if err != nil { + p.pending = nil + return nil, fmt.Errorf("Error creating group %s: %s", groupName, err) + } + } + + p.pending.groups[groupName] = group + result[i] = group + } + + return result, nil +} + +// ProposeValue called when config is added to a proposal +func (p *Proposer) ProposeValue(key string, configValue *cb.ConfigValue) error { + msg, ok := p.pending.allocated.ProtoMsg(key) + if !ok { + return fmt.Errorf("Unknown value key: %s", key) + } + + if err := proto.Unmarshal(configValue.Value, msg); err != nil { + return fmt.Errorf("Error unmarshaling key to proto message: %s", err) + } + + return nil +} + +// Validate ensures that the new config values is a valid change +func (p *Proposer) PreCommit() error { + return p.pending.allocated.Validate() +} + +// RollbackProposals called when a config proposal is abandoned +func (p *Proposer) RollbackProposals() { + p.pending = nil +} + +// CommitProposals called when a config proposal is committed +func (p *Proposer) CommitProposals() { + if p.pending == nil { + logger.Panicf("Attempted to commit with no pending values (indicates no Begin invoked)") + } + p.current = p.pending + p.current.allocated.Commit() + p.pending = nil +} diff --git a/common/configvalues/root/proposer_test.go b/common/configvalues/root/proposer_test.go new file mode 100644 index 00000000000..dba004f8b80 --- /dev/null +++ b/common/configvalues/root/proposer_test.go @@ -0,0 +1,162 @@ +/* +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 config + +import ( + "fmt" + "testing" + + api "github.com/hyperledger/fabric/common/configvalues" + cb "github.com/hyperledger/fabric/protos/common" + "github.com/hyperledger/fabric/protos/utils" + + "github.com/golang/protobuf/proto" + logging "github.com/op/go-logging" + "github.com/stretchr/testify/assert" +) + +func init() { + logging.SetLevel(logging.DEBUG, "") +} + +type mockValues struct { + ProtoMsgMap map[string]proto.Message + ValidateReturn error +} + +func (v *mockValues) ProtoMsg(key string) (proto.Message, bool) { + msg, ok := v.ProtoMsgMap[key] + return msg, ok +} + +func (v *mockValues) Validate() error { + return v.ValidateReturn +} + +func (v *mockValues) Commit() {} + +func newMockValues() *mockValues { + return &mockValues{ + ProtoMsgMap: make(map[string]proto.Message), + } +} + +type mockHandler struct { + AllocateReturn *mockValues + NewGroupMap map[string]api.ValueProposer + NewGroupError error +} + +func (h *mockHandler) Allocate() Values { + return h.AllocateReturn +} + +func (h *mockHandler) NewGroup(name string) (api.ValueProposer, error) { + group, ok := h.NewGroupMap[name] + if !ok { + return nil, fmt.Errorf("Missing group implies error") + } + return group, nil +} + +func newMockHandler() *mockHandler { + return &mockHandler{ + AllocateReturn: newMockValues(), + NewGroupMap: make(map[string]api.ValueProposer), + } +} + +func TestDoubleBegin(t *testing.T) { + p := NewProposer(&mockHandler{AllocateReturn: &mockValues{}}) + p.BeginValueProposals(nil) + assert.Panics(t, func() { p.BeginValueProposals(nil) }, "Two begins back to back should have caused a panic") +} + +func TestCommitWithoutBegin(t *testing.T) { + p := NewProposer(&mockHandler{AllocateReturn: &mockValues{}}) + assert.Panics(t, func() { p.CommitProposals() }, "Commit without begin should have caused a panic") +} + +func TestRollback(t *testing.T) { + p := NewProposer(&mockHandler{AllocateReturn: &mockValues{}}) + p.pending = &config{} + p.RollbackProposals() + assert.Nil(t, p.pending, "Should have cleared pending config on rollback") +} + +func TestGoodKeys(t *testing.T) { + mh := newMockHandler() + mh.AllocateReturn.ProtoMsgMap["Envelope"] = &cb.Envelope{} + mh.AllocateReturn.ProtoMsgMap["Payload"] = &cb.Payload{} + + p := NewProposer(mh) + _, err := p.BeginValueProposals(nil) + assert.NoError(t, err) + + env := &cb.Envelope{Payload: []byte("SOME DATA")} + pay := &cb.Payload{Data: []byte("SOME OTHER DATA")} + + assert.NoError(t, p.ProposeValue("Envelope", &cb.ConfigValue{Value: utils.MarshalOrPanic(env)})) + assert.NoError(t, p.ProposeValue("Payload", &cb.ConfigValue{Value: utils.MarshalOrPanic(pay)})) + + assert.Equal(t, mh.AllocateReturn.ProtoMsgMap["Envelope"], env) + assert.Equal(t, mh.AllocateReturn.ProtoMsgMap["Payload"], pay) +} + +func TestBadMarshaling(t *testing.T) { + mh := newMockHandler() + mh.AllocateReturn.ProtoMsgMap["Envelope"] = &cb.Envelope{} + + p := NewProposer(mh) + _, err := p.BeginValueProposals(nil) + assert.NoError(t, err) + + assert.Error(t, p.ProposeValue("Envelope", &cb.ConfigValue{Value: []byte("GARBAGE")}), "Should have errored unmarshaling") +} + +func TestBadMissingMessage(t *testing.T) { + mh := newMockHandler() + mh.AllocateReturn.ProtoMsgMap["Payload"] = &cb.Payload{} + + p := NewProposer(mh) + _, err := p.BeginValueProposals(nil) + assert.NoError(t, err) + + assert.Error(t, p.ProposeValue("Envelope", &cb.ConfigValue{Value: utils.MarshalOrPanic(&cb.Envelope{})}), "Should have errored on unexpected message") +} + +func TestGroups(t *testing.T) { + mh := newMockHandler() + mh.NewGroupMap["foo"] = nil + mh.NewGroupMap["bar"] = nil + + p := NewProposer(mh) + _, err := p.BeginValueProposals([]string{"foo", "bar"}) + assert.NoError(t, err, "Both groups were present") + p.CommitProposals() + + mh.NewGroupMap = make(map[string]api.ValueProposer) + _, err = p.BeginValueProposals([]string{"foo", "bar"}) + assert.NoError(t, err, "Should not have tried to recreate the groups") + p.CommitProposals() + + _, err = p.BeginValueProposals([]string{"foo", "other"}) + assert.Error(t, err, "Should not have errored when trying to create 'other'") + + _, err = p.BeginValueProposals([]string{"foo"}) + assert.NoError(t, err, "Should be able to begin again without rolling back because of error") +}