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 repo object on created and updated #913

Merged
merged 3 commits into from
Jul 25, 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/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

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

c.JSON(http.StatusOK, input)
c.JSON(http.StatusOK, r)
}
2 changes: 1 addition & 1 deletion api/build/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion api/build/restart.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion api/repo/chown.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
10 changes: 2 additions & 8 deletions api/repo/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion api/repo/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion api/repo/repair.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
5 changes: 1 addition & 4 deletions api/repo/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
}
2 changes: 1 addition & 1 deletion api/scm/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion api/scm/sync_org.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
6 changes: 3 additions & 3 deletions api/webhook/post.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion cmd/vela-server/schedule.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/repo/count_org_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/repo/count_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions database/repo/count_user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
25 changes: 18 additions & 7 deletions database/repo/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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
}
8 changes: 7 additions & 1 deletion database/repo/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package repo

import (
"reflect"
"testing"

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