From 3a6a89b09f707693b30e7f2c4e46ee1970bba0d7 Mon Sep 17 00:00:00 2001 From: ecrupper Date: Fri, 7 Feb 2025 10:53:33 -0600 Subject: [PATCH] enhance(pagination): remove total results calculation for performance improvement --- api/build/graph.go | 4 +- api/build/list_org.go | 5 +- api/build/list_repo.go | 5 +- api/build/update.go | 4 +- api/deployment/list.go | 12 +---- api/hook/list.go | 4 +- api/log/list_build.go | 4 +- api/pagination.go | 21 ++------ api/pipeline/list.go | 4 +- api/repo/list.go | 4 +- api/repo/list_org.go | 4 +- api/schedule/list.go | 4 +- api/scm/sync_org.go | 2 +- api/secret/list.go | 12 +---- api/service/list.go | 4 +- api/step/list.go | 4 +- api/user/get_source.go | 2 +- api/user/list.go | 4 +- api/webhook/post.go | 4 +- database/build/interface.go | 4 +- database/build/list.go | 14 +---- database/build/list_org.go | 20 ++----- database/build/list_org_test.go | 16 +----- database/build/list_repo.go | 20 ++----- database/build/list_repo_test.go | 10 +--- database/build/list_test.go | 8 +-- database/hook/interface.go | 2 +- database/hook/list.go | 14 +---- database/hook/list_repo.go | 20 ++----- database/hook/list_repo_test.go | 10 +--- database/hook/list_test.go | 8 +-- database/integration_test.go | 76 ++++++--------------------- database/log/interface.go | 2 +- database/log/list.go | 14 +---- database/log/list_build.go | 20 ++----- database/log/list_build_test.go | 10 +--- database/log/list_test.go | 8 +-- database/pipeline/interface.go | 2 +- database/pipeline/list.go | 14 +---- database/pipeline/list_repo.go | 22 ++------ database/pipeline/list_repo_test.go | 10 +--- database/pipeline/list_test.go | 8 +-- database/repo/interface.go | 4 +- database/repo/list.go | 14 +---- database/repo/list_org.go | 26 +++------ database/repo/list_org_test.go | 16 +----- database/repo/list_test.go | 8 +-- database/repo/list_user.go | 26 +++------ database/repo/list_user_test.go | 16 +----- database/schedule/interface.go | 2 +- database/schedule/list.go | 14 +---- database/schedule/list_active.go | 14 +---- database/schedule/list_active_test.go | 8 +-- database/schedule/list_repo.go | 20 ++----- database/schedule/list_repo_test.go | 10 +--- database/schedule/list_test.go | 8 +-- database/secret/interface.go | 8 +-- database/secret/list.go | 14 +---- database/secret/list_org.go | 20 ++----- database/secret/list_org_test.go | 11 +--- database/secret/list_repo.go | 20 ++----- database/secret/list_repo_test.go | 11 +--- database/secret/list_team.go | 40 +++----------- database/secret/list_team_test.go | 22 ++------ database/secret/list_test.go | 8 +-- database/service/interface.go | 2 +- database/service/list.go | 14 +---- database/service/list_build.go | 20 ++----- database/service/list_build_test.go | 10 +--- database/service/list_test.go | 8 +-- database/step/interface.go | 2 +- database/step/list.go | 14 +---- database/step/list_build.go | 20 ++----- database/step/list_build_test.go | 10 +--- database/step/list_test.go | 8 +-- database/user/interface.go | 2 +- database/user/list.go | 14 +---- database/user/list_lite.go | 20 ++----- database/user/list_lite_test.go | 10 +--- database/user/list_test.go | 8 +-- secret/native/list.go | 8 +-- 81 files changed, 189 insertions(+), 760 deletions(-) diff --git a/api/build/graph.go b/api/build/graph.go index 5db83eaa4..c57d97feb 100644 --- a/api/build/graph.go +++ b/api/build/graph.go @@ -249,7 +249,7 @@ func GetBuildGraph(c *gin.Context) { if len(p.Stages) > 0 || len(p.Steps) > 0 { for page > 0 { // retrieve build steps (per page) from the database - stepsPart, _, err := database.FromContext(c).ListStepsForBuild(ctx, b, nil, page, perPage) + stepsPart, err := database.FromContext(c).ListStepsForBuild(ctx, b, nil, page, perPage) if err != nil { retErr := fmt.Errorf("unable to retrieve steps for build %s: %w", entry, err) @@ -287,7 +287,7 @@ func GetBuildGraph(c *gin.Context) { if len(p.Services) > 0 { for page > 0 { // retrieve build services (per page) from the database - servicesPart, _, err := database.FromContext(c).ListServicesForBuild(ctx, b, nil, page, perPage) + servicesPart, err := database.FromContext(c).ListServicesForBuild(ctx, b, nil, page, perPage) if err != nil { retErr := fmt.Errorf("unable to retrieve services for build %s: %w", entry, err) diff --git a/api/build/list_org.go b/api/build/list_org.go index fd00559ef..f5f4b45a3 100644 --- a/api/build/list_org.go +++ b/api/build/list_org.go @@ -107,7 +107,6 @@ func ListBuildsForOrg(c *gin.Context) { var ( filters = map[string]interface{}{} b []*types.Build - t int64 ) // capture middleware values @@ -199,7 +198,7 @@ func ListBuildsForOrg(c *gin.Context) { } // send API call to capture the list of builds for the org (and event type if passed in) - b, t, err = database.FromContext(c).ListBuildsForOrg(ctx, o, filters, page, perPage) + b, err = database.FromContext(c).ListBuildsForOrg(ctx, o, filters, page, perPage) if err != nil { retErr := fmt.Errorf("unable to list builds for org %s: %w", o, err) @@ -212,7 +211,7 @@ func ListBuildsForOrg(c *gin.Context) { pagination := api.Pagination{ Page: page, PerPage: perPage, - Total: t, + Results: len(b), } // set pagination headers pagination.SetHeaderLink(c) diff --git a/api/build/list_repo.go b/api/build/list_repo.go index 1d7fae833..1d06fff45 100644 --- a/api/build/list_repo.go +++ b/api/build/list_repo.go @@ -129,7 +129,6 @@ func ListBuildsForRepo(c *gin.Context) { var ( filters = map[string]interface{}{} b []*types.Build - t int64 ) // capture middleware values @@ -237,7 +236,7 @@ func ListBuildsForRepo(c *gin.Context) { return } - b, t, err = database.FromContext(c).ListBuildsForRepo(ctx, r, filters, before, after, page, perPage) + b, err = database.FromContext(c).ListBuildsForRepo(ctx, r, filters, before, after, page, perPage) if err != nil { retErr := fmt.Errorf("unable to list builds for repo %s: %w", r.GetFullName(), err) @@ -250,7 +249,7 @@ func ListBuildsForRepo(c *gin.Context) { pagination := api.Pagination{ Page: page, PerPage: perPage, - Total: t, + Results: len(b), } // set pagination headers pagination.SetHeaderLink(c) diff --git a/api/build/update.go b/api/build/update.go index 83fc3f9b3..f7d0ea97d 100644 --- a/api/build/update.go +++ b/api/build/update.go @@ -196,7 +196,7 @@ func UpdateComponentStatuses(c *gin.Context, b *types.Build, status string) erro for page > 0 { // retrieve build steps (per page) from the database - stepsPart, _, err := database.FromContext(c).ListStepsForBuild(ctx, b, map[string]interface{}{}, page, perPage) + stepsPart, err := database.FromContext(c).ListStepsForBuild(ctx, b, map[string]interface{}{}, page, perPage) if err != nil { return err } @@ -234,7 +234,7 @@ func UpdateComponentStatuses(c *gin.Context, b *types.Build, status string) erro for page > 0 { // retrieve build services (per page) from the database - servicesPart, _, err := database.FromContext(c).ListServicesForBuild(ctx, b, map[string]interface{}{}, page, perPage) + servicesPart, err := database.FromContext(c).ListServicesForBuild(ctx, b, map[string]interface{}{}, page, perPage) if err != nil { return err } diff --git a/api/deployment/list.go b/api/deployment/list.go index 447fe55f3..1e87555e0 100644 --- a/api/deployment/list.go +++ b/api/deployment/list.go @@ -109,16 +109,6 @@ func ListDeployments(c *gin.Context) { // ensure per_page isn't above or below allowed values perPage = max(1, min(100, perPage)) - // send API call to capture the total number of deployments for the repo - t, err := database.FromContext(c).CountDeploymentsForRepo(c, r) - if err != nil { - retErr := fmt.Errorf("unable to get deployment count for %s: %w", r.GetFullName(), err) - - util.HandleError(c, http.StatusInternalServerError, retErr) - - return - } - // send API call to capture the list of deployments for the repo d, err := database.FromContext(c).ListDeploymentsForRepo(c, r, page, perPage) if err != nil { @@ -133,7 +123,7 @@ func ListDeployments(c *gin.Context) { pagination := api.Pagination{ Page: page, PerPage: perPage, - Total: t, + Results: len(d), } // set pagination headers pagination.SetHeaderLink(c) diff --git a/api/hook/list.go b/api/hook/list.go index 35724b64b..b13b37b93 100644 --- a/api/hook/list.go +++ b/api/hook/list.go @@ -112,7 +112,7 @@ func ListHooks(c *gin.Context) { perPage = max(1, min(100, perPage)) // send API call to capture the list of steps for the build - h, t, err := database.FromContext(c).ListHooksForRepo(ctx, r, page, perPage) + h, err := database.FromContext(c).ListHooksForRepo(ctx, r, page, perPage) if err != nil { retErr := fmt.Errorf("unable to get hooks for repo %s: %w", r.GetFullName(), err) @@ -125,7 +125,7 @@ func ListHooks(c *gin.Context) { pagination := api.Pagination{ Page: page, PerPage: perPage, - Total: t, + Results: len(h), } // set pagination headers pagination.SetHeaderLink(c) diff --git a/api/log/list_build.go b/api/log/list_build.go index ceb30806d..ef6d61d6d 100644 --- a/api/log/list_build.go +++ b/api/log/list_build.go @@ -112,7 +112,7 @@ func ListLogsForBuild(c *gin.Context) { perPage = max(1, min(100, perPage)) // send API call to capture the list of logs for the build - bl, t, err := database.FromContext(c).ListLogsForBuild(ctx, b, page, perPage) + bl, err := database.FromContext(c).ListLogsForBuild(ctx, b, page, perPage) if err != nil { retErr := fmt.Errorf("unable to list logs for build %s: %w", entry, err) @@ -125,7 +125,7 @@ func ListLogsForBuild(c *gin.Context) { pagination := api.Pagination{ Page: page, PerPage: perPage, - Total: t, + Results: len(bl), } // set pagination headers pagination.SetHeaderLink(c) diff --git a/api/pagination.go b/api/pagination.go index 126b36372..b2096bae3 100644 --- a/api/pagination.go +++ b/api/pagination.go @@ -4,7 +4,6 @@ package api import ( "fmt" - "math" "net/http" "strconv" "strings" @@ -16,7 +15,7 @@ import ( type Pagination struct { PerPage int Page int - Total int64 + Results int } // HeaderLink will hold the information needed to form a link element in the header. @@ -35,7 +34,6 @@ func (p *Pagination) SetHeaderLink(c *gin.Context) { hl := HeaderLink{ "first": 1, - "last": p.TotalPages(), "next": p.NextPage(), "prev": p.PrevPage(), } @@ -55,7 +53,7 @@ func (p *Pagination) SetHeaderLink(c *gin.Context) { } // drop last, next on the last page - if p.Page == p.TotalPages() { + if p.Results < p.PerPage { delete(hl, "last") delete(hl, "next") } @@ -77,7 +75,6 @@ func (p *Pagination) SetHeaderLink(c *gin.Context) { l = append(l, ls) } - c.Header("X-Total-Count", strconv.FormatInt(p.Total, 10)) c.Header("Link", strings.Join(l, ", ")) } @@ -106,22 +103,12 @@ func (p *Pagination) HasPrev() bool { // HasNext will return true if there is a next page. func (p *Pagination) HasNext() bool { - return p.Page < p.TotalPages() + return p.PerPage == p.Results } // HasPages returns true if there is need to deal with pagination. func (p *Pagination) HasPages() bool { - return p.Total > int64(p.PerPage) -} - -// TotalPages will return the total number of pages. -func (p *Pagination) TotalPages() int { - n := int(math.Ceil(float64(p.Total) / float64(p.PerPage))) - if n == 0 { - n = 1 - } - - return n + return !(p.Page == 1 && p.Results < p.PerPage) } // resolveScheme is a helper to determine the protocol scheme diff --git a/api/pipeline/list.go b/api/pipeline/list.go index f16717213..64071f8d4 100644 --- a/api/pipeline/list.go +++ b/api/pipeline/list.go @@ -115,7 +115,7 @@ func ListPipelines(c *gin.Context) { //nolint:gomnd // ignore magic number perPage = max(1, min(100, perPage)) - p, t, err := database.FromContext(c).ListPipelinesForRepo(ctx, r, page, perPage) + p, err := database.FromContext(c).ListPipelinesForRepo(ctx, r, page, perPage) if err != nil { retErr := fmt.Errorf("unable to list pipelines for repo %s: %w", r.GetFullName(), err) @@ -128,7 +128,7 @@ func ListPipelines(c *gin.Context) { pagination := api.Pagination{ Page: page, PerPage: perPage, - Total: t, + Results: len(p), } // set pagination headers pagination.SetHeaderLink(c) diff --git a/api/repo/list.go b/api/repo/list.go index eb584cba3..cc99e7ba2 100644 --- a/api/repo/list.go +++ b/api/repo/list.go @@ -108,7 +108,7 @@ func ListRepos(c *gin.Context) { } // send API call to capture the list of repos for the user - r, t, err := database.FromContext(c).ListReposForUser(ctx, u, sortBy, filters, page, perPage) + r, err := database.FromContext(c).ListReposForUser(ctx, u, sortBy, filters, page, perPage) if err != nil { retErr := fmt.Errorf("unable to get repos for user %s: %w", u.GetName(), err) @@ -121,7 +121,7 @@ func ListRepos(c *gin.Context) { pagination := api.Pagination{ Page: page, PerPage: perPage, - Total: t, + Results: len(r), } // set pagination headers pagination.SetHeaderLink(c) diff --git a/api/repo/list_org.go b/api/repo/list_org.go index 75f6a2aa1..270c1e0f4 100644 --- a/api/repo/list_org.go +++ b/api/repo/list_org.go @@ -145,7 +145,7 @@ func ListReposForOrg(c *gin.Context) { } // send API call to capture the list of repos for the org - r, t, err := database.FromContext(c).ListReposForOrg(ctx, o, sortBy, filters, page, perPage) + r, err := database.FromContext(c).ListReposForOrg(ctx, o, sortBy, filters, page, perPage) if err != nil { retErr := fmt.Errorf("unable to get repos for org %s: %w", o, err) @@ -158,7 +158,7 @@ func ListReposForOrg(c *gin.Context) { pagination := api.Pagination{ Page: page, PerPage: perPage, - Total: t, + Results: len(r), } // set pagination headers pagination.SetHeaderLink(c) diff --git a/api/schedule/list.go b/api/schedule/list.go index a8d7ed059..2cafe6311 100644 --- a/api/schedule/list.go +++ b/api/schedule/list.go @@ -111,7 +111,7 @@ func ListSchedules(c *gin.Context) { perPage = max(1, min(100, perPage)) // send API call to capture the list of schedules for the repo - s, t, err := database.FromContext(c).ListSchedulesForRepo(ctx, r, page, perPage) + s, err := database.FromContext(c).ListSchedulesForRepo(ctx, r, page, perPage) if err != nil { retErr := fmt.Errorf("unable to get schedules for repo %s: %w", r.GetFullName(), err) @@ -124,7 +124,7 @@ func ListSchedules(c *gin.Context) { pagination := api.Pagination{ Page: page, PerPage: perPage, - Total: t, + Results: len(s), } // set pagination headers pagination.SetHeaderLink(c) diff --git a/api/scm/sync_org.go b/api/scm/sync_org.go index e8efee734..649116388 100644 --- a/api/scm/sync_org.go +++ b/api/scm/sync_org.go @@ -105,7 +105,7 @@ func SyncReposForOrg(c *gin.Context) { page := 0 // capture all repos belonging to a certain org in database for orgRepos := int64(0); orgRepos < t; orgRepos += 100 { - r, _, err := database.FromContext(c).ListReposForOrg(ctx, o, "name", map[string]interface{}{}, page, 100) + r, err := database.FromContext(c).ListReposForOrg(ctx, o, "name", map[string]interface{}{}, page, 100) if err != nil { retErr := fmt.Errorf("unable to get repo count for org %s: %w", o, err) diff --git a/api/secret/list.go b/api/secret/list.go index f7e21e706..42f53ab91 100644 --- a/api/secret/list.go +++ b/api/secret/list.go @@ -162,16 +162,6 @@ func ListSecrets(c *gin.Context) { return } - // send API call to capture the total number of secrets - total, err := secret.FromContext(c, e).Count(ctx, t, o, n, teams) - if err != nil { - retErr := fmt.Errorf("unable to get secret count for %s from %s service: %w", entry, e, err) - - util.HandleError(c, http.StatusInternalServerError, retErr) - - return - } - // ensure per_page isn't above or below allowed values perPage = max(1, min(100, perPage)) @@ -189,7 +179,7 @@ func ListSecrets(c *gin.Context) { pagination := api.Pagination{ Page: page, PerPage: perPage, - Total: total, + Results: len(s), } // set pagination headers pagination.SetHeaderLink(c) diff --git a/api/service/list.go b/api/service/list.go index 0c1045867..373d16794 100644 --- a/api/service/list.go +++ b/api/service/list.go @@ -120,7 +120,7 @@ func ListServices(c *gin.Context) { perPage = max(1, min(100, perPage)) // send API call to capture the list of services for the build - s, t, err := database.FromContext(c).ListServicesForBuild(ctx, b, map[string]interface{}{}, page, perPage) + s, err := database.FromContext(c).ListServicesForBuild(ctx, b, map[string]interface{}{}, page, perPage) if err != nil { retErr := fmt.Errorf("unable to get services for build %s: %w", entry, err) @@ -133,7 +133,7 @@ func ListServices(c *gin.Context) { pagination := api.Pagination{ Page: page, PerPage: perPage, - Total: t, + Results: len(s), } // set pagination headers pagination.SetHeaderLink(c) diff --git a/api/step/list.go b/api/step/list.go index 08edc0189..fb29dc7c2 100644 --- a/api/step/list.go +++ b/api/step/list.go @@ -120,7 +120,7 @@ func ListSteps(c *gin.Context) { perPage = max(1, min(100, perPage)) // send API call to capture the list of steps for the build - s, t, err := database.FromContext(c).ListStepsForBuild(ctx, b, map[string]interface{}{}, page, perPage) + s, err := database.FromContext(c).ListStepsForBuild(ctx, b, map[string]interface{}{}, page, perPage) if err != nil { retErr := fmt.Errorf("unable to list steps for build %s: %w", entry, err) @@ -133,7 +133,7 @@ func ListSteps(c *gin.Context) { pagination := api.Pagination{ Page: page, PerPage: perPage, - Total: t, + Results: len(s), } // set pagination headers pagination.SetHeaderLink(c) diff --git a/api/user/get_source.go b/api/user/get_source.go index 4a7fe2bf2..03678a14d 100644 --- a/api/user/get_source.go +++ b/api/user/get_source.go @@ -87,7 +87,7 @@ func GetSourceRepos(c *gin.Context) { for page > 0 { // send API call to capture the list of repos for the org - dbReposPart, _, err := database.FromContext(c).ListReposForOrg(ctx, org, "name", filters, page, 100) + dbReposPart, err := database.FromContext(c).ListReposForOrg(ctx, org, "name", filters, page, 100) if err != nil { retErr := fmt.Errorf("unable to get repos for org %s: %w", org, err) diff --git a/api/user/list.go b/api/user/list.go index 9b1c91d11..bd58ef4fc 100644 --- a/api/user/list.go +++ b/api/user/list.go @@ -95,7 +95,7 @@ func ListUsers(c *gin.Context) { perPage = max(1, min(100, perPage)) // send API call to capture the list of users - users, t, err := database.FromContext(c).ListLiteUsers(ctx, page, perPage) + users, err := database.FromContext(c).ListLiteUsers(ctx, page, perPage) if err != nil { retErr := fmt.Errorf("unable to get users: %w", err) @@ -108,7 +108,7 @@ func ListUsers(c *gin.Context) { pagination := api.Pagination{ Page: page, PerPage: perPage, - Total: t, + Results: len(users), } // set pagination headers pagination.SetHeaderLink(c) diff --git a/api/webhook/post.go b/api/webhook/post.go index e5f644889..3e04c0ffe 100644 --- a/api/webhook/post.go +++ b/api/webhook/post.go @@ -757,7 +757,7 @@ func RenameRepository(ctx context.Context, l *logrus.Entry, db database.Interfac page := 1 // capture all secrets belonging to certain repo in database for repoSecrets := int64(0); repoSecrets < t; repoSecrets += 100 { - s, _, err := db.ListSecretsForRepo(ctx, dbR, map[string]interface{}{}, page, 100) + s, err := db.ListSecretsForRepo(ctx, dbR, map[string]interface{}{}, page, 100) if err != nil { return nil, fmt.Errorf("unable to get secret list for repo %s/%s: %w", dbR.GetOrg(), dbR.GetName(), err) } @@ -794,7 +794,7 @@ func RenameRepository(ctx context.Context, l *logrus.Entry, db database.Interfac page = 1 // capture all builds belonging to repo in database for build := int64(0); build < t; build += 100 { - b, _, err := db.ListBuildsForRepo(ctx, dbR, nil, time.Now().Unix(), 0, page, 100) + b, err := db.ListBuildsForRepo(ctx, dbR, nil, time.Now().Unix(), 0, page, 100) if err != nil { return nil, fmt.Errorf("unable to get build list for repo %s: %w", dbR.GetFullName(), err) } diff --git a/database/build/interface.go b/database/build/interface.go index 4b4f91f3e..5f62a01aa 100644 --- a/database/build/interface.go +++ b/database/build/interface.go @@ -51,11 +51,11 @@ type BuildInterface interface { // ListBuilds defines a function that gets a list of all builds. ListBuilds(context.Context) ([]*api.Build, error) // ListBuildsForOrg defines a function that gets a list of builds by org name. - ListBuildsForOrg(context.Context, string, map[string]interface{}, int, int) ([]*api.Build, int64, error) + ListBuildsForOrg(context.Context, string, map[string]interface{}, int, int) ([]*api.Build, error) // ListBuildsForDashboardRepo defines a function that gets a list of builds based on dashboard filters. ListBuildsForDashboardRepo(context.Context, *api.Repo, []string, []string) ([]*api.Build, error) // ListBuildsForRepo defines a function that gets a list of builds by repo ID. - ListBuildsForRepo(context.Context, *api.Repo, map[string]interface{}, int64, int64, int, int) ([]*api.Build, int64, error) + ListBuildsForRepo(context.Context, *api.Repo, map[string]interface{}, int64, int64, int, int) ([]*api.Build, error) // ListPendingAndRunningBuilds defines a function that gets a list of pending and running builds. ListPendingAndRunningBuilds(context.Context, string) ([]*api.QueueBuild, error) // ListPendingAndRunningBuildsForRepo defines a function that gets a list of pending and running builds for a repo. diff --git a/database/build/list.go b/database/build/list.go index 1a3e22a5b..8d79778fe 100644 --- a/database/build/list.go +++ b/database/build/list.go @@ -15,23 +15,11 @@ func (e *engine) ListBuilds(ctx context.Context) ([]*api.Build, error) { e.logger.Trace("listing all builds") // variables to store query results and return value - count := int64(0) b := new([]types.Build) builds := []*api.Build{} - // count the results - count, err := e.CountBuilds(ctx) - if err != nil { - return nil, err - } - - // short-circuit if there are no results - if count == 0 { - return builds, nil - } - // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Preload("Repo"). Preload("Repo.Owner"). diff --git a/database/build/list_org.go b/database/build/list_org.go index 5f8259753..6dcedca38 100644 --- a/database/build/list_org.go +++ b/database/build/list_org.go @@ -15,31 +15,19 @@ import ( // ListBuildsForOrg gets a list of builds by org name from the database. // //nolint:lll // ignore long line length due to variable names -func (e *engine) ListBuildsForOrg(ctx context.Context, org string, filters map[string]interface{}, page, perPage int) ([]*api.Build, int64, error) { +func (e *engine) ListBuildsForOrg(ctx context.Context, org string, filters map[string]interface{}, page, perPage int) ([]*api.Build, error) { e.logger.WithFields(logrus.Fields{ "org": org, }).Tracef("listing builds for org %s", org) // variables to store query results and return values - count := int64(0) b := new([]types.Build) builds := []*api.Build{} - // count the results - count, err := e.CountBuildsForOrg(ctx, org, filters) - if err != nil { - return builds, 0, err - } - - // short-circuit if there are no results - if count == 0 { - return builds, 0, nil - } - // calculate offset for pagination through results offset := perPage * (page - 1) - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableBuild). Preload("Repo"). @@ -55,7 +43,7 @@ func (e *engine) ListBuildsForOrg(ctx context.Context, org string, filters map[s Find(&b). Error if err != nil { - return nil, count, err + return nil, err } // iterate through all query results @@ -71,5 +59,5 @@ func (e *engine) ListBuildsForOrg(ctx context.Context, org string, filters map[s builds = append(builds, tmp.ToAPI()) } - return builds, count, nil + return builds, nil } diff --git a/database/build/list_org_test.go b/database/build/list_org_test.go index 965955891..7ad6c0b8b 100644 --- a/database/build/list_org_test.go +++ b/database/build/list_org_test.go @@ -63,12 +63,8 @@ func TestBuild_Engine_ListBuildsForOrg(t *testing.T) { _postgres, _mock := testPostgres(t) defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() - // create expected count query without filters result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - // ensure the mock expects the count query without filters - _mock.ExpectQuery(`SELECT count(*) FROM "builds" JOIN repos ON builds.repo_id = repos.id WHERE repos.org = $1`).WithArgs("foo").WillReturnRows(_rows) // create expected query without filters result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "repo_id", "pipeline_id", "number", "parent", "event", "event_action", "status", "error", "enqueued", "created", "started", "finished", "deploy", "deploy_payload", "clone", "source", "title", "message", "commit", "sender", "author", "email", "link", "branch", "ref", "base_ref", "head_ref", "host", "runtime", "distribution", "approved_at", "approved_by", "timestamp"}). AddRow(1, 1, nil, 1, 0, "push", "", "", "", 0, 0, 0, 0, "", nil, "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", 0, "", 0). AddRow(2, 2, nil, 2, 0, "push", "", "", "", 0, 0, 0, 0, "", nil, "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", 0, "", 0) @@ -87,10 +83,6 @@ func TestBuild_Engine_ListBuildsForOrg(t *testing.T) { _mock.ExpectQuery(`SELECT * FROM "repos" WHERE "repos"."id" IN ($1,$2)`).WithArgs(1, 2).WillReturnRows(_repoRows) _mock.ExpectQuery(`SELECT * FROM "users" WHERE "users"."id" = $1`).WithArgs(1).WillReturnRows(_userRows) - // create expected count query with event filter result in mock - _rows = sqlmock.NewRows([]string{"count"}).AddRow(2) - // ensure the mock expects the count query with event filter - _mock.ExpectQuery(`SELECT count(*) FROM "builds" JOIN repos ON builds.repo_id = repos.id WHERE repos.org = $1 AND "event" = $2`).WithArgs("foo", "push").WillReturnRows(_rows) // create expected query with event filter result in mock _rows = sqlmock.NewRows( []string{"id", "repo_id", "pipeline_id", "number", "parent", "event", "event_action", "status", "error", "enqueued", "created", "started", "finished", "deploy", "deploy_payload", "clone", "source", "title", "message", "commit", "sender", "author", "email", "link", "branch", "ref", "base_ref", "head_ref", "host", "runtime", "distribution", "approved_at", "approved_by", "timestamp"}). @@ -111,10 +103,6 @@ func TestBuild_Engine_ListBuildsForOrg(t *testing.T) { _mock.ExpectQuery(`SELECT * FROM "repos" WHERE "repos"."id" IN ($1,$2)`).WithArgs(1, 2).WillReturnRows(_repoRows) _mock.ExpectQuery(`SELECT * FROM "users" WHERE "users"."id" = $1`).WithArgs(1).WillReturnRows(_userRows) - // create expected count query with visibility filter result in mock - _rows = sqlmock.NewRows([]string{"count"}).AddRow(2) - // ensure the mock expects the count query with visibility filter - _mock.ExpectQuery(`SELECT count(*) FROM "builds" JOIN repos ON builds.repo_id = repos.id WHERE repos.org = $1 AND "visibility" = $2`).WithArgs("foo", "public").WillReturnRows(_rows) // create expected query with visibility filter result in mock _rows = sqlmock.NewRows( []string{"id", "repo_id", "pipeline_id", "number", "parent", "event", "event_action", "status", "error", "enqueued", "created", "started", "finished", "deploy", "deploy_payload", "clone", "source", "title", "message", "commit", "sender", "author", "email", "link", "branch", "ref", "base_ref", "head_ref", "host", "runtime", "distribution", "approved_at", "approved_by", "timestamp"}). @@ -236,7 +224,7 @@ func TestBuild_Engine_ListBuildsForOrg(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, _, err := test.database.ListBuildsForOrg(context.TODO(), "foo", test.filters, 1, 10) + got, err := test.database.ListBuildsForOrg(context.TODO(), "foo", test.filters, 1, 10) if test.failure { if err == nil { diff --git a/database/build/list_repo.go b/database/build/list_repo.go index 3de7b2da4..0bb4931ca 100644 --- a/database/build/list_repo.go +++ b/database/build/list_repo.go @@ -15,32 +15,20 @@ import ( // ListBuildsForRepo gets a list of builds by repo ID from the database. // //nolint:lll // ignore long line length due to variable names -func (e *engine) ListBuildsForRepo(ctx context.Context, r *api.Repo, filters map[string]interface{}, before, after int64, page, perPage int) ([]*api.Build, int64, error) { +func (e *engine) ListBuildsForRepo(ctx context.Context, r *api.Repo, filters map[string]interface{}, before, after int64, page, perPage int) ([]*api.Build, error) { e.logger.WithFields(logrus.Fields{ "org": r.GetOrg(), "repo": r.GetName(), }).Tracef("listing builds for repo %s", r.GetFullName()) // variables to store query results and return values - count := int64(0) b := new([]types.Build) builds := []*api.Build{} - // count the results - count, err := e.CountBuildsForRepo(ctx, r, filters, before, after) - if err != nil { - return builds, 0, err - } - - // short-circuit if there are no results - if count == 0 { - return builds, 0, nil - } - // calculate offset for pagination through results offset := perPage * (page - 1) - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableBuild). Where("repo_id = ?", r.GetID()). @@ -53,7 +41,7 @@ func (e *engine) ListBuildsForRepo(ctx context.Context, r *api.Repo, filters map Find(&b). Error if err != nil { - return nil, count, err + return nil, err } // iterate through all query results @@ -67,5 +55,5 @@ func (e *engine) ListBuildsForRepo(ctx context.Context, r *api.Repo, filters map builds = append(builds, result) } - return builds, count, nil + return builds, nil } diff --git a/database/build/list_repo_test.go b/database/build/list_repo_test.go index c384405bf..7f42f1216 100644 --- a/database/build/list_repo_test.go +++ b/database/build/list_repo_test.go @@ -51,14 +51,8 @@ func TestBuild_Engine_ListBuildsForRepo(t *testing.T) { _postgres, _mock := testPostgres(t) defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() - // create expected count query result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the count query - _mock.ExpectQuery(`SELECT count(*) FROM "builds" WHERE repo_id = $1 AND created < $2 AND created > $3`).WithArgs(1, AnyArgument{}, 0).WillReturnRows(_rows) - // create expected query result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "repo_id", "pipeline_id", "number", "parent", "event", "event_action", "status", "error", "enqueued", "created", "started", "finished", "deploy", "deploy_payload", "clone", "source", "title", "message", "commit", "sender", "author", "email", "link", "branch", "ref", "base_ref", "head_ref", "host", "runtime", "distribution", "approved_at", "approved_by", "timestamp"}). AddRow(2, 1, nil, 2, 0, "", "", "", "", 0, 2, 0, 0, "", nil, "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", 0, "", 0). AddRow(1, 1, nil, 1, 0, "", "", "", "", 0, 1, 0, 0, "", nil, "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", 0, "", 0) @@ -105,7 +99,7 @@ func TestBuild_Engine_ListBuildsForRepo(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, _, err := test.database.ListBuildsForRepo(context.TODO(), _repo, filters, time.Now().UTC().Unix(), 0, 1, 10) + got, err := test.database.ListBuildsForRepo(context.TODO(), _repo, filters, time.Now().UTC().Unix(), 0, 1, 10) if test.failure { if err == nil { diff --git a/database/build/list_test.go b/database/build/list_test.go index e440a162c..d4caa9843 100644 --- a/database/build/list_test.go +++ b/database/build/list_test.go @@ -52,13 +52,7 @@ func TestBuild_Engine_ListBuilds(t *testing.T) { defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "builds"`).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "repo_id", "pipeline_id", "number", "parent", "event", "event_action", "status", "error", "enqueued", "created", "started", "finished", "deploy", "deploy_number", "deploy_payload", "clone", "source", "title", "message", "commit", "sender", "author", "email", "link", "branch", "ref", "base_ref", "head_ref", "host", "runtime", "distribution", "timestamp"}). AddRow(1, 1, nil, 1, 0, "", "", "", "", 0, 0, 0, 0, "", 0, nil, "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", 0). AddRow(2, 1, nil, 2, 0, "", "", "", "", 0, 0, 0, 0, "", 0, nil, "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", 0) diff --git a/database/hook/interface.go b/database/hook/interface.go index aad449413..e839bf0e0 100644 --- a/database/hook/interface.go +++ b/database/hook/interface.go @@ -45,7 +45,7 @@ type HookInterface interface { // ListHooks defines a function that gets a list of all hooks. ListHooks(context.Context) ([]*api.Hook, error) // ListHooksForRepo defines a function that gets a list of hooks by repo ID. - ListHooksForRepo(context.Context, *api.Repo, int, int) ([]*api.Hook, int64, error) + ListHooksForRepo(context.Context, *api.Repo, int, int) ([]*api.Hook, error) // UpdateHook defines a function that updates an existing hook. UpdateHook(context.Context, *api.Hook) (*api.Hook, error) } diff --git a/database/hook/list.go b/database/hook/list.go index 95da18ca6..67331645b 100644 --- a/database/hook/list.go +++ b/database/hook/list.go @@ -15,23 +15,11 @@ func (e *engine) ListHooks(ctx context.Context) ([]*api.Hook, error) { e.logger.Trace("listing all hooks") // variables to store query results and return value - count := int64(0) h := new([]types.Hook) hooks := []*api.Hook{} - // count the results - count, err := e.CountHooks(ctx) - if err != nil { - return nil, err - } - - // short-circuit if there are no results - if count == 0 { - return hooks, nil - } - // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableHook). Preload("Repo"). diff --git a/database/hook/list_repo.go b/database/hook/list_repo.go index dea9b350e..6304c4978 100644 --- a/database/hook/list_repo.go +++ b/database/hook/list_repo.go @@ -13,33 +13,21 @@ import ( ) // ListHooksForRepo gets a list of hooks by repo ID from the database. -func (e *engine) ListHooksForRepo(ctx context.Context, r *api.Repo, page, perPage int) ([]*api.Hook, int64, error) { +func (e *engine) ListHooksForRepo(ctx context.Context, r *api.Repo, page, perPage int) ([]*api.Hook, error) { e.logger.WithFields(logrus.Fields{ "org": r.GetOrg(), "repo": r.GetName(), }).Tracef("listing hooks for repo %s", r.GetFullName()) // variables to store query results and return value - count := int64(0) h := new([]types.Hook) hooks := []*api.Hook{} - // count the results - count, err := e.CountHooksForRepo(ctx, r) - if err != nil { - return nil, 0, err - } - - // short-circuit if there are no results - if count == 0 { - return hooks, 0, nil - } - // calculate offset for pagination through results offset := perPage * (page - 1) // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableHook). Preload("Build"). @@ -50,7 +38,7 @@ func (e *engine) ListHooksForRepo(ctx context.Context, r *api.Repo, page, perPag Find(&h). Error if err != nil { - return nil, count, err + return nil, err } // iterate through all query results @@ -64,5 +52,5 @@ func (e *engine) ListHooksForRepo(ctx context.Context, r *api.Repo, page, perPag hooks = append(hooks, result) } - return hooks, count, nil + return hooks, nil } diff --git a/database/hook/list_repo_test.go b/database/hook/list_repo_test.go index ea3301f2c..9cd02200b 100644 --- a/database/hook/list_repo_test.go +++ b/database/hook/list_repo_test.go @@ -61,13 +61,7 @@ func TestHook_Engine_ListHooksForRepo(t *testing.T) { defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "hooks" WHERE repo_id = $1`).WithArgs(1).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "repo_id", "build_id", "number", "source_id", "created", "host", "event", "event_action", "branch", "error", "status", "link", "webhook_id"}). AddRow(2, 1, 0, 2, "c8da1302-07d6-11ea-882f-4893bca275b8", 0, "", "", "", "", "", "", "", 1). AddRow(1, 1, 1, 1, "c8da1302-07d6-11ea-882f-4893bca275b8", 0, "", "", "", "", "", "", "", 1) @@ -116,7 +110,7 @@ func TestHook_Engine_ListHooksForRepo(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, _, err := test.database.ListHooksForRepo(context.TODO(), _repo, 1, 10) + got, err := test.database.ListHooksForRepo(context.TODO(), _repo, 1, 10) // empty values of build are different in testing but still empty // TODO: fix complex types such as deploy payload, dashboards, favorites, etc for empty comps diff --git a/database/hook/list_test.go b/database/hook/list_test.go index 5d8aa4fe9..3dd167cd0 100644 --- a/database/hook/list_test.go +++ b/database/hook/list_test.go @@ -61,13 +61,7 @@ func TestHook_Engine_ListHooks(t *testing.T) { defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "hooks"`).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "repo_id", "build_id", "number", "source_id", "created", "host", "event", "event_action", "branch", "error", "status", "link", "webhook_id"}). AddRow(1, 1, 1, 1, "c8da1302-07d6-11ea-882f-4893bca275b8", 0, "", "", "", "", "", "", "", 1). AddRow(2, 1, 0, 2, "c8da1302-07d6-11ea-882f-4893bca275b8", 0, "", "", "", "", "", "", "", 1) diff --git a/database/integration_test.go b/database/integration_test.go index a629c1e95..a0a49ba17 100644 --- a/database/integration_test.go +++ b/database/integration_test.go @@ -288,26 +288,20 @@ func testBuilds(t *testing.T, db Interface, resources *Resources) { methods["ListBuilds"] = true // list the builds for an org - list, count, err = db.ListBuildsForOrg(context.TODO(), resources.Repos[0].GetOrg(), nil, 1, 10) + list, err = db.ListBuildsForOrg(context.TODO(), resources.Repos[0].GetOrg(), nil, 1, 10) if err != nil { t.Errorf("unable to list builds for org %s: %v", resources.Repos[0].GetOrg(), err) } - if int(count) != len(resources.Builds) { - t.Errorf("ListBuildsForOrg() is %v, want %v", count, len(resources.Builds)) - } if diff := cmp.Diff(resources.Builds, list); diff != "" { t.Errorf("ListBuildsForOrg() mismatch (-want +got):\n%s", diff) } methods["ListBuildsForOrg"] = true // list the builds for a repo - list, count, err = db.ListBuildsForRepo(context.TODO(), resources.Repos[0], nil, time.Now().UTC().Unix(), 0, 1, 10) + list, err = db.ListBuildsForRepo(context.TODO(), resources.Repos[0], nil, time.Now().UTC().Unix(), 0, 1, 10) if err != nil { t.Errorf("unable to list builds for repo %d: %v", resources.Repos[0].GetID(), err) } - if int(count) != len(resources.Builds) { - t.Errorf("ListBuildsForRepo() is %v, want %v", count, len(resources.Builds)) - } if diff := cmp.Diff([]*api.Build{resources.Builds[1], resources.Builds[0]}, list); diff != "" { t.Errorf("ListBuildsForRepo() mismatch (-want +got):\n%s", diff) } @@ -873,14 +867,10 @@ func testHooks(t *testing.T, db Interface, resources *Resources) { methods["ListHooks"] = true // list the hooks for a repo - list, count, err = db.ListHooksForRepo(context.TODO(), resources.Repos[0], 1, 10) + list, err = db.ListHooksForRepo(context.TODO(), resources.Repos[0], 1, 10) if err != nil { t.Errorf("unable to list hooks for repo %d: %v", resources.Repos[0].GetID(), err) } - // only 2 of 3 hooks belong to Repos[0] repo - if int(count) != len(resources.Hooks)-1 { - t.Errorf("ListHooksForRepo() is %v, want %v", count, len(resources.Hooks)) - } if diff := cmp.Diff([]*api.Hook{resources.Hooks[2], resources.Hooks[0]}, list); diff != "" { t.Errorf("ListHooksForRepo() mismatch (-want +got):\n%s", diff) } @@ -1120,13 +1110,10 @@ func testLogs(t *testing.T, db Interface, resources *Resources) { methods["ListLogs"] = true // list the logs for a build - list, count, err = db.ListLogsForBuild(context.TODO(), resources.Builds[0], 1, 10) + list, err = db.ListLogsForBuild(context.TODO(), resources.Builds[0], 1, 10) if err != nil { t.Errorf("unable to list logs for build %d: %v", resources.Builds[0].GetID(), err) } - if int(count) != len(resources.Logs) { - t.Errorf("ListLogsForBuild() is %v, want %v", count, len(resources.Logs)) - } if diff := cmp.Diff(resources.Logs, list); diff != "" { t.Errorf("ListLogsForBuild() mismatch (-want +got):\n%s", diff) } @@ -1269,13 +1256,10 @@ func testPipelines(t *testing.T, db Interface, resources *Resources) { methods["ListPipelines"] = true // list the pipelines for a repo - list, count, err = db.ListPipelinesForRepo(context.TODO(), resources.Repos[0], 1, 10) + list, err = db.ListPipelinesForRepo(context.TODO(), resources.Repos[0], 1, 10) if err != nil { t.Errorf("unable to list pipelines for repo %d: %v", resources.Repos[0].GetID(), err) } - if int(count) != len(resources.Pipelines) { - t.Errorf("ListPipelinesForRepo() is %v, want %v", count, len(resources.Pipelines)) - } if diff := cmp.Diff(resources.Pipelines, list); diff != "" { t.Errorf("ListPipelines() mismatch (-want +got):\n%s", diff) } @@ -1423,26 +1407,20 @@ func testRepos(t *testing.T, db Interface, resources *Resources) { methods["ListRepos"] = true // list the repos for an org - list, count, err = db.ListReposForOrg(context.TODO(), resources.Repos[0].GetOrg(), "name", nil, 1, 10) + list, err = db.ListReposForOrg(context.TODO(), resources.Repos[0].GetOrg(), "name", nil, 1, 10) if err != nil { t.Errorf("unable to list repos for org %s: %v", resources.Repos[0].GetOrg(), err) } - if int(count) != len(resources.Repos) { - t.Errorf("ListReposForOrg() is %v, want %v", count, len(resources.Repos)) - } if diff := cmp.Diff(resources.Repos, list); diff != "" { t.Errorf("ListReposForOrg() mismatch (-want +got):\n%s", diff) } methods["ListReposForOrg"] = true // list the repos for a user - list, count, err = db.ListReposForUser(context.TODO(), resources.Users[0], "name", nil, 1, 10) + list, err = db.ListReposForUser(context.TODO(), resources.Users[0], "name", nil, 1, 10) if err != nil { t.Errorf("unable to list repos for user %d: %v", resources.Users[0].GetID(), err) } - if int(count) != len(resources.Repos) { - t.Errorf("ListReposForUser() is %v, want %v", count, len(resources.Repos)) - } if diff := cmp.Diff(resources.Repos, list); diff != "" { t.Errorf("ListReposForUser() mismatch (-want +got):\n%s", diff) } @@ -1589,13 +1567,10 @@ func testSchedules(t *testing.T, db Interface, resources *Resources) { methods["ListActiveSchedules"] = true // list the schedules for a repo - list, count, err = db.ListSchedulesForRepo(context.TODO(), resources.Repos[0], 1, 10) + list, err = db.ListSchedulesForRepo(context.TODO(), resources.Repos[0], 1, 10) if err != nil { t.Errorf("unable to count schedules for repo %d: %v", resources.Repos[0].GetID(), err) } - if int(count) != len(resources.Schedules) { - t.Errorf("ListSchedulesForRepo() is %v, want %v", count, len(resources.Schedules)) - } if !cmp.Equal(list, []*api.Schedule{resources.Schedules[1], resources.Schedules[0]}, CmpOptApproxUpdatedAt()) { t.Errorf("ListSchedulesForRepo() is %v, want %v", list, []*api.Schedule{resources.Schedules[1], resources.Schedules[0]}) } @@ -1760,52 +1735,40 @@ func testSecrets(t *testing.T, db Interface, resources *Resources) { switch secret.GetType() { case constants.SecretOrg: // list the secrets for an org - list, count, err = db.ListSecretsForOrg(context.TODO(), secret.GetOrg(), nil, 1, 10) + list, err = db.ListSecretsForOrg(context.TODO(), secret.GetOrg(), nil, 1, 10) if err != nil { t.Errorf("unable to list secrets for org %s: %v", secret.GetOrg(), err) } - if int(count) != 1 { - t.Errorf("ListSecretsForOrg() is %v, want %v", count, 1) - } if !cmp.Equal(list, []*api.Secret{secret}) { t.Errorf("ListSecretsForOrg() is %v, want %v", list, []*api.Secret{secret}) } methods["ListSecretsForOrg"] = true case constants.SecretRepo: // list the secrets for a repo - list, count, err = db.ListSecretsForRepo(context.TODO(), resources.Repos[0], nil, 1, 10) + list, err = db.ListSecretsForRepo(context.TODO(), resources.Repos[0], nil, 1, 10) if err != nil { t.Errorf("unable to list secrets for repo %d: %v", resources.Repos[0].GetID(), err) } - if int(count) != 1 { - t.Errorf("ListSecretsForRepo() is %v, want %v", count, 1) - } if !cmp.Equal(list, []*api.Secret{secret}, CmpOptApproxUpdatedAt()) { t.Errorf("ListSecretsForRepo() is %v, want %v", list, []*api.Secret{secret}) } methods["ListSecretsForRepo"] = true case constants.SecretShared: // list the secrets for a team - list, count, err = db.ListSecretsForTeam(context.TODO(), secret.GetOrg(), secret.GetTeam(), nil, 1, 10) + list, err = db.ListSecretsForTeam(context.TODO(), secret.GetOrg(), secret.GetTeam(), nil, 1, 10) if err != nil { t.Errorf("unable to list secrets for team %s: %v", secret.GetTeam(), err) } - if int(count) != 1 { - t.Errorf("ListSecretsForTeam() is %v, want %v", count, 1) - } if !cmp.Equal(list, []*api.Secret{secret}, CmpOptApproxUpdatedAt()) { t.Errorf("ListSecretsForTeam() is %v, want %v", list, []*api.Secret{secret}) } methods["ListSecretsForTeam"] = true // list the secrets for a list of teams - list, count, err = db.ListSecretsForTeams(context.TODO(), secret.GetOrg(), []string{secret.GetTeam()}, nil, 1, 10) + list, err = db.ListSecretsForTeams(context.TODO(), secret.GetOrg(), []string{secret.GetTeam()}, nil, 1, 10) if err != nil { t.Errorf("unable to list secrets for teams %s: %v", []string{secret.GetTeam()}, err) } - if int(count) != 1 { - t.Errorf("ListSecretsForTeams() is %v, want %v", count, 1) - } if !cmp.Equal(list, []*api.Secret{secret}, CmpOptApproxUpdatedAt()) { t.Errorf("ListSecretsForTeams() is %v, want %v", list, []*api.Secret{secret}) } @@ -1942,16 +1905,13 @@ func testServices(t *testing.T, db Interface, resources *Resources) { methods["ListServices"] = true // list the services for a build - list, count, err = db.ListServicesForBuild(context.TODO(), resources.Builds[0], nil, 1, 10) + list, err = db.ListServicesForBuild(context.TODO(), resources.Builds[0], nil, 1, 10) if err != nil { t.Errorf("unable to list services for build %d: %v", resources.Builds[0].GetID(), err) } if !cmp.Equal(list, []*api.Service{resources.Services[1], resources.Services[0]}) { t.Errorf("ListServicesForBuild() is %v, want %v", list, []*api.Service{resources.Services[1], resources.Services[0]}) } - if int(count) != len(resources.Services) { - t.Errorf("ListServicesForBuild() is %v, want %v", count, len(resources.Services)) - } methods["ListServicesForBuild"] = true expected := map[string]float64{ @@ -2097,16 +2057,13 @@ func testSteps(t *testing.T, db Interface, resources *Resources) { methods["ListSteps"] = true // list the steps for a build - list, count, err = db.ListStepsForBuild(ctx, resources.Builds[0], nil, 1, 10) + list, err = db.ListStepsForBuild(ctx, resources.Builds[0], nil, 1, 10) if err != nil { t.Errorf("unable to list steps for build %d: %v", resources.Builds[0].GetID(), err) } if !cmp.Equal(list, []*api.Step{resources.Steps[1], resources.Steps[0]}) { t.Errorf("ListStepsForBuild() is %v, want %v", list, []*api.Step{resources.Steps[1], resources.Steps[0]}) } - if int(count) != len(resources.Steps) { - t.Errorf("ListStepsForBuild() is %v, want %v", count, len(resources.Steps)) - } methods["ListStepsForBuild"] = true expected := map[string]float64{ @@ -2263,16 +2220,13 @@ func testUsers(t *testing.T, db Interface, resources *Resources) { methods["ListUsers"] = true // lite list the users - list, count, err = db.ListLiteUsers(context.TODO(), 1, 10) + list, err = db.ListLiteUsers(context.TODO(), 1, 10) if err != nil { t.Errorf("unable to list lite users: %v", err) } if !cmp.Equal(list, liteUsers) { t.Errorf("ListLiteUsers() is %v, want %v", list, liteUsers) } - if int(count) != len(liteUsers) { - t.Errorf("ListLiteUsers() is %v, want %v", count, len(liteUsers)) - } methods["ListLiteUsers"] = true // lookup the users by name diff --git a/database/log/interface.go b/database/log/interface.go index d5e77ead3..6f78c2c53 100644 --- a/database/log/interface.go +++ b/database/log/interface.go @@ -43,7 +43,7 @@ type LogInterface interface { // ListLogs defines a function that gets a list of all logs. ListLogs(context.Context) ([]*api.Log, error) // ListLogsForBuild defines a function that gets a list of logs by build ID. - ListLogsForBuild(context.Context, *api.Build, int, int) ([]*api.Log, int64, error) + ListLogsForBuild(context.Context, *api.Build, int, int) ([]*api.Log, error) // UpdateLog defines a function that updates an existing log. UpdateLog(context.Context, *api.Log) error } diff --git a/database/log/list.go b/database/log/list.go index aa0e816b1..adb0a1a4a 100644 --- a/database/log/list.go +++ b/database/log/list.go @@ -15,23 +15,11 @@ func (e *engine) ListLogs(ctx context.Context) ([]*api.Log, error) { e.logger.Trace("listing all logs") // variables to store query results and return value - count := int64(0) l := new([]types.Log) logs := []*api.Log{} - // count the results - count, err := e.CountLogs(ctx) - if err != nil { - return nil, err - } - - // short-circuit if there are no results - if count == 0 { - return logs, nil - } - // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableLog). Find(&l). diff --git a/database/log/list_build.go b/database/log/list_build.go index 4318a3b5a..1f7e1f80c 100644 --- a/database/log/list_build.go +++ b/database/log/list_build.go @@ -11,30 +11,18 @@ import ( ) // ListLogsForBuild gets a list of logs by build ID from the database. -func (e *engine) ListLogsForBuild(ctx context.Context, b *api.Build, page, perPage int) ([]*api.Log, int64, error) { +func (e *engine) ListLogsForBuild(ctx context.Context, b *api.Build, page, perPage int) ([]*api.Log, error) { e.logger.Tracef("listing logs for build %d", b.GetID()) // variables to store query results and return value - count := int64(0) l := new([]types.Log) logs := []*api.Log{} - // count the results - count, err := e.CountLogsForBuild(ctx, b) - if err != nil { - return nil, 0, err - } - - // short-circuit if there are no results - if count == 0 { - return logs, 0, nil - } - // calculate offset for pagination through results offset := perPage * (page - 1) // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableLog). Where("build_id = ?", b.GetID()). @@ -45,7 +33,7 @@ func (e *engine) ListLogsForBuild(ctx context.Context, b *api.Build, page, perPa Find(&l). Error if err != nil { - return nil, count, err + return nil, err } // iterate through all query results @@ -66,5 +54,5 @@ func (e *engine) ListLogsForBuild(ctx context.Context, b *api.Build, page, perPa logs = append(logs, tmp.ToAPI()) } - return logs, count, nil + return logs, nil } diff --git a/database/log/list_build_test.go b/database/log/list_build_test.go index 29ad6a162..9b7dfdaa0 100644 --- a/database/log/list_build_test.go +++ b/database/log/list_build_test.go @@ -42,13 +42,7 @@ func TestLog_Engine_ListLogsForBuild(t *testing.T) { defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "logs" WHERE build_id = $1`).WithArgs(1).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "build_id", "repo_id", "service_id", "step_id", "data"}). AddRow(1, 1, 1, 1, 0, []byte{}).AddRow(2, 1, 1, 0, 1, []byte{}) @@ -92,7 +86,7 @@ func TestLog_Engine_ListLogsForBuild(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, _, err := test.database.ListLogsForBuild(context.TODO(), _build, 1, 10) + got, err := test.database.ListLogsForBuild(context.TODO(), _build, 1, 10) if test.failure { if err == nil { diff --git a/database/log/list_test.go b/database/log/list_test.go index d2249d0c1..495f57b55 100644 --- a/database/log/list_test.go +++ b/database/log/list_test.go @@ -33,13 +33,7 @@ func TestLog_Engine_ListLogs(t *testing.T) { defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "logs"`).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "build_id", "repo_id", "service_id", "step_id", "data"}). AddRow(1, 1, 1, 1, 0, []byte{}).AddRow(2, 1, 1, 0, 1, []byte{}) diff --git a/database/pipeline/interface.go b/database/pipeline/interface.go index 62ffac726..27d6f44f6 100644 --- a/database/pipeline/interface.go +++ b/database/pipeline/interface.go @@ -41,7 +41,7 @@ type PipelineInterface interface { // ListPipelines defines a function that gets a list of all pipelines. ListPipelines(context.Context) ([]*api.Pipeline, error) // ListPipelinesForRepo defines a function that gets a list of pipelines by repo ID. - ListPipelinesForRepo(context.Context, *api.Repo, int, int) ([]*api.Pipeline, int64, error) + ListPipelinesForRepo(context.Context, *api.Repo, int, int) ([]*api.Pipeline, error) // UpdatePipeline defines a function that updates an existing pipeline. UpdatePipeline(context.Context, *api.Pipeline) (*api.Pipeline, error) } diff --git a/database/pipeline/list.go b/database/pipeline/list.go index 85e39a3cd..5c850a797 100644 --- a/database/pipeline/list.go +++ b/database/pipeline/list.go @@ -15,23 +15,11 @@ func (e *engine) ListPipelines(ctx context.Context) ([]*api.Pipeline, error) { e.logger.Trace("listing all pipelines") // variables to store query results and return value - count := int64(0) p := new([]types.Pipeline) pipelines := []*api.Pipeline{} - // count the results - count, err := e.CountPipelines(ctx) - if err != nil { - return nil, err - } - - // short-circuit if there are no results - if count == 0 { - return pipelines, nil - } - // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TablePipeline). Preload("Repo"). diff --git a/database/pipeline/list_repo.go b/database/pipeline/list_repo.go index cc179febe..0f03869bc 100644 --- a/database/pipeline/list_repo.go +++ b/database/pipeline/list_repo.go @@ -15,32 +15,20 @@ import ( // ListPipelinesForRepo gets a list of pipelines by repo ID from the database. // //nolint:lll // ignore long line length due to variable names -func (e *engine) ListPipelinesForRepo(ctx context.Context, r *api.Repo, page, perPage int) ([]*api.Pipeline, int64, error) { +func (e *engine) ListPipelinesForRepo(ctx context.Context, r *api.Repo, page, perPage int) ([]*api.Pipeline, error) { e.logger.WithFields(logrus.Fields{ "org": r.GetOrg(), "repo": r.GetName(), }).Tracef("listing pipelines for repo %s", r.GetFullName()) // variables to store query results and return values - count := int64(0) p := new([]types.Pipeline) pipelines := []*api.Pipeline{} - // count the results - count, err := e.CountPipelinesForRepo(ctx, r) - if err != nil { - return pipelines, 0, err - } - - // short-circuit if there are no results - if count == 0 { - return pipelines, 0, nil - } - // calculate offset for pagination through results offset := perPage * (page - 1) - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TablePipeline). Where("repo_id = ?", r.GetID()). @@ -49,7 +37,7 @@ func (e *engine) ListPipelinesForRepo(ctx context.Context, r *api.Repo, page, pe Find(&p). Error if err != nil { - return nil, count, err + return nil, err } // iterate through all query results @@ -59,7 +47,7 @@ func (e *engine) ListPipelinesForRepo(ctx context.Context, r *api.Repo, page, pe err = tmp.Decompress() if err != nil { - return nil, count, err + return nil, err } result := tmp.ToAPI() @@ -68,5 +56,5 @@ func (e *engine) ListPipelinesForRepo(ctx context.Context, r *api.Repo, page, pe pipelines = append(pipelines, result) } - return pipelines, count, nil + return pipelines, nil } diff --git a/database/pipeline/list_repo_test.go b/database/pipeline/list_repo_test.go index 77696e05d..c122c97c4 100644 --- a/database/pipeline/list_repo_test.go +++ b/database/pipeline/list_repo_test.go @@ -55,13 +55,7 @@ func TestPipeline_Engine_ListPipelinesForRepo(t *testing.T) { defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(1) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "pipelines" WHERE repo_id = $1`).WithArgs(1).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "repo_id", "commit", "flavor", "platform", "ref", "type", "version", "services", "stages", "steps", "templates", "data"}). AddRow(1, 1, "48afb5bdc41ad69bf22588491333f7cf71135163", "", "", "refs/heads/main", "yaml", "1", false, false, false, false, []byte{120, 94, 74, 203, 207, 7, 4, 0, 0, 255, 255, 2, 130, 1, 69}). AddRow(2, 1, "a49aaf4afae6431a79239c95247a2b169fd9f067", "", "", "refs/heads/main", "yaml", "1", false, false, false, false, []byte{120, 94, 74, 203, 207, 7, 4, 0, 0, 255, 255, 2, 130, 1, 69}) @@ -104,7 +98,7 @@ func TestPipeline_Engine_ListPipelinesForRepo(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, _, err := test.database.ListPipelinesForRepo(context.TODO(), _repo, 1, 10) + got, err := test.database.ListPipelinesForRepo(context.TODO(), _repo, 1, 10) if test.failure { if err == nil { diff --git a/database/pipeline/list_test.go b/database/pipeline/list_test.go index 4dc624a55..a8113300d 100644 --- a/database/pipeline/list_test.go +++ b/database/pipeline/list_test.go @@ -67,13 +67,7 @@ func TestPipeline_Engine_ListPipelines(t *testing.T) { defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "pipelines"`).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "repo_id", "commit", "flavor", "platform", "ref", "type", "version", "services", "stages", "steps", "templates", "data"}). AddRow(1, 1, "48afb5bdc41ad69bf22588491333f7cf71135163", "", "", "refs/heads/main", "yaml", "1", false, false, false, false, []byte{120, 94, 74, 203, 207, 7, 4, 0, 0, 255, 255, 2, 130, 1, 69}). AddRow(2, 2, "a49aaf4afae6431a79239c95247a2b169fd9f067", "", "", "refs/heads/main", "yaml", "1", false, false, false, false, []byte{120, 94, 74, 203, 207, 7, 4, 0, 0, 255, 255, 2, 130, 1, 69}) diff --git a/database/repo/interface.go b/database/repo/interface.go index 8c4a4e466..15d4908d4 100644 --- a/database/repo/interface.go +++ b/database/repo/interface.go @@ -43,9 +43,9 @@ type RepoInterface interface { // 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. - ListReposForOrg(context.Context, string, string, map[string]interface{}, int, int) ([]*api.Repo, int64, error) + ListReposForOrg(context.Context, string, string, map[string]interface{}, int, int) ([]*api.Repo, error) // ListReposForUser defines a function that gets a list of repos by user ID. - ListReposForUser(context.Context, *api.User, string, map[string]interface{}, int, int) ([]*api.Repo, int64, error) + ListReposForUser(context.Context, *api.User, string, map[string]interface{}, int, int) ([]*api.Repo, error) // UpdateRepo defines a function that updates an existing repo. UpdateRepo(context.Context, *api.Repo) (*api.Repo, error) } diff --git a/database/repo/list.go b/database/repo/list.go index 5fd587dd4..ba819eae0 100644 --- a/database/repo/list.go +++ b/database/repo/list.go @@ -15,23 +15,11 @@ func (e *engine) ListRepos(ctx context.Context) ([]*api.Repo, error) { e.logger.Trace("listing all repos") // variables to store query results and return value - count := int64(0) r := new([]types.Repo) repos := []*api.Repo{} - // count the results - count, err := e.CountRepos(ctx) - if err != nil { - return nil, err - } - - // short-circuit if there are no results - if count == 0 { - return repos, nil - } - // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableRepo). Preload("Owner"). diff --git a/database/repo/list_org.go b/database/repo/list_org.go index b390192d4..90f6fbcc0 100644 --- a/database/repo/list_org.go +++ b/database/repo/list_org.go @@ -15,27 +15,15 @@ import ( // ListReposForOrg gets a list of repos by org name from the database. // //nolint:lll // ignore long line length due to variable names -func (e *engine) ListReposForOrg(ctx context.Context, org, sortBy string, filters map[string]interface{}, page, perPage int) ([]*api.Repo, int64, error) { +func (e *engine) ListReposForOrg(ctx context.Context, org, sortBy string, filters map[string]interface{}, page, perPage int) ([]*api.Repo, error) { e.logger.WithFields(logrus.Fields{ "org": org, }).Tracef("listing repos for org %s", org) // variables to store query results and return values - count := int64(0) r := new([]types.Repo) repos := []*api.Repo{} - // count the results - count, err := e.CountReposForOrg(ctx, org, filters) - if err != nil { - return repos, 0, err - } - - // short-circuit if there are no results - if count == 0 { - return repos, 0, nil - } - // calculate offset for pagination through results offset := perPage * (page - 1) @@ -49,7 +37,7 @@ func (e *engine) ListReposForOrg(ctx context.Context, org, sortBy string, filter Where("repos.org = ?", org). Group("repos.id") - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableRepo). Preload("Owner"). @@ -61,12 +49,12 @@ func (e *engine) ListReposForOrg(ctx context.Context, org, sortBy string, filter Find(&r). Error if err != nil { - return nil, count, err + return nil, err } case "name": fallthrough default: - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableRepo). Preload("Owner"). @@ -78,7 +66,7 @@ func (e *engine) ListReposForOrg(ctx context.Context, org, sortBy string, filter Find(&r). Error if err != nil { - return nil, count, err + return nil, err } } @@ -88,7 +76,7 @@ func (e *engine) ListReposForOrg(ctx context.Context, org, sortBy string, filter tmp := repo // decrypt the fields for the repo - err = tmp.Decrypt(e.config.EncryptionKey) + err := tmp.Decrypt(e.config.EncryptionKey) if err != nil { // TODO: remove backwards compatibility before 1.x.x release // @@ -102,5 +90,5 @@ func (e *engine) ListReposForOrg(ctx context.Context, org, sortBy string, filter repos = append(repos, tmp.ToAPI()) } - return repos, count, nil + return repos, nil } diff --git a/database/repo/list_org_test.go b/database/repo/list_org_test.go index 9aad55864..72a32395b 100644 --- a/database/repo/list_org_test.go +++ b/database/repo/list_org_test.go @@ -62,14 +62,8 @@ func TestRepo_Engine_ListReposForOrg(t *testing.T) { _postgres, _mock := testPostgres(t) defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() - // create expected name count query result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the name count query - _mock.ExpectQuery(`SELECT count(*) FROM "repos" WHERE org = $1`).WithArgs("foo").WillReturnRows(_rows) - // create expected name query result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "user_id", "hash", "org", "name", "full_name", "link", "clone", "branch", "topics", "timeout", "counter", "visibility", "private", "trusted", "active", "allow_events", "pipeline_type", "previous_name", "approve_build"}). AddRow(1, 1, "baz", "foo", "bar", "foo/bar", "", "", "", "{}", 0, 0, "public", false, false, false, 1, "yaml", nil, nil). AddRow(2, 1, "bar", "foo", "baz", "foo/baz", "", "", "", "{}", 0, 0, "public", false, false, false, 1, "yaml", nil, nil) @@ -82,12 +76,6 @@ func TestRepo_Engine_ListReposForOrg(t *testing.T) { _mock.ExpectQuery(`SELECT * FROM "repos" WHERE org = $1 ORDER BY name LIMIT $2`).WithArgs("foo", 10).WillReturnRows(_rows) _mock.ExpectQuery(`SELECT * FROM "users" WHERE "users"."id" = $1`).WithArgs(1).WillReturnRows(_userRows) - // create expected latest count query result in mock - _rows = sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the latest count query - _mock.ExpectQuery(`SELECT count(*) FROM "repos" WHERE org = $1`).WithArgs("foo").WillReturnRows(_rows) - // create expected latest query result in mock _rows = sqlmock.NewRows( []string{"id", "user_id", "hash", "org", "name", "full_name", "link", "clone", "branch", "topics", "timeout", "counter", "visibility", "private", "trusted", "active", "allow_events", "pipeline_type", "previous_name", "approve_build"}). @@ -183,7 +171,7 @@ func TestRepo_Engine_ListReposForOrg(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, _, err := test.database.ListReposForOrg(context.TODO(), "foo", test.sort, filters, 1, 10) + got, err := test.database.ListReposForOrg(context.TODO(), "foo", test.sort, filters, 1, 10) if test.failure { if err == nil { diff --git a/database/repo/list_test.go b/database/repo/list_test.go index 176d70369..270229da8 100644 --- a/database/repo/list_test.go +++ b/database/repo/list_test.go @@ -50,13 +50,7 @@ func TestRepo_Engine_ListRepos(t *testing.T) { defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "repos"`).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "user_id", "hash", "org", "name", "full_name", "link", "clone", "branch", "topics", "timeout", "counter", "visibility", "private", "trusted", "active", "allow_events", "pipeline_type", "previous_name", "approve_build"}). AddRow(1, 1, "baz", "foo", "bar", "foo/bar", "", "", "", "{}", 0, 0, "public", false, false, false, 1, "yaml", nil, nil). AddRow(2, 1, "baz", "bar", "foo", "bar/foo", "", "", "", "{}", 0, 0, "public", false, false, false, 1, "yaml", nil, nil) diff --git a/database/repo/list_user.go b/database/repo/list_user.go index c0bedbe66..fb6b0992a 100644 --- a/database/repo/list_user.go +++ b/database/repo/list_user.go @@ -15,27 +15,15 @@ import ( // ListReposForUser gets a list of repos by user ID from the database. // //nolint:lll // ignore long line length due to variable names -func (e *engine) ListReposForUser(ctx context.Context, u *api.User, sortBy string, filters map[string]interface{}, page, perPage int) ([]*api.Repo, int64, error) { +func (e *engine) ListReposForUser(ctx context.Context, u *api.User, sortBy string, filters map[string]interface{}, page, perPage int) ([]*api.Repo, error) { e.logger.WithFields(logrus.Fields{ "user": u.GetName(), }).Tracef("listing repos for user %s", u.GetName()) // variables to store query results and return values - count := int64(0) r := new([]types.Repo) repos := []*api.Repo{} - // count the results - count, err := e.CountReposForUser(ctx, u, filters) - if err != nil { - return repos, 0, err - } - - // short-circuit if there are no results - if count == 0 { - return repos, 0, nil - } - // calculate offset for pagination through results offset := perPage * (page - 1) @@ -49,7 +37,7 @@ func (e *engine) ListReposForUser(ctx context.Context, u *api.User, sortBy strin Where("repos.user_id = ?", u.GetID()). Group("repos.id") - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableRepo). Preload("Owner"). @@ -61,12 +49,12 @@ func (e *engine) ListReposForUser(ctx context.Context, u *api.User, sortBy strin Find(&r). Error if err != nil { - return nil, count, err + return nil, err } case "name": fallthrough default: - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableRepo). Preload("Owner"). @@ -78,7 +66,7 @@ func (e *engine) ListReposForUser(ctx context.Context, u *api.User, sortBy strin Find(&r). Error if err != nil { - return nil, count, err + return nil, err } } @@ -88,7 +76,7 @@ func (e *engine) ListReposForUser(ctx context.Context, u *api.User, sortBy strin tmp := repo // decrypt the fields for the repo - err = tmp.Decrypt(e.config.EncryptionKey) + err := tmp.Decrypt(e.config.EncryptionKey) if err != nil { // TODO: remove backwards compatibility before 1.x.x release // @@ -102,5 +90,5 @@ func (e *engine) ListReposForUser(ctx context.Context, u *api.User, sortBy strin repos = append(repos, tmp.ToAPI()) } - return repos, count, nil + return repos, nil } diff --git a/database/repo/list_user_test.go b/database/repo/list_user_test.go index 5b9fda28a..5f485c214 100644 --- a/database/repo/list_user_test.go +++ b/database/repo/list_user_test.go @@ -62,14 +62,8 @@ func TestRepo_Engine_ListReposForUser(t *testing.T) { _postgres, _mock := testPostgres(t) defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() - // create expected name count query result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the name count query - _mock.ExpectQuery(`SELECT count(*) FROM "repos" WHERE user_id = $1`).WithArgs(1).WillReturnRows(_rows) - // create expected name query result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "user_id", "hash", "org", "name", "full_name", "link", "clone", "branch", "topics", "timeout", "counter", "visibility", "private", "trusted", "active", "allow_events", "pipeline_type", "previous_name", "approve_build"}). AddRow(1, 1, "baz", "foo", "bar", "foo/bar", "", "", "", "{}", 0, 0, "public", false, false, false, 1, "yaml", nil, nil). AddRow(2, 1, "baz", "bar", "foo", "bar/foo", "", "", "", "{}", 0, 0, "public", false, false, false, 1, "yaml", nil, nil) @@ -82,12 +76,6 @@ func TestRepo_Engine_ListReposForUser(t *testing.T) { _mock.ExpectQuery(`SELECT * FROM "repos" WHERE user_id = $1 ORDER BY name LIMIT $2`).WithArgs(1, 10).WillReturnRows(_rows) _mock.ExpectQuery(`SELECT * FROM "users" WHERE "users"."id" = $1`).WithArgs(1).WillReturnRows(_userRows) - // create expected latest count query result in mock - _rows = sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the latest count query - _mock.ExpectQuery(`SELECT count(*) FROM "repos" WHERE user_id = $1`).WithArgs(1).WillReturnRows(_rows) - // create expected latest query result in mock _rows = sqlmock.NewRows( []string{"id", "user_id", "hash", "org", "name", "full_name", "link", "clone", "branch", "topics", "timeout", "counter", "visibility", "private", "trusted", "active", "allow_events", "pipeline_type", "previous_name", "approve_build"}). @@ -183,7 +171,7 @@ func TestRepo_Engine_ListReposForUser(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, _, err := test.database.ListReposForUser(context.TODO(), _owner, test.sort, filters, 1, 10) + got, err := test.database.ListReposForUser(context.TODO(), _owner, test.sort, filters, 1, 10) if test.failure { if err == nil { diff --git a/database/schedule/interface.go b/database/schedule/interface.go index f4d284b45..65a6ed1c9 100644 --- a/database/schedule/interface.go +++ b/database/schedule/interface.go @@ -43,7 +43,7 @@ type ScheduleInterface interface { // ListSchedules defines a function that gets a list of all schedules. ListSchedules(context.Context) ([]*api.Schedule, error) // ListSchedulesForRepo defines a function that gets a list of schedules by repo ID. - ListSchedulesForRepo(context.Context, *api.Repo, int, int) ([]*api.Schedule, int64, error) + ListSchedulesForRepo(context.Context, *api.Repo, int, int) ([]*api.Schedule, error) // UpdateSchedule defines a function that updates an existing schedule. UpdateSchedule(context.Context, *api.Schedule, bool) (*api.Schedule, error) } diff --git a/database/schedule/list.go b/database/schedule/list.go index 53e3c6361..7bd2bd700 100644 --- a/database/schedule/list.go +++ b/database/schedule/list.go @@ -15,23 +15,11 @@ func (e *engine) ListSchedules(ctx context.Context) ([]*api.Schedule, error) { e.logger.Trace("listing all schedules") // variables to store query results and return value - count := int64(0) s := new([]types.Schedule) schedules := []*api.Schedule{} - // count the results - count, err := e.CountSchedules(ctx) - if err != nil { - return nil, err - } - - // short-circuit if there are no results - if count == 0 { - return schedules, nil - } - // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableSchedule). Preload("Repo"). diff --git a/database/schedule/list_active.go b/database/schedule/list_active.go index 665682e2c..ddfcdbf01 100644 --- a/database/schedule/list_active.go +++ b/database/schedule/list_active.go @@ -15,23 +15,11 @@ func (e *engine) ListActiveSchedules(ctx context.Context) ([]*api.Schedule, erro e.logger.Trace("listing all active schedules") // variables to store query results and return value - count := int64(0) s := new([]types.Schedule) schedules := []*api.Schedule{} - // count the results - count, err := e.CountActiveSchedules(ctx) - if err != nil { - return nil, err - } - - // short-circuit if there are no results - if count == 0 { - return schedules, nil - } - // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableSchedule). Preload("Repo"). diff --git a/database/schedule/list_active_test.go b/database/schedule/list_active_test.go index d015f0e38..c0f891d0c 100644 --- a/database/schedule/list_active_test.go +++ b/database/schedule/list_active_test.go @@ -92,13 +92,7 @@ func TestSchedule_Engine_ListActiveSchedules(t *testing.T) { defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(1) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "schedules" WHERE active = $1`).WithArgs(true).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "repo_id", "active", "name", "entry", "created_at", "created_by", "updated_at", "updated_by", "scheduled_at", "branch", "error"}). AddRow(1, 1, true, "nightly", "0 0 * * *", 1713476291, "octocat", 3013476291, "octokitty", 2013476291, "main", "no version: YAML property provided") diff --git a/database/schedule/list_repo.go b/database/schedule/list_repo.go index 2f23d985d..f2f9dbd08 100644 --- a/database/schedule/list_repo.go +++ b/database/schedule/list_repo.go @@ -13,33 +13,21 @@ import ( ) // ListSchedulesForRepo gets a list of schedules by repo ID from the database. -func (e *engine) ListSchedulesForRepo(ctx context.Context, r *api.Repo, page, perPage int) ([]*api.Schedule, int64, error) { +func (e *engine) ListSchedulesForRepo(ctx context.Context, r *api.Repo, page, perPage int) ([]*api.Schedule, error) { e.logger.WithFields(logrus.Fields{ "org": r.GetOrg(), "repo": r.GetName(), }).Tracef("listing schedules for repo %s", r.GetFullName()) // variables to store query results and return value - count := int64(0) s := new([]types.Schedule) schedules := []*api.Schedule{} - // count the results - count, err := e.CountSchedulesForRepo(ctx, r) - if err != nil { - return nil, 0, err - } - - // short-circuit if there are no results - if count == 0 { - return schedules, 0, nil - } - // calculate offset for pagination through results offset := perPage * (page - 1) // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableSchedule). Where("repo_id = ?", r.GetID()). @@ -49,7 +37,7 @@ func (e *engine) ListSchedulesForRepo(ctx context.Context, r *api.Repo, page, pe Find(&s). Error if err != nil { - return nil, count, err + return nil, err } // iterate through all query results @@ -64,5 +52,5 @@ func (e *engine) ListSchedulesForRepo(ctx context.Context, r *api.Repo, page, pe schedules = append(schedules, result) } - return schedules, count, nil + return schedules, nil } diff --git a/database/schedule/list_repo_test.go b/database/schedule/list_repo_test.go index a1b8f7131..0775203f4 100644 --- a/database/schedule/list_repo_test.go +++ b/database/schedule/list_repo_test.go @@ -91,13 +91,7 @@ func TestSchedule_Engine_ListSchedulesForRepo(t *testing.T) { defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "schedules" WHERE repo_id = $1`).WithArgs(1).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "repo_id", "active", "name", "entry", "created_at", "created_by", "updated_at", "updated_by", "scheduled_at", "branch", "error"}). AddRow(1, 1, true, "nightly", "0 0 * * *", 1713476291, "octocat", 3013476291, "octokitty", 2013476291, "main", "no version: YAML property provided"). AddRow(2, 1, false, "hourly", "0 * * * *", 1713476291, "octocat", 3013476291, "octokitty", 2013476291, "main", "no version: YAML property provided") @@ -142,7 +136,7 @@ func TestSchedule_Engine_ListSchedulesForRepo(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, _, err := test.database.ListSchedulesForRepo(context.TODO(), _repo, 1, 10) + got, err := test.database.ListSchedulesForRepo(context.TODO(), _repo, 1, 10) if test.failure { if err == nil { diff --git a/database/schedule/list_test.go b/database/schedule/list_test.go index 5d8b5d738..a51d936aa 100644 --- a/database/schedule/list_test.go +++ b/database/schedule/list_test.go @@ -92,13 +92,7 @@ func TestSchedule_Engine_ListSchedules(t *testing.T) { defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "schedules"`).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "repo_id", "active", "name", "entry", "created_at", "created_by", "updated_at", "updated_by", "scheduled_at", "branch", "error"}). AddRow(1, 1, true, "nightly", "0 0 * * *", 1713476291, "octocat", 3013476291, "octokitty", 2013476291, "main", "no version: YAML property provided"). AddRow(2, 1, false, "hourly", "0 * * * *", 1713476291, "octocat", 3013476291, "octokitty", 2013476291, "main", "no version: YAML property provided") diff --git a/database/secret/interface.go b/database/secret/interface.go index f38cf6a75..02d444c49 100644 --- a/database/secret/interface.go +++ b/database/secret/interface.go @@ -51,13 +51,13 @@ type SecretInterface interface { // ListSecrets defines a function that gets a list of all secrets. ListSecrets(context.Context) ([]*api.Secret, error) // ListSecretsForOrg defines a function that gets a list of secrets by org name. - ListSecretsForOrg(context.Context, string, map[string]interface{}, int, int) ([]*api.Secret, int64, error) + ListSecretsForOrg(context.Context, string, map[string]interface{}, int, int) ([]*api.Secret, error) // ListSecretsForRepo defines a function that gets a list of secrets by org and repo name. - ListSecretsForRepo(context.Context, *api.Repo, map[string]interface{}, int, int) ([]*api.Secret, int64, error) + ListSecretsForRepo(context.Context, *api.Repo, map[string]interface{}, int, int) ([]*api.Secret, error) // ListSecretsForTeam defines a function that gets a list of secrets by org and team name. - ListSecretsForTeam(context.Context, string, string, map[string]interface{}, int, int) ([]*api.Secret, int64, error) + ListSecretsForTeam(context.Context, string, string, map[string]interface{}, int, int) ([]*api.Secret, error) // ListSecretsForTeams defines a function that gets a list of secrets by teams within an org. - ListSecretsForTeams(context.Context, string, []string, map[string]interface{}, int, int) ([]*api.Secret, int64, error) + ListSecretsForTeams(context.Context, string, []string, map[string]interface{}, int, int) ([]*api.Secret, error) // UpdateSecret defines a function that updates an existing secret. UpdateSecret(context.Context, *api.Secret) (*api.Secret, error) } diff --git a/database/secret/list.go b/database/secret/list.go index 0f11b750f..4c129b533 100644 --- a/database/secret/list.go +++ b/database/secret/list.go @@ -15,23 +15,11 @@ func (e *engine) ListSecrets(ctx context.Context) ([]*api.Secret, error) { e.logger.Trace("listing all secrets") // variables to store query results and return value - count := int64(0) s := new([]types.Secret) secrets := []*api.Secret{} - // count the results - count, err := e.CountSecrets(ctx) - if err != nil { - return nil, err - } - - // short-circuit if there are no results - if count == 0 { - return secrets, nil - } - // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableSecret). Find(&s). diff --git a/database/secret/list_org.go b/database/secret/list_org.go index a32ddda34..0150e0b8c 100644 --- a/database/secret/list_org.go +++ b/database/secret/list_org.go @@ -15,33 +15,21 @@ import ( // ListSecretsForOrg gets a list of secrets by org name from the database. // //nolint:lll // ignore long line length due to variable names -func (e *engine) ListSecretsForOrg(ctx context.Context, org string, filters map[string]interface{}, page, perPage int) ([]*api.Secret, int64, error) { +func (e *engine) ListSecretsForOrg(ctx context.Context, org string, filters map[string]interface{}, page, perPage int) ([]*api.Secret, error) { e.logger.WithFields(logrus.Fields{ "org": org, "type": constants.SecretOrg, }).Tracef("listing secrets for org %s", org) // variables to store query results and return values - count := int64(0) s := new([]types.Secret) secrets := []*api.Secret{} - // count the results - count, err := e.CountSecretsForOrg(ctx, org, filters) - if err != nil { - return secrets, 0, err - } - - // short-circuit if there are no results - if count == 0 { - return secrets, 0, nil - } - // calculate offset for pagination through results offset := perPage * (page - 1) // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableSecret). Where("type = ?", constants.SecretOrg). @@ -53,7 +41,7 @@ func (e *engine) ListSecretsForOrg(ctx context.Context, org string, filters map[ Find(&s). Error if err != nil { - return nil, count, err + return nil, err } // iterate through all query results @@ -74,5 +62,5 @@ func (e *engine) ListSecretsForOrg(ctx context.Context, org string, filters map[ secrets = append(secrets, tmp.ToAPI()) } - return secrets, count, nil + return secrets, nil } diff --git a/database/secret/list_org_test.go b/database/secret/list_org_test.go index f5a274679..124428ae6 100644 --- a/database/secret/list_org_test.go +++ b/database/secret/list_org_test.go @@ -45,15 +45,8 @@ func TestSecret_Engine_ListSecretsForOrg(t *testing.T) { _postgres, _mock := testPostgres(t) defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() - // create expected name count query result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the name count query - _mock.ExpectQuery(`SELECT count(*) FROM "secrets" WHERE type = $1 AND org = $2`). - WithArgs(constants.SecretOrg, "foo").WillReturnRows(_rows) - // create expected name query result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "type", "org", "repo", "team", "name", "value", "images", "allow_events", "allow_command", "allow_substitution", "created_at", "created_by", "updated_at", "updated_by"}). AddRow(2, "org", "foo", "*", "", "bar", "baz", nil, 1, false, false, 1, "user", 1, "user2"). AddRow(1, "org", "foo", "*", "", "baz", "bar", nil, 1, false, false, 1, "user", 1, "user2") @@ -101,7 +94,7 @@ func TestSecret_Engine_ListSecretsForOrg(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, _, err := test.database.ListSecretsForOrg(context.TODO(), "foo", filters, 1, 10) + got, err := test.database.ListSecretsForOrg(context.TODO(), "foo", filters, 1, 10) if test.failure { if err == nil { diff --git a/database/secret/list_repo.go b/database/secret/list_repo.go index 59520ea12..a89373234 100644 --- a/database/secret/list_repo.go +++ b/database/secret/list_repo.go @@ -15,7 +15,7 @@ import ( // ListSecretsForRepo gets a list of secrets by org name from the database. // //nolint:lll // ignore long line length due to variable names -func (e *engine) ListSecretsForRepo(ctx context.Context, r *api.Repo, filters map[string]interface{}, page, perPage int) ([]*api.Secret, int64, error) { +func (e *engine) ListSecretsForRepo(ctx context.Context, r *api.Repo, filters map[string]interface{}, page, perPage int) ([]*api.Secret, error) { e.logger.WithFields(logrus.Fields{ "org": r.GetOrg(), "repo": r.GetName(), @@ -23,26 +23,14 @@ func (e *engine) ListSecretsForRepo(ctx context.Context, r *api.Repo, filters ma }).Tracef("listing secrets for repo %s", r.GetFullName()) // variables to store query results and return values - count := int64(0) s := new([]types.Secret) secrets := []*api.Secret{} - // count the results - count, err := e.CountSecretsForRepo(ctx, r, filters) - if err != nil { - return secrets, 0, err - } - - // short-circuit if there are no results - if count == 0 { - return secrets, 0, nil - } - // calculate offset for pagination through results offset := perPage * (page - 1) // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableSecret). Where("type = ?", constants.SecretRepo). @@ -55,7 +43,7 @@ func (e *engine) ListSecretsForRepo(ctx context.Context, r *api.Repo, filters ma Find(&s). Error if err != nil { - return nil, count, err + return nil, err } // iterate through all query results @@ -76,5 +64,5 @@ func (e *engine) ListSecretsForRepo(ctx context.Context, r *api.Repo, filters ma secrets = append(secrets, tmp.ToAPI()) } - return secrets, count, nil + return secrets, nil } diff --git a/database/secret/list_repo_test.go b/database/secret/list_repo_test.go index 685cc52bb..36ee569ea 100644 --- a/database/secret/list_repo_test.go +++ b/database/secret/list_repo_test.go @@ -55,15 +55,8 @@ func TestSecret_Engine_ListSecretsForRepo(t *testing.T) { _postgres, _mock := testPostgres(t) defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() - // create expected name count query result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the name count query - _mock.ExpectQuery(`SELECT count(*) FROM "secrets" WHERE type = $1 AND org = $2 AND repo = $3`). - WithArgs(constants.SecretRepo, "foo", "bar").WillReturnRows(_rows) - // create expected name query result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "type", "org", "repo", "team", "name", "value", "images", "allow_events", "allow_command", "allow_substitution", "created_at", "created_by", "updated_at", "updated_by"}). AddRow(2, "repo", "foo", "bar", "", "foob", "baz", nil, 1, false, false, 1, "user", 1, "user2"). AddRow(1, "repo", "foo", "bar", "", "baz", "foob", nil, 1, false, false, 1, "user", 1, "user2") @@ -111,7 +104,7 @@ func TestSecret_Engine_ListSecretsForRepo(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, _, err := test.database.ListSecretsForRepo(context.TODO(), _repo, filters, 1, 10) + got, err := test.database.ListSecretsForRepo(context.TODO(), _repo, filters, 1, 10) if test.failure { if err == nil { diff --git a/database/secret/list_team.go b/database/secret/list_team.go index ae56a3da4..1fbbc4806 100644 --- a/database/secret/list_team.go +++ b/database/secret/list_team.go @@ -16,7 +16,7 @@ import ( // ListSecretsForTeam gets a list of secrets by org and team name from the database. // //nolint:lll // ignore long line length due to variable names -func (e *engine) ListSecretsForTeam(ctx context.Context, org, team string, filters map[string]interface{}, page, perPage int) ([]*api.Secret, int64, error) { +func (e *engine) ListSecretsForTeam(ctx context.Context, org, team string, filters map[string]interface{}, page, perPage int) ([]*api.Secret, error) { e.logger.WithFields(logrus.Fields{ "org": org, "team": team, @@ -24,26 +24,14 @@ func (e *engine) ListSecretsForTeam(ctx context.Context, org, team string, filte }).Tracef("listing secrets for team %s/%s", org, team) // variables to store query results and return values - count := int64(0) s := new([]types.Secret) secrets := []*api.Secret{} - // count the results - count, err := e.CountSecretsForTeam(ctx, org, team, filters) - if err != nil { - return secrets, 0, err - } - - // short-circuit if there are no results - if count == 0 { - return secrets, 0, nil - } - // calculate offset for pagination through results offset := perPage * (page - 1) // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableSecret). Where("type = ?", constants.SecretShared). @@ -56,7 +44,7 @@ func (e *engine) ListSecretsForTeam(ctx context.Context, org, team string, filte Find(&s). Error if err != nil { - return nil, count, err + return nil, err } // iterate through all query results @@ -77,11 +65,11 @@ func (e *engine) ListSecretsForTeam(ctx context.Context, org, team string, filte secrets = append(secrets, tmp.ToAPI()) } - return secrets, count, nil + return secrets, nil } // ListSecretsForTeams gets a list of secrets by teams within an org from the database. -func (e *engine) ListSecretsForTeams(ctx context.Context, org string, teams []string, filters map[string]interface{}, page, perPage int) ([]*api.Secret, int64, error) { +func (e *engine) ListSecretsForTeams(ctx context.Context, org string, teams []string, filters map[string]interface{}, page, perPage int) ([]*api.Secret, error) { // iterate through the list of teams provided for index, team := range teams { // ensure the team name is lower case @@ -95,26 +83,14 @@ func (e *engine) ListSecretsForTeams(ctx context.Context, org string, teams []st }).Tracef("listing secrets for teams %s in org %s", teams, org) // variables to store query results and return values - count := int64(0) s := new([]types.Secret) secrets := []*api.Secret{} - // count the results - count, err := e.CountSecretsForTeams(ctx, org, teams, filters) - if err != nil { - return secrets, 0, err - } - - // short-circuit if there are no results - if count == 0 { - return secrets, 0, nil - } - // calculate offset for pagination through results offset := perPage * (page - 1) // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableSecret). Where("type = ?", constants.SecretShared). @@ -127,7 +103,7 @@ func (e *engine) ListSecretsForTeams(ctx context.Context, org string, teams []st Find(&s). Error if err != nil { - return nil, count, err + return nil, err } // iterate through all query results @@ -148,5 +124,5 @@ func (e *engine) ListSecretsForTeams(ctx context.Context, org string, teams []st secrets = append(secrets, tmp.ToAPI()) } - return secrets, count, nil + return secrets, nil } diff --git a/database/secret/list_team_test.go b/database/secret/list_team_test.go index 382f6864c..54dfce245 100644 --- a/database/secret/list_team_test.go +++ b/database/secret/list_team_test.go @@ -45,15 +45,8 @@ func TestSecret_Engine_ListSecretsForTeam(t *testing.T) { _postgres, _mock := testPostgres(t) defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() - // create expected name count query result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the name count query - _mock.ExpectQuery(`SELECT count(*) FROM "secrets" WHERE type = $1 AND org = $2 AND team = $3`). - WithArgs(constants.SecretShared, "foo", "bar").WillReturnRows(_rows) - // create expected name query result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "type", "org", "repo", "team", "name", "value", "images", "allow_events", "allow_command", "allow_substitution", "created_at", "created_by", "updated_at", "updated_by"}). AddRow(2, "shared", "foo", "", "bar", "foob", "baz", nil, 1, false, false, 1, "user", 1, "user2"). AddRow(1, "shared", "foo", "", "bar", "baz", "foob", nil, 1, false, false, 1, "user", 1, "user2") @@ -101,7 +94,7 @@ func TestSecret_Engine_ListSecretsForTeam(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, _, err := test.database.ListSecretsForTeam(context.TODO(), "foo", "bar", filters, 1, 10) + got, err := test.database.ListSecretsForTeam(context.TODO(), "foo", "bar", filters, 1, 10) if test.failure { if err == nil { @@ -153,15 +146,8 @@ func TestSecret_Engine_ListSecretsForTeams(t *testing.T) { _postgres, _mock := testPostgres(t) defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() - // create expected name count query result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the name count query - _mock.ExpectQuery(`SELECT count(*) FROM "secrets" WHERE type = $1 AND org = $2 AND LOWER(team) IN ($3,$4)`). - WithArgs(constants.SecretShared, "foo", "foo", "bar").WillReturnRows(_rows) - // create expected name query result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "type", "org", "repo", "team", "name", "value", "images", "allow_events", "allow_command", "created_at", "created_by", "updated_at", "updated_by"}). AddRow(2, "shared", "foo", "", "bar", "foob", "baz", nil, 1, false, 1, "user", 1, "user2"). AddRow(1, "shared", "foo", "", "bar", "baz", "foob", nil, 1, false, 1, "user", 1, "user2") @@ -209,7 +195,7 @@ func TestSecret_Engine_ListSecretsForTeams(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, _, err := test.database.ListSecretsForTeams(context.TODO(), "foo", []string{"foo", "bar"}, filters, 1, 10) + got, err := test.database.ListSecretsForTeams(context.TODO(), "foo", []string{"foo", "bar"}, filters, 1, 10) if test.failure { if err == nil { diff --git a/database/secret/list_test.go b/database/secret/list_test.go index b744c434a..cf98e059f 100644 --- a/database/secret/list_test.go +++ b/database/secret/list_test.go @@ -45,13 +45,7 @@ func TestSecret_Engine_ListSecrets(t *testing.T) { defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "secrets"`).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "type", "org", "repo", "team", "name", "value", "images", "allow_events", "allow_command", "allow_substitution", "created_at", "created_by", "updated_at", "updated_by"}). AddRow(1, "repo", "foo", "bar", "", "baz", "foob", nil, 1, false, false, 1, "user", 1, "user2"). AddRow(2, "repo", "foo", "bar", "", "foob", "baz", nil, 1, false, false, 1, "user", 1, "user2") diff --git a/database/service/interface.go b/database/service/interface.go index 161103d26..784809879 100644 --- a/database/service/interface.go +++ b/database/service/interface.go @@ -41,7 +41,7 @@ type ServiceInterface interface { // ListServices defines a function that gets a list of all services. ListServices(context.Context) ([]*api.Service, error) // ListServicesForBuild defines a function that gets a list of services by build ID. - ListServicesForBuild(context.Context, *api.Build, map[string]interface{}, int, int) ([]*api.Service, int64, error) + ListServicesForBuild(context.Context, *api.Build, map[string]interface{}, int, int) ([]*api.Service, error) // ListServiceImageCount defines a function that gets a list of all service images and the count of their occurrence. ListServiceImageCount(context.Context) (map[string]float64, error) // ListServiceStatusCount defines a function that gets a list of all service statuses and the count of their occurrence. diff --git a/database/service/list.go b/database/service/list.go index 6f09c6dcd..c4b77dd6d 100644 --- a/database/service/list.go +++ b/database/service/list.go @@ -15,23 +15,11 @@ func (e *engine) ListServices(ctx context.Context) ([]*api.Service, error) { e.logger.Trace("listing all services") // variables to store query results and return value - count := int64(0) w := new([]types.Service) services := []*api.Service{} - // count the results - count, err := e.CountServices(ctx) - if err != nil { - return nil, err - } - - // short-circuit if there are no results - if count == 0 { - return services, nil - } - // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableService). Find(&w). diff --git a/database/service/list_build.go b/database/service/list_build.go index 7aa897aa4..b67d6adeb 100644 --- a/database/service/list_build.go +++ b/database/service/list_build.go @@ -13,32 +13,20 @@ import ( ) // ListServicesForBuild gets a list of all services from the database. -func (e *engine) ListServicesForBuild(ctx context.Context, b *api.Build, filters map[string]interface{}, page int, perPage int) ([]*api.Service, int64, error) { +func (e *engine) ListServicesForBuild(ctx context.Context, b *api.Build, filters map[string]interface{}, page int, perPage int) ([]*api.Service, error) { e.logger.WithFields(logrus.Fields{ "build": b.GetNumber(), }).Tracef("listing services for build %d", b.GetNumber()) // variables to store query results and return value - count := int64(0) s := new([]types.Service) services := []*api.Service{} - // count the results - count, err := e.CountServicesForBuild(ctx, b, filters) - if err != nil { - return services, 0, err - } - - // short-circuit if there are no results - if count == 0 { - return services, 0, nil - } - // calculate offset for pagination through results offset := perPage * (page - 1) // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableService). Where("build_id = ?", b.GetID()). @@ -49,7 +37,7 @@ func (e *engine) ListServicesForBuild(ctx context.Context, b *api.Build, filters Find(&s). Error if err != nil { - return nil, count, err + return nil, err } // iterate through all query results @@ -60,5 +48,5 @@ func (e *engine) ListServicesForBuild(ctx context.Context, b *api.Build, filters services = append(services, tmp.ToAPI()) } - return services, count, nil + return services, nil } diff --git a/database/service/list_build_test.go b/database/service/list_build_test.go index cf00dbc54..75f203345 100644 --- a/database/service/list_build_test.go +++ b/database/service/list_build_test.go @@ -40,13 +40,7 @@ func TestService_Engine_ListServicesForBuild(t *testing.T) { defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "services" WHERE build_id = $1`).WithArgs(1).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "repo_id", "build_id", "number", "name", "image", "stage", "status", "error", "exit_code", "created", "started", "finished", "host", "runtime", "distribution"}). AddRow(2, 1, 1, 2, "foo", "bar", "", "", "", 0, 0, 0, 0, "", "", ""). AddRow(1, 1, 1, 1, "foo", "bar", "", "", "", 0, 0, 0, 0, "", "", "") @@ -93,7 +87,7 @@ func TestService_Engine_ListServicesForBuild(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, _, err := test.database.ListServicesForBuild(context.TODO(), _build, filters, 1, 10) + got, err := test.database.ListServicesForBuild(context.TODO(), _build, filters, 1, 10) if test.failure { if err == nil { diff --git a/database/service/list_test.go b/database/service/list_test.go index 7b6afa58e..9d6959f6f 100644 --- a/database/service/list_test.go +++ b/database/service/list_test.go @@ -35,13 +35,7 @@ func TestService_Engine_ListServices(t *testing.T) { defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "services"`).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "repo_id", "build_id", "number", "name", "image", "stage", "status", "error", "exit_code", "created", "started", "finished", "host", "runtime", "distribution"}). AddRow(1, 1, 1, 1, "foo", "bar", "", "", "", 0, 0, 0, 0, "", "", ""). AddRow(2, 1, 2, 1, "bar", "foo", "", "", "", 0, 0, 0, 0, "", "", "") diff --git a/database/step/interface.go b/database/step/interface.go index ef74d05ea..87d3f2108 100644 --- a/database/step/interface.go +++ b/database/step/interface.go @@ -41,7 +41,7 @@ type StepInterface interface { // ListSteps defines a function that gets a list of all steps. ListSteps(ctx context.Context) ([]*api.Step, error) // ListStepsForBuild defines a function that gets a list of steps by build ID. - ListStepsForBuild(context.Context, *api.Build, map[string]interface{}, int, int) ([]*api.Step, int64, error) + ListStepsForBuild(context.Context, *api.Build, map[string]interface{}, int, int) ([]*api.Step, error) // ListStepImageCount defines a function that gets a list of all step images and the count of their occurrence. ListStepImageCount(context.Context) (map[string]float64, error) // ListStepStatusCount defines a function that gets a list of all step statuses and the count of their occurrence. diff --git a/database/step/list.go b/database/step/list.go index 3904d9efa..f4b42719a 100644 --- a/database/step/list.go +++ b/database/step/list.go @@ -15,23 +15,11 @@ func (e *engine) ListSteps(ctx context.Context) ([]*api.Step, error) { e.logger.Trace("listing all steps") // variables to store query results and return value - count := int64(0) w := new([]types.Step) steps := []*api.Step{} - // count the results - count, err := e.CountSteps(ctx) - if err != nil { - return nil, err - } - - // short-circuit if there are no results - if count == 0 { - return steps, nil - } - // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableStep). Find(&w). diff --git a/database/step/list_build.go b/database/step/list_build.go index 819156746..80144122b 100644 --- a/database/step/list_build.go +++ b/database/step/list_build.go @@ -13,32 +13,20 @@ import ( ) // ListStepsForBuild gets a list of all steps from the database. -func (e *engine) ListStepsForBuild(ctx context.Context, b *api.Build, filters map[string]interface{}, page int, perPage int) ([]*api.Step, int64, error) { +func (e *engine) ListStepsForBuild(ctx context.Context, b *api.Build, filters map[string]interface{}, page int, perPage int) ([]*api.Step, error) { e.logger.WithFields(logrus.Fields{ "build": b.GetNumber(), }).Tracef("listing steps for build %d", b.GetNumber()) // variables to store query results and return value - count := int64(0) s := new([]types.Step) steps := []*api.Step{} - // count the results - count, err := e.CountStepsForBuild(ctx, b, filters) - if err != nil { - return steps, 0, err - } - - // short-circuit if there are no results - if count == 0 { - return steps, 0, nil - } - // calculate offset for pagination through results offset := perPage * (page - 1) // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableStep). Where("build_id = ?", b.GetID()). @@ -49,7 +37,7 @@ func (e *engine) ListStepsForBuild(ctx context.Context, b *api.Build, filters ma Find(&s). Error if err != nil { - return nil, count, err + return nil, err } // iterate through all query results @@ -60,5 +48,5 @@ func (e *engine) ListStepsForBuild(ctx context.Context, b *api.Build, filters ma steps = append(steps, tmp.ToAPI()) } - return steps, count, nil + return steps, nil } diff --git a/database/step/list_build_test.go b/database/step/list_build_test.go index 4bb43b044..2f679ca56 100644 --- a/database/step/list_build_test.go +++ b/database/step/list_build_test.go @@ -43,13 +43,7 @@ func TestStep_Engine_ListStepsForBuild(t *testing.T) { defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "steps" WHERE build_id = $1`).WithArgs(1).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "repo_id", "build_id", "number", "name", "image", "stage", "status", "error", "exit_code", "created", "started", "finished", "host", "runtime", "distribution", "report_as"}). AddRow(2, 1, 1, 2, "foo", "bar", "", "", "", 0, 0, 0, 0, "", "", "", "test"). AddRow(1, 1, 1, 1, "foo", "bar", "", "", "", 0, 0, 0, 0, "", "", "", "") @@ -96,7 +90,7 @@ func TestStep_Engine_ListStepsForBuild(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, _, err := test.database.ListStepsForBuild(ctx, _build, filters, 1, 10) + got, err := test.database.ListStepsForBuild(ctx, _build, filters, 1, 10) if test.failure { if err == nil { diff --git a/database/step/list_test.go b/database/step/list_test.go index 99819c59f..b2b3cd845 100644 --- a/database/step/list_test.go +++ b/database/step/list_test.go @@ -37,13 +37,7 @@ func TestStep_Engine_ListSteps(t *testing.T) { defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "steps"`).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "repo_id", "build_id", "number", "name", "image", "stage", "status", "error", "exit_code", "created", "started", "finished", "host", "runtime", "distribution", "report_as"}). AddRow(1, 1, 1, 1, "foo", "bar", "", "", "", 0, 0, 0, 0, "", "", "", ""). AddRow(2, 1, 2, 1, "bar", "foo", "", "", "", 0, 0, 0, 0, "", "", "", "") diff --git a/database/user/interface.go b/database/user/interface.go index 55a1b2d62..73f1fd73a 100644 --- a/database/user/interface.go +++ b/database/user/interface.go @@ -39,7 +39,7 @@ type UserInterface interface { // ListUsers defines a function that gets a list of all users. ListUsers(context.Context) ([]*api.User, error) // ListLiteUsers defines a function that gets a lite list of users. - ListLiteUsers(context.Context, int, int) ([]*api.User, int64, error) + ListLiteUsers(context.Context, int, int) ([]*api.User, error) // UpdateUser defines a function that updates an existing user. UpdateUser(context.Context, *api.User) (*api.User, error) } diff --git a/database/user/list.go b/database/user/list.go index ae2858686..b7b36db5d 100644 --- a/database/user/list.go +++ b/database/user/list.go @@ -15,23 +15,11 @@ func (e *engine) ListUsers(ctx context.Context) ([]*api.User, error) { e.logger.Trace("listing all users") // variables to store query results and return value - count := int64(0) u := new([]types.User) users := []*api.User{} - // count the results - count, err := e.CountUsers(ctx) - if err != nil { - return nil, err - } - - // short-circuit if there are no results - if count == 0 { - return users, nil - } - // send query to the database and store result in variable - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableUser). Find(&u). diff --git a/database/user/list_lite.go b/database/user/list_lite.go index f1719e0d5..7faafbcce 100644 --- a/database/user/list_lite.go +++ b/database/user/list_lite.go @@ -13,29 +13,17 @@ import ( // ListLiteUsers gets a lite (only: id, name) list of users from the database. // //nolint:lll // ignore long line length due to variable names -func (e *engine) ListLiteUsers(ctx context.Context, page, perPage int) ([]*api.User, int64, error) { +func (e *engine) ListLiteUsers(ctx context.Context, page, perPage int) ([]*api.User, error) { e.logger.Trace("listing lite users") // variables to store query results and return values - count := int64(0) u := new([]types.User) users := []*api.User{} - // count the results - count, err := e.CountUsers(ctx) - if err != nil { - return users, 0, err - } - - // short-circuit if there are no results - if count == 0 { - return users, 0, nil - } - // calculate offset for pagination through results offset := perPage * (page - 1) - err = e.client. + err := e.client. WithContext(ctx). Table(constants.TableUser). Select("id", "name"). @@ -44,7 +32,7 @@ func (e *engine) ListLiteUsers(ctx context.Context, page, perPage int) ([]*api.U Find(&u). Error if err != nil { - return nil, count, err + return nil, err } // iterate through all query results @@ -56,5 +44,5 @@ func (e *engine) ListLiteUsers(ctx context.Context, page, perPage int) ([]*api.U users = append(users, tmp.ToAPI()) } - return users, count, nil + return users, nil } diff --git a/database/user/list_lite_test.go b/database/user/list_lite_test.go index 31a4b1f0f..52259f951 100644 --- a/database/user/list_lite_test.go +++ b/database/user/list_lite_test.go @@ -33,13 +33,7 @@ func TestUser_Engine_ListLiteUsers(t *testing.T) { defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(1) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "users"`).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "name"}). AddRow(1, "foo"). AddRow(2, "baz") @@ -95,7 +89,7 @@ func TestUser_Engine_ListLiteUsers(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, _, err := test.database.ListLiteUsers(context.TODO(), 1, 10) + got, err := test.database.ListLiteUsers(context.TODO(), 1, 10) if test.failure { if err == nil { diff --git a/database/user/list_test.go b/database/user/list_test.go index ce2e7b3f7..e6a6c0608 100644 --- a/database/user/list_test.go +++ b/database/user/list_test.go @@ -33,13 +33,7 @@ func TestUser_Engine_ListUsers(t *testing.T) { defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "users"`).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "name", "refresh_token", "token", "hash", "favorites", "active", "admin", "dashboards"}). AddRow(1, "foo", "", "bar", "baz", "{}", false, false, "{}"). AddRow(2, "baz", "", "bar", "foo", "{}", false, false, "{}") diff --git a/secret/native/list.go b/secret/native/list.go index 1f6604f91..9c561e303 100644 --- a/secret/native/list.go +++ b/secret/native/list.go @@ -23,7 +23,7 @@ func (c *client) List(ctx context.Context, sType, org, name string, page, perPag }).Tracef("listing native %s secrets for %s", sType, org) // capture the list of org secrets from the native service - secrets, _, err := c.Database.ListSecretsForOrg(ctx, org, nil, page, perPage) + secrets, err := c.Database.ListSecretsForOrg(ctx, org, nil, page, perPage) if err != nil { return nil, err } @@ -43,7 +43,7 @@ func (c *client) List(ctx context.Context, sType, org, name string, page, perPag r.SetFullName(fmt.Sprintf("%s/%s", org, name)) // capture the list of repo secrets from the native service - secrets, _, err := c.Database.ListSecretsForRepo(ctx, r, nil, page, perPage) + secrets, err := c.Database.ListSecretsForRepo(ctx, r, nil, page, perPage) if err != nil { return nil, err } @@ -59,7 +59,7 @@ func (c *client) List(ctx context.Context, sType, org, name string, page, perPag }).Tracef("listing native %s secrets for teams %s in org %s", sType, teams, org) // capture the list of shared secrets for multiple teams from the native service - secrets, _, err := c.Database.ListSecretsForTeams(ctx, org, teams, nil, page, perPage) + secrets, err := c.Database.ListSecretsForTeams(ctx, org, teams, nil, page, perPage) if err != nil { return nil, err } @@ -74,7 +74,7 @@ func (c *client) List(ctx context.Context, sType, org, name string, page, perPag }).Tracef("listing native %s secrets for %s/%s", sType, org, name) // capture the list of shared secrets from the native service - secrets, _, err := c.Database.ListSecretsForTeam(ctx, org, name, nil, page, perPage) + secrets, err := c.Database.ListSecretsForTeam(ctx, org, name, nil, page, perPage) if err != nil { return nil, err }