refactor(db): return service on created and updated (#932)

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

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(),

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

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

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

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

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

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

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

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

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
 }

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

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

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

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

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

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

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

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

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

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
 }

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