diff --git a/api/admin/service.go b/api/admin/service.go index 2cd295c4c..24ddae3dc 100644 --- a/api/admin/service.go +++ b/api/admin/service.go @@ -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) @@ -76,5 +76,5 @@ func UpdateService(c *gin.Context) { return } - c.JSON(http.StatusOK, input) + c.JSON(http.StatusOK, s) } diff --git a/api/build/cancel.go b/api/build/cancel.go index 923d2eb77..d8d4e6c06 100644 --- a/api/build/cancel.go +++ b/api/build/cancel.go @@ -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(), diff --git a/api/build/clean.go b/api/build/clean.go index 8344694b7..a93a9355c 100644 --- a/api/build/clean.go +++ b/api/build/clean.go @@ -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) } diff --git a/api/service/create.go b/api/service/create.go index 426f13760..2dcccbd26 100644 --- a/api/service/create.go +++ b/api/service/create.go @@ -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) @@ -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) } diff --git a/api/service/plan.go b/api/service/plan.go index 4912f38d1..a0200767e 100644 --- a/api/service/plan.go +++ b/api/service/plan.go @@ -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 diff --git a/api/service/update.go b/api/service/update.go index bed781f88..7c1a5859f 100644 --- a/api/service/update.go +++ b/api/service/update.go @@ -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) @@ -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) } diff --git a/database/integration_test.go b/database/integration_test.go index d8dede18b..cd4e3f9a7 100644 --- a/database/integration_test.go +++ b/database/integration_test.go @@ -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) } @@ -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 diff --git a/database/service/clean_test.go b/database/service/clean_test.go index 3bd9baf15..17301e119 100644 --- a/database/service/clean_test.go +++ b/database/service/clean_test.go @@ -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) } diff --git a/database/service/count_build_test.go b/database/service/count_build_test.go index 08c964da7..160dfa5f8 100644 --- a/database/service/count_build_test.go +++ b/database/service/count_build_test.go @@ -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) } diff --git a/database/service/count_test.go b/database/service/count_test.go index 226d4b879..4152c18e9 100644 --- a/database/service/count_test.go +++ b/database/service/count_test.go @@ -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) } diff --git a/database/service/create.go b/database/service/create.go index 56574e23b..f748be077 100644 --- a/database/service/create.go +++ b/database/service/create.go @@ -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()) @@ -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 } diff --git a/database/service/create_test.go b/database/service/create_test.go index 4e84ed090..26771f31c 100644 --- a/database/service/create_test.go +++ b/database/service/create_test.go @@ -5,6 +5,7 @@ package service 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) 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 { @@ -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) + } }) } } diff --git a/database/service/delete_test.go b/database/service/delete_test.go index d63c55c05..04d08fdc8 100644 --- a/database/service/delete_test.go +++ b/database/service/delete_test.go @@ -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) } diff --git a/database/service/get_build_test.go b/database/service/get_build_test.go index 36747ad65..4b6a629de 100644 --- a/database/service/get_build_test.go +++ b/database/service/get_build_test.go @@ -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) } diff --git a/database/service/get_test.go b/database/service/get_test.go index 13bd6691a..1c2734b36 100644 --- a/database/service/get_test.go +++ b/database/service/get_test.go @@ -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) } diff --git a/database/service/interface.go b/database/service/interface.go index d22595859..0a05c626d 100644 --- a/database/service/interface.go +++ b/database/service/interface.go @@ -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. @@ -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) } diff --git a/database/service/list_build_test.go b/database/service/list_build_test.go index 3b6d97161..1f0a860f4 100644 --- a/database/service/list_build_test.go +++ b/database/service/list_build_test.go @@ -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) } diff --git a/database/service/list_image_test.go b/database/service/list_image_test.go index f4e2f228e..8f0d588d0 100644 --- a/database/service/list_image_test.go +++ b/database/service/list_image_test.go @@ -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) } diff --git a/database/service/list_status_test.go b/database/service/list_status_test.go index 85b7808af..d1cb46649 100644 --- a/database/service/list_status_test.go +++ b/database/service/list_status_test.go @@ -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) } diff --git a/database/service/list_test.go b/database/service/list_test.go index 72b60d5cf..a7351a8b6 100644 --- a/database/service/list_test.go +++ b/database/service/list_test.go @@ -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) } diff --git a/database/service/update.go b/database/service/update.go index ceaa7b6b8..d9b0bd595 100644 --- a/database/service/update.go +++ b/database/service/update.go @@ -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()) @@ -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 } diff --git a/database/service/update_test.go b/database/service/update_test.go index e9fe61955..12979dce9 100644 --- a/database/service/update_test.go +++ b/database/service/update_test.go @@ -5,6 +5,7 @@ package service import ( + "reflect" "testing" "github.com/DATA-DOG/go-sqlmock" @@ -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) } @@ -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 { @@ -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) + } }) } } diff --git a/router/middleware/service/service_test.go b/router/middleware/service/service_test.go index d900394b7..70a8024c3 100644 --- a/router/middleware/service/service_test.go +++ b/router/middleware/service/service_test.go @@ -87,7 +87,7 @@ func TestService_Establish(t *testing.T) { _, _ = db.CreateRepo(context.TODO(), r) _, _ = db.CreateBuild(context.TODO(), b) - _ = db.CreateService(want) + _, _ = db.CreateService(want) // setup context gin.SetMode(gin.TestMode)