Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(db): return step on created and updated #933

Merged
merged 3 commits into from
Aug 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -1480,7 +1480,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 @@ -1585,16 +1585,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