Skip to content

Commit

Permalink
refactor(db): Removing pagination based on OFFSET
Browse files Browse the repository at this point in the history 
Signed-off-by: Vincent Boutour <[email protected]>
  • Loading branch information
@ViBiOh
ViBiOh committed May 14, 2021
1 parent af01852 commit a05c2ca
Show file tree
Hide file tree
Showing 15 changed files with 96 additions and 75 deletions.
9 changes: 8 additions & 1 deletion pkg/ketchup/renderer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package ketchup

import (
"net/http"
"strings"

"github.com/ViBiOh/httputils/v4/pkg/logger"
"github.com/ViBiOh/httputils/v4/pkg/query"
)

const (
Expand Down Expand Up @@ -37,7 +39,12 @@ func min(a, b uint64) uint64 {
}

func (a app) AppTemplateFunc(r *http.Request) (string, int, map[string]interface{}, error) {
ketchups, _, err := a.ketchupService.List(r.Context(), 1, 100)
pagination, err := query.ParsePagination(r, 1, 100, 100)
if err != nil {
return "", http.StatusBadRequest, nil, err
}

ketchups, _, err := a.ketchupService.List(r.Context(), pagination.PageSize, strings.TrimSpace(r.URL.Query().Get("lastKey")))
if err != nil {
return "", http.StatusInternalServerError, nil, err
}
Expand Down
8 changes: 6 additions & 2 deletions pkg/notifier/releases.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ func (a app) getNewGithubReleases(ctx context.Context) ([]model.Release, uint64,
var newReleases []model.Release
var count uint64
page := uint(1)
lastKey := ""

for {
repositories, totalCount, err := a.repositoryService.ListByKind(ctx, page, pageSize, model.Github)
repositories, totalCount, err := a.repositoryService.ListByKind(ctx, pageSize, lastKey, model.Github)
if err != nil {
return nil, count, fmt.Errorf("unable to fetch page %d of GitHub repositories: %s", page, err)
}
Expand All @@ -40,6 +41,7 @@ func (a app) getNewGithubReleases(ctx context.Context) ([]model.Release, uint64,
}

if uint64(page*pageSize) < totalCount {
lastKey = fmt.Sprintf("%d", repositories[len(repositories)-1].ID)
page++
} else {
logger.Info("%d GitHub repositories checked, %d new releases", count, len(newReleases))
Expand Down Expand Up @@ -68,9 +70,10 @@ func (a app) getNewHelmReleases(ctx context.Context) ([]model.Release, uint64, e
var newReleases []model.Release
var count uint64
page := uint(1)
lastKey := ""

for {
repositories, totalCount, err := a.repositoryService.ListByKind(ctx, page, pageSize, model.Helm)
repositories, totalCount, err := a.repositoryService.ListByKind(ctx, pageSize, lastKey, model.Helm)
if err != nil {
return nil, 0, fmt.Errorf("unable to fetch page %d of Helm repositories: %s", page, err)
}
Expand Down Expand Up @@ -98,6 +101,7 @@ func (a app) getNewHelmReleases(ctx context.Context) ([]model.Release, uint64, e
newReleases = append(newReleases, a.getFetchHelmSources(repoWithNames)...)

if uint64(page*pageSize) < totalCount {
lastKey = fmt.Sprintf("%d", repositories[len(repositories)-1].ID)
page++
} else {
logger.Info("%d Helm repositories checked, %d new releases", count, len(newReleases))
Expand Down
6 changes: 3 additions & 3 deletions pkg/service/ketchup/ketchup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (

// App of package
type App interface {
List(ctx context.Context, page, pageSize uint) ([]model.Ketchup, uint64, error)
List(ctx context.Context, pageSize uint, lastKey string) ([]model.Ketchup, uint64, error)
ListForRepositories(ctx context.Context, repositories []model.Repository, frequency model.KetchupFrequency) ([]model.Ketchup, error)
ListOutdatedByFrequency(ctx context.Context, frequency model.KetchupFrequency) ([]model.Ketchup, error)
Create(ctx context.Context, item model.Ketchup) (model.Ketchup, error)
Expand All @@ -37,8 +37,8 @@ func New(ketchupStore ketchup.App, repositoryService repository.App) App {
}
}

func (a app) List(ctx context.Context, page, pageSize uint) ([]model.Ketchup, uint64, error) {
list, total, err := a.ketchupStore.List(ctx, page, pageSize)
func (a app) List(ctx context.Context, pageSize uint, lastKey string) ([]model.Ketchup, uint64, error) {
list, total, err := a.ketchupStore.List(ctx, pageSize, lastKey)
if err != nil {
return nil, 0, httpModel.WrapInternal(fmt.Errorf("unable to list: %w", err))
}
Expand Down
12 changes: 4 additions & 8 deletions pkg/service/ketchup/ketchup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ var (

func TestList(t *testing.T) {
type args struct {
page uint
pageSize uint
lastKey string
}

var cases = []struct {
Expand All @@ -41,9 +41,7 @@ func TestList(t *testing.T) {
model.NewKetchup(model.DefaultPattern, "1.2.3", model.Daily, model.NewGithubRepository(2, viwsRepository).AddVersion(model.DefaultPattern, "1.2.3")),
model.NewKetchup(model.DefaultPattern, "1.0.0", model.Daily, model.NewGithubRepository(1, ketchupRepository).AddVersion(model.DefaultPattern, "1.0.2")),
}, 2, nil), nil),
args{
page: 1,
},
args{},
[]model.Ketchup{
{Pattern: model.DefaultPattern, Version: "1.0.0", Frequency: model.Daily, Semver: "Patch", Repository: model.NewGithubRepository(1, ketchupRepository).AddVersion(model.DefaultPattern, "1.0.2")},
{Pattern: model.DefaultPattern, Version: "1.2.3", Frequency: model.Daily, Repository: model.NewGithubRepository(2, viwsRepository).AddVersion(model.DefaultPattern, "1.2.3")},
Expand All @@ -54,9 +52,7 @@ func TestList(t *testing.T) {
{
"error",
New(ketchuptest.New().SetList(nil, 0, errors.New("failed")), nil),
args{
page: 0,
},
args{},
nil,
0,
httpModel.ErrInternalError,
Expand All @@ -65,7 +61,7 @@ func TestList(t *testing.T) {

for _, tc := range cases {
t.Run(tc.intention, func(t *testing.T) {
got, gotCount, gotErr := tc.instance.List(context.Background(), tc.args.page, tc.args.pageSize)
got, gotCount, gotErr := tc.instance.List(context.Background(), tc.args.pageSize, tc.args.lastKey)

failed := false

Expand Down
2 changes: 1 addition & 1 deletion pkg/service/ketchup/ketchuptest/ketchuptest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (a *App) SetListOutdatedByFrequency(list []model.Ketchup, err error) *App {
}

// List mocks
func (a App) List(_ context.Context, _, _ uint) ([]model.Ketchup, uint64, error) {
func (a App) List(_ context.Context, _ uint, _ string) ([]model.Ketchup, uint64, error) {
return a.listKetchups, a.listTotal, a.listErr
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/service/repository/repository.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ var (

// App of package
type App interface {
List(ctx context.Context, page, pageSize uint) ([]model.Repository, uint64, error)
ListByKind(ctx context.Context, page, pageSize uint, kind model.RepositoryKind) ([]model.Repository, uint64, error)
List(ctx context.Context, pageSize uint, lastKey string) ([]model.Repository, uint64, error)
ListByKind(ctx context.Context, pageSize uint, lastKey string, kind model.RepositoryKind) ([]model.Repository, uint64, error)
Suggest(ctx context.Context, ignoreIds []uint64, count uint64) ([]model.Repository, error)
GetOrCreate(ctx context.Context, kind model.RepositoryKind, name, part, pattern string) (model.Repository, error)
Update(ctx context.Context, item model.Repository) error
Expand All @@ -45,17 +45,17 @@ func New(repositoryStore repository.App, githubApp github.App, helmApp helm.App)
}
}

func (a app) List(ctx context.Context, page, pageSize uint) ([]model.Repository, uint64, error) {
list, total, err := a.repositoryStore.List(ctx, page, pageSize)
func (a app) List(ctx context.Context, pageSize uint, lastKey string) ([]model.Repository, uint64, error) {
list, total, err := a.repositoryStore.List(ctx, pageSize, lastKey)
if err != nil {
return nil, 0, httpModel.WrapInternal(fmt.Errorf("unable to list: %w", err))
}

return list, total, nil
}

func (a app) ListByKind(ctx context.Context, page, pageSize uint, kind model.RepositoryKind) ([]model.Repository, uint64, error) {
list, total, err := a.repositoryStore.ListByKind(ctx, page, pageSize, kind)
func (a app) ListByKind(ctx context.Context, pageSize uint, lastKey string, kind model.RepositoryKind) ([]model.Repository, uint64, error) {
list, total, err := a.repositoryStore.ListByKind(ctx, pageSize, lastKey, kind)
if err != nil {
return nil, 0, httpModel.WrapInternal(fmt.Errorf("unable to list by kind: %w", err))
}
Expand Down
12 changes: 4 additions & 8 deletions pkg/service/repository/repository_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ func safeParse(version string) semver.Version {

func TestList(t *testing.T) {
type args struct {
page uint
pageSize uint
lastKey string
}

var cases = []struct {
Expand All @@ -52,9 +52,7 @@ func TestList(t *testing.T) {
model.NewGithubRepository(1, ketchupRepository).AddVersion(model.DefaultPattern, "1.0.0"),
model.NewGithubRepository(2, viwsRepository).AddVersion(model.DefaultPattern, "1.2.3"),
}, 2, nil), nil, nil),
args{
page: 1,
},
args{},
[]model.Repository{
model.NewGithubRepository(1, ketchupRepository).AddVersion(model.DefaultPattern, "1.0.0"),
model.NewGithubRepository(2, viwsRepository).AddVersion(model.DefaultPattern, "1.2.3"),
Expand All @@ -65,9 +63,7 @@ func TestList(t *testing.T) {
{
"error",
New(repositorytest.New().SetList(nil, 0, errors.New("failed")), nil, nil),
args{
page: 0,
},
args{},
nil,
0,
httpModel.ErrInternalError,
Expand All @@ -76,7 +72,7 @@ func TestList(t *testing.T) {

for _, tc := range cases {
t.Run(tc.intention, func(t *testing.T) {
got, gotCount, gotErr := tc.instance.List(context.Background(), tc.args.page, tc.args.pageSize)
got, gotCount, gotErr := tc.instance.List(context.Background(), tc.args.pageSize, tc.args.lastKey)

failed := false

Expand Down
4 changes: 2 additions & 2 deletions pkg/service/repository/repositorytest/repositorytest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,12 @@ func (a *App) SetLatestVersions(latestVersions map[string]semver.Version, err er
}

// List mocks
func (a *App) List(_ context.Context, _, _ uint) ([]model.Repository, uint64, error) {
func (a *App) List(_ context.Context, _ uint, _ string) ([]model.Repository, uint64, error) {
return a.listRepositories, a.listTotal, a.listErr
}

// ListByKind mocks
func (a *App) ListByKind(_ context.Context, _, _ uint, _ model.RepositoryKind) ([]model.Repository, uint64, error) {
func (a *App) ListByKind(_ context.Context, _ uint, _ string, _ model.RepositoryKind) ([]model.Repository, uint64, error) {
return a.listByKindRepositories, a.listByKindTotal, a.listByKindErr
}

Expand Down
33 changes: 29 additions & 4 deletions pkg/store/ketchup/ketchup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,18 @@ import (
"context"
"database/sql"
"errors"
"fmt"
"strings"

"github.com/ViBiOh/httputils/v4/pkg/db"
"github.com/ViBiOh/ketchup/pkg/model"
"github.com/ViBiOh/ketchup/pkg/store"
"github.com/lib/pq"
)

// App of package
type App interface {
DoAtomic(ctx context.Context, action func(context.Context) error) error
List(ctx context.Context, page, pageSize uint) ([]model.Ketchup, uint64, error)
List(ctx context.Context, page uint, lastKey string) ([]model.Ketchup, uint64, error)
ListByRepositoriesID(ctx context.Context, ids []uint64, frequency model.KetchupFrequency) ([]model.Ketchup, error)
ListOutdatedByFrequency(ctx context.Context, frequency model.KetchupFrequency) ([]model.Ketchup, error)
GetByRepositoryID(ctx context.Context, id uint64, forUpdate bool) (model.Ketchup, error)
Expand Down Expand Up @@ -61,7 +61,18 @@ WHERE
AND rv.pattern = k.pattern
`

func (a app) List(ctx context.Context, page, pageSize uint) ([]model.Ketchup, uint64, error) {
const listQueryRestart = `
AND (
(
k.repository_id = $%d
AND k.pattern > $%d
) OR (
k.repository_id > $%d
)
)
`

func (a app) List(ctx context.Context, pageSize uint, lastKey string) ([]model.Ketchup, uint64, error) {
user := model.ReadUser(ctx)

var totalCount uint64
Expand Down Expand Up @@ -102,7 +113,21 @@ func (a app) List(ctx context.Context, page, pageSize uint) ([]model.Ketchup, ui
user.ID,
}

queryArgs = append(queryArgs, store.AddPagination(&query, len(queryArgs), page, pageSize)...)
if len(lastKey) != 0 {
parts := strings.Split(lastKey, "|")
if len(parts) != 2 {
return nil, 0, fmt.Errorf("invalid last key format: %s", lastKey)
}

value := len(queryArgs)
query.WriteString(fmt.Sprintf(listQueryRestart, value+1, value+2, value+3))
queryArgs = append(queryArgs, parts[0], parts[1], parts[0])
}

query.WriteString(" ORDER BY k.repository_id ASC, k.pattern ASC")

query.WriteString(fmt.Sprintf(" LIMIT $%d", len(queryArgs)+1))
queryArgs = append(queryArgs, pageSize)

return list, totalCount, db.List(ctx, a.db, scanner, query.String(), queryArgs...)
}
Expand Down
9 changes: 2 additions & 7 deletions pkg/store/ketchup/ketchup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ func testWithTransaction(t *testing.T, mockDb *sql.DB, mock sqlmock.Sqlmock) con

func TestList(t *testing.T) {
type args struct {
page uint
pageSize uint
}

Expand All @@ -67,7 +66,6 @@ func TestList(t *testing.T) {
{
"simple",
args{
page: 1,
pageSize: 20,
},
[]model.Ketchup{
Expand All @@ -92,7 +90,6 @@ func TestList(t *testing.T) {
{
"timeout",
args{
page: 1,
pageSize: 20,
},
[]model.Ketchup{},
Expand All @@ -102,7 +99,6 @@ func TestList(t *testing.T) {
{
"invalid rows",
args{
page: 1,
pageSize: 20,
},
[]model.Ketchup{},
Expand All @@ -112,7 +108,6 @@ func TestList(t *testing.T) {
{
"invalid kind",
args{
page: 1,
pageSize: 20,
},
[]model.Ketchup{},
Expand All @@ -125,7 +120,7 @@ func TestList(t *testing.T) {
t.Run(tc.intention, func(t *testing.T) {
testWithMock(t, func(mockDb *sql.DB, mock sqlmock.Sqlmock) {
rows := sqlmock.NewRows([]string{"pattern", "version", "frequency", "repository_id", "name", "part", "kind", "repository_version", "full_count"})
expectedQuery := mock.ExpectQuery("SELECT k.pattern, k.version, k.frequency, k.repository_id, r.name, r.part, r.kind, rv.version, .+ AS full_count FROM ketchup.ketchup k, ketchup.repository r, ketchup.repository_version rv WHERE user_id = .+ AND k.repository_id = r.id AND rv.repository_id = r.id AND rv.pattern = k.pattern").WithArgs(3, 20, 0).WillReturnRows(rows)
expectedQuery := mock.ExpectQuery("SELECT k.pattern, k.version, k.frequency, k.repository_id, r.name, r.part, r.kind, rv.version, .+ AS full_count FROM ketchup.ketchup k, ketchup.repository r, ketchup.repository_version rv WHERE user_id = .+ AND k.repository_id = r.id AND rv.repository_id = r.id AND rv.pattern = k.pattern ORDER BY k.repository_id ASC, k.pattern ASC").WithArgs(3, 20).WillReturnRows(rows)

switch tc.intention {
case "simple":
Expand All @@ -146,7 +141,7 @@ func TestList(t *testing.T) {
rows.AddRow(model.DefaultPattern, repositoryVersion, "Daily", 2, viwsRepository, "", "wrong", repositoryVersion, 1)
}

got, gotCount, gotErr := New(mockDb).List(testCtx, tc.args.page, tc.args.pageSize)
got, gotCount, gotErr := New(mockDb).List(testCtx, tc.args.pageSize, "")
failed := false

if tc.wantErr == nil && gotErr != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/store/ketchup/ketchuptest/ketchuptest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (a *App) DoAtomic(ctx context.Context, action func(context.Context) error)
}

// List mocks
func (a *App) List(_ context.Context, page, pageSize uint) ([]model.Ketchup, uint64, error) {
func (a *App) List(_ context.Context, _ uint, _ string) ([]model.Ketchup, uint64, error) {
return a.listKetchups, a.listTotal, a.listErr
}

Expand Down
Loading

0 comments on commit a05c2ca

Please sign in to comment.