Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding paging to get repositories from the database #1793

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
adding paging to get repositories from the database
  • Loading branch information
thiago committed Sep 15, 2016
commit 239c36434279d6241555e9f8079405f950a39fc4
34 changes: 25 additions & 9 deletions store/datastore/repos.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,37 @@ func (db *datastore) GetRepoName(name string) (*model.Repo, error) {

func (db *datastore) GetRepoListOf(listof []*model.RepoLite) ([]*model.Repo, error) {
var (
repos []*model.Repo
args []interface{}
stmt string
err error
err error
repos = []*model.Repo{}
toListRepoLite func([]*model.RepoLite) (string, []interface{})
total = len(listof)
)

if total == 0 {
return repos, nil
}

switch meddler.Default {
case meddler.PostgreSQL:
stmt, args = toListPosgres(listof)
toListRepoLite = toListPosgres
default:
stmt, args = toList(listof)
toListRepoLite = toList
}
if len(args) > 0 {
err = meddler.QueryAll(db, &repos, fmt.Sprintf(repoListOfQuery, stmt), args...)

pages := calculatePagination(total, maxRepoPage)

for i := 0; i < pages; i++ {
stmt, args := toListRepoLite(resizeList(listof, i, maxRepoPage))

var tmpRepos []*model.Repo
err = meddler.QueryAll(db, &tmpRepos, fmt.Sprintf(repoListOfQuery, stmt), args...)
if err != nil {
return nil, err
}

repos = append(repos, tmpRepos...)
}
return repos, err
return repos, nil
}

func (db *datastore) GetRepoCount() (int, error) {
Expand Down
47 changes: 47 additions & 0 deletions store/datastore/repos_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package datastore

import (
"fmt"
"testing"

"github.com/drone/drone/model"
Expand Down Expand Up @@ -138,6 +139,52 @@ func TestRepos(t *testing.T) {
g.Assert(count).Equal(2)
})

g.It("Should Get a Repo List Paginated", func() {
_repos := []*model.RepoLite{
{FullName: "bradrydzewski/drone"},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the newer versions of the Go linter will complain when underscores are used in variable names, so we've tried to start removing these from our code. So perhaps repoLiteList instead?

{FullName: "drone/drone"},
}
_repo := []*model.Repo{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps repoList instead of _repo since it is an array. When I see _repo I think it is a single repo

{
UserID: 1,
Owner: "bradrydzewski",
Name: "drone",
FullName: "bradrydzewski/drone",
},
{
UserID: 2,
Owner: "drone",
Name: "drone",
FullName: "drone/drone",
},
}
s.CreateRepo(_repo[0])
s.CreateRepo(_repo[1])

for i := 0; i < 3000; i++ {
// find by 3000 repos
_repos = append(_repos, &model.RepoLite{FullName: "octocat/hello-world" + fmt.Sprint(i)})
// but create only 2000 repos
if i >= 1999 && i < 2999 {
continue
}
repo := &model.Repo{
UserID: 2,
Owner: "octocat",
Name: "hello-world" + fmt.Sprint(i),
FullName: "octocat/hello-world" + fmt.Sprint(i),
}
_repo = append(_repo, repo)
s.CreateRepo(repo)
}
repos, err := s.GetRepoListOf(_repos)
g.Assert(err == nil).IsTrue()
g.Assert(len(repos)).Equal(2002)
g.Assert(repos[0].ID).Equal(_repo[0].ID)
g.Assert(repos[1].ID).Equal(_repo[1].ID)
g.Assert(repos[2001].ID).Equal(_repo[2001].ID)
})

g.It("Should Delete a Repo", func() {
repo := model.Repo{
UserID: 1,
Expand Down
75 changes: 55 additions & 20 deletions store/datastore/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package datastore

import (
"fmt"
"sort"

"github.com/drone/drone/model"
"github.com/russross/meddler"
Expand All @@ -27,42 +28,76 @@ func (db *datastore) GetUserList() ([]*model.User, error) {

func (db *datastore) GetUserFeed(listof []*model.RepoLite) ([]*model.Feed, error) {
var (
args []interface{}
stmt string
err error

feed = []*model.Feed{}
err error
feed = feedHelper{}
total = len(listof)
toListRepoLite func([]*model.RepoLite) (string, []interface{})
)

if total == 0 {
return feed, nil
}

switch meddler.Default {
case meddler.PostgreSQL:
stmt, args = toListPosgres(listof)
toListRepoLite = toListPosgres
default:
stmt, args = toList(listof)
toListRepoLite = toList
}
if len(args) > 0 {
err = meddler.QueryAll(db, &feed, fmt.Sprintf(userFeedQuery, stmt), args...)

pages := calculatePagination(total, maxRepoPage)
for i := 0; i < pages; i++ {
stmt, args := toListRepoLite(resizeList(listof, i, maxRepoPage))

var tmpFeed []*model.Feed
err = meddler.QueryAll(db, &tmpFeed, fmt.Sprintf(userFeedQuery, stmt), args...)
if err != nil {
return nil, err
}

feed = append(feed, tmpFeed...)
}

if len(feed) <= 50 {
return feed, nil
}
return feed, err

sort.Sort(sort.Reverse(feed))

return feed[:50], nil
}

func (db *datastore) GetUserFeedLatest(listof []*model.RepoLite) ([]*model.Feed, error) {
var (
args []interface{}
stmt string
err error

err error
feed = []*model.Feed{}
toListRepoLite func([]*model.RepoLite) (string, []interface{})
total = len(listof)
)

if total == 0 {
return feed, nil
}

switch meddler.Default {
case meddler.PostgreSQL:
stmt, args = toListPosgres(listof)
toListRepoLite = toListPosgres
default:
stmt, args = toList(listof)
toListRepoLite = toList
}
if len(args) > 0 {
err = meddler.QueryAll(db, &feed, fmt.Sprintf(userFeedLatest, stmt), args...)

pages := calculatePagination(total, maxRepoPage)
for i := 0; i < pages; i++ {
stmt, args := toListRepoLite(resizeList(listof, i, maxRepoPage))
var tmpFeed []*model.Feed
err = meddler.QueryAll(db, &tmpFeed, fmt.Sprintf(userFeedLatest, stmt), args...)
if err != nil {
return nil, err
}

feed = append(feed, tmpFeed...)
}
return feed, err
return feed, nil
}

func (db *datastore) GetUserCount() (int, error) {
Expand Down Expand Up @@ -170,4 +205,4 @@ FROM repos LEFT OUTER JOIN builds ON build_id = (
LIMIT 1
)
WHERE repo_full_name IN (%s)
`
`
48 changes: 40 additions & 8 deletions store/datastore/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,24 @@ import (
"github.com/russross/meddler"
)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import grouping / ordering


// indicate max of each page when paginated repo list
const maxRepoPage = 999

// helper type that sort Feed List
type feedHelper []*model.Feed

func (slice feedHelper) Len() int {
return len(slice)
}

func (slice feedHelper) Less(i, j int) bool {
return slice[i].Created < slice[j].Created;
}

func (slice feedHelper) Swap(i, j int) {
slice[i], slice[j] = slice[j], slice[i]
}

// rebind is a helper function that changes the sql
// bind type from ? to $ for postgres queries.
func rebind(query string) string {
Expand Down Expand Up @@ -36,14 +54,32 @@ func rebind(query string) string {
return string(rqb)
}

// helper function that calculate pagination
func calculatePagination(total, limit int) (pages int) {
pages = total / limit
if total % limit != 0 {
pages++
}
return
}

// helper function that resize list of repo to pagination
func resizeList(listof []*model.RepoLite, page, limit int) []*model.RepoLite {
var total = len(listof)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we avoid returning a [][]interface{} and instead do something like:

const limit = 999

total := len(listof)
if total == 0 {
    return feed
}

pages := total / limit
if total % limit != 0 {
  pages++
}

for i:=0;i<pages;i++ {
    stmt, args := toList(listof, i*limit, limit)

    var tmpFeed []*model.Feed
    err = meddler.QueryAll(db, &tmpFeed, 
      fmt.Sprintf(userFeedQuery, stmt), args...)
    if err != nil {
      return nil, err
    }

    feed = append(feed, tmpFeed...)
}

if len(feed) <= 50 {
   return feed, nil
}

// sort

return feed[:50], nil

where start and limit indicate where to slice

func toList(listof []*model.RepoLite, start, limit int) ([]string, []interface{}) {
   if len(listof) != limit {
     listof = listof[start,limit]
   }

it could make sense to slice the listof before invoking toList as well ... I don't have big preference either way

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was my first solution... It did not seem attractive for me because the limit variable and the core logic for pagination was replicated on the repos and users functions.

Copy link

@bradrydzewski bradrydzewski Sep 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It did not seem attractive for me because the limit variable

this limit could be a global constant, for example:

const maxPlaceholder = 999

core logic for pagination was replicated

I think this is OK

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

core logic for pagination was replicated

note that it could always get split out into a separate helper function:

func calculatePagination(total, limit int) (pages int) {
  pages = total / limit
  if total % limit != 0 {
    pages++
  }
  return
}

var end = (page * limit) + limit
if end > total{
end = total
}
if total > limit{
return listof[page * limit:end]
}
return listof
}

// helper function that converts a simple repsitory list
// to a sql IN statment.
func toList(listof []*model.RepoLite) (string, []interface{}) {
var size = len(listof)
if size > 999 {
size = 999
listof = listof[:999]
}
var qs = make([]string, size, size)
var in = make([]interface{}, size, size)
for i, repo := range listof {
Expand All @@ -57,10 +93,6 @@ func toList(listof []*model.RepoLite) (string, []interface{}) {
// to a sql IN statement compatible with postgres.
func toListPosgres(listof []*model.RepoLite) (string, []interface{}) {
var size = len(listof)
if size > 999 {
size = 999
listof = listof[:999]
}
var qs = make([]string, size, size)
var in = make([]interface{}, size, size)
for i, repo := range listof {
Expand Down
47 changes: 47 additions & 0 deletions store/datastore/utils_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package datastore

import (
"testing"

"github.com/drone/drone/model"
"github.com/franela/goblin"
)

func TestUtils(t *testing.T) {
g := goblin.Goblin(t)
g.Describe("Utils", func() {

g.It("Should Calculate Pagination", func() {
g.Assert(calculatePagination(10, 100)).Equal(1)
g.Assert(calculatePagination(10, 5)).Equal(2)
g.Assert(calculatePagination(10, 3)).Equal(4)
})

g.It("Should Resize a RepoLite List", func() {
repoLiteList := []*model.RepoLite{
{FullName: "bradrydzewski/drone"},
{FullName: "drone/drone"},
{FullName: "octocat/hello-world-1"},
{FullName: "octocat/hello-world-2"},
{FullName: "octocat/hello-world-3"},
}
notResized := resizeList(repoLiteList, 0, 10)
page1 := resizeList(repoLiteList, 0, 2)
page2 := resizeList(repoLiteList, 1, 2)
page3 := resizeList(repoLiteList, 2, 2)

g.Assert(len(notResized)).Equal(5)

g.Assert(len(page1)).Equal(2)
g.Assert(page1[0].FullName).Equal("bradrydzewski/drone")
g.Assert(page1[1].FullName).Equal("drone/drone")

g.Assert(len(page2)).Equal(2)
g.Assert(page2[0].FullName).Equal("octocat/hello-world-1")
g.Assert(page2[1].FullName).Equal("octocat/hello-world-2")

g.Assert(len(page3)).Equal(1)
g.Assert(page3[0].FullName).Equal("octocat/hello-world-3")
})
})
}