refactor(database): return build object on created and updated (#884)

* refactor(database): return build object on created and updated

* address feedback

api/admin/build.go

@@ -116,7 +116,7 @@ func UpdateBuild(c *gin.Context) {
 	}
 
 	// send API call to update the build
-	err = database.FromContext(c).UpdateBuild(input)
+	b, err := database.FromContext(c).UpdateBuild(input)
 	if err != nil {
 		retErr := fmt.Errorf("unable to update build %d: %w", input.GetID(), err)
 
@@ -125,5 +125,5 @@ func UpdateBuild(c *gin.Context) {
 		return
 	}
 
-	c.JSON(http.StatusOK, input)
+	c.JSON(http.StatusOK, b)
 }

api/build/cancel.go

@@ -190,7 +190,7 @@ func CancelBuild(c *gin.Context) {
 	// update the status in the build table
 	b.SetStatus(constants.StatusCanceled)
 
-	err := database.FromContext(c).UpdateBuild(b)
+	b, err := database.FromContext(c).UpdateBuild(b)
 	if err != nil {
 		retErr := fmt.Errorf("unable to update status for build %s: %w", entry, err)
 		util.HandleError(c, http.StatusInternalServerError, retErr)

api/build/clean.go

@@ -25,7 +25,7 @@ func CleanBuild(database database.Interface, b *library.Build, services []*libra
 	b.SetFinished(time.Now().UTC().Unix())
 
 	// send API call to update the build
-	err := database.UpdateBuild(b)
+	b, err := database.UpdateBuild(b)
 	if err != nil {
 		logrus.Errorf("unable to kill build %d: %v", b.GetNumber(), err)
 	}

api/build/plan.go

@@ -26,11 +26,10 @@ func PlanBuild(database database.Interface, p *pipeline.Build, b *library.Build,
 
 	// send API call to create the build
 	// TODO: return created build and error instead of just error
-	err := database.CreateBuild(b)
+	b, err := database.CreateBuild(b)
 	if err != nil {
 		// clean up the objects from the pipeline in the database
 		// TODO:
-		// - return build in CreateBuild
 		// - even if it was created, we need to get the new build id
 		//   otherwise it will be 0, which attempts to INSERT instead
 		//   of UPDATE-ing the existing build - which results in
@@ -41,14 +40,6 @@ func PlanBuild(database database.Interface, p *pipeline.Build, b *library.Build,
 		return fmt.Errorf("unable to create new build for %s: %w", r.GetFullName(), err)
 	}
 
-	// send API call to capture the created build
-	// TODO: this can be dropped once we return
-	// the created build above
-	b, err = database.GetBuildForRepo(r, b.GetNumber())
-	if err != nil {
-		return fmt.Errorf("unable to get new build for %s: %w", r.GetFullName(), err)
-	}
-
 	// plan all services for the build
 	services, err := service.PlanServices(database, p, b)
 	if err != nil {

api/build/publish.go

@@ -67,7 +67,7 @@ func PublishToQueue(queue queue.Service, db database.Interface, p *pipeline.Buil
 	b.SetEnqueued(time.Now().UTC().Unix())
 
 	// update the build in the db to reflect the time it was enqueued
-	err = db.UpdateBuild(b)
+	_, err = db.UpdateBuild(b)
 	if err != nil {
 		logrus.Errorf("Failed to update build %d during publish to queue for %s: %v", b.GetNumber(), r.GetFullName(), err)
 	}

api/build/update.go

@@ -151,7 +151,7 @@ func UpdateBuild(c *gin.Context) {
 	}
 
 	// send API call to update the build
-	err = database.FromContext(c).UpdateBuild(b)
+	b, err = database.FromContext(c).UpdateBuild(b)
 	if err != nil {
 		retErr := fmt.Errorf("unable to update build %s: %w", entry, err)
 
@@ -160,9 +160,6 @@ func UpdateBuild(c *gin.Context) {
 		return
 	}
 
-	// send API call to capture the updated build
-	b, _ = database.FromContext(c).GetBuildForRepo(r, b.GetNumber())
-
 	c.JSON(http.StatusOK, b)
 
 	// check if the build is in a "final" state

api/webhook/post.go

@@ -893,7 +893,7 @@ func renameRepository(h *library.Hook, r *library.Repo, c *gin.Context, m *types
 			fmt.Sprintf("%s/%s/%d", m.Vela.WebAddress, dbR.GetFullName(), build.GetNumber()),
 		)
 
-		err = database.FromContext(c).UpdateBuild(build)
+		_, err = database.FromContext(c).UpdateBuild(build)
 		if err != nil {
 			return nil, fmt.Errorf("unable to update build for repo %s: %w", dbR.GetFullName(), err)
 		}

database/build/clean_test.go

@@ -54,22 +54,22 @@ func TestBuild_Engine_CleanBuilds(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateBuild(_buildOne)
+	_, err := _sqlite.CreateBuild(_buildOne)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateBuild(_buildTwo)
+	_, err = _sqlite.CreateBuild(_buildTwo)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateBuild(_buildThree)
+	_, err = _sqlite.CreateBuild(_buildThree)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateBuild(_buildFour)
+	_, err = _sqlite.CreateBuild(_buildFour)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}

database/build/count_deployment_test.go

@@ -44,12 +44,12 @@ func TestBuild_Engine_CountBuildsForDeployment(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateBuild(_buildOne)
+	_, err := _sqlite.CreateBuild(_buildOne)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateBuild(_buildTwo)
+	_, err = _sqlite.CreateBuild(_buildTwo)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}

database/build/count_org_test.go

@@ -67,12 +67,12 @@ func TestBuild_Engine_CountBuildsForOrg(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateBuild(_buildOne)
+	_, err := _sqlite.CreateBuild(_buildOne)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateBuild(_buildTwo)
+	_, err = _sqlite.CreateBuild(_buildTwo)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}

database/build/count_repo_test.go

@@ -46,12 +46,12 @@ func TestBuild_Engine_CountBuildsForRepo(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateBuild(_buildOne)
+	_, err := _sqlite.CreateBuild(_buildOne)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateBuild(_buildTwo)
+	_, err = _sqlite.CreateBuild(_buildTwo)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}

database/build/count_status_test.go

@@ -39,12 +39,12 @@ func TestBuild_Engine_CountBuildsForStatus(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateBuild(_buildOne)
+	_, err := _sqlite.CreateBuild(_buildOne)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateBuild(_buildTwo)
+	_, err = _sqlite.CreateBuild(_buildTwo)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}

database/build/count_test.go

@@ -37,12 +37,12 @@ func TestBuild_Engine_CountBuilds(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateBuild(_buildOne)
+	_, err := _sqlite.CreateBuild(_buildOne)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateBuild(_buildTwo)
+	_, err = _sqlite.CreateBuild(_buildTwo)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}

database/build/create.go

@@ -13,7 +13,7 @@ import (
 )
 
 // CreateBuild creates a new build in the database.
-func (e *engine) CreateBuild(b *library.Build) error {
+func (e *engine) CreateBuild(b *library.Build) (*library.Build, error) {
 	e.logger.WithFields(logrus.Fields{
 		"build": b.GetNumber(),
 	}).Tracef("creating build %d in the database", b.GetNumber())
@@ -28,12 +28,14 @@ func (e *engine) CreateBuild(b *library.Build) error {
 	// https://pkg.go.dev/github.com/go-vela/types/database#Build.Validate
 	err := build.Validate()
 	if err != nil {
-		return err
+		return nil, err
 	}
 
+	// crop build if any columns are too large
+	build = build.Crop()
+
 	// send query to the database
-	return e.client.
-		Table(constants.TableBuild).
-		Create(build.Crop()).
-		Error
+	result := e.client.Table(constants.TableBuild).Create(build)
+
+	return build.ToLibrary(), result.Error
 }

database/build/create_test.go

@@ -5,6 +5,7 @@
 package build
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/DATA-DOG/go-sqlmock"
@@ -55,7 +56,7 @@ VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20,$
 	// run tests
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			err := test.database.CreateBuild(_build)
+			got, err := test.database.CreateBuild(_build)
 
 			if test.failure {
 				if err == nil {
@@ -68,6 +69,10 @@ VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20,$
 			if err != nil {
 				t.Errorf("CreateBuild for %s returned err: %v", test.name, err)
 			}
+
+			if !reflect.DeepEqual(got, _build) {
+				t.Errorf("CreateBuild for %s returned %s, want %s", test.name, got, _build)
+			}
 		})
 	}
 }

database/build/delete_test.go

@@ -29,7 +29,7 @@ func TestBuild_Engine_DeleteBuild(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateBuild(_build)
+	_, err := _sqlite.CreateBuild(_build)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}

database/build/get_repo_test.go

@@ -43,7 +43,7 @@ func TestBuild_Engine_GetBuildForRepo(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateBuild(_build)
+	_, err := _sqlite.CreateBuild(_build)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}

database/build/get_test.go

@@ -34,7 +34,7 @@ func TestBuild_Engine_GetBuild(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateBuild(_build)
+	_, err := _sqlite.CreateBuild(_build)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}

database/build/interface.go

@@ -39,7 +39,7 @@ type BuildInterface interface {
 	// CountBuildsForStatus defines a function that gets the count of builds by status.
 	CountBuildsForStatus(string, map[string]interface{}) (int64, error)
 	// CreateBuild defines a function that creates a new build.
-	CreateBuild(*library.Build) error
+	CreateBuild(*library.Build) (*library.Build, error)
 	// DeleteBuild defines a function that deletes an existing build.
 	DeleteBuild(*library.Build) error
 	// GetBuild defines a function that gets a build by ID.
@@ -59,5 +59,5 @@ type BuildInterface interface {
 	// ListPendingAndRunningBuilds defines a function that gets a list of pending and running builds.
 	ListPendingAndRunningBuilds(string) ([]*library.BuildQueue, error)
 	// UpdateBuild defines a function that updates an existing build.
-	UpdateBuild(*library.Build) error
+	UpdateBuild(*library.Build) (*library.Build, error)
 }

database/build/last_repo_test.go

@@ -44,7 +44,7 @@ func TestBuild_Engine_LastBuildForRepo(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateBuild(_build)
+	_, err := _sqlite.CreateBuild(_build)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}

database/build/list_deployment_test.go

@@ -54,12 +54,12 @@ func TestBuild_Engine_ListBuildsForDeployment(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateBuild(_buildOne)
+	_, err := _sqlite.CreateBuild(_buildOne)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateBuild(_buildTwo)
+	_, err = _sqlite.CreateBuild(_buildTwo)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}

database/build/list_org_test.go

@@ -94,12 +94,12 @@ func TestBuild_Engine_ListBuildsForOrg(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateBuild(_buildOne)
+	_, err := _sqlite.CreateBuild(_buildOne)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateBuild(_buildTwo)
+	_, err = _sqlite.CreateBuild(_buildTwo)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}

database/build/list_pending_running_test.go

@@ -65,12 +65,12 @@ func TestBuild_Engine_ListPendingAndRunningBuilds(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateBuild(_buildOne)
+	_, err := _sqlite.CreateBuild(_buildOne)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateBuild(_buildTwo)
+	_, err = _sqlite.CreateBuild(_buildTwo)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}

database/build/list_repo_test.go

@@ -59,12 +59,12 @@ func TestBuild_Engine_ListBuildsForRepo(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateBuild(_buildOne)
+	_, err := _sqlite.CreateBuild(_buildOne)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateBuild(_buildTwo)
+	_, err = _sqlite.CreateBuild(_buildTwo)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}

database/build/list_test.go

@@ -47,12 +47,12 @@ func TestBuild_Engine_ListBuilds(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateBuild(_buildOne)
+	_, err := _sqlite.CreateBuild(_buildOne)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateBuild(_buildTwo)
+	_, err = _sqlite.CreateBuild(_buildTwo)
 	if err != nil {
 		t.Errorf("unable to create test build for sqlite: %v", err)
 	}