From d23283642393fbd8b1a859113eed738af7b64081 Mon Sep 17 00:00:00 2001 From: konrad Date: Fri, 9 Nov 2018 10:30:17 +0000 Subject: [PATCH] Added pagination (#16) --- Featurecreep.md | 2 +- REST-Tests/auth.http | 2 +- REST-Tests/lists.http | 2 +- config.yml.sample | 4 +++- docs/config.md | 2 ++ pkg/config/config.go | 1 + pkg/models/crudable.go | 2 +- pkg/models/list.go | 15 ++++++++------- pkg/models/list_read_test.go | 4 ++-- pkg/models/list_users_readall.go | 3 ++- pkg/models/models.go | 12 ++++++++++++ pkg/models/namespace.go | 3 ++- pkg/models/namespace_test.go | 2 +- pkg/models/namespace_users_readall.go | 3 ++- pkg/models/team_list_readall.go | 3 ++- pkg/models/team_list_test.go | 8 ++++---- pkg/models/team_namespace_readall.go | 3 ++- pkg/models/team_namespace_test.go | 6 +++--- pkg/models/teams.go | 3 ++- pkg/models/teams_test.go | 2 +- pkg/routes/api/v1/caldav.go | 2 +- pkg/routes/crud/read_all.go | 18 +++++++++++++++++- 22 files changed, 71 insertions(+), 31 deletions(-) diff --git a/Featurecreep.md b/Featurecreep.md index 5b43106c1..3a2ed6ba4 100644 --- a/Featurecreep.md +++ b/Featurecreep.md @@ -222,7 +222,7 @@ Teams sind global, d.h. Ein Team kann mehrere Namespaces verwalten. * [ ] Wir brauchen noch ne gute idee, wie man die listen kriegt, auf die man nur so Zugriff hat (ohne namespace) * Dazu am Besten nen pseudonamespace anlegen (id -1 oder so), der hat das dann alles * [ ] Validation der ankommenden structs, am besten mit https://github.com/go-validator/validator oder mit dem Ding von echo -* [ ] Pagination +* [x] Pagination * Sollte in der Config definierbar sein, wie viel pro Seite angezeigt werden soll, die CRUD-Methoden übergeben dann ein "gibt mir die Seite sowieso" an die CRUDable-Funktionenen, die müssen das dann Auswerten. Geht leider nicht anders, wenn man erst 2342352 Einträge hohlt und die dann nachträglich auf 200 begrenzt ist das ne massive Ressourcenverschwendung. * [ ] Testing mit locust: https://locust.io/ * [ ] Methode einbauen, um mit einem gültigen token ein neues gültiges zu kriegen diff --git a/REST-Tests/auth.http b/REST-Tests/auth.http index acba01457..7da9d12d3 100644 --- a/REST-Tests/auth.http +++ b/REST-Tests/auth.http @@ -4,7 +4,7 @@ Content-Type: application/json { "username": "user", - "password": "12345" + "password": "1234" } > {% client.global.set("auth_token", response.body.token); %} diff --git a/REST-Tests/lists.http b/REST-Tests/lists.http index fcf19fca5..91cbf7de8 100644 --- a/REST-Tests/lists.http +++ b/REST-Tests/lists.http @@ -1,5 +1,5 @@ # Get all lists -GET http://localhost:8080/api/v1/lists +GET http://localhost:8080/api/v1/lists?page=0 Authorization: Bearer {{auth_token}} ### diff --git a/config.yml.sample b/config.yml.sample index 7d4af66a3..f3e32b384 100644 --- a/config.yml.sample +++ b/config.yml.sample @@ -7,6 +7,8 @@ service: interface: ":3456" # The URL of the frontend, used to send password reset emails. frontendurl: "" + # The number of items which gets returned per page + pagecount: 50 database: # Database type to use. Supported types are mysql and sqlite. @@ -48,4 +50,4 @@ mailer: # Wether to skip verification of the tls certificate on the server skiptlsverify: false # The default from address when sending emails - fromemail: 'mail@vikunja' \ No newline at end of file + fromemail: 'mail@vikunja' diff --git a/docs/config.md b/docs/config.md index 1e63c9c74..5bde996e2 100644 --- a/docs/config.md +++ b/docs/config.md @@ -33,6 +33,8 @@ service: frontendurl: "" # The base path on the file system where the binary and assets are rootpath: + # The number of items which gets returned per page + pagecount: 50 database: # Database type to use. Supported types are mysql and sqlite. diff --git a/pkg/config/config.go b/pkg/config/config.go index 7504879be..fd0966341 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -29,6 +29,7 @@ func InitConfig() (err error) { } exPath := filepath.Dir(ex) viper.SetDefault("service.rootpath", exPath) + viper.SetDefault("service.pagecount", 50) // Database viper.SetDefault("database.type", "sqlite") viper.SetDefault("database.host", "localhost") diff --git a/pkg/models/crudable.go b/pkg/models/crudable.go index 0480e90ed..3f9e9bed9 100644 --- a/pkg/models/crudable.go +++ b/pkg/models/crudable.go @@ -4,7 +4,7 @@ package models type CRUDable interface { Create(*User) error ReadOne() error - ReadAll(*User) (interface{}, error) + ReadAll(*User, int) (interface{}, error) Update() error Delete() error } diff --git a/pkg/models/list.go b/pkg/models/list.go index 17cd65711..fe9c4bf7c 100644 --- a/pkg/models/list.go +++ b/pkg/models/list.go @@ -27,8 +27,8 @@ func GetListsByNamespaceID(nID int64) (lists []*List, err error) { } // ReadAll gets all lists a user has access to -func (l *List) ReadAll(u *User) (interface{}, error) { - lists, err := getRawListsForUser(u) +func (l *List) ReadAll(u *User, page int) (interface{}, error) { + lists, err := getRawListsForUser(u, page) if err != nil { return nil, err } @@ -80,7 +80,7 @@ func (l *List) GetSimpleByID() (err error) { } // Gets the lists only, without any tasks or so -func getRawListsForUser(u *User) (lists []*List, err error) { +func getRawListsForUser(u *User, page int) (lists []*List, err error) { fullUser, err := GetUserByID(u.ID) if err != nil { return lists, err @@ -104,6 +104,7 @@ func getRawListsForUser(u *User) (lists []*List, err error) { Or("ul.user_id = ?", fullUser.ID). Or("un.user_id = ?", fullUser.ID). GroupBy("l.id"). + Limit(getLimitFromPageIndex(page)). Find(&lists) return lists, err @@ -160,14 +161,14 @@ type ListTasksDummy struct { } // ReadAll gets all tasks for a user -func (lt *ListTasksDummy) ReadAll(u *User) (interface{}, error) { - return GetTasksByUser(u) +func (lt *ListTasksDummy) ReadAll(u *User, page int) (interface{}, error) { + return GetTasksByUser(u, page) } //GetTasksByUser returns all tasks for a user -func GetTasksByUser(u *User) (tasks []*ListTask, err error) { +func GetTasksByUser(u *User, page int) (tasks []*ListTask, err error) { // Get all lists - lists, err := getRawListsForUser(u) + lists, err := getRawListsForUser(u, page) if err != nil { return nil, err } diff --git a/pkg/models/list_read_test.go b/pkg/models/list_read_test.go index 8d01efe5c..adf24e988 100644 --- a/pkg/models/list_read_test.go +++ b/pkg/models/list_read_test.go @@ -20,14 +20,14 @@ func TestList_ReadAll(t *testing.T) { assert.NoError(t, err) lists2 := List{} - lists3, err := lists2.ReadAll(&u) + lists3, err := lists2.ReadAll(&u, 1) assert.NoError(t, err) assert.Equal(t, reflect.TypeOf(lists3).Kind(), reflect.Slice) s := reflect.ValueOf(lists3) assert.Equal(t, s.Len(), 1) // Try getting lists for a nonexistant user - _, err = lists2.ReadAll(&User{ID: 984234}) + _, err = lists2.ReadAll(&User{ID: 984234}, 1) assert.Error(t, err) assert.True(t, IsErrUserDoesNotExist(err)) } diff --git a/pkg/models/list_users_readall.go b/pkg/models/list_users_readall.go index fe1a978e7..73b858403 100644 --- a/pkg/models/list_users_readall.go +++ b/pkg/models/list_users_readall.go @@ -1,7 +1,7 @@ package models // ReadAll gets all users who have access to a list -func (ul *ListUser) ReadAll(u *User) (interface{}, error) { +func (ul *ListUser) ReadAll(u *User, page int) (interface{}, error) { // Check if the user has access to the list l := &List{ID: ul.ListID} if err := l.GetSimpleByID(); err != nil { @@ -16,6 +16,7 @@ func (ul *ListUser) ReadAll(u *User) (interface{}, error) { err := x. Join("INNER", "users_list", "user_id = users.id"). Where("users_list.list_id = ?", ul.ListID). + Limit(getLimitFromPageIndex(page)). Find(&all) return all, err diff --git a/pkg/models/models.go b/pkg/models/models.go index 71c8fb5d2..196146fbc 100644 --- a/pkg/models/models.go +++ b/pkg/models/models.go @@ -90,3 +90,15 @@ func SetEngine() (err error) { return nil } + +func getLimitFromPageIndex(page int) (limit, start int) { + + // Get everything when page index is -1 + if page < 0 { + return 0, 0 + } + + limit = viper.GetInt("service.pagecount") + start = limit * (page - 1) + return +} diff --git a/pkg/models/namespace.go b/pkg/models/namespace.go index b72fd0a8a..94003f742 100644 --- a/pkg/models/namespace.go +++ b/pkg/models/namespace.go @@ -53,7 +53,7 @@ func (n *Namespace) ReadOne() (err error) { } // ReadAll gets all namespaces a user has access to -func (n *Namespace) ReadAll(doer *User) (interface{}, error) { +func (n *Namespace) ReadAll(doer *User, page int) (interface{}, error) { type namespaceWithLists struct { Namespace `xorm:"extends"` @@ -71,6 +71,7 @@ func (n *Namespace) ReadAll(doer *User) (interface{}, error) { Or("namespaces.owner_id = ?", doer.ID). Or("users_namespace.user_id = ?", doer.ID). GroupBy("namespaces.id"). + Limit(getLimitFromPageIndex(page)). Find(&all) if err != nil { diff --git a/pkg/models/namespace_test.go b/pkg/models/namespace_test.go index 140f32397..05ddd4b70 100644 --- a/pkg/models/namespace_test.go +++ b/pkg/models/namespace_test.go @@ -85,7 +85,7 @@ func TestNamespace_Create(t *testing.T) { assert.True(t, IsErrNamespaceDoesNotExist(err)) // Get all namespaces of a user - nsps, err := n.ReadAll(&doer) + nsps, err := n.ReadAll(&doer, 1) assert.NoError(t, err) assert.Equal(t, reflect.TypeOf(nsps).Kind(), reflect.Slice) s := reflect.ValueOf(nsps) diff --git a/pkg/models/namespace_users_readall.go b/pkg/models/namespace_users_readall.go index d31080e02..b2939f040 100644 --- a/pkg/models/namespace_users_readall.go +++ b/pkg/models/namespace_users_readall.go @@ -1,7 +1,7 @@ package models // ReadAll gets all users who have access to a namespace -func (un *NamespaceUser) ReadAll(u *User) (interface{}, error) { +func (un *NamespaceUser) ReadAll(u *User, page int) (interface{}, error) { // Check if the user has access to the namespace l, err := GetNamespaceByID(un.NamespaceID) if err != nil { @@ -16,6 +16,7 @@ func (un *NamespaceUser) ReadAll(u *User) (interface{}, error) { err = x. Join("INNER", "users_namespace", "user_id = users.id"). Where("users_namespace.namespace_id = ?", un.NamespaceID). + Limit(getLimitFromPageIndex(page)). Find(&all) return all, err diff --git a/pkg/models/team_list_readall.go b/pkg/models/team_list_readall.go index 09572496d..4e3a3a056 100644 --- a/pkg/models/team_list_readall.go +++ b/pkg/models/team_list_readall.go @@ -1,7 +1,7 @@ package models // ReadAll implements the method to read all teams of a list -func (tl *TeamList) ReadAll(u *User) (interface{}, error) { +func (tl *TeamList) ReadAll(u *User, page int) (interface{}, error) { // Check if the user can read the namespace l := &List{ID: tl.ListID} if err := l.GetSimpleByID(); err != nil { @@ -17,6 +17,7 @@ func (tl *TeamList) ReadAll(u *User) (interface{}, error) { Table("teams"). Join("INNER", "team_list", "team_id = teams.id"). Where("team_list.list_id = ?", tl.ListID). + Limit(getLimitFromPageIndex(page)). Find(&all) return all, err diff --git a/pkg/models/team_list_test.go b/pkg/models/team_list_test.go index 926bff325..76b7b8242 100644 --- a/pkg/models/team_list_test.go +++ b/pkg/models/team_list_test.go @@ -50,27 +50,27 @@ func TestTeamList(t *testing.T) { assert.True(t, IsErrListDoesNotExist(err)) // Test Read all - teams, err := tl.ReadAll(&u) + teams, err := tl.ReadAll(&u, 1) assert.NoError(t, err) assert.Equal(t, reflect.TypeOf(teams).Kind(), reflect.Slice) s := reflect.ValueOf(teams) assert.Equal(t, s.Len(), 1) // Test Read all for nonexistant list - _, err = tl4.ReadAll(&u) + _, err = tl4.ReadAll(&u, 1) assert.Error(t, err) assert.True(t, IsErrListDoesNotExist(err)) // Test Read all for a list where the user is owner of the namespace this list belongs to tl5 := tl tl5.ListID = 2 - _, err = tl5.ReadAll(&u) + _, err = tl5.ReadAll(&u, 1) assert.NoError(t, err) // Test read all for a list where the user not has access tl6 := tl tl6.ListID = 3 - _, err = tl6.ReadAll(&u) + _, err = tl6.ReadAll(&u, 1) assert.Error(t, err) assert.True(t, IsErrNeedToHaveListReadAccess(err)) diff --git a/pkg/models/team_namespace_readall.go b/pkg/models/team_namespace_readall.go index 8b49ae62f..cef5d5bba 100644 --- a/pkg/models/team_namespace_readall.go +++ b/pkg/models/team_namespace_readall.go @@ -1,7 +1,7 @@ package models // ReadAll implements the method to read all teams of a namespace -func (tn *TeamNamespace) ReadAll(user *User) (interface{}, error) { +func (tn *TeamNamespace) ReadAll(user *User, page int) (interface{}, error) { // Check if the user can read the namespace n, err := GetNamespaceByID(tn.NamespaceID) if err != nil { @@ -17,6 +17,7 @@ func (tn *TeamNamespace) ReadAll(user *User) (interface{}, error) { err = x.Table("teams"). Join("INNER", "team_namespaces", "team_id = teams.id"). Where("team_namespaces.namespace_id = ?", tn.NamespaceID). + Limit(getLimitFromPageIndex(page)). Find(&all) return all, err diff --git a/pkg/models/team_namespace_test.go b/pkg/models/team_namespace_test.go index ed5c4c2ac..6773eba5f 100644 --- a/pkg/models/team_namespace_test.go +++ b/pkg/models/team_namespace_test.go @@ -49,20 +49,20 @@ func TestTeamNamespace(t *testing.T) { assert.True(t, IsErrNamespaceDoesNotExist(err)) // Check readall - teams, err := tn.ReadAll(&dummyuser) + teams, err := tn.ReadAll(&dummyuser, 1) assert.NoError(t, err) assert.Equal(t, reflect.TypeOf(teams).Kind(), reflect.Slice) s := reflect.ValueOf(teams) assert.Equal(t, s.Len(), 1) // Check readall for a nonexistant namespace - _, err = tn4.ReadAll(&dummyuser) + _, err = tn4.ReadAll(&dummyuser, 1) assert.Error(t, err) assert.True(t, IsErrNamespaceDoesNotExist(err)) // Check with no right to read the namespace nouser := &User{ID: 393} - _, err = tn.ReadAll(nouser) + _, err = tn.ReadAll(nouser, 1) assert.Error(t, err) assert.True(t, IsErrNeedToHaveNamespaceReadAccess(err)) diff --git a/pkg/models/teams.go b/pkg/models/teams.go index 44f837d84..c6228f84c 100644 --- a/pkg/models/teams.go +++ b/pkg/models/teams.go @@ -84,12 +84,13 @@ func (t *Team) ReadOne() (err error) { } // ReadAll gets all teams the user is part of -func (t *Team) ReadAll(user *User) (teams interface{}, err error) { +func (t *Team) ReadAll(user *User, page int) (teams interface{}, err error) { all := []*Team{} err = x.Select("teams.*"). Table("teams"). Join("INNER", "team_members", "team_members.team_id = teams.id"). Where("team_members.user_id = ?", user.ID). + Limit(getLimitFromPageIndex(page)). Find(&all) return all, err diff --git a/pkg/models/teams_test.go b/pkg/models/teams_test.go index bd311850b..4f5cc76dd 100644 --- a/pkg/models/teams_test.go +++ b/pkg/models/teams_test.go @@ -32,7 +32,7 @@ func TestTeam_Create(t *testing.T) { assert.True(t, dummyteam.CanRead(&doer)) // Get all teams the user is part of - ts, err := tm.ReadAll(&doer) + ts, err := tm.ReadAll(&doer, 1) assert.NoError(t, err) assert.Equal(t, reflect.TypeOf(ts).Kind(), reflect.Slice) s := reflect.ValueOf(ts) diff --git a/pkg/routes/api/v1/caldav.go b/pkg/routes/api/v1/caldav.go index 5e28a904c..cff688273 100644 --- a/pkg/routes/api/v1/caldav.go +++ b/pkg/routes/api/v1/caldav.go @@ -39,7 +39,7 @@ func Caldav(c echo.Context) error { } // Get all tasks for that user - tasks, err := models.GetTasksByUser(&u) + tasks, err := models.GetTasksByUser(&u, -1) if err != nil { return crud.HandleHTTPError(err) } diff --git a/pkg/routes/crud/read_all.go b/pkg/routes/crud/read_all.go index b6e53c2c8..b47808ae1 100644 --- a/pkg/routes/crud/read_all.go +++ b/pkg/routes/crud/read_all.go @@ -1,9 +1,11 @@ package crud import ( + "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/models" "github.com/labstack/echo" "net/http" + "strconv" ) // ReadAllWeb is the webhandler to get all objects of a type @@ -21,7 +23,21 @@ func (c *WebHandler) ReadAllWeb(ctx echo.Context) error { return echo.NewHTTPError(http.StatusBadRequest, "No or invalid model provided.") } - lists, err := currentStruct.ReadAll(¤tUser) + // Pagination + page := ctx.QueryParam("page") + if page == "" { + page = "1" + } + pageNumber, err := strconv.Atoi(page) + if err != nil { + log.Log.Error(err.Error()) + return echo.NewHTTPError(http.StatusBadRequest, "Bad page requested.") + } + if pageNumber < 0 { + return echo.NewHTTPError(http.StatusBadRequest, "Bad page requested.") + } + + lists, err := currentStruct.ReadAll(¤tUser, pageNumber) if err != nil { return HandleHTTPError(err) }