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

fix: use keyset pagination list identities #3565

Merged
merged 3 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
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"
}
}
5 changes: 2 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,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
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 +93,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 +261,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 h1:a3hyQZIwokuRCeoPzMxbewY/y6C6r1NgX4Jn3csVZv0=
github.com/ory/x v0.0.591/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
82 changes: 34 additions & 48 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 @@ -1490,53 +1486,46 @@ func TestHandler(t *testing.T) {
perPage := perPage
t.Run(fmt.Sprintf("perPage=%d", perPage), func(t *testing.T) {
t.Parallel()
body, res := getFull(t, ts, fmt.Sprintf("/identities?per_page=%d", perPage), http.StatusOK)
body, _ := 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"))
})
}

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, res := run(t, path, knownIDs)
assert.NotContains(t, res.Header, "X-Total-Count", "not supported in token pagination")
if next == nil {
break
}
assert.NotContains(t, next.Query(), "page")
assert.NotContains(t, next.Query(), "per_page")
path = next.Path + "?" + next.Query().Encode()
}

assert.Len(t, knownIDs, count)
Expand All @@ -1545,19 +1534,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)
assert.Equal(t, strconv.Itoa(count), res.Header.Get("X-Total-Count"))
if next == nil {
break
}
path = next.Path + "?" + next.Query().Encode()
}

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())
alnr marked this conversation as resolved.
Show resolved Hide resolved
}

// 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
15 changes: 9 additions & 6 deletions identity/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,9 @@ func (m *Manager) UpdateTraits(ctx context.Context, id uuid.UUID, traits Traits,
}

func (m *Manager) ValidateIdentity(ctx context.Context, i *Identity, o *ManagerOptions) (err error) {
ctx, span := m.r.Tracer(ctx).Tracer().Start(ctx, "identity.Manager.validate")
defer otelx.End(span, &err)
// This trace is more noisy than it's worth in diagnostic power.
// ctx, span := m.r.Tracer(ctx).Tracer().Start(ctx, "identity.Manager.validate")
// defer otelx.End(span, &err)

if err := m.r.IdentityValidator().Validate(ctx, i); err != nil {
if _, ok := errorsx.Cause(err).(*jsonschema.ValidationError); ok && !o.ExposeValidationErrors {
Expand All @@ -429,8 +430,9 @@ func (m *Manager) ValidateIdentity(ctx context.Context, i *Identity, o *ManagerO
}

func (m *Manager) CountActiveFirstFactorCredentials(ctx context.Context, i *Identity) (count int, err error) {
ctx, span := m.r.Tracer(ctx).Tracer().Start(ctx, "identity.Manager.CountActiveFirstFactorCredentials")
defer otelx.End(span, &err)
// This trace is more noisy than it's worth in diagnostic power.
// ctx, span := m.r.Tracer(ctx).Tracer().Start(ctx, "identity.Manager.CountActiveFirstFactorCredentials")
// defer otelx.End(span, &err)

for _, strategy := range m.r.ActiveCredentialsCounterStrategies(ctx) {
current, err := strategy.CountActiveFirstFactorCredentials(i.Credentials)
Expand All @@ -444,8 +446,9 @@ func (m *Manager) CountActiveFirstFactorCredentials(ctx context.Context, i *Iden
}

func (m *Manager) CountActiveMultiFactorCredentials(ctx context.Context, i *Identity) (count int, err error) {
ctx, span := m.r.Tracer(ctx).Tracer().Start(ctx, "identity.Manager.CountActiveMultiFactorCredentials")
defer otelx.End(span, &err)
// This trace is more noisy than it's worth in diagnostic power.
// ctx, span := m.r.Tracer(ctx).Tracer().Start(ctx, "identity.Manager.CountActiveMultiFactorCredentials")
// defer otelx.End(span, &err)

for _, strategy := range m.r.ActiveCredentialsCounterStrategies(ctx) {
current, err := strategy.CountActiveMultiFactorCredentials(i.Credentials)
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
Loading