Skip to content

Commit

Permalink
fix(build): ensure count/pagination links account for filter/query pa…
Browse files Browse the repository at this point in the history
…rams (#1242)
  • Loading branch information
wass3rw3rk authored and TimHuynh committed Jan 17, 2025
1 parent 4eee25d commit df21b7a
Show file tree
Hide file tree
Showing 9 changed files with 26 additions and 12 deletions.
2 changes: 1 addition & 1 deletion api/build/compile_publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func CompileAndPublish(
}

// send API call to capture the number of pending or running builds for the repo
builds, err := database.CountBuildsForRepo(ctx, r, filters)
builds, err := database.CountBuildsForRepo(ctx, r, filters, time.Now().Unix(), 0)
if err != nil {
retErr := fmt.Errorf("%s: unable to get count of builds for repo %s", baseErr, r.GetFullName())

Expand Down
15 changes: 12 additions & 3 deletions api/pagination.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ func (p *Pagination) SetHeaderLink(c *gin.Context) {
l := []string{}
r := c.Request

// grab the current query params
q := r.URL.Query()

hl := HeaderLink{
"first": 1,
"last": p.TotalPages(),
Expand All @@ -42,6 +45,9 @@ func (p *Pagination) SetHeaderLink(c *gin.Context) {
return
}

// reset per config
q.Set("per_page", strconv.Itoa(p.PerPage))

// drop first, prev on the first page
if p.Page == 1 {
delete(hl, "first")
Expand All @@ -54,14 +60,17 @@ func (p *Pagination) SetHeaderLink(c *gin.Context) {
delete(hl, "next")
}

// loop over the fields that make up the header links
for rel, page := range hl {
// set the page info for the current field
q.Set("page", strconv.Itoa(page))

ls := fmt.Sprintf(
`<%s://%s%s?per_page=%d&page=%d>; rel="%s"`,
`<%s://%s%s?%s>; rel="%s"`,
resolveScheme(r),
r.Host,
r.URL.Path,
p.PerPage,
page,
q.Encode(),
rel,
)

Expand Down
2 changes: 1 addition & 1 deletion api/webhook/post.go
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,7 @@ func RenameRepository(ctx context.Context, l *logrus.Entry, db database.Interfac
}

// get total number of builds associated with repository
t, err = db.CountBuildsForRepo(ctx, dbR, nil)
t, err = db.CountBuildsForRepo(ctx, dbR, nil, time.Now().Unix(), 0)
if err != nil {
return nil, fmt.Errorf("unable to get build count for repo %s: %w", dbR.GetFullName(), err)
}
Expand Down
4 changes: 3 additions & 1 deletion database/build/count_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
)

// CountBuildsForRepo gets the count of builds by repo ID from the database.
func (e *engine) CountBuildsForRepo(ctx context.Context, r *api.Repo, filters map[string]interface{}) (int64, error) {
func (e *engine) CountBuildsForRepo(ctx context.Context, r *api.Repo, filters map[string]interface{}, before, after int64) (int64, error) {
e.logger.WithFields(logrus.Fields{
"org": r.GetOrg(),
"repo": r.GetName(),
Expand All @@ -26,6 +26,8 @@ func (e *engine) CountBuildsForRepo(ctx context.Context, r *api.Repo, filters ma
WithContext(ctx).
Table(constants.TableBuild).
Where("repo_id = ?", r.GetID()).
Where("created < ?", before).
Where("created > ?", after).
Where(filters).
Count(&b).
Error
Expand Down
7 changes: 5 additions & 2 deletions database/build/count_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"context"
"reflect"
"testing"
"time"

"github.com/DATA-DOG/go-sqlmock"

Expand Down Expand Up @@ -33,12 +34,14 @@ func TestBuild_Engine_CountBuildsForRepo(t *testing.T) {
_buildOne.SetRepo(_repo)
_buildOne.SetNumber(1)
_buildOne.SetDeployPayload(nil)
_buildOne.SetCreated(1)

_buildTwo := testutils.APIBuild()
_buildTwo.SetID(2)
_buildTwo.SetRepo(_repo)
_buildTwo.SetNumber(2)
_buildTwo.SetDeployPayload(nil)
_buildTwo.SetCreated(2)

_postgres, _mock := testPostgres(t)
defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }()
Expand All @@ -47,7 +50,7 @@ func TestBuild_Engine_CountBuildsForRepo(t *testing.T) {
_rows := sqlmock.NewRows([]string{"count"}).AddRow(2)

// ensure the mock expects the query
_mock.ExpectQuery(`SELECT count(*) FROM "builds" WHERE repo_id = $1`).WithArgs(1).WillReturnRows(_rows)
_mock.ExpectQuery(`SELECT count(*) FROM "builds" WHERE repo_id = $1 AND created < $2 AND created > $3`).WithArgs(1, AnyArgument{}, 0).WillReturnRows(_rows)

_sqlite := testSqlite(t)
defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
Expand Down Expand Up @@ -88,7 +91,7 @@ func TestBuild_Engine_CountBuildsForRepo(t *testing.T) {
// run tests
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
got, err := test.database.CountBuildsForRepo(context.TODO(), _repo, filters)
got, err := test.database.CountBuildsForRepo(context.TODO(), _repo, filters, time.Now().Unix(), 0)

if test.failure {
if err == nil {
Expand Down
2 changes: 1 addition & 1 deletion database/build/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type BuildInterface interface {
// CountBuildsForOrg defines a function that gets the count of builds by org name.
CountBuildsForOrg(context.Context, string, map[string]interface{}) (int64, error)
// CountBuildsForRepo defines a function that gets the count of builds by repo ID.
CountBuildsForRepo(context.Context, *api.Repo, map[string]interface{}) (int64, error)
CountBuildsForRepo(context.Context, *api.Repo, map[string]interface{}, int64, int64) (int64, error)
// CountBuildsForStatus defines a function that gets the count of builds by status.
CountBuildsForStatus(context.Context, string, map[string]interface{}) (int64, error)
// CreateBuild defines a function that creates a new build.
Expand Down
2 changes: 1 addition & 1 deletion database/build/list_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func (e *engine) ListBuildsForRepo(ctx context.Context, r *api.Repo, filters map
builds := []*api.Build{}

// count the results
count, err := e.CountBuildsForRepo(ctx, r, filters)
count, err := e.CountBuildsForRepo(ctx, r, filters, before, after)
if err != nil {
return builds, 0, err
}
Expand Down
2 changes: 1 addition & 1 deletion database/build/list_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestBuild_Engine_ListBuildsForRepo(t *testing.T) {
_rows := sqlmock.NewRows([]string{"count"}).AddRow(2)

// ensure the mock expects the count query
_mock.ExpectQuery(`SELECT count(*) FROM "builds" WHERE repo_id = $1`).WithArgs(1).WillReturnRows(_rows)
_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(
Expand Down
2 changes: 1 addition & 1 deletion database/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ func testBuilds(t *testing.T, db Interface, resources *Resources) {
methods["CountBuildsForOrg"] = true

// count the builds for a repo
count, err = db.CountBuildsForRepo(context.TODO(), resources.Repos[0], nil)
count, err = db.CountBuildsForRepo(context.TODO(), resources.Repos[0], nil, time.Now().Unix(), 0)
if err != nil {
t.Errorf("unable to count builds for repo %d: %v", resources.Repos[0].GetID(), err)
}
Expand Down

0 comments on commit df21b7a

Please sign in to comment.