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(database): return build object on created and updated #884

Merged
merged 4 commits into from
Jun 15, 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/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

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

c.JSON(http.StatusOK, input)
c.JSON(http.StatusOK, b)
}
2 changes: 1 addition & 1 deletion api/build/cancel.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion api/build/clean.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
11 changes: 1 addition & 10 deletions api/build/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion api/build/publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
5 changes: 1 addition & 4 deletions api/build/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion api/webhook/post.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
8 changes: 4 additions & 4 deletions database/build/clean_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/build/count_deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/build/count_org_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/build/count_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/build/count_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/build/count_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
14 changes: 8 additions & 6 deletions database/build/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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
}

ecrupper marked this conversation as resolved.
Show resolved Hide resolved
// 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
}
7 changes: 6 additions & 1 deletion database/build/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package build

import (
"reflect"
"testing"

"github.com/DATA-DOG/go-sqlmock"
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}
})
}
}
2 changes: 1 addition & 1 deletion database/build/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion database/build/get_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion database/build/get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/build/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
}
2 changes: 1 addition & 1 deletion database/build/last_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/build/list_deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/build/list_org_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/build/list_pending_running_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/build/list_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/build/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Loading