Skip to content

Commit

Permalink
refactor(db/get-repos): utilize db indexing
Browse files Browse the repository at this point in the history
  • Loading branch information
wass3rw3rk committed Sep 20, 2024
1 parent 59553f5 commit fa013a7
Show file tree
Hide file tree
Showing 9 changed files with 21 additions and 28 deletions.
2 changes: 1 addition & 1 deletion api/build/compile_publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ func CompileAndPublish(
}

// send API call to capture repo for the counter (grabbing repo again to ensure counter is correct)
repo, err = database.GetRepoForOrg(ctx, r.GetOrg(), r.GetName())
repo, err = database.GetRepoForOrg(ctx, r.GetFullName())
if err != nil {
retErr := fmt.Errorf("%s: unable to get repo %s: %w", baseErr, r.GetFullName(), err)

Expand Down
9 changes: 1 addition & 8 deletions api/dashboard/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"context"
"fmt"
"net/http"
"strings"
"time"

"github.com/gin-gonic/gin"
Expand Down Expand Up @@ -173,14 +172,8 @@ func createAdminSet(c context.Context, caller *types.User, users []*types.User)
// in the database while also confirming the IDs match when saving.
func validateRepoSet(c context.Context, repos []*types.DashboardRepo) error {
for _, repo := range repos {
// verify format (org/repo)
parts := strings.Split(repo.GetName(), "/")
if len(parts) != 2 {
return fmt.Errorf("unable to create dashboard: %s is not a valid repo", repo.GetName())
}

// fetch repo from database
dbRepo, err := database.FromContext(c).GetRepoForOrg(c, parts[0], parts[1])
dbRepo, err := database.FromContext(c).GetRepoForOrg(c, repo.GetName())
if err != nil || !dbRepo.GetActive() {
return fmt.Errorf("unable to create dashboard: could not get repo %s: %w", repo.GetName(), err)
}
Expand Down
2 changes: 1 addition & 1 deletion api/repo/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ func CreateRepo(c *gin.Context) {
}

// send API call to capture the repo from the database
dbRepo, err := database.FromContext(c).GetRepoForOrg(ctx, r.GetOrg(), r.GetName())
dbRepo, err := database.FromContext(c).GetRepoForOrg(ctx, r.GetFullName())
if err == nil && dbRepo.GetActive() {
retErr := fmt.Errorf("unable to activate repo: %s is already active", r.GetFullName())

Expand Down
4 changes: 2 additions & 2 deletions api/webhook/post.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func PostWebhook(c *gin.Context) {
}()

// send API call to capture parsed repo from webhook
repo, err := database.FromContext(c).GetRepoForOrg(ctx, r.GetOrg(), r.GetName())
repo, err := database.FromContext(c).GetRepoForOrg(ctx, r.GetFullName())
if err != nil {
retErr := fmt.Errorf("%s: failed to get repo %s: %w", baseErr, r.GetFullName(), err)
util.HandleError(c, http.StatusBadRequest, retErr)
Expand Down Expand Up @@ -659,7 +659,7 @@ func handleRepositoryEvent(ctx context.Context, c *gin.Context, m *internal.Meta
case "archived", "unarchived", constants.ActionEdited:
l.Debugf("repository action %s for %s", h.GetEventAction(), r.GetFullName())
// send call to get repository from database
dbRepo, err := database.FromContext(c).GetRepoForOrg(ctx, r.GetOrg(), r.GetName())
dbRepo, err := database.FromContext(c).GetRepoForOrg(ctx, r.GetFullName())

Check failure on line 662 in api/webhook/post.go

View workflow job for this annotation

GitHub Actions / golangci

[golangci] api/webhook/post.go#L662

Non-inherited new context, use function like `context.WithXXX` instead (contextcheck)
Raw output
api/webhook/post.go:662:38: Non-inherited new context, use function like `context.WithXXX` instead (contextcheck)
		dbRepo, err := database.FromContext(c).GetRepoForOrg(ctx, r.GetFullName())
		                                   ^
if err != nil {
retErr := fmt.Errorf("%s: failed to get repo %s: %w", baseErr, r.GetFullName(), err)

Expand Down
2 changes: 1 addition & 1 deletion database/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1364,7 +1364,7 @@ func testRepos(t *testing.T, db Interface, resources *Resources) {

// lookup the repos by name
for _, repo := range resources.Repos {
got, err := db.GetRepoForOrg(context.TODO(), repo.GetOrg(), repo.GetName())
got, err := db.GetRepoForOrg(context.TODO(), repo.GetFullName())
if err != nil {
t.Errorf("unable to get repo %d by org: %v", repo.GetID(), err)
}
Expand Down
17 changes: 7 additions & 10 deletions database/repo/get_org.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,17 @@ package repo
import (
"context"

"github.com/sirupsen/logrus"

api "github.com/go-vela/server/api/types"
"github.com/go-vela/server/database/types"
"github.com/go-vela/types/constants"
)

// GetRepoForOrg gets a repo by org and repo name from the database.
func (e *engine) GetRepoForOrg(ctx context.Context, org, name string) (*api.Repo, error) {
e.logger.WithFields(logrus.Fields{
"org": org,
"repo": name,
}).Tracef("getting repo %s/%s", org, name)
func (e *engine) GetRepoForOrg(ctx context.Context, fullName string) (*api.Repo, error) {

Check failure on line 14 in database/repo/get_org.go

View workflow job for this annotation

GitHub Actions / golangci

[golangci] database/repo/get_org.go#L14

block should not start with a whitespace (wsl)
Raw output
database/repo/get_org.go:14:90: block should not start with a whitespace (wsl)
func (e *engine) GetRepoForOrg(ctx context.Context, fullName string) (*api.Repo, error) {
                                                                                         ^
// e.logger.WithFields(logrus.Fields{
// "org": org,
// "repo": name,
// }).Tracef("getting repo %s/%s", org, name)

// variable to store query results
r := new(types.Repo)
Expand All @@ -27,8 +25,7 @@ func (e *engine) GetRepoForOrg(ctx context.Context, org, name string) (*api.Repo
WithContext(ctx).
Table(constants.TableRepo).
Preload("Owner").
Where("org = ?", org).
Where("name = ?", name).
Where("full_name = ?", fullName).
Take(r).
Error
if err != nil {
Expand All @@ -43,7 +40,7 @@ func (e *engine) GetRepoForOrg(ctx context.Context, org, name string) (*api.Repo
// ensures that the change is backwards compatible
// by logging the error instead of returning it
// which allows us to fetch unencrypted repos
e.logger.Errorf("unable to decrypt repo %s/%s: %v", org, name, err)
e.logger.Errorf("unable to decrypt repo %s: %v", fullName, err)

// return the unencrypted repo
return r.ToAPI(), nil
Expand Down
4 changes: 2 additions & 2 deletions database/repo/get_org_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestRepo_Engine_GetRepoForOrg(t *testing.T) {
AddRow(1, "foo", "bar", "baz", false, false)

// ensure the mock expects the query
_mock.ExpectQuery(`SELECT * FROM "repos" WHERE org = $1 AND name = $2 LIMIT $3`).WithArgs("foo", "bar", 1).WillReturnRows(_rows)
_mock.ExpectQuery(`SELECT * FROM "repos" WHERE full_name = $1 LIMIT $2`).WithArgs("foo/bar", 1).WillReturnRows(_rows)
_mock.ExpectQuery(`SELECT * FROM "users" WHERE "users"."id" = $1`).WithArgs(1).WillReturnRows(_userRows)

_sqlite := testSqlite(t)
Expand Down Expand Up @@ -93,7 +93,7 @@ func TestRepo_Engine_GetRepoForOrg(t *testing.T) {
// run tests
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
got, err := test.database.GetRepoForOrg(context.TODO(), "foo", "bar")
got, err := test.database.GetRepoForOrg(context.TODO(), "foo/bar")

if test.failure {
if err == nil {
Expand Down
2 changes: 1 addition & 1 deletion database/repo/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ type RepoInterface interface {
// GetRepo defines a function that gets a repo by ID.
GetRepo(context.Context, int64) (*api.Repo, error)
// GetRepoForOrg defines a function that gets a repo by org and repo name.
GetRepoForOrg(context.Context, string, string) (*api.Repo, error)
GetRepoForOrg(context.Context, string) (*api.Repo, error)
// ListRepos defines a function that gets a list of all repos.
ListRepos(context.Context) ([]*api.Repo, error)
// ListReposForOrg defines a function that gets a list of repos by org name.
Expand Down
7 changes: 5 additions & 2 deletions router/middleware/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,12 @@ func Establish() gin.HandlerFunc {

l.Debugf("reading repo %s", rParam)

r, err := database.FromContext(c).GetRepoForOrg(ctx, o, rParam)
// construct full name
fullName := fmt.Sprintf("%s/%s", o, rParam)

r, err := database.FromContext(c).GetRepoForOrg(ctx, fullName)
if err != nil {
retErr := fmt.Errorf("unable to read repo %s/%s: %w", o, rParam, err)
retErr := fmt.Errorf("unable to read repo %s: %w", fullName, err)
util.HandleError(c, http.StatusNotFound, retErr)

return
Expand Down

0 comments on commit fa013a7

Please sign in to comment.