Skip to content

Commit

Permalink
refactor(db): return step on created and updated
Browse files Browse the repository at this point in the history
  • Loading branch information
ecrupper committed Aug 22, 2023
1 parent ee0d2a0 commit 746aa91
Show file tree
Hide file tree
Showing 23 changed files with 54 additions and 63 deletions.
4 changes: 2 additions & 2 deletions api/admin/step.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func UpdateStep(c *gin.Context) {
}

// send API call to update the step
err = database.FromContext(c).UpdateStep(input)
s, err := database.FromContext(c).UpdateStep(input)
if err != nil {
retErr := fmt.Errorf("unable to update step %d: %w", input.GetID(), err)

Expand All @@ -75,5 +75,5 @@ func UpdateStep(c *gin.Context) {
return
}

c.JSON(http.StatusOK, input)
c.JSON(http.StatusOK, s)
}
2 changes: 1 addition & 1 deletion api/build/cancel.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ func CancelBuild(c *gin.Context) {
if step.GetStatus() == constants.StatusRunning || step.GetStatus() == constants.StatusPending {
step.SetStatus(constants.StatusCanceled)

err = database.FromContext(c).UpdateStep(step)
_, err = database.FromContext(c).UpdateStep(step)
if err != nil {
retErr := fmt.Errorf("unable to update step %s for build %s: %w", step.GetName(), entry, err)
util.HandleError(c, http.StatusNotFound, retErr)
Expand Down
2 changes: 1 addition & 1 deletion api/build/clean.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func CleanBuild(ctx context.Context, database database.Interface, b *library.Bui
s.SetFinished(time.Now().UTC().Unix())

// send API call to update the step
err := database.UpdateStep(s)
_, err := database.UpdateStep(s)
if err != nil {
logrus.Errorf("unable to kill step %s for build %d: %v", s.GetName(), b.GetNumber(), err)
}
Expand Down
5 changes: 1 addition & 4 deletions api/step/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func CreateStep(c *gin.Context) {
}

// send API call to create the step
err = database.FromContext(c).CreateStep(input)
s, err := database.FromContext(c).CreateStep(input)
if err != nil {
retErr := fmt.Errorf("unable to create step for build %s: %w", entry, err)

Expand All @@ -121,8 +121,5 @@ func CreateStep(c *gin.Context) {
return
}

// send API call to capture the created step
s, _ := database.FromContext(c).GetStepForBuild(b, input.GetNumber())

c.JSON(http.StatusCreated, s)
}
8 changes: 1 addition & 7 deletions api/step/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,11 @@ func planStep(database database.Interface, b *library.Build, c *pipeline.Contain
s.SetCreated(time.Now().UTC().Unix())

// send API call to create the step
err := database.CreateStep(s)
s, err := database.CreateStep(s)
if err != nil {
return nil, fmt.Errorf("unable to create step %s: %w", s.GetName(), err)
}

// send API call to capture the created step
s, err = database.GetStepForBuild(b, s.GetNumber())
if err != nil {
return nil, fmt.Errorf("unable to get step %s: %w", s.GetName(), err)
}

// populate environment variables from step library
//
// https://pkg.go.dev/github.com/go-vela/types/library#step.Environment
Expand Down
5 changes: 1 addition & 4 deletions api/step/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func UpdateStep(c *gin.Context) {
}

// send API call to update the step
err = database.FromContext(c).UpdateStep(s)
s, err = database.FromContext(c).UpdateStep(s)
if err != nil {
retErr := fmt.Errorf("unable to update step %s: %w", entry, err)

Expand All @@ -156,8 +156,5 @@ func UpdateStep(c *gin.Context) {
return
}

// send API call to capture the updated step
s, _ = database.FromContext(c).GetStepForBuild(b, s.GetNumber())

c.JSON(http.StatusOK, s)
}
9 changes: 2 additions & 7 deletions database/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1429,7 +1429,7 @@ func testSteps(t *testing.T, db Interface, resources *Resources) {

// create the steps
for _, step := range resources.Steps {
err := db.CreateStep(step)
_, err := db.CreateStep(step)
if err != nil {
t.Errorf("unable to create step %d: %v", step.GetID(), err)
}
Expand Down Expand Up @@ -1534,16 +1534,11 @@ func testSteps(t *testing.T, db Interface, resources *Resources) {
// update the steps
for _, step := range resources.Steps {
step.SetStatus("success")
err = db.UpdateStep(step)
got, err := db.UpdateStep(step)
if err != nil {
t.Errorf("unable to update step %d: %v", step.GetID(), err)
}

// lookup the step by ID
got, err := db.GetStep(step.GetID())
if err != nil {
t.Errorf("unable to get step %d by ID: %v", step.GetID(), err)
}
if !reflect.DeepEqual(got, step) {
t.Errorf("GetStep() is %v, want %v", got, step)
}
Expand Down
8 changes: 4 additions & 4 deletions database/step/clean_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,22 +64,22 @@ func TestStep_Engine_CleanStep(t *testing.T) {
_sqlite := testSqlite(t)
defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()

err := _sqlite.CreateStep(_stepOne)
_, err := _sqlite.CreateStep(_stepOne)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}

err = _sqlite.CreateStep(_stepTwo)
_, err = _sqlite.CreateStep(_stepTwo)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}

err = _sqlite.CreateStep(_stepThree)
_, err = _sqlite.CreateStep(_stepThree)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}

err = _sqlite.CreateStep(_stepFour)
_, err = _sqlite.CreateStep(_stepFour)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions database/step/count_build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,12 @@ func TestStep_Engine_CountStepsForBuild(t *testing.T) {
_sqlite := testSqlite(t)
defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()

err := _sqlite.CreateStep(_stepOne)
_, err := _sqlite.CreateStep(_stepOne)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}

err = _sqlite.CreateStep(_stepTwo)
_, err = _sqlite.CreateStep(_stepTwo)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions database/step/count_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ func TestStep_Engine_CountSteps(t *testing.T) {
_sqlite := testSqlite(t)
defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()

err := _sqlite.CreateStep(_stepOne)
_, err := _sqlite.CreateStep(_stepOne)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}

err = _sqlite.CreateStep(_stepTwo)
_, err = _sqlite.CreateStep(_stepTwo)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}
Expand Down
11 changes: 5 additions & 6 deletions database/step/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
)

// CreateStep creates a new step in the database.
func (e *engine) CreateStep(s *library.Step) error {
func (e *engine) CreateStep(s *library.Step) (*library.Step, error) {
e.logger.WithFields(logrus.Fields{
"step": s.GetNumber(),
}).Tracef("creating step %s in the database", s.GetName())
Expand All @@ -27,12 +27,11 @@ func (e *engine) CreateStep(s *library.Step) error {
// https://pkg.go.dev/github.com/go-vela/types/database#Step.Validate
err := step.Validate()
if err != nil {
return err
return nil, err
}

// send query to the database
return e.client.
Table(constants.TableStep).
Create(step).
Error
result := e.client.Table(constants.TableStep).Create(step)

return step.ToLibrary(), result.Error
}
7 changes: 6 additions & 1 deletion database/step/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package step

import (
"reflect"
"testing"

"github.com/DATA-DOG/go-sqlmock"
Expand Down Expand Up @@ -57,7 +58,7 @@ VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16) RETURNING "id"`)
// run tests
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
err := test.database.CreateStep(_step)
got, err := test.database.CreateStep(_step)

if test.failure {
if err == nil {
Expand All @@ -70,6 +71,10 @@ VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16) RETURNING "id"`)
if err != nil {
t.Errorf("CreateStep for %s returned err: %v", test.name, err)
}

if !reflect.DeepEqual(got, _step) {
t.Errorf("CreateStep for %s returned %s, want %s", test.name, got, _step)
}
})
}
}
2 changes: 1 addition & 1 deletion database/step/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestStep_Engine_DeleteStep(t *testing.T) {
_sqlite := testSqlite(t)
defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()

err := _sqlite.CreateStep(_step)
_, err := _sqlite.CreateStep(_step)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion database/step/get_build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func TestStep_Engine_GetStepForBuild(t *testing.T) {
_sqlite := testSqlite(t)
defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()

err := _sqlite.CreateStep(_step)
_, err := _sqlite.CreateStep(_step)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion database/step/get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestStep_Engine_GetStep(t *testing.T) {
_sqlite := testSqlite(t)
defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()

err := _sqlite.CreateStep(_step)
_, err := _sqlite.CreateStep(_step)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions database/step/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type StepInterface interface {
// CountStepsForBuild defines a function that gets the count of steps by build ID.
CountStepsForBuild(*library.Build, map[string]interface{}) (int64, error)
// CreateStep defines a function that creates a new step.
CreateStep(*library.Step) error
CreateStep(*library.Step) (*library.Step, error)
// DeleteStep defines a function that deletes an existing step.
DeleteStep(*library.Step) error
// GetStep defines a function that gets a step by ID.
Expand All @@ -47,5 +47,5 @@ type StepInterface interface {
// ListStepStatusCount defines a function that gets a list of all step statuses and the count of their occurrence.
ListStepStatusCount() (map[string]float64, error)
// UpdateStep defines a function that updates an existing step.
UpdateStep(*library.Step) error
UpdateStep(*library.Step) (*library.Step, error)
}
4 changes: 2 additions & 2 deletions database/step/list_build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ func TestStep_Engine_ListStepsForBuild(t *testing.T) {
_sqlite := testSqlite(t)
defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()

err := _sqlite.CreateStep(_stepOne)
_, err := _sqlite.CreateStep(_stepOne)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}

err = _sqlite.CreateStep(_stepTwo)
_, err = _sqlite.CreateStep(_stepTwo)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions database/step/list_image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ func TestStep_Engine_ListStepImageCount(t *testing.T) {
_sqlite := testSqlite(t)
defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()

err := _sqlite.CreateStep(_stepOne)
_, err := _sqlite.CreateStep(_stepOne)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}

err = _sqlite.CreateStep(_stepTwo)
_, err = _sqlite.CreateStep(_stepTwo)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions database/step/list_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,12 @@ func TestStep_Engine_ListStepStatusCount(t *testing.T) {
_sqlite := testSqlite(t)
defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()

err := _sqlite.CreateStep(_stepOne)
_, err := _sqlite.CreateStep(_stepOne)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}

err = _sqlite.CreateStep(_stepTwo)
_, err = _sqlite.CreateStep(_stepTwo)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions database/step/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ func TestStep_Engine_ListSteps(t *testing.T) {
_sqlite := testSqlite(t)
defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()

err := _sqlite.CreateStep(_stepOne)
_, err := _sqlite.CreateStep(_stepOne)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}

err = _sqlite.CreateStep(_stepTwo)
_, err = _sqlite.CreateStep(_stepTwo)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}
Expand Down
11 changes: 5 additions & 6 deletions database/step/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
)

// UpdateStep updates an existing step in the database.
func (e *engine) UpdateStep(s *library.Step) error {
func (e *engine) UpdateStep(s *library.Step) (*library.Step, error) {
e.logger.WithFields(logrus.Fields{
"step": s.GetNumber(),
}).Tracef("updating step %s in the database", s.GetName())
Expand All @@ -27,12 +27,11 @@ func (e *engine) UpdateStep(s *library.Step) error {
// https://pkg.go.dev/github.com/go-vela/types/database#Step.Validate
err := step.Validate()
if err != nil {
return err
return nil, err
}

// send query to the database
return e.client.
Table(constants.TableStep).
Save(step).
Error
result := e.client.Table(constants.TableStep).Save(step)

return step.ToLibrary(), result.Error
}
9 changes: 7 additions & 2 deletions database/step/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package step

import (
"reflect"
"testing"

"github.com/DATA-DOG/go-sqlmock"
Expand Down Expand Up @@ -33,7 +34,7 @@ WHERE "id" = $16`).
_sqlite := testSqlite(t)
defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()

err := _sqlite.CreateStep(_step)
_, err := _sqlite.CreateStep(_step)
if err != nil {
t.Errorf("unable to create test step for sqlite: %v", err)
}
Expand All @@ -59,7 +60,7 @@ WHERE "id" = $16`).
// run tests
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
err = test.database.UpdateStep(_step)
got, err := test.database.UpdateStep(_step)

if test.failure {
if err == nil {
Expand All @@ -72,6 +73,10 @@ WHERE "id" = $16`).
if err != nil {
t.Errorf("UpdateStep for %s returned err: %v", test.name, err)
}

if !reflect.DeepEqual(got, _step) {
t.Errorf("UpdateStep for %s returned %s, want %s", test.name, got, _step)
}
})
}
}
Loading

0 comments on commit 746aa91

Please sign in to comment.