fix: improve identity list performance

Closes #1412 Closes ory-corp/cloud#975
ory · Jun 15, 2021 · 14762d2 · 14762d2
1 parent d3c526b
commit 14762d2
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 8 deletions.
diff --git a/identity/identity.go b/identity/identity.go
@@ -259,3 +259,30 @@ func (i IdentityWithCredentialsMetadataInJSON) MarshalJSON() ([]byte, error) {
 	}
 	return result, nil
 }
+
+func (i *Identity) ValidateNID() error {
+	expected := i.NID
+	if expected == uuid.Nil {
+		return errors.WithStack(herodot.ErrInternalServerError.WithReason("Received empty nid."))
+	}
+
+	for _, r := range i.RecoveryAddresses {
+		if r.NID != expected {
+			return errors.WithStack(herodot.ErrInternalServerError.WithReason("Mismatching nid for recovery addresses."))
+		}
+	}
+
+	for _, r := range i.VerifiableAddresses {
+		if r.NID != expected {
+			return errors.WithStack(herodot.ErrInternalServerError.WithReason("Mismatching nid for verifiable addresses."))
+		}
+	}
+
+	for _, r := range i.Credentials {
+		if r.NID != expected {
+			return errors.WithStack(herodot.ErrInternalServerError.WithReason("Mismatching nid for credentials."))
+		}
+	}
+
+	return nil
+}
diff --git a/identity/identity_recovery.go b/identity/identity_recovery.go
@@ -53,6 +53,10 @@ func (a RecoveryAddress) TableName(ctx context.Context) string {
 	return corp.ContextualizeTableName(ctx, "identity_recovery_addresses")
 }
 
+func (a RecoveryAddress) ValidateNID() error {
+	return nil
+}
+
 func NewRecoveryEmailAddress(
 	value string,
 	identity uuid.UUID,

diff --git a/identity/identity_test.go b/identity/identity_test.go
@@ -3,8 +3,11 @@ package identity
 import (
 	"bytes"
 	"encoding/json"
+	"fmt"
 	"testing"
 
+	"github.com/ory/kratos/x"
+
 	"github.com/stretchr/testify/require"
 	"github.com/tidwall/gjson"
 
@@ -99,3 +102,30 @@ func TestMarshalIdentityWithCredentials(t *testing.T) {
 	assert.JSONEq(t, "{\"password\":{\"type\":\"password\",\"identifiers\":null,\"updated_at\":\"0001-01-01T00:00:00Z\",\"created_at\":\"0001-01-01T00:00:00Z\"}}", credentialsInJson.Raw)
 	assert.Equal(t, credentials, i.Credentials, "Original credentials should not be touched by marshalling")
 }
+
+func TestValidateNID(t *testing.T) {
+	nid := x.NewUUID()
+	for k, tc := range []struct {
+		i           *Identity
+		expectedErr bool
+	}{
+		{i: &Identity{NID: nid}},
+		{i: &Identity{NID: nid, RecoveryAddresses: []RecoveryAddress{{NID: x.NewUUID()}}}, expectedErr: true},
+		{i: &Identity{NID: nid, VerifiableAddresses: []VerifiableAddress{{NID: x.NewUUID()}}}, expectedErr: true},
+		{i: &Identity{NID: nid, Credentials: map[CredentialsType]Credentials{CredentialsTypePassword: {NID: x.NewUUID()}}}, expectedErr: true},
+		{i: &Identity{NID: nid, RecoveryAddresses: []RecoveryAddress{{NID: nid}}, VerifiableAddresses: []VerifiableAddress{{NID: x.NewUUID()}}}, expectedErr: true},
+		{i: &Identity{NID: nid, RecoveryAddresses: []RecoveryAddress{{NID: x.NewUUID()}}, VerifiableAddresses: []VerifiableAddress{{NID: nid}}}, expectedErr: true},
+		{i: &Identity{NID: nid, RecoveryAddresses: []RecoveryAddress{{NID: nid}}, VerifiableAddresses: []VerifiableAddress{{NID: nid}}}, expectedErr: false},
+		{i: &Identity{NID: nid, Credentials: map[CredentialsType]Credentials{CredentialsTypePassword: {NID: x.NewUUID()}}, RecoveryAddresses: []RecoveryAddress{{NID: nid}}, VerifiableAddresses: []VerifiableAddress{{NID: nid}}}, expectedErr: true},
+		{i: &Identity{NID: nid, Credentials: map[CredentialsType]Credentials{CredentialsTypePassword: {NID: nid}}, RecoveryAddresses: []RecoveryAddress{{NID: nid}}, VerifiableAddresses: []VerifiableAddress{{NID: nid}}}},
+	} {
+		t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
+			err := tc.i.ValidateNID()
+			if tc.expectedErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}
diff --git a/identity/identity_verification.go b/identity/identity_verification.go
@@ -115,3 +115,7 @@ func (a VerifiableAddress) GetID() uuid.UUID {
 func (a VerifiableAddress) GetNID() uuid.UUID {
 	return a.NID
 }
+
+func (a VerifiableAddress) ValidateNID() error {
+	return nil
+}
diff --git a/identity/test/pool.go b/identity/test/pool.go
@@ -3,6 +3,7 @@ package test
 import (
 	"context"
 	"fmt"
+	"github.com/ory/x/assertx"
 	"strconv"
 	"strings"
 	"testing"
@@ -397,10 +398,13 @@ func TestPool(ctx context.Context, conf *config.Config, p interface {
 				var found bool
 				for _, i := range is {
 					if i.ID == id {
+						expected, err := p.GetIdentity(ctx, id)
+						require.NoError(t, err)
+						assertx.EqualAsJSON(t, expected, i)
 						found = true
 					}
 				}
-				assert.True(t, found, id)
+				require.True(t, found, id)
 			}
 
 			t.Run("no results on other network", func(t *testing.T) {

diff --git a/persistence/sql/persister_identity.go b/persistence/sql/persister_identity.go
@@ -252,23 +252,27 @@ func (p *Persister) ListIdentities(ctx context.Context, page, perPage int) ([]id
 
 	/* #nosec G201 TableName is static */
 	if err := sqlcon.HandleError(p.GetConnection(ctx).Where("nid = ?", corp.ContextualizeNID(ctx, p.nid)).
+		Eager("VerifiableAddresses", "RecoveryAddresses").
 		Paginate(page, perPage).Order("id DESC").
 		All(&is)); err != nil {
 		return nil, err
 	}
 
+	schemaCache := map[string]string{}
+
 	for k := range is {
 		i := &is[k]
-		if err := p.findVerifiableAddresses(ctx, i); err != nil {
+		if err := i.ValidateNID(); err != nil {
 			return nil, sqlcon.HandleError(err)
 		}
 
-		if err := p.findRecoveryAddresses(ctx, i); err != nil {
-			return nil, sqlcon.HandleError(err)
-		}
-
-		if err := p.injectTraitsSchemaURL(ctx, i); err != nil {
-			return nil, err
+		if u, ok := schemaCache[i.SchemaID]; ok {
+			i.SchemaURL = u
+		} else {
+			if err := p.injectTraitsSchemaURL(ctx, i); err != nil {
+				return nil, err
+			}
+			schemaCache[i.SchemaID] = i.SchemaURL
 		}
 
 		is[k] = *i