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 service on created and updated #932

Merged
merged 1 commit into from
Aug 21, 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/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func UpdateService(c *gin.Context) {
}

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

Expand All @@ -76,5 +76,5 @@ func UpdateService(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 @@ -272,7 +272,7 @@ func CancelBuild(c *gin.Context) {
if service.GetStatus() == constants.StatusRunning || service.GetStatus() == constants.StatusPending {
service.SetStatus(constants.StatusCanceled)

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

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

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

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

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

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

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

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

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

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

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

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

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

// create the services
for _, service := range resources.Services {
err := db.CreateService(service)
_, err := db.CreateService(service)
if err != nil {
t.Errorf("unable to create service %d: %v", service.GetID(), err)
}
Expand Down Expand Up @@ -1380,18 +1380,13 @@ func testServices(t *testing.T, db Interface, resources *Resources) {
// update the services
for _, service := range resources.Services {
service.SetStatus("success")
err = db.UpdateService(service)
got, err := db.UpdateService(service)
if err != nil {
t.Errorf("unable to update service %d: %v", service.GetID(), err)
}

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

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

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

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

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

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

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

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

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

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

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

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

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) RETURNING "id"`).
// run tests
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
err := test.database.CreateService(_service)
got, err := test.database.CreateService(_service)

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) RETURNING "id"`).
if err != nil {
t.Errorf("CreateService for %s returned err: %v", test.name, err)
}

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

import (
"reflect"
"testing"

"github.com/DATA-DOG/go-sqlmock"
Expand All @@ -31,7 +32,7 @@ func TestService_Engine_UpdateService(t *testing.T) {
_sqlite := testSqlite(t)
defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()

err := _sqlite.CreateService(_service)
_, err := _sqlite.CreateService(_service)
if err != nil {
t.Errorf("unable to create test service for sqlite: %v", err)
}
Expand All @@ -57,7 +58,7 @@ func TestService_Engine_UpdateService(t *testing.T) {
// run tests
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
err = test.database.UpdateService(_service)
got, err := test.database.UpdateService(_service)

if test.failure {
if err == nil {
Expand All @@ -70,6 +71,10 @@ func TestService_Engine_UpdateService(t *testing.T) {
if err != nil {
t.Errorf("UpdateService for %s returned err: %v", test.name, err)
}

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