Skip to content

Commit

Permalink
fix: change ListIdentities to keyset pagination
Browse files Browse the repository at this point in the history
  • Loading branch information
alnr committed Oct 9, 2023
1 parent 3efd0c2 commit fe52c87
Show file tree
Hide file tree
Showing 12 changed files with 212 additions and 144 deletions.
6 changes: 6 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"gopls": {
"formatting.gofumpt": true,
"formatting.local": "github.com/ory"
}
}
7 changes: 4 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ module github.com/ory/kratos

go 1.19

// replace github.com/ory/x => ../x

replace (
github.com/bradleyjkemp/cupaloy/v2 => github.com/aeneasr/cupaloy/v2 v2.6.1-0.20210924214125-3dfdd01210a3

Expand Down Expand Up @@ -77,7 +79,8 @@ require (
github.com/ory/jsonschema/v3 v3.0.8
github.com/ory/mail/v3 v3.0.0
github.com/ory/nosurf v1.2.7
github.com/ory/x v0.0.590
github.com/ory/x v0.0.591-0.20231009141749-f6c175b9bd1a
github.com/peterhellberg/link v1.2.0
github.com/phayes/freeport v0.0.0-20180830031419-95f893ade6f2
github.com/pkg/errors v0.9.1
github.com/pquerna/otp v1.4.0
Expand All @@ -92,7 +95,6 @@ require (
github.com/stretchr/testify v1.8.4
github.com/tidwall/gjson v1.14.3
github.com/tidwall/sjson v1.2.5
github.com/tomnomnom/linkheader v0.0.0-20180905144013-02ca5825eb80
github.com/urfave/negroni v1.0.0
github.com/zmb3/spotify/v2 v2.0.0
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.36.4
Expand Down Expand Up @@ -261,7 +263,6 @@ require (
github.com/openzipkin/zipkin-go v0.4.1 // indirect
github.com/pelletier/go-toml v1.9.5 // indirect
github.com/pelletier/go-toml/v2 v2.0.7 // indirect
github.com/peterhellberg/link v1.2.0 // indirect
github.com/philhofer/fwd v1.1.2 // indirect
github.com/pkg/profile v1.7.0 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
Expand Down
6 changes: 2 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -847,8 +847,8 @@ github.com/ory/nosurf v1.2.7 h1:YrHrbSensQyU6r6HT/V5+HPdVEgrOTMJiLoJABSBOp4=
github.com/ory/nosurf v1.2.7/go.mod h1:d4L3ZBa7Amv55bqxCBtCs63wSlyaiCkWVl4vKf3OUxA=
github.com/ory/sessions v1.2.2-0.20220110165800-b09c17334dc2 h1:zm6sDvHy/U9XrGpixwHiuAwpp0Ock6khSVHkrv6lQQU=
github.com/ory/sessions v1.2.2-0.20220110165800-b09c17334dc2/go.mod h1:dk2InVEVJ0sfLlnXv9EAgkf6ecYs/i80K/zI+bUmuGM=
github.com/ory/x v0.0.590 h1:t0+XlSlDw5pzZhdAxOB8uFp1Dp+MStPRTG8Nn/fm1PE=
github.com/ory/x v0.0.590/go.mod h1:ksLBEd6iW6czGpE6eNA0gCIxO1FFeqIxCZgsgwNrzMM=
github.com/ory/x v0.0.591-0.20231009141749-f6c175b9bd1a h1:Cts5bFV13FeL9guuolHhdq/ARwJCiqA8TD38Mi51mgU=
github.com/ory/x v0.0.591-0.20231009141749-f6c175b9bd1a/go.mod h1:ksLBEd6iW6czGpE6eNA0gCIxO1FFeqIxCZgsgwNrzMM=
github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
github.com/pascaldekloe/goe v0.1.0 h1:cBOtyMzM9HTpWjXfbbunk26uA6nG3a8n06Wieeh0MwY=
github.com/pascaldekloe/goe v0.1.0/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
Expand Down Expand Up @@ -1032,8 +1032,6 @@ github.com/timtadh/lexmachine v0.2.2 h1:g55RnjdYazm5wnKv59pwFcBJHOyvTPfDEoz21s4P
github.com/timtadh/lexmachine v0.2.2/go.mod h1:GBJvD5OAfRn/gnp92zb9KTgHLB7akKyxmVivoYCcjQI=
github.com/tinylib/msgp v1.1.8 h1:FCXC1xanKO4I8plpHGH2P7koL/RzZs12l/+r7vakfm0=
github.com/tinylib/msgp v1.1.8/go.mod h1:qkpG+2ldGg4xRFmx+jfTvZPxfGFhi64BcnL9vkCm/Tw=
github.com/tomnomnom/linkheader v0.0.0-20180905144013-02ca5825eb80 h1:nrZ3ySNYwJbSpD6ce9duiP+QkD3JuLCcWkdaehUS/3Y=
github.com/tomnomnom/linkheader v0.0.0-20180905144013-02ca5825eb80/go.mod h1:iFyPdL66DjUD96XmzVL3ZntbzcflLnznH0fr99w5VqE=
github.com/toqueteos/webbrowser v1.2.0 h1:tVP/gpK69Fx+qMJKsLE7TD8LuGWPnEV71wBN9rrstGQ=
github.com/toqueteos/webbrowser v1.2.0/go.mod h1:XWoZq4cyp9WeUeak7w7LXRUQf1F1ATJMir8RTqb4ayM=
github.com/tv42/httpunix v0.0.0-20150427012821-b75d8614f926/go.mod h1:9ESjWnEqriFuLhtthL60Sar/7RFoluCcXsuvEwTV5KM=
Expand Down
48 changes: 31 additions & 17 deletions identity/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import (
"net/http"
"time"

"github.com/ory/x/pagination/keysetpagination"
"github.com/ory/x/pagination/migrationpagination"
"github.com/ory/x/pagination/pagepagination"
"github.com/ory/x/sqlcon"

"github.com/ory/kratos/hash"
Expand Down Expand Up @@ -169,32 +171,45 @@ type listIdentitiesParameters struct {
// 200: listIdentities
// default: errorGeneric
func (h *Handler) list(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
page, itemsPerPage := x.ParsePagination(r)

params := ListIdentityParameters{
Expand: ExpandDefault,
Page: page,
PerPage: itemsPerPage,
CredentialsIdentifier: r.URL.Query().Get("credentials_identifier"),
CredentialsIdentifierSimilar: r.URL.Query().Get("preview_credentials_identifier_similar"),
var (
err error
params = ListIdentityParameters{
Expand: ExpandDefault,
CredentialsIdentifier: r.URL.Query().Get("credentials_identifier"),
CredentialsIdentifierSimilar: r.URL.Query().Get("preview_credentials_identifier_similar"),
}
)
if params.CredentialsIdentifier != "" && params.CredentialsIdentifierSimilar != "" {
h.r.Writer().WriteError(w, r, herodot.ErrBadRequest.WithReason("Cannot pass both credentials_identifier and preview_credentials_identifier_similar."))
return
}
if params.CredentialsIdentifier != "" {
if params.CredentialsIdentifier != "" || params.CredentialsIdentifierSimilar != "" {
params.Expand = ExpandEverything
}
params.KeySetPagination, params.PagePagination, err = x.ParseKeysetOrPagePagination(r)
if err != nil {
h.r.Writer().WriteError(w, r, err)
return
}

is, err := h.r.IdentityPool().ListIdentities(r.Context(), params)
is, nextPage, err := h.r.IdentityPool().ListIdentities(r.Context(), params)
if err != nil {
h.r.Writer().WriteError(w, r, err)
return
}

total := int64(len(is))
if params.CredentialsIdentifier == "" {
total, err = h.r.IdentityPool().CountIdentities(r.Context())
if err != nil {
h.r.Writer().WriteError(w, r, err)
return
if params.PagePagination != nil {
total := int64(len(is))
if params.CredentialsIdentifier == "" {
total, err = h.r.IdentityPool().CountIdentities(r.Context())
if err != nil {
h.r.Writer().WriteError(w, r, err)
return
}
}
pagepagination.PaginationHeader(w, urlx.AppendPaths(h.r.Config().SelfAdminURL(r.Context()), RouteCollection), total, params.PagePagination.Page, params.PagePagination.ItemsPerPage)
} else {
keysetpagination.Header(w, urlx.AppendPaths(h.r.Config().SelfAdminURL(r.Context()), RouteCollection), nextPage)
}

// Identities using the marshaler for including metadata_admin
Expand All @@ -203,7 +218,6 @@ func (h *Handler) list(w http.ResponseWriter, r *http.Request, _ httprouter.Para
isam[i] = WithCredentialsMetadataAndAdminMetadataInJSON(identity)
}

migrationpagination.PaginationHeader(w, urlx.AppendPaths(h.r.Config().SelfAdminURL(r.Context()), RouteCollection), total, page, itemsPerPage)
h.r.Writer().Write(w, r, isam)
}

Expand Down
78 changes: 31 additions & 47 deletions identity/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,11 @@ import (

"github.com/bxcodec/faker/v3"
"github.com/gofrs/uuid"
"github.com/peterhellberg/link"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/tidwall/gjson"

"github.com/tomnomnom/linkheader"

"github.com/ory/kratos/driver/config"
"github.com/ory/kratos/hash"
"github.com/ory/kratos/identity"
Expand Down Expand Up @@ -683,7 +682,6 @@ func TestHandler(t *testing.T) {
req := &identity.BatchPatchIdentitiesBody{Identities: validPatches}
send(t, adminTS, "PATCH", "/identities", http.StatusOK, req)
})

})

t.Run("case=ignores create nil bodies", func(t *testing.T) {
Expand Down Expand Up @@ -787,7 +785,6 @@ func TestHandler(t *testing.T) {

for name, ts := range map[string]*httptest.Server{"public": publicTS, "admin": adminTS} {
t.Run("endpoint="+name, func(t *testing.T) {

email := "UPPER" + x.NewUUID().String() + "@ory.sh"
lowercaseEmail := strings.ToLower(email)
var cr identity.CreateIdentityBody
Expand Down Expand Up @@ -820,7 +817,6 @@ func TestHandler(t *testing.T) {
assert.EqualValues(t, identity.StateActive, res.Get("state").String(), "%s", res.Raw)
})
}

})

t.Run("case=PATCH update should not persist if schema id is invalid", func(t *testing.T) {
Expand Down Expand Up @@ -1492,51 +1488,42 @@ func TestHandler(t *testing.T) {
t.Parallel()
body, res := getFull(t, ts, fmt.Sprintf("/identities?per_page=%d", perPage), http.StatusOK)
assert.Len(t, body.Array(), perPage)
assert.Equal(t, strconv.Itoa(count), res.Header.Get("X-Total-Count"))
_ = res
})
}

t.Run("iterate over next page", func(t *testing.T) {
perPage := 10
pagePath := fmt.Sprintf("/identities?per_page=%d", perPage)

run := func(t *testing.T, path string, knownIDs map[string]struct{}) (isLast bool, parsed *url.URL) {
var err error
run := func(t *testing.T, path string, knownIDs map[string]struct{}) (next *url.URL, res *http.Response) {
t.Logf("Requesting %s", path)
body, res := getFull(t, ts, path, http.StatusOK)
for _, link := range linkheader.Parse(res.Header.Get("Link")) {
if link.Rel != "next" {
isLast = true
continue
}
parsed, err = url.Parse(link.URL)
require.NoError(t, err)
isLast = false
break
}

for _, i := range body.Array() {
assert.NotContains(t, knownIDs, i.Get("id").String())
knownIDs[i.Get("id").String()] = struct{}{}
id := i.Get("id").String()
_, seen := knownIDs[id]
require.Falsef(t, seen, "ID %s was previously returned from the API", id)
knownIDs[id] = struct{}{}
}
links := link.ParseResponse(res)
if link, ok := links["next"]; ok {
next, err := url.Parse(link.URI)
require.NoError(t, err)
return next, res
}
return isLast, parsed
return nil, res
}

t.Run("using token pagination", func(t *testing.T) {
knownIDs := make(map[string]struct{})
var isLast bool
var pages int
path := pagePath
for !isLast {
t.Run(fmt.Sprintf("page=%d", pages), func(t *testing.T) {
var res *url.URL
pages++
isLast, res = run(t, path, knownIDs)
if isLast {
return
}
path = fmt.Sprintf("/identities?page_size=%s&page_token=%s", res.Query().Get("page_size"), res.Query().Get("page_token"))
})
path := fmt.Sprintf("/identities?page_size=%d", perPage)
for {
pages++
next, _ := run(t, path, knownIDs)
if next == nil {
break
}
path = next.Path + "?" + next.Query().Encode()
}

assert.Len(t, knownIDs, count)
Expand All @@ -1545,19 +1532,16 @@ func TestHandler(t *testing.T) {

t.Run("using page pagination", func(t *testing.T) {
knownIDs := make(map[string]struct{})
var isLast bool
var pages int
path := pagePath
for !isLast {
t.Run(fmt.Sprintf("page=%d", pages), func(t *testing.T) {
var res *url.URL
pages++
isLast, res = run(t, path, knownIDs)
if isLast {
return
}
path = fmt.Sprintf("/identities?per_page=%s&page=%s", res.Query().Get("per_page"), res.Query().Get("page"))
})
path := fmt.Sprintf("/identities?page=0&per_page=%d", perPage)
for {
pages++
next, res := run(t, path, knownIDs)
if next == nil {
break
}
path = next.Path + "?" + next.Query().Encode()
assert.Equal(t, strconv.Itoa(count), res.Header.Get("X-Total-Count"))
}

assert.Len(t, knownIDs, count)
Expand Down
9 changes: 9 additions & 0 deletions identity/identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/ory/kratos/cipher"

"github.com/ory/herodot"
"github.com/ory/x/pagination/keysetpagination"
"github.com/ory/x/sqlxx"

"github.com/ory/kratos/driver/config"
Expand Down Expand Up @@ -132,6 +133,14 @@ type Identity struct {
OrganizationID uuid.NullUUID `json:"organization_id,omitempty" faker:"-" db:"organization_id"`
}

func (i *Identity) PageToken() keysetpagination.PageToken {
return keysetpagination.StringPageToken(i.ID.String())
}

func DefaultPageToken() keysetpagination.PageToken {
return keysetpagination.StringPageToken(uuid.Nil.String())
}

// Traits represent an identity's traits. The identity is able to create, modify, and delete traits
// in a self-service manner. The input will always be validated against the JSON Schema defined
// in `schema_url`.
Expand Down
9 changes: 6 additions & 3 deletions identity/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ package identity
import (
"context"

"github.com/ory/kratos/x"
"github.com/ory/x/pagination/keysetpagination"
"github.com/ory/x/sqlxx"

"github.com/gofrs/uuid"
Expand All @@ -16,13 +18,14 @@ type (
Expand Expandables
CredentialsIdentifier string
CredentialsIdentifierSimilar string
Page int
PerPage int
KeySetPagination []keysetpagination.Option
// DEPRECATED
PagePagination *x.Page
}

Pool interface {
// ListIdentities lists all identities in the store given the page and itemsPerPage.
ListIdentities(ctx context.Context, params ListIdentityParameters) ([]Identity, error)
ListIdentities(ctx context.Context, params ListIdentityParameters) ([]Identity, *keysetpagination.Paginator, error)

// CountIdentities counts the number of identities in the store.
CountIdentities(ctx context.Context) (int64, error)
Expand Down
Loading

0 comments on commit fe52c87

Please sign in to comment.