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

api/admin/step.go

@@ -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)
 
@@ -75,5 +75,5 @@ func UpdateStep(c *gin.Context) {
 		return
 	}
 
-	c.JSON(http.StatusOK, input)
+	c.JSON(http.StatusOK, s)
 }

api/build/cancel.go

@@ -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)

api/build/clean.go

@@ -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)
 		}

api/step/create.go

@@ -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)
 
@@ -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)
 }

api/step/plan.go

@@ -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

api/step/update.go

@@ -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)
 
@@ -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)
 }

database/integration_test.go

@@ -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)
 		}
@@ -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)
 		}

database/step/clean_test.go

@@ -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)
 	}

database/step/count_build_test.go

@@ -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)
 	}

database/step/count_test.go

@@ -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)
 	}

database/step/create.go

@@ -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())
@@ -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
 }

database/step/create_test.go

@@ -5,6 +5,7 @@
 package step
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/DATA-DOG/go-sqlmock"
@@ -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 {
@@ -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)
+			}
 		})
 	}
 }

database/step/delete_test.go

@@ -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)
 	}

database/step/get_build_test.go

@@ -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)
 	}

database/step/get_test.go

@@ -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)
 	}

database/step/interface.go

@@ -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.
@@ -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)
 }

database/step/list_build_test.go

@@ -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)
 	}

database/step/list_image_test.go

@@ -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)
 	}

database/step/list_status_test.go

@@ -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)
 	}

database/step/list_test.go

@@ -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)
 	}

database/step/update.go

@@ -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())
@@ -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
 }

database/step/update_test.go

@@ -5,6 +5,7 @@
 package step
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/DATA-DOG/go-sqlmock"
@@ -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)
 	}
@@ -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 {
@@ -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)
+			}
 		})
 	}
 }