From 9592a0069ed4b851cec8591038f9be5ce6d81a28 Mon Sep 17 00:00:00 2001 From: arekkas Date: Thu, 18 Jan 2018 15:11:06 +0100 Subject: [PATCH] Resolves issues with pagination --- Gopkg.lock | 12 +++++-- Gopkg.toml | 2 +- pkg/paginator_test.go | 60 ---------------------------------- policy/handler.go | 5 +-- warden/group/handler.go | 8 ++--- warden/group/manager.go | 4 +-- warden/group/manager_memory.go | 22 +++++-------- warden/group/manager_sql.go | 4 +-- 8 files changed, 29 insertions(+), 88 deletions(-) delete mode 100644 pkg/paginator_test.go diff --git a/Gopkg.lock b/Gopkg.lock index 27b7e9142b9..f646850fe32 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -249,8 +249,14 @@ [[projects]] name = "github.com/ory/ladon" packages = [".","compiler","manager/memory","manager/sql"] - revision = "941d70faef8cc9656a9f0d6403a108a08d2ab631" - version = "v0.8.6" + revision = "73217e44ed7e4a6bbd44ae7dd858d11932f08cf8" + version = "v0.8.8" + +[[projects]] + name = "github.com/ory/pagination" + packages = ["."] + revision = "abd7ec33a01fdec119267449c8f3bad187f881f6" + version = "v0.0.1" [[projects]] name = "github.com/pborman/uuid" @@ -435,6 +441,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "74f885ed4e8bfad02ad2497ef672db4b7bfc39a820677d4caca1bd1eb3f15c8b" + inputs-digest = "3ac73b4a2792495a72e7cb50575db622556e1e100b19956fd7a3ca6e3f7e9def" solver-name = "gps-cdcl" solver-version = 1 diff --git a/Gopkg.toml b/Gopkg.toml index 36fa57d215b..b691ff42aec 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -87,7 +87,7 @@ [[constraint]] name = "github.com/ory/ladon" - version = "0.8.4" + version = "0.8.8" [[constraint]] name = "github.com/pborman/uuid" diff --git a/pkg/paginator_test.go b/pkg/paginator_test.go deleted file mode 100644 index 4e8d3f0dbe2..00000000000 --- a/pkg/paginator_test.go +++ /dev/null @@ -1,60 +0,0 @@ -package pkg - -import ( - "net/http" - "net/url" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestParsePagination(t *testing.T) { - t.Run("case=normal", func(t *testing.T) { - u, _ := url.Parse("http://localhost/foo?limit=10&offset=10") - limit, offset := ParsePagination(&http.Request{URL: u}, 0, 0, 10) - assert.EqualValues(t, limit, 10) - assert.EqualValues(t, offset, 10) - }) - - t.Run("case=defaults", func(t *testing.T) { - u, _ := url.Parse("http://localhost/foo") - limit, offset := ParsePagination(&http.Request{URL: u}, 5, 5, 10) - assert.EqualValues(t, limit, 5) - assert.EqualValues(t, offset, 5) - }) - - t.Run("case=defaults_and_limits", func(t *testing.T) { - u, _ := url.Parse("http://localhost/foo") - limit, offset := ParsePagination(&http.Request{URL: u}, 5, 5, 2) - assert.EqualValues(t, limit, 2) - assert.EqualValues(t, offset, 5) - }) - - t.Run("case=limits", func(t *testing.T) { - u, _ := url.Parse("http://localhost/foo?limit=10&offset=10") - limit, offset := ParsePagination(&http.Request{URL: u}, 0, 0, 5) - assert.EqualValues(t, limit, 5) - assert.EqualValues(t, offset, 10) - }) - - t.Run("case=negatives", func(t *testing.T) { - u, _ := url.Parse("http://localhost/foo?limit=-1&offset=-1") - limit, offset := ParsePagination(&http.Request{URL: u}, 0, 0, 5) - assert.EqualValues(t, limit, 0) - assert.EqualValues(t, offset, 0) - }) - - t.Run("case=default_negatives", func(t *testing.T) { - u, _ := url.Parse("http://localhost/foo") - limit, offset := ParsePagination(&http.Request{URL: u}, -1, -1, 5) - assert.EqualValues(t, limit, 0) - assert.EqualValues(t, offset, 0) - }) - - t.Run("case=invalid_defaults", func(t *testing.T) { - u, _ := url.Parse("http://localhost/foo?offset=a&limit=b") - limit, offset := ParsePagination(&http.Request{URL: u}, 10, 10, 15) - assert.EqualValues(t, limit, 10) - assert.EqualValues(t, offset, 10) - }) -} diff --git a/policy/handler.go b/policy/handler.go index a14114dd8d5..ea8c35bac95 100644 --- a/policy/handler.go +++ b/policy/handler.go @@ -24,6 +24,7 @@ import ( "github.com/ory/hydra/firewall" "github.com/ory/hydra/pkg" "github.com/ory/ladon" + "github.com/ory/pagination" "github.com/pborman/uuid" "github.com/pkg/errors" ) @@ -102,8 +103,8 @@ func (h *Handler) List(w http.ResponseWriter, r *http.Request, _ httprouter.Para return } - limit, offset := pkg.ParsePagination(r, 500, 0, 1000) - policies, err := h.Manager.GetAll(limit, offset) + limit, offset := pagination.Parse(r, 500, 0, 1000) + policies, err := h.Manager.GetAll(int64(limit), int64(offset)) if err != nil { h.H.WriteError(w, r, errors.WithStack(err)) return diff --git a/warden/group/handler.go b/warden/group/handler.go index 95386d36c89..d11ff6b329e 100644 --- a/warden/group/handler.go +++ b/warden/group/handler.go @@ -22,7 +22,7 @@ import ( "github.com/julienschmidt/httprouter" "github.com/ory/herodot" "github.com/ory/hydra/firewall" - "github.com/ory/hydra/pkg" + "github.com/ory/pagination" "github.com/pkg/errors" ) @@ -111,7 +111,7 @@ func (h *Handler) ListGroupsHandler(w http.ResponseWriter, r *http.Request, _ ht return } - limit, offset := pkg.ParsePagination(r, 100, 0, 500) + limit, offset := pagination.Parse(r, 100, 0, 500) if member := r.URL.Query().Get("member"); member != "" { h.FindGroupNames(w, r, member, limit, offset) return @@ -121,7 +121,7 @@ func (h *Handler) ListGroupsHandler(w http.ResponseWriter, r *http.Request, _ ht } } -func (h *Handler) ListGroups(w http.ResponseWriter, r *http.Request, limit, offset int64) { +func (h *Handler) ListGroups(w http.ResponseWriter, r *http.Request, limit, offset int) { groups, err := h.Manager.ListGroups(limit, offset) if err != nil { h.H.WriteError(w, r, err) @@ -131,7 +131,7 @@ func (h *Handler) ListGroups(w http.ResponseWriter, r *http.Request, limit, offs h.H.Write(w, r, groups) } -func (h *Handler) FindGroupNames(w http.ResponseWriter, r *http.Request, member string, limit, offset int64) { +func (h *Handler) FindGroupNames(w http.ResponseWriter, r *http.Request, member string, limit, offset int) { groups, err := h.Manager.FindGroupsByMember(member, limit, offset) if err != nil { h.H.WriteError(w, r, err) diff --git a/warden/group/manager.go b/warden/group/manager.go index 7d046615a70..039854c3143 100644 --- a/warden/group/manager.go +++ b/warden/group/manager.go @@ -33,6 +33,6 @@ type Manager interface { AddGroupMembers(group string, members []string) error RemoveGroupMembers(group string, members []string) error - FindGroupsByMember(subject string, limit, offset int64) ([]Group, error) - ListGroups(limit, offset int64) ([]Group, error) + FindGroupsByMember(subject string, limit, offset int) ([]Group, error) + ListGroups(limit, offset int) ([]Group, error) } diff --git a/warden/group/manager_memory.go b/warden/group/manager_memory.go index 8761e02edf1..64871928ff8 100644 --- a/warden/group/manager_memory.go +++ b/warden/group/manager_memory.go @@ -18,6 +18,7 @@ import ( "sync" "github.com/ory/hydra/pkg" + "github.com/ory/pagination" "github.com/pborman/uuid" "github.com/pkg/errors" ) @@ -91,7 +92,7 @@ func (m *MemoryManager) RemoveGroupMembers(group string, subjects []string) erro return m.CreateGroup(g) } -func (m *MemoryManager) FindGroupsByMember(subject string, limit, offset int64) ([]Group, error) { +func (m *MemoryManager) FindGroupsByMember(subject string, limit, offset int) ([]Group, error) { if m.Groups == nil { m.Groups = map[string]Group{} } @@ -106,29 +107,22 @@ func (m *MemoryManager) FindGroupsByMember(subject string, limit, offset int64) } } - if offset+limit > int64(len(res)) { - return []Group{}, nil - } - - return res[offset:limit], nil + start, end := pagination.Index(limit, offset, len(res)) + return res[start:end], nil } -func (m *MemoryManager) ListGroups(limit, offset int64) ([]Group, error) { +func (m *MemoryManager) ListGroups(limit, offset int) ([]Group, error) { if m.Groups == nil { m.Groups = map[string]Group{} } - if offset+limit > int64(len(m.Groups)) { - return []Group{}, nil - } - - i := int64(0) + i := 0 res := make([]Group, len(m.Groups)) for _, g := range m.Groups { res[i] = g i++ } - - return res[offset:limit], nil + start, end := pagination.Index(limit, offset, len(res)) + return res[start:end], nil } diff --git a/warden/group/manager_sql.go b/warden/group/manager_sql.go index 71d4f55e088..871218bcdd4 100644 --- a/warden/group/manager_sql.go +++ b/warden/group/manager_sql.go @@ -142,7 +142,7 @@ func (m *SQLManager) RemoveGroupMembers(group string, subjects []string) error { return nil } -func (m *SQLManager) FindGroupsByMember(subject string, limit, offset int64) ([]Group, error) { +func (m *SQLManager) FindGroupsByMember(subject string, limit, offset int) ([]Group, error) { var ids []string if err := m.DB.Select(&ids, m.DB.Rebind("SELECT group_id from hydra_warden_group_member WHERE member = ? GROUP BY group_id ORDER BY group_id LIMIT ? OFFSET ?"), subject, limit, offset); err == sql.ErrNoRows { return nil, errors.WithStack(pkg.ErrNotFound) @@ -163,7 +163,7 @@ func (m *SQLManager) FindGroupsByMember(subject string, limit, offset int64) ([] return groups, nil } -func (m *SQLManager) ListGroups(limit, offset int64) ([]Group, error) { +func (m *SQLManager) ListGroups(limit, offset int) ([]Group, error) { var ids []string if err := m.DB.Select(&ids, m.DB.Rebind("SELECT group_id from hydra_warden_group_member GROUP BY group_id ORDER BY group_id LIMIT ? OFFSET ?"), limit, offset); err == sql.ErrNoRows { return nil, errors.WithStack(pkg.ErrNotFound)