From 6b649d132ed1099593b96a1d4e3cf48c0a3acfff Mon Sep 17 00:00:00 2001 From: Thibault Robert Date: Wed, 22 Jan 2020 14:35:03 +0100 Subject: [PATCH 1/5] Specify bad request error --- chaincode/common.go | 4 ++-- chaincode/compute_plan.go | 11 +++++------ chaincode/main.go | 2 +- chaincode/objective.go | 3 +-- chaincode/permissions.go | 6 ++---- chaincode/tuple.go | 3 +-- 6 files changed, 12 insertions(+), 17 deletions(-) diff --git a/chaincode/common.go b/chaincode/common.go index 612e4f84..29ec5c2c 100644 --- a/chaincode/common.go +++ b/chaincode/common.go @@ -15,7 +15,7 @@ package main import ( - "fmt" + "chaincode/errors" "strings" ) @@ -39,7 +39,7 @@ func queryFilter(db *LedgerDB, args []string) (elements interface{}, err error) "aggregatetuple~tag", } if !stringInSlice(inp.IndexName, validIndexNames) { - err = fmt.Errorf("invalid indexName filter query: %s", inp.IndexName) + err = errors.BadRequest("invalid indexName filter query: %s", inp.IndexName) return } indexName := inp.IndexName + "~key" diff --git a/chaincode/compute_plan.go b/chaincode/compute_plan.go index 45bc5f7b..fd9c005e 100644 --- a/chaincode/compute_plan.go +++ b/chaincode/compute_plan.go @@ -16,7 +16,6 @@ package main import ( "chaincode/errors" - "fmt" "strconv" ) @@ -31,7 +30,7 @@ func (inpTraintuple *inputTraintuple) Fill(inpCP inputComputePlanTraintuple, tra for _, InModelID := range inpCP.InModelsIDs { inModelKey, ok := traintupleKeysByID[InModelID] if !ok { - return fmt.Errorf("model ID %s not found", InModelID) + return errors.BadRequest("model ID %s not found", InModelID) } inpTraintuple.InModels = append(inpTraintuple.InModels, inModelKey) } @@ -49,7 +48,7 @@ func (inpAggregatetuple *inputAggregatetuple) Fill(inpCP inputComputePlanAggrega for _, InModelID := range inpCP.InModelsIDs { inModelKey, ok := aggregatetupleKeysByID[InModelID] if !ok { - return fmt.Errorf("model ID %s not found", InModelID) + return errors.BadRequest("model ID %s not found", InModelID) } inpAggregatetuple.InModels = append(inpAggregatetuple.InModels, inModelKey) } @@ -70,14 +69,14 @@ func (inpCompositeTraintuple *inputCompositeTraintuple) Fill(inpCP inputComputeP var ok bool inpCompositeTraintuple.InHeadModelKey, ok = traintupleKeysByID[inpCP.InHeadModelID] if !ok { - return fmt.Errorf("head model ID %s not found", inpCP.InHeadModelID) + return errors.BadRequest("head model ID %s not found", inpCP.InHeadModelID) } } if inpCP.InTrunkModelID != "" { var ok bool inpCompositeTraintuple.InTrunkModelKey, ok = traintupleKeysByID[inpCP.InTrunkModelID] if !ok { - return fmt.Errorf("trunk model ID %s not found", inpCP.InTrunkModelID) + return errors.BadRequest("trunk model ID %s not found", inpCP.InTrunkModelID) } } return nil @@ -86,7 +85,7 @@ func (inpCompositeTraintuple *inputCompositeTraintuple) Fill(inpCP inputComputeP func (inpTesttuple *inputTesttuple) Fill(inpCP inputComputePlanTesttuple, traintupleKeysByID map[string]string) error { traintupleKey, ok := traintupleKeysByID[inpCP.TraintupleID] if !ok { - return fmt.Errorf("traintuple ID %s not found", inpCP.TraintupleID) + return errors.BadRequest("traintuple ID %s not found", inpCP.TraintupleID) } inpTesttuple.TraintupleKey = traintupleKey inpTesttuple.DataManagerKey = inpCP.DataManagerKey diff --git a/chaincode/main.go b/chaincode/main.go index c67d920a..c24d8c97 100644 --- a/chaincode/main.go +++ b/chaincode/main.go @@ -167,7 +167,7 @@ func (t *SubstraChaincode) Invoke(stub shim.ChaincodeStubInterface) peer.Respons case "queryNodes": result, err = queryNodes(db, args) default: - err = fmt.Errorf("function \"%s\" not implemented", fn) + err = errors.BadRequest("function \"%s\" not implemented", fn) } logger.Infof("Response from chaincode: %#v, error: %s", result, err) // Return the result as success payload diff --git a/chaincode/objective.go b/chaincode/objective.go index e102a3af..5dde6d4c 100644 --- a/chaincode/objective.go +++ b/chaincode/objective.go @@ -16,7 +16,6 @@ package main import ( "chaincode/errors" - "fmt" "sort" ) @@ -116,7 +115,7 @@ func queryObjective(db *LedgerDB, args []string) (out outputObjective, err error func queryObjectives(db *LedgerDB, args []string) (outObjectives []outputObjective, err error) { outObjectives = []outputObjective{} if len(args) != 0 { - err = fmt.Errorf("incorrect number of arguments, expecting nothing") + err = errors.BadRequest("incorrect number of arguments, expecting nothing") return } elementsKeys, err := db.GetIndexKeys("objective~owner~key", []string{"objective"}) diff --git a/chaincode/permissions.go b/chaincode/permissions.go index 189f5175..450e373b 100644 --- a/chaincode/permissions.go +++ b/chaincode/permissions.go @@ -14,9 +14,7 @@ package main -import ( - "fmt" -) +import "chaincode/errors" // Permission represents one permission based on an action type type Permission struct { @@ -73,7 +71,7 @@ func NewPermissions(db *LedgerDB, in inputPermissions) (Permissions, error) { } if !stringInSlice(authorizedID, nodesIDs) { - return Permissions{}, fmt.Errorf("invalid permission input values") + return Permissions{}, errors.BadRequest("invalid permission input values") } } diff --git a/chaincode/tuple.go b/chaincode/tuple.go index e8c225d2..2e6d4927 100644 --- a/chaincode/tuple.go +++ b/chaincode/tuple.go @@ -18,7 +18,6 @@ import ( "chaincode/errors" "crypto/sha256" "encoding/hex" - "fmt" "sort" ) @@ -174,7 +173,7 @@ func validateTupleOwner(db *LedgerDB, worker string) error { return err } if txCreator != worker { - return fmt.Errorf("%s is not allowed to update tuple (%s)", txCreator, worker) + return errors.Forbidden("%s is not allowed to update tuple (%s)", txCreator, worker) } return nil } From 44fe3861bc0fb7d034594c49363ccef7816a2a54 Mon Sep 17 00:00:00 2001 From: Thibault Robert Date: Wed, 22 Jan 2020 14:47:23 +0100 Subject: [PATCH 2/5] Use Internal when possible --- chaincode/compute_plan_dag.go | 6 +++--- chaincode/compute_plan_test.go | 3 +-- chaincode/data.go | 5 ++--- chaincode/ledger_db.go | 13 ++++++------- chaincode/main.go | 4 ++-- chaincode/output.go | 18 +++++++++--------- chaincode/output_aggregate.go | 6 +++--- chaincode/output_composite.go | 10 ++++------ chaincode/testtuple.go | 5 ++--- chaincode/traintuple.go | 17 ++++++++--------- chaincode/traintuple_composite.go | 5 ++--- chaincode/tuple_aggregate.go | 11 +++++------ 12 files changed, 47 insertions(+), 56 deletions(-) diff --git a/chaincode/compute_plan_dag.go b/chaincode/compute_plan_dag.go index 36a83225..8774e28e 100644 --- a/chaincode/compute_plan_dag.go +++ b/chaincode/compute_plan_dag.go @@ -1,6 +1,6 @@ package main -import "fmt" +import "chaincode/errors" // TrainingTask is a node of a ComputeDAG. It represents a training task // (i.e. a Traintuple, a CompositeTraintuple or an Aggregatetuple) @@ -75,7 +75,7 @@ func (dag *ComputeDAG) sort() error { current[i].Depth = depth final = append(final, current[i]) if _, ok := IDPresents[current[i].ID]; ok { - return fmt.Errorf("compute plan error: Duplicate training task ID: %s", current[i].ID) + return errors.Internal("compute plan error: Duplicate training task ID: %s", current[i].ID) } IDPresents[current[i].ID] = current[i].Depth } else { @@ -90,7 +90,7 @@ func (dag *ComputeDAG) sort() error { for _, c := range current { errorIDs = append(errorIDs, c.ID) } - return fmt.Errorf("compute plan error: Cyclic or missing dependency among inModels IDs: %v", errorIDs) + return errors.Internal("compute plan error: Cyclic or missing dependency among inModels IDs: %v", errorIDs) } i = 0 current = temp diff --git a/chaincode/compute_plan_test.go b/chaincode/compute_plan_test.go index 0e87a7de..347786b6 100644 --- a/chaincode/compute_plan_test.go +++ b/chaincode/compute_plan_test.go @@ -15,7 +15,6 @@ package main import ( - "fmt" "testing" "github.com/stretchr/testify/assert" @@ -240,7 +239,7 @@ func validateTupleRank(t *testing.T, db *LedgerDB, expectedRank int, key string, assert.NoError(t, err) rank = tuple.Rank default: - assert.NoError(t, fmt.Errorf("not implemented: %v", assetType)) + t.Errorf("not implemented: %s", assetType) } assert.Equal(t, expectedRank, rank, "Rank for tuple of type %v with key \"%s\" should be %d", assetType, key, expectedRank) } diff --git a/chaincode/data.go b/chaincode/data.go index 7e33a7b7..263caa4f 100644 --- a/chaincode/data.go +++ b/chaincode/data.go @@ -16,7 +16,6 @@ package main import ( "chaincode/errors" - "fmt" "strconv" "strings" ) @@ -323,7 +322,7 @@ func queryDataset(db *LedgerDB, args []string) (outputDataset, error) { func queryDataSamples(db *LedgerDB, args []string) ([]outputDataSample, error) { outDataSamples := []outputDataSample{} if len(args) != 0 { - err := fmt.Errorf("incorrect number of arguments, expecting nothing") + err := errors.BadRequest("incorrect number of arguments, expecting nothing") return outDataSamples, err } elementsKeys, err := db.GetIndexKeys("dataSample~dataManager~key", []string{"dataSample"}) @@ -393,7 +392,7 @@ func checkSameDataManager(db *LedgerDB, dataManagerKey string, dataSampleKeys [] return testOnly, trainOnly, err } if !stringInSlice(dataManagerKey, dataSample.DataManagerKeys) { - err = fmt.Errorf("dataSample do not belong to the same dataManager") + err = errors.BadRequest("dataSample do not belong to the same dataManager") return testOnly, trainOnly, err } testOnly = testOnly && dataSample.TestOnly diff --git a/chaincode/ledger_db.go b/chaincode/ledger_db.go index b3a67204..1574c82a 100644 --- a/chaincode/ledger_db.go +++ b/chaincode/ledger_db.go @@ -17,7 +17,6 @@ package main import ( "chaincode/errors" "encoding/json" - "fmt" "sync" "github.com/hyperledger/fabric/core/chaincode/shim" @@ -129,11 +128,11 @@ func (db *LedgerDB) Add(key string, object interface{}) error { func (db *LedgerDB) CreateIndex(index string, attributes []string) error { compositeKey, err := db.cc.CreateCompositeKey(index, attributes) if err != nil { - return fmt.Errorf("cannot create index %s: %s", index, err.Error()) + return errors.Internal("cannot create index %s: %s", index, err.Error()) } value := []byte{0x00} if err = db.cc.PutState(compositeKey, value); err != nil { - return fmt.Errorf("cannot create index %s: %s", index, err.Error()) + return errors.Internal("cannot create index %s: %s", index, err.Error()) } return nil } @@ -160,7 +159,7 @@ func (db *LedgerDB) GetIndexKeys(index string, attributes []string) ([]string, e keys := make([]string, 0) iterator, err := db.cc.GetStateByPartialCompositeKey(index, attributes) if err != nil { - return nil, fmt.Errorf("get index %s failed: %s", index, err.Error()) + return nil, errors.Internal("get index %s failed: %s", index, err.Error()) } defer iterator.Close() for iterator.HasNext() { @@ -170,7 +169,7 @@ func (db *LedgerDB) GetIndexKeys(index string, attributes []string) ([]string, e } _, keyParts, err := db.cc.SplitCompositeKey(compositeKey.Key) if err != nil { - return nil, fmt.Errorf("get index %s failed: cannot split key %s: %s", index, compositeKey.Key, err.Error()) + return nil, errors.Internal("get index %s failed: cannot split key %s: %s", index, compositeKey.Key, err.Error()) } keys = append(keys, keyParts[len(keyParts)-1]) } @@ -393,7 +392,7 @@ func (db *LedgerDB) GetOutModelHashDress(traintupleKey string, modelType Composi case TrunkType: return tuple.OutTrunkModel.OutModel, nil default: - return nil, fmt.Errorf("GetOutModelHashDress: Unsupported composite model type %s", modelType) + return nil, errors.Internal("GetOutModelHashDress: Unsupported composite model type %s", modelType) } case TraintupleType: @@ -407,7 +406,7 @@ func (db *LedgerDB) GetOutModelHashDress(traintupleKey string, modelType Composi return tuple.OutModel, nil } default: - return nil, fmt.Errorf("GetOutModelHashDress: Unsupported asset type %s", assetType) + return nil, errors.Internal("GetOutModelHashDress: Unsupported asset type %s", assetType) } } diff --git a/chaincode/main.go b/chaincode/main.go index c24d8c97..6c9c2aef 100644 --- a/chaincode/main.go +++ b/chaincode/main.go @@ -178,12 +178,12 @@ func (t *SubstraChaincode) Invoke(stub shim.ChaincodeStubInterface) peer.Respons // one event per call err = db.SendTuplesEvent() if err != nil { - return formatErrorResponse(fmt.Errorf("could not send event: %s", err.Error())) + return formatErrorResponse(errors.Internal("could not send event: %s", err.Error())) } // Marshal to json the smartcontract result resp, err := json.Marshal(result) if err != nil { - return formatErrorResponse(fmt.Errorf("could not format response: %s", err.Error())) + return formatErrorResponse(errors.Internal("could not format response: %s", err.Error())) } return shim.Success(resp) diff --git a/chaincode/output.go b/chaincode/output.go index bd6c7c4c..c2875590 100644 --- a/chaincode/output.go +++ b/chaincode/output.go @@ -15,7 +15,7 @@ package main import ( - "fmt" + "chaincode/errors" "math" ) @@ -151,7 +151,7 @@ func (outputTraintuple *outputTraintuple) Fill(db *LedgerDB, traintuple Traintup // fill algo algo, err := db.GetAlgo(traintuple.AlgoKey) if err != nil { - err = fmt.Errorf("could not retrieve algo with key %s - %s", traintuple.AlgoKey, err.Error()) + err = errors.Internal("could not retrieve algo with key %s - %s", traintuple.AlgoKey, err.Error()) return } outputTraintuple.Algo = &HashDressName{ @@ -166,7 +166,7 @@ func (outputTraintuple *outputTraintuple) Fill(db *LedgerDB, traintuple Traintup } parentTraintuple, err := db.GetTraintuple(inModelKey) if err != nil { - return fmt.Errorf("could not retrieve parent traintuple with key %s - %s", inModelKey, err.Error()) + return errors.Internal("could not retrieve parent traintuple with key %s - %s", inModelKey, err.Error()) } inModel := &Model{ TraintupleKey: inModelKey, @@ -219,7 +219,7 @@ func (out *outputTesttuple) Fill(db *LedgerDB, key string, in Testtuple) error { // fill type traintupleType, err := db.GetAssetType(in.TraintupleKey) if err != nil { - return fmt.Errorf("could not retrieve traintuple type with key %s - %s", in.TraintupleKey, err.Error()) + return errors.Internal("could not retrieve traintuple type with key %s - %s", in.TraintupleKey, err.Error()) } out.TraintupleType = LowerFirst(traintupleType.String()) @@ -229,18 +229,18 @@ func (out *outputTesttuple) Fill(db *LedgerDB, key string, in Testtuple) error { case TraintupleType: algo, err = db.GetAlgo(in.AlgoKey) if err != nil { - return fmt.Errorf("could not retrieve algo with key %s - %s", in.AlgoKey, err.Error()) + return errors.Internal("could not retrieve algo with key %s - %s", in.AlgoKey, err.Error()) } case CompositeTraintupleType: compositeAlgo, err := db.GetCompositeAlgo(in.AlgoKey) if err != nil { - return fmt.Errorf("could not retrieve composite algo with key %s - %s", in.AlgoKey, err.Error()) + return errors.Internal("could not retrieve composite algo with key %s - %s", in.AlgoKey, err.Error()) } algo = compositeAlgo.Algo case AggregatetupleType: aggregateAlgo, err := db.GetAggregateAlgo(in.AlgoKey) if err != nil { - return fmt.Errorf("could not retrieve aggregate algo with key %s - %s", in.AlgoKey, err.Error()) + return errors.Internal("could not retrieve aggregate algo with key %s - %s", in.AlgoKey, err.Error()) } algo = aggregateAlgo.Algo } @@ -252,10 +252,10 @@ func (out *outputTesttuple) Fill(db *LedgerDB, key string, in Testtuple) error { // fill objective objective, err := db.GetObjective(in.ObjectiveKey) if err != nil { - return fmt.Errorf("could not retrieve associated objective with key %s- %s", in.ObjectiveKey, err.Error()) + return errors.Internal("could not retrieve associated objective with key %s- %s", in.ObjectiveKey, err.Error()) } if objective.Metrics == nil { - return fmt.Errorf("objective %s is missing metrics values", in.ObjectiveKey) + return errors.Internal("objective %s is missing metrics values", in.ObjectiveKey) } metrics := HashDress{ Hash: objective.Metrics.Hash, diff --git a/chaincode/output_aggregate.go b/chaincode/output_aggregate.go index 0107da0d..ef204294 100644 --- a/chaincode/output_aggregate.go +++ b/chaincode/output_aggregate.go @@ -14,7 +14,7 @@ package main -import "fmt" +import "chaincode/errors" type outputAggregatetuple struct { Key string `json:"key"` @@ -51,7 +51,7 @@ func (outputAggregatetuple *outputAggregatetuple) Fill(db *LedgerDB, traintuple outputAggregatetuple.Tag = traintuple.Tag algo, err := db.GetAggregateAlgo(traintuple.AlgoKey) if err != nil { - err = fmt.Errorf("could not retrieve aggregate algo with key %s - %s", traintuple.AlgoKey, err.Error()) + err = errors.Internal("could not retrieve aggregate algo with key %s - %s", traintuple.AlgoKey, err.Error()) return } outputAggregatetuple.Algo = &HashDressName{ @@ -66,7 +66,7 @@ func (outputAggregatetuple *outputAggregatetuple) Fill(db *LedgerDB, traintuple } hashDress, _err := db.GetOutModelHashDress(inModelKey, TrunkType, []AssetType{TraintupleType, CompositeTraintupleType, AggregatetupleType}) if _err != nil { - err = fmt.Errorf("could not fill in-model with key \"%s\": %s", inModelKey, _err.Error()) + err = errors.Internal("could not fill in-model with key \"%s\": %s", inModelKey, _err.Error()) return } inModel := &Model{ diff --git a/chaincode/output_composite.go b/chaincode/output_composite.go index 3c1392c8..0ac7934f 100644 --- a/chaincode/output_composite.go +++ b/chaincode/output_composite.go @@ -14,9 +14,7 @@ package main -import ( - "fmt" -) +import "chaincode/errors" type outputCompositeAlgo struct { outputAlgo @@ -62,7 +60,7 @@ func (outputCompositeTraintuple *outputCompositeTraintuple) Fill(db *LedgerDB, t // fill algo algo, err := db.GetCompositeAlgo(traintuple.AlgoKey) if err != nil { - err = fmt.Errorf("could not retrieve composite algo with key %s - %s", traintuple.AlgoKey, err.Error()) + err = errors.Internal("could not retrieve composite algo with key %s - %s", traintuple.AlgoKey, err.Error()) return } outputCompositeTraintuple.Algo = &HashDressName{ @@ -75,7 +73,7 @@ func (outputCompositeTraintuple *outputCompositeTraintuple) Fill(db *LedgerDB, t // Head can only be a composite traintuple's head out model hashDress, _err := db.GetOutModelHashDress(traintuple.InHeadModel, HeadType, []AssetType{CompositeTraintupleType}) if _err != nil { - err = fmt.Errorf("could not fill (head) in-model with key \"%s\": %s", traintuple.InHeadModel, _err.Error()) + err = errors.Internal("could not fill (head) in-model with key \"%s\": %s", traintuple.InHeadModel, _err.Error()) return } outputCompositeTraintuple.InHeadModel = &Model{ @@ -95,7 +93,7 @@ func (outputCompositeTraintuple *outputCompositeTraintuple) Fill(db *LedgerDB, t // - an aggregate tuple's out model hashDress, _err := db.GetOutModelHashDress(traintuple.InTrunkModel, TrunkType, []AssetType{TraintupleType, CompositeTraintupleType, AggregatetupleType}) if _err != nil { - err = fmt.Errorf("could not fill (trunk) in-model with key \"%s\": %s", traintuple.InTrunkModel, _err.Error()) + err = errors.Internal("could not fill (trunk) in-model with key \"%s\": %s", traintuple.InTrunkModel, _err.Error()) return } outputCompositeTraintuple.InTrunkModel = &Model{ diff --git a/chaincode/testtuple.go b/chaincode/testtuple.go index 18e84656..5cac8e77 100644 --- a/chaincode/testtuple.go +++ b/chaincode/testtuple.go @@ -16,7 +16,6 @@ package main import ( "chaincode/errors" - "fmt" "reflect" "sort" "strconv" @@ -448,7 +447,7 @@ func (testtuple *Testtuple) validateNewStatus(db *LedgerDB, status string) error // commitStatusUpdate update the testtuple status in the ledger func (testtuple *Testtuple) commitStatusUpdate(db *LedgerDB, testtupleKey string, newStatus string) error { if err := testtuple.validateNewStatus(db, newStatus); err != nil { - return fmt.Errorf("update testtuple %s failed: %s", testtupleKey, err.Error()) + return errors.Internal("update testtuple %s failed: %s", testtupleKey, err.Error()) } // do not update if previous status is already Done, Failed, Todo, Doing @@ -460,7 +459,7 @@ func (testtuple *Testtuple) commitStatusUpdate(db *LedgerDB, testtupleKey string testtuple.Status = newStatus if err := db.Put(testtupleKey, testtuple); err != nil { - return fmt.Errorf("failed to update testtuple status to %s with key %s", newStatus, testtupleKey) + return errors.Internal("failed to update testtuple status to %s with key %s", newStatus, testtupleKey) } // update associated composite key diff --git a/chaincode/traintuple.go b/chaincode/traintuple.go index 4d5e483d..2476dc51 100644 --- a/chaincode/traintuple.go +++ b/chaincode/traintuple.go @@ -16,7 +16,6 @@ package main import ( "chaincode/errors" - "fmt" "strconv" ) @@ -93,7 +92,7 @@ func (traintuple *Traintuple) SetFromParents(db *LedgerDB, inModels []string) er return errors.BadRequest(err, "could not retrieve parent traintuple with key %s", parentTraintupleKey) } if !typeInSlice(tuple.AssetType, []AssetType{TraintupleType, CompositeTraintupleType, AggregatetupleType}) { - return fmt.Errorf("aggregate.SetFromParents: Unsupported parent type %s", tuple.AssetType) + return errors.Internal("aggregate.SetFromParents: Unsupported parent type %s", tuple.AssetType) } parentStatuses = append(parentStatuses, tuple.Status) inModelKeys = append(inModelKeys, parentTraintupleKey) @@ -471,15 +470,15 @@ func UpdateTraintupleChildren(db *LedgerDB, traintupleKey string, traintupleStat // get traintuples having as inModels the input traintuple childTraintupleKeys, err := db.GetIndexKeys("traintuple~inModel~key", []string{"traintuple", traintupleKey}) if err != nil { - return fmt.Errorf("error while getting associated tuples to update their inModel, traintupleKey=%s traintupleStatus=%s %s", traintupleKey, traintupleStatus, err) + return errors.Internal("error while getting associated tuples to update their inModel, traintupleKey=%s traintupleStatus=%s %s", traintupleKey, traintupleStatus, err) } childCompositeTraintupleKeys, err := db.GetIndexKeys("compositeTraintuple~inModel~key", []string{"compositeTraintuple", traintupleKey}) if err != nil { - return fmt.Errorf("error while getting associated composite traintuples to update their inModel") + return errors.Internal("error while getting associated composite traintuples to update their inModel") } childAggregatetupleKeys, err := db.GetIndexKeys("aggregatetuple~inModel~key", []string{"aggregatetuple", traintupleKey}) if err != nil { - return fmt.Errorf("error while getting associated aggregate tuples to update their inModel") + return errors.Internal("error while getting associated aggregate tuples to update their inModel") } allChildKeys := append(append(childTraintupleKeys, childCompositeTraintupleKeys...), childAggregatetupleKeys...) @@ -499,7 +498,7 @@ func UpdateTraintupleChildren(db *LedgerDB, traintupleKey string, traintupleStat } if child.Status != StatusWaiting { - return fmt.Errorf("traintuple %s has invalid status : '%s' instead of waiting", childTraintupleKey, child.Status) + return errors.Internal("traintuple %s has invalid status : '%s' instead of waiting", childTraintupleKey, child.Status) } childTraintupleStatus := child.Status @@ -522,7 +521,7 @@ func UpdateTraintupleChildren(db *LedgerDB, traintupleKey string, traintupleStat return err } default: - return fmt.Errorf("Unknown child traintuple type: %s", child.AssetType) + return errors.Internal("Unknown child traintuple type: %s", child.AssetType) } alreadyUpdatedKeys = append(alreadyUpdatedKeys, childTraintupleKey) @@ -617,13 +616,13 @@ func (traintuple *Traintuple) commitStatusUpdate(db *LedgerDB, traintupleKey str } if err := traintuple.validateNewStatus(db, newStatus); err != nil { - return fmt.Errorf("update traintuple %s failed: %s", traintupleKey, err.Error()) + return errors.Internal("update traintuple %s failed: %s", traintupleKey, err.Error()) } oldStatus := traintuple.Status traintuple.Status = newStatus if err := db.Put(traintupleKey, traintuple); err != nil { - return fmt.Errorf("failed to update traintuple %s - %s", traintupleKey, err.Error()) + return errors.Internal("failed to update traintuple %s - %s", traintupleKey, err.Error()) } // update associated composite keys diff --git a/chaincode/traintuple_composite.go b/chaincode/traintuple_composite.go index ee380c3b..de45b2e9 100644 --- a/chaincode/traintuple_composite.go +++ b/chaincode/traintuple_composite.go @@ -16,7 +16,6 @@ package main import ( "chaincode/errors" - "fmt" "strconv" ) @@ -560,13 +559,13 @@ func (traintuple *CompositeTraintuple) commitStatusUpdate(db *LedgerDB, traintup } if err := traintuple.validateNewStatus(db, newStatus); err != nil { - return fmt.Errorf("update traintuple %s failed: %s", traintupleKey, err.Error()) + return errors.Internal("update traintuple %s failed: %s", traintupleKey, err.Error()) } oldStatus := traintuple.Status traintuple.Status = newStatus if err := db.Put(traintupleKey, traintuple); err != nil { - return fmt.Errorf("failed to update traintuple %s - %s", traintupleKey, err.Error()) + return errors.Internal("failed to update traintuple %s - %s", traintupleKey, err.Error()) } // update associated composite keys diff --git a/chaincode/tuple_aggregate.go b/chaincode/tuple_aggregate.go index 3277b4c3..6b458a44 100644 --- a/chaincode/tuple_aggregate.go +++ b/chaincode/tuple_aggregate.go @@ -16,7 +16,6 @@ package main import ( "chaincode/errors" - "fmt" "strconv" ) @@ -65,7 +64,7 @@ func (tuple *Aggregatetuple) SetFromParents(db *LedgerDB, inModels []string) err for _, parentTraintupleKey := range inModels { parentType, err := db.GetAssetType(parentTraintupleKey) if err != nil { - return fmt.Errorf("could not retrieve traintuple type with key %s - %s", parentTraintupleKey, err.Error()) + return errors.Internal("could not retrieve traintuple type with key %s - %s", parentTraintupleKey, err.Error()) } parentPermissions := Permissions{} @@ -92,11 +91,11 @@ func (tuple *Aggregatetuple) SetFromParents(db *LedgerDB, inModels []string) err parentStatuses = append(parentStatuses, tuple.Status) } default: - return fmt.Errorf("aggregate.SetFromParents: Unsupported parent type %s", parentType) + return errors.Internal("aggregate.SetFromParents: Unsupported parent type %s", parentType) } if err != nil { - return fmt.Errorf("could not retrieve traintuple type with key %s - %s", parentTraintupleKey, err.Error()) + return errors.Internal("could not retrieve traintuple type with key %s - %s", parentTraintupleKey, err.Error()) } inModelKeys = append(inModelKeys, parentTraintupleKey) @@ -512,13 +511,13 @@ func (tuple *Aggregatetuple) commitStatusUpdate(db *LedgerDB, aggregatetupleKey } if err := tuple.validateNewStatus(db, newStatus); err != nil { - return fmt.Errorf("update aggregatetuple %s failed: %s", aggregatetupleKey, err.Error()) + return errors.Internal("update aggregatetuple %s failed: %s", aggregatetupleKey, err.Error()) } oldStatus := tuple.Status tuple.Status = newStatus if err := db.Put(aggregatetupleKey, tuple); err != nil { - return fmt.Errorf("failed to update aggregatetuple %s - %s", aggregatetupleKey, err.Error()) + return errors.Internal("failed to update aggregatetuple %s - %s", aggregatetupleKey, err.Error()) } // update associated composite keys From 3d45c98e42fd158a6ba39d053a49b1bd80a65050 Mon Sep 17 00:00:00 2001 From: Thibault Robert Date: Wed, 22 Jan 2020 14:52:20 +0100 Subject: [PATCH 3/5] Clean leftover in utils --- chaincode/algo_aggregate_test.go | 2 +- chaincode/algo_composite_test.go | 2 +- chaincode/algo_test.go | 2 +- chaincode/data.go | 9 -------- chaincode/data_test.go | 2 +- chaincode/generate_examples_test.go | 4 ++-- chaincode/objective_test.go | 2 +- chaincode/testtuple_test.go | 6 ++--- chaincode/utils.go | 35 ----------------------------- 9 files changed, 10 insertions(+), 54 deletions(-) diff --git a/chaincode/algo_aggregate_test.go b/chaincode/algo_aggregate_test.go index e936ab7b..4ea28dfb 100644 --- a/chaincode/algo_aggregate_test.go +++ b/chaincode/algo_aggregate_test.go @@ -51,7 +51,7 @@ func TestAggregateAlgo(t *testing.T) { resp = mockStub.MockInvoke("42", args) assert.EqualValuesf(t, 200, resp.Status, "when querying an aggregate algo with status %d and message %s", resp.Status, resp.Message) algo := outputAggregateAlgo{} - err = bytesToStruct(resp.Payload, &algo) + err = json.Unmarshal(resp.Payload, &algo) assert.NoError(t, err, "when unmarshalling queried aggregate algo") expectedAlgo := outputAggregateAlgo{ outputAlgo: outputAlgo{ diff --git a/chaincode/algo_composite_test.go b/chaincode/algo_composite_test.go index f06f1fdb..5ad12bc1 100644 --- a/chaincode/algo_composite_test.go +++ b/chaincode/algo_composite_test.go @@ -51,7 +51,7 @@ func TestCompositeAlgo(t *testing.T) { resp = mockStub.MockInvoke("42", args) assert.EqualValuesf(t, 200, resp.Status, "when querying a composite algo with status %d and message %s", resp.Status, resp.Message) algo := outputCompositeAlgo{} - err = bytesToStruct(resp.Payload, &algo) + err = json.Unmarshal(resp.Payload, &algo) assert.NoError(t, err, "when unmarshalling queried composite algo") expectedAlgo := outputCompositeAlgo{ outputAlgo: outputAlgo{ diff --git a/chaincode/algo_test.go b/chaincode/algo_test.go index d7d97a7a..d005800b 100644 --- a/chaincode/algo_test.go +++ b/chaincode/algo_test.go @@ -49,7 +49,7 @@ func TestAlgo(t *testing.T) { resp = mockStub.MockInvoke("42", args) assert.EqualValuesf(t, 200, resp.Status, "when querying an algo with status %d and message %s", resp.Status, resp.Message) algo := outputAlgo{} - err = bytesToStruct(resp.Payload, &algo) + err = json.Unmarshal(resp.Payload, &algo) assert.NoError(t, err, "when unmarshalling queried objective") expectedAlgo := outputAlgo{ Key: algoKey, diff --git a/chaincode/data.go b/chaincode/data.go index 263caa4f..985bc446 100644 --- a/chaincode/data.go +++ b/chaincode/data.go @@ -52,10 +52,6 @@ func (dataManager *DataManager) Set(db *LedgerDB, inp inputDataManager) (string, // and returning corresponding dataSample hashes, associated dataManagers, testOnly and errors func setDataSample(db *LedgerDB, inp inputDataSample) (dataSampleHashes []string, dataSample DataSample, err error) { dataSampleHashes = inp.Hashes - if err = checkHashes(dataSampleHashes); err != nil { - err = errors.BadRequest(err) - return - } // check dataSample is not already in the ledger if existingKeys := checkDataSamplesExist(db, dataSampleHashes); existingKeys != nil { err = errors.Conflict("data samples with keys %s already exist", existingKeys).WithKeys(existingKeys) @@ -91,11 +87,6 @@ func setDataSample(db *LedgerDB, inp inputDataSample) (dataSampleHashes []string // one or more dataSamplef func validateUpdateDataSample(db *LedgerDB, inp inputUpdateDataSample) (dataSampleHashes []string, dataManagerKeys []string, err error) { // TODO return full dataSample - // check validity of dataSampleHashes - if err = checkHashes(inp.Hashes); err != nil { - err = errors.BadRequest(err) - return - } // check dataManagers exist and are owned by the transaction requester if err = checkDataManagerOwner(db, inp.DataManagerKeys); err != nil { return diff --git a/chaincode/data_test.go b/chaincode/data_test.go index a67d38e8..d82bd5c6 100644 --- a/chaincode/data_test.go +++ b/chaincode/data_test.go @@ -66,7 +66,7 @@ func TestDataManager(t *testing.T) { resp = mockStub.MockInvoke("42", args) assert.EqualValuesf(t, 200, resp.Status, "when querying the dataManager, status %d and message %s", resp.Status, resp.Message) dataManager := outputDataManager{} - err = bytesToStruct(resp.Payload, &dataManager) + err = json.Unmarshal(resp.Payload, &dataManager) assert.NoError(t, err, "when unmarshalling queried dataManager") expectedDataManager := outputDataManager{ ObjectiveKey: inpDataManager.ObjectiveKey, diff --git a/chaincode/generate_examples_test.go b/chaincode/generate_examples_test.go index ebac03da..ee8b32e1 100644 --- a/chaincode/generate_examples_test.go +++ b/chaincode/generate_examples_test.go @@ -123,7 +123,7 @@ func TestPipeline(t *testing.T) { assert.EqualValuesf(t, 200, resp.Status, "when adding traintuple with status %d and message %s", resp.Status, resp.Message) traintuple := outputTraintuple{} respTraintuple := resp.Payload - if err := bytesToStruct(respTraintuple, &traintuple); err != nil { + if err := json.Unmarshal(respTraintuple, &traintuple); err != nil { t.Errorf("when unmarshalling queried traintuple with error %s", err) } trainWorker := traintuple.Dataset.Worker @@ -185,7 +185,7 @@ func TestPipeline(t *testing.T) { resp = mockStub.MockInvoke("42", args) respTesttuple := resp.Payload testtuple := outputTesttuple{} - if err := bytesToStruct(respTesttuple, &testtuple); err != nil { + if err := json.Unmarshal(respTesttuple, &testtuple); err != nil { t.Errorf("when unmarshalling queried testtuple with error %s", err) } testWorker := testtuple.Dataset.Worker diff --git a/chaincode/objective_test.go b/chaincode/objective_test.go index 3410a77d..a32f19c4 100644 --- a/chaincode/objective_test.go +++ b/chaincode/objective_test.go @@ -145,7 +145,7 @@ func TestObjective(t *testing.T) { resp = mockStub.MockInvoke("42", args) assert.EqualValuesf(t, 200, resp.Status, "when querying a dataManager with status %d and message %s", resp.Status, resp.Message) objective := outputObjective{} - err = bytesToStruct(resp.Payload, &objective) + err = json.Unmarshal(resp.Payload, &objective) assert.NoError(t, err, "when unmarshalling queried objective") expectedObjective := outputObjective{ Key: objectiveKey, diff --git a/chaincode/testtuple_test.go b/chaincode/testtuple_test.go index 8a2a33e7..8cb66cee 100644 --- a/chaincode/testtuple_test.go +++ b/chaincode/testtuple_test.go @@ -169,7 +169,7 @@ func TestQueryTesttuple(t *testing.T) { resp = mockStub.MockInvoke("42", args) respTesttuple := resp.Payload testtuple := outputTesttuple{} - bytesToStruct(respTesttuple, &testtuple) + json.Unmarshal(respTesttuple, &testtuple) // assert assert.Equal(t, worker, testtuple.Creator) @@ -210,7 +210,7 @@ func TestTesttupleOnCompositeTraintuple(t *testing.T) { resp := mockStub.MockInvoke("42", args) assert.EqualValues(t, http.StatusOK, resp.Status, resp.Message) values := map[string]string{} - bytesToStruct(resp.Payload, &values) + json.Unmarshal(resp.Payload, &values) testTupleKey := values["key"] // Start training @@ -254,7 +254,7 @@ func TestTesttupleOnCompositeTraintuple(t *testing.T) { case StatusDone: assert.EqualValues(t, http.StatusOK, resp.Status, resp.Message) values = map[string]string{} - bytesToStruct(resp.Payload, &values) + json.Unmarshal(resp.Payload, &values) testTupleKey = values["key"] testTuple, err := queryTesttuple(db, assetToArgs(inputHash{Key: testTupleKey})) assert.NoError(t, err) diff --git a/chaincode/utils.go b/chaincode/utils.go index 40fcbdf2..faaebbec 100644 --- a/chaincode/utils.go +++ b/chaincode/utils.go @@ -19,7 +19,6 @@ import ( "encoding/json" "fmt" "math/rand" - "reflect" "unicode" "unicode/utf8" @@ -49,40 +48,6 @@ func typeInSlice(a AssetType, list []AssetType) bool { return false } -// inputStructToBytes converts fields of a struct (with string fields only, such as input struct defined in ledger.go) to a [][]byte -func inputStructToBytes(v interface{}) (sb [][]byte, err error) { - - e := reflect.Indirect(reflect.ValueOf(v)) - for i := 0; i < e.NumField(); i++ { - v := e.Field(i) - if v.Type().Name() != "string" { - err = fmt.Errorf("struct should contain only string values") - return - } - varValue := v.String() - sb = append(sb, []byte(varValue)) - } - return - -} - -// bytesToStruct converts bytes to one a the struct corresponding to elements stored in the ledger -func bytesToStruct(elementBytes []byte, element interface{}) error { - return json.Unmarshal(elementBytes, &element) -} - -// checkHashes checks if all elements in a slice are all hashes, returns error if not the case -func checkHashes(hashes []string) (err error) { - for _, hash := range hashes { - // check validity of dataSampleHashes - if len(hash) != 64 { - err = fmt.Errorf("invalid hash %s", hash) - return - } - } - return -} - // AssetFromJSON unmarshal a stringify json into the passed interface func AssetFromJSON(args []string, asset interface{}) error { if len(args) != 1 { From 61a1ee723690f79b5745c66e5f875b187a48a529 Mon Sep 17 00:00:00 2001 From: Thibault Robert Date: Wed, 22 Jan 2020 15:58:00 +0100 Subject: [PATCH 4/5] Fix after reviews --- chaincode/compute_plan_dag.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chaincode/compute_plan_dag.go b/chaincode/compute_plan_dag.go index 8774e28e..36361ce6 100644 --- a/chaincode/compute_plan_dag.go +++ b/chaincode/compute_plan_dag.go @@ -75,7 +75,7 @@ func (dag *ComputeDAG) sort() error { current[i].Depth = depth final = append(final, current[i]) if _, ok := IDPresents[current[i].ID]; ok { - return errors.Internal("compute plan error: Duplicate training task ID: %s", current[i].ID) + return errors.Conflict("compute plan error: Duplicate training task ID: %s", current[i].ID) } IDPresents[current[i].ID] = current[i].Depth } else { From 400f330255dc5686f680b417ff078cb34d135068 Mon Sep 17 00:00:00 2001 From: Thibault Robert Date: Wed, 22 Jan 2020 16:25:50 +0100 Subject: [PATCH 5/5] BadRequest for DAG --- chaincode/compute_plan_dag.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/chaincode/compute_plan_dag.go b/chaincode/compute_plan_dag.go index 36361ce6..69793859 100644 --- a/chaincode/compute_plan_dag.go +++ b/chaincode/compute_plan_dag.go @@ -75,7 +75,7 @@ func (dag *ComputeDAG) sort() error { current[i].Depth = depth final = append(final, current[i]) if _, ok := IDPresents[current[i].ID]; ok { - return errors.Conflict("compute plan error: Duplicate training task ID: %s", current[i].ID) + return errors.BadRequest("compute plan error: Duplicate training task ID: %s", current[i].ID) } IDPresents[current[i].ID] = current[i].Depth } else { @@ -90,7 +90,7 @@ func (dag *ComputeDAG) sort() error { for _, c := range current { errorIDs = append(errorIDs, c.ID) } - return errors.Internal("compute plan error: Cyclic or missing dependency among inModels IDs: %v", errorIDs) + return errors.BadRequest("compute plan error: Cyclic or missing dependency among inModels IDs: %v", errorIDs) } i = 0 current = temp