From a54f76d2e9a4d47335630335502242870d275bd8 Mon Sep 17 00:00:00 2001 From: parauliya Date: Thu, 4 Mar 2021 16:25:44 +0530 Subject: [PATCH 1/2] Fixed #381: 'tink workflow state' command returns an error if workflow does not exist Signed-off-by: parauliya --- db/workflow.go | 7 ++++--- grpc-server/workflow.go | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/db/workflow.go b/db/workflow.go index 0bd3056a4..533fa0767 100644 --- a/db/workflow.go +++ b/db/workflow.go @@ -350,13 +350,13 @@ func (d TinkDB) GetWorkflow(ctx context.Context, id string) (Workflow, error) { if err == nil { return Workflow{ID: id, Template: tmp, Hardware: tar}, nil } - if err != sql.ErrNoRows { err = errors.Wrap(err, "SELECT") d.logger.Error(err) + return Workflow{}, err } - return Workflow{}, nil + return Workflow{}, errors.New("Workflow with id " + id + " does not exist") } // DeleteWorkflow deletes a workflow @@ -549,8 +549,9 @@ func (d TinkDB) GetWorkflowContexts(ctx context.Context, wfID string) (*pb.Workf if err != sql.ErrNoRows { err = errors.Wrap(err, "SELECT from worflow_state") d.logger.Error(err) + return &pb.WorkflowContext{}, err } - return &pb.WorkflowContext{}, nil + return &pb.WorkflowContext{}, errors.New("Workflow with id " + wfID + " does not exist") } // GetWorkflowActions : gives you the action list of workflow diff --git a/grpc-server/workflow.go b/grpc-server/workflow.go index cd3c54299..d74a222e4 100644 --- a/grpc-server/workflow.go +++ b/grpc-server/workflow.go @@ -102,8 +102,8 @@ func (s *server) GetWorkflow(ctx context.Context, in *workflow.GetRequest) (*wor l = l.With("detail", pqErr.Detail, "where", pqErr.Where) } l.Error(err) + return &workflow.Workflow{}, err } - fields := map[string]string{ "id": w.Template, } @@ -200,7 +200,7 @@ func (s *server) GetWorkflowContext(ctx context.Context, in *workflow.GetRequest metrics.CacheInFlight.With(labels).Inc() defer metrics.CacheInFlight.With(labels).Dec() - const msg = "getting a workflow" + const msg = "getting a workflow context" labels["op"] = "get" metrics.CacheTotals.With(labels).Inc() From e7fdb8deaaddf683a8a05b8d9811967157e69949 Mon Sep 17 00:00:00 2001 From: parauliya Date: Mon, 8 Mar 2021 18:46:18 +0530 Subject: [PATCH 2/2] Added UTs for the fix Signed-off-by: parauliya --- grpc-server/workflow.go | 1 + grpc-server/workflow_test.go | 61 ++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/grpc-server/workflow.go b/grpc-server/workflow.go index d74a222e4..cab36c733 100644 --- a/grpc-server/workflow.go +++ b/grpc-server/workflow.go @@ -216,6 +216,7 @@ func (s *server) GetWorkflowContext(ctx context.Context, in *workflow.GetRequest l = l.With("detail", pqErr.Detail, "where", pqErr.Where) } l.Error(err) + return &workflow.WorkflowContext{}, err } wf := &workflow.WorkflowContext{ WorkflowId: w.WorkflowId, diff --git a/grpc-server/workflow_test.go b/grpc-server/workflow_test.go index 7969f398d..8efcdca8c 100644 --- a/grpc-server/workflow_test.go +++ b/grpc-server/workflow_test.go @@ -147,6 +147,18 @@ func TestGetWorkflow(t *testing.T) { expectedError: false, }, }, + "WorkflowDoesNotExist": { + args: args{ + db: mock.DB{ + GetWorkflowFunc: func(ctx context.Context, workflowID string) (db.Workflow, error) { + return db.Workflow{}, errors.New("Workflow with id " + workflowID + " does not exist") + }, + }, + }, + want: want{ + expectedError: true, + }, + }, } ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) @@ -169,3 +181,52 @@ func TestGetWorkflow(t *testing.T) { }) } } + +func TestGetWorkflowContext(t *testing.T) { + type ( + args struct { + db mock.DB + } + want struct { + expectedError bool + } + ) + testCases := map[string]struct { + args args + want want + }{ + "WorkflowDoesNotExist": { + args: args{ + db: mock.DB{ + GetWorkflowContextsFunc: func(ctx context.Context, workflowID string) (*workflow.WorkflowContext, error) { + w := workflow.WorkflowContext{} + return &w, errors.New("Workflow with id " + workflowID + " does not exist") + }, + }, + }, + want: want{ + expectedError: true, + }, + }, + } + + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + s := testServer(t, tc.args.db) + res, err := s.GetWorkflowContext(ctx, &workflow.GetRequest{ + Id: workflowID, + }) + if err != nil { + assert.Error(t, err) + assert.Empty(t, res) + assert.True(t, tc.want.expectedError) + return + } + assert.NoError(t, err) + assert.NotEmpty(t, res) + assert.False(t, tc.want.expectedError) + }) + } +}