refactor(database): return repo object on created and updated (#913)

* init commit

* fix tests

api/admin/repo.go

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

api/build/create.go

@@ -329,7 +329,7 @@ func CreateBuild(c *gin.Context) {
 	}
 
 	// send API call to update repo for ensuring counter is incremented
-	err = database.FromContext(c).UpdateRepo(r)
+	r, err = database.FromContext(c).UpdateRepo(r)
 	if err != nil {
 		retErr := fmt.Errorf("unable to create new build: failed to update repo %s: %w", r.GetFullName(), err)
 

api/build/restart.go

@@ -320,7 +320,7 @@ func RestartBuild(c *gin.Context) {
 	}
 
 	// send API call to update repo for ensuring counter is incremented
-	err = database.FromContext(c).UpdateRepo(r)
+	r, err = database.FromContext(c).UpdateRepo(r)
 	if err != nil {
 		retErr := fmt.Errorf("unable to restart build: failed to update repo %s: %w", r.GetFullName(), err)
 		util.HandleError(c, http.StatusBadRequest, retErr)

api/repo/chown.go

@@ -68,7 +68,7 @@ func ChownRepo(c *gin.Context) {
 	r.SetUserID(u.GetID())
 
 	// send API call to update the repo
-	err := database.FromContext(c).UpdateRepo(r)
+	_, err := database.FromContext(c).UpdateRepo(r)
 	if err != nil {
 		retErr := fmt.Errorf("unable to change owner of repo %s to %s: %w", r.GetFullName(), u.GetName(), err)
 

api/repo/create.go

@@ -285,30 +285,24 @@ func CreateRepo(c *gin.Context) {
 		dbRepo.SetActive(true)
 
 		// send API call to update the repo
-		err = database.FromContext(c).UpdateRepo(dbRepo)
+		r, err = database.FromContext(c).UpdateRepo(dbRepo)
 		if err != nil {
 			retErr := fmt.Errorf("unable to set repo %s to active: %w", dbRepo.GetFullName(), err)
 
 			util.HandleError(c, http.StatusInternalServerError, retErr)
 
 			return
 		}
-
-		// send API call to capture the updated repo
-		r, _ = database.FromContext(c).GetRepoForOrg(dbRepo.GetOrg(), dbRepo.GetName())
 	} else {
 		// send API call to create the repo
-		err = database.FromContext(c).CreateRepo(r)
+		r, err = database.FromContext(c).CreateRepo(r)
 		if err != nil {
 			retErr := fmt.Errorf("unable to create new repo %s: %w", r.GetFullName(), err)
 
 			util.HandleError(c, http.StatusInternalServerError, retErr)
 
 			return
 		}
-
-		// send API call to capture the created repo
-		r, _ = database.FromContext(c).GetRepoForOrg(r.GetOrg(), r.GetName())
 	}
 
 	// create init hook in the DB after repo has been added in order to capture its ID

api/repo/delete.go

@@ -88,7 +88,7 @@ func DeleteRepo(c *gin.Context) {
 	// Mark the repo as inactive
 	r.SetActive(false)
 
-	err = database.FromContext(c).UpdateRepo(r)
+	_, err = database.FromContext(c).UpdateRepo(r)
 	if err != nil {
 		retErr := fmt.Errorf("unable to set repo %s to inactive: %w", r.GetFullName(), err)
 

api/repo/repair.go

@@ -122,7 +122,7 @@ func RepairRepo(c *gin.Context) {
 		r.SetActive(true)
 
 		// send API call to update the repo
-		err := database.FromContext(c).UpdateRepo(r)
+		_, err := database.FromContext(c).UpdateRepo(r)
 		if err != nil {
 			retErr := fmt.Errorf("unable to set repo %s to active: %w", r.GetFullName(), err)
 

api/repo/update.go

@@ -295,7 +295,7 @@ func UpdateRepo(c *gin.Context) {
 	}
 
 	// send API call to update the repo
-	err = database.FromContext(c).UpdateRepo(r)
+	r, err = database.FromContext(c).UpdateRepo(r)
 	if err != nil {
 		retErr := fmt.Errorf("unable to update repo %s: %w", r.GetFullName(), err)
 
@@ -304,8 +304,5 @@ func UpdateRepo(c *gin.Context) {
 		return
 	}
 
-	// send API call to capture the updated repo
-	r, _ = database.FromContext(c).GetRepoForOrg(r.GetOrg(), r.GetName())
-
 	c.JSON(http.StatusOK, r)
 }

api/scm/sync.go

@@ -80,7 +80,7 @@ func SyncRepo(c *gin.Context) {
 		r.SetActive(false)
 
 		// update repo in database
-		err := database.FromContext(c).UpdateRepo(r)
+		_, err := database.FromContext(c).UpdateRepo(r)
 		if err != nil {
 			retErr := fmt.Errorf("unable to update repo for org %s: %w", o, err)
 

api/scm/sync_org.go

@@ -113,7 +113,7 @@ func SyncReposForOrg(c *gin.Context) {
 		if err != nil {
 			repo.SetActive(false)
 
-			err := database.FromContext(c).UpdateRepo(repo)
+			_, err := database.FromContext(c).UpdateRepo(repo)
 			if err != nil {
 				retErr := fmt.Errorf("unable to update repo for org %s: %w", o, err)
 

api/webhook/post.go

@@ -621,7 +621,7 @@ func PostWebhook(c *gin.Context) {
 	} // end of retry loop
 
 	// send API call to update repo for ensuring counter is incremented
-	err = database.FromContext(c).UpdateRepo(repo)
+	repo, err = database.FromContext(c).UpdateRepo(repo)
 	if err != nil {
 		retErr := fmt.Errorf("%s: failed to update repo %s: %w", baseErr, repo.GetFullName(), err)
 		util.HandleError(c, http.StatusBadRequest, retErr)
@@ -759,7 +759,7 @@ func handleRepositoryEvent(c *gin.Context, m *types.Metadata, h *library.Hook, r
 		}
 
 		// update repo object in the database after applying edits
-		err = database.FromContext(c).UpdateRepo(dbRepo)
+		dbRepo, err = database.FromContext(c).UpdateRepo(dbRepo)
 		if err != nil {
 			retErr := fmt.Errorf("%s: failed to update repo %s: %w", baseErr, r.GetFullName(), err)
 
@@ -891,7 +891,7 @@ func renameRepository(h *library.Hook, r *library.Repo, c *gin.Context, m *types
 	dbR.SetPreviousName(r.GetPreviousName())
 
 	// update the repo in the database
-	err = database.FromContext(c).UpdateRepo(dbR)
+	dbR, err = database.FromContext(c).UpdateRepo(dbR)
 	if err != nil {
 		retErr := fmt.Errorf("%s: failed to update repo %s/%s in database", baseErr, prevOrg, prevRepo)
 		util.HandleError(c, http.StatusBadRequest, retErr)

cmd/vela-server/schedule.go

@@ -373,7 +373,7 @@ func processSchedule(s *library.Schedule, compiler compiler.Engine, database dat
 	} // end of retry loop
 
 	// send API call to update repo for ensuring counter is incremented
-	err = database.UpdateRepo(r)
+	r, err = database.UpdateRepo(r)
 	if err != nil {
 		return fmt.Errorf("unable to update repo %s: %w", r.GetFullName(), err)
 	}

database/repo/count_org_test.go

@@ -43,12 +43,12 @@ func TestRepo_Engine_CountReposForOrg(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateRepo(_repoOne)
+	_, err := _sqlite.CreateRepo(_repoOne)
 	if err != nil {
 		t.Errorf("unable to create test repo for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateRepo(_repoTwo)
+	_, err = _sqlite.CreateRepo(_repoTwo)
 	if err != nil {
 		t.Errorf("unable to create test repo for sqlite: %v", err)
 	}

database/repo/count_test.go

@@ -43,12 +43,12 @@ func TestRepo_Engine_CountRepos(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateRepo(_repoOne)
+	_, err := _sqlite.CreateRepo(_repoOne)
 	if err != nil {
 		t.Errorf("unable to create test repo for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateRepo(_repoTwo)
+	_, err = _sqlite.CreateRepo(_repoTwo)
 	if err != nil {
 		t.Errorf("unable to create test repo for sqlite: %v", err)
 	}

database/repo/count_user_test.go

@@ -49,12 +49,12 @@ func TestRepo_Engine_CountReposForUser(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateRepo(_repoOne)
+	_, err := _sqlite.CreateRepo(_repoOne)
 	if err != nil {
 		t.Errorf("unable to create test repo for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateRepo(_repoTwo)
+	_, err = _sqlite.CreateRepo(_repoTwo)
 	if err != nil {
 		t.Errorf("unable to create test repo for sqlite: %v", err)
 	}

database/repo/create.go

@@ -15,7 +15,7 @@ import (
 )
 
 // CreateRepo creates a new repo in the database.
-func (e *engine) CreateRepo(r *library.Repo) error {
+func (e *engine) CreateRepo(r *library.Repo) (*library.Repo, error) {
 	e.logger.WithFields(logrus.Fields{
 		"org":  r.GetOrg(),
 		"repo": r.GetName(),
@@ -31,20 +31,31 @@ func (e *engine) CreateRepo(r *library.Repo) error {
 	// https://pkg.go.dev/github.com/go-vela/types/database#Repo.Validate
 	err := repo.Validate()
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	// encrypt the fields for the repo
 	//
 	// https://pkg.go.dev/github.com/go-vela/types/database#Repo.Encrypt
 	err = repo.Encrypt(e.config.EncryptionKey)
 	if err != nil {
-		return fmt.Errorf("unable to encrypt repo %s: %w", r.GetFullName(), err)
+		return nil, fmt.Errorf("unable to encrypt repo %s: %w", r.GetFullName(), err)
 	}
 
 	// send query to the database
-	return e.client.
-		Table(constants.TableRepo).
-		Create(repo).
-		Error
+	err = e.client.Table(constants.TableRepo).Create(repo).Error
+	if err != nil {
+		return nil, err
+	}
+
+	// decrypt the fields for the repo
+	err = repo.Decrypt(e.config.EncryptionKey)
+	if err != nil {
+		// only log to preserve backwards compatibility
+		e.logger.Errorf("unable to decrypt repo %d: %v", r.GetID(), err)
+
+		return repo.ToLibrary(), nil
+	}
+
+	return repo.ToLibrary(), nil
 }

database/repo/create_test.go

@@ -5,6 +5,7 @@
 package repo
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/DATA-DOG/go-sqlmock"
@@ -22,6 +23,7 @@ func TestRepo_Engine_CreateRepo(t *testing.T) {
 	_repo.SetVisibility("public")
 	_repo.SetPipelineType("yaml")
 	_repo.SetPreviousName("oldName")
+	_repo.SetTopics([]string{})
 
 	_postgres, _mock := testPostgres(t)
 	defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }()
@@ -60,7 +62,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.CreateRepo(_repo)
+			got, err := test.database.CreateRepo(_repo)
 
 			if test.failure {
 				if err == nil {
@@ -73,6 +75,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("CreateRepo for %s returned err: %v", test.name, err)
 			}
+
+			if !reflect.DeepEqual(got, _repo) {
+				t.Errorf("CreateRepo for %s returned %s, want %s", test.name, got, _repo)
+			}
 		})
 	}
 }

database/repo/delete_test.go

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

database/repo/get_org_test.go

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

database/repo/get_test.go

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

database/repo/interface.go

@@ -33,7 +33,7 @@ type RepoInterface interface {
 	// CountReposForUser defines a function that gets the count of repos by user ID.
 	CountReposForUser(*library.User, map[string]interface{}) (int64, error)
 	// CreateRepo defines a function that creates a new repo.
-	CreateRepo(*library.Repo) error
+	CreateRepo(*library.Repo) (*library.Repo, error)
 	// DeleteRepo defines a function that deletes an existing repo.
 	DeleteRepo(*library.Repo) error
 	// GetRepo defines a function that gets a repo by ID.
@@ -47,5 +47,5 @@ type RepoInterface interface {
 	// ListReposForUser defines a function that gets a list of repos by user ID.
 	ListReposForUser(*library.User, string, map[string]interface{}, int, int) ([]*library.Repo, int64, error)
 	// UpdateRepo defines a function that updates an existing repo.
-	UpdateRepo(*library.Repo) error
+	UpdateRepo(*library.Repo) (*library.Repo, error)
 }

database/repo/list_org_test.go

@@ -87,12 +87,12 @@ func TestRepo_Engine_ListReposForOrg(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateRepo(_repoOne)
+	_, err := _sqlite.CreateRepo(_repoOne)
 	if err != nil {
 		t.Errorf("unable to create test repo for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateRepo(_repoTwo)
+	_, err = _sqlite.CreateRepo(_repoTwo)
 	if err != nil {
 		t.Errorf("unable to create test repo for sqlite: %v", err)
 	}

database/repo/list_test.go

@@ -57,12 +57,12 @@ func TestRepo_Engine_ListRepos(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateRepo(_repoOne)
+	_, err := _sqlite.CreateRepo(_repoOne)
 	if err != nil {
 		t.Errorf("unable to create test repo for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateRepo(_repoTwo)
+	_, err = _sqlite.CreateRepo(_repoTwo)
 	if err != nil {
 		t.Errorf("unable to create test repo for sqlite: %v", err)
 	}

database/repo/list_user_test.go

@@ -92,12 +92,12 @@ func TestRepo_Engine_ListReposForUser(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateRepo(_repoOne)
+	_, err := _sqlite.CreateRepo(_repoOne)
 	if err != nil {
 		t.Errorf("unable to create test repo for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateRepo(_repoTwo)
+	_, err = _sqlite.CreateRepo(_repoTwo)
 	if err != nil {
 		t.Errorf("unable to create test repo for sqlite: %v", err)
 	}