feat (cache): impl. soft-delete of users in cache

Previously, when all the auth tokens for a user
where deleted, we deleted the user along with all
the cached resources for that user.  This can
create a lot of churn in the cache, especially if
the auth tokens have a short TTL (like 8 hrs), which
can cause all the resources to be reloaded every
morning when the user logs in.

This change introduces a soft-delete of users in
the cache.  When all the auth tokens for a user are
deleted, but there's still a valid refresh token
that's less than 20 days old, we mark the user as
deleted in the cache and do not return the user in
the list of users returned by the cache, but we
still keep the user in the cache (all all their
cached resources). This way, when the user logs in
again, we can just mark the user as active again
and not have to reload all the resources.  If
the refresh token is older than 20 days, we delete
the user from the cache along with all their cached
resources.

internal/clientcache/internal/cache/refresh_test.go

@@ -104,7 +104,7 @@ func testResolvableAliasStaticResourceRetrievalFunc(inFunc func(ctx context.Cont
 
 // testNoRefreshRetrievalFunc simulates a controller that doesn't support refresh
 // since it does not return any refresh token.
-func testNoRefreshRetrievalFunc[T any](t *testing.T) func(context.Context, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
+func testNoRefreshRetrievalFunc[T any](_ *testing.T) func(context.Context, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
 	return func(_ context.Context, _, _ string, _ RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
 		return nil, nil, "", ErrRefreshNotSupported
 	}
@@ -113,7 +113,7 @@ func testNoRefreshRetrievalFunc[T any](t *testing.T) func(context.Context, strin
 // testErroringForRefreshTokenRetrievalFunc returns a refresh token error when
 // the refresh token is not empty.  This is useful for testing behavior when
 // the refresh token has expired or is otherwise invalid.
-func testErroringForRefreshTokenRetrievalFunc[T any](t *testing.T, ret []T) func(context.Context, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
+func testErroringForRefreshTokenRetrievalFunc[T any](_ *testing.T, ret []T) func(context.Context, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
 	return func(ctx context.Context, s1, s2 string, refToken RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
 		if refToken != "" {
 			return nil, nil, "", api.ErrInvalidListToken
@@ -158,7 +158,7 @@ func testStaticResourceRetrievalFuncForId[T any](t *testing.T, ret [][]T, remove
 // since it does not return any refresh token. This is for retrieval
 // functions that require an id be provided for listing purposes like when
 // listing resolvable aliases.
-func testNoRefreshRetrievalFuncForId[T any](t *testing.T) func(context.Context, string, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
+func testNoRefreshRetrievalFuncForId[T any](_ *testing.T) func(context.Context, string, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
 	return func(_ context.Context, _, _, _ string, _ RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
 		return nil, nil, "", ErrRefreshNotSupported
 	}
@@ -169,7 +169,7 @@ func testNoRefreshRetrievalFuncForId[T any](t *testing.T) func(context.Context,
 // the refresh token has expired or is otherwise invalid. This is for retrieval
 // functions that require an id be provided for listing purposes like when
 // listing resolvable aliases.
-func testErroringForRefreshTokenRetrievalFuncForId[T any](t *testing.T, ret []T) func(context.Context, string, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
+func testErroringForRefreshTokenRetrievalFuncForId[T any](_ *testing.T, ret []T) func(context.Context, string, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
 	return func(ctx context.Context, s1, s2, s3 string, refToken RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
 		if refToken != "" {
 			return nil, nil, "", api.ErrInvalidListToken

internal/clientcache/internal/cache/repository_refresh_token_test.go

@@ -189,7 +189,7 @@ func TestLookupRefreshToken(t *testing.T) {
 	})
 
 	t.Run("unknown user", func(t *testing.T) {
-		got, err := r.lookupRefreshToken(ctx, &user{Id: "unkonwnUser", Address: "addr"}, targetResourceType)
+		got, err := r.lookupRefreshToken(ctx, &user{Id: "unknownUser", Address: "addr"}, targetResourceType)
 		assert.NoError(t, err)
 		assert.Empty(t, got)
 	})
@@ -209,10 +209,11 @@ func TestLookupRefreshToken(t *testing.T) {
 		require.NoError(t, r.rw.Create(ctx, known))
 
 		before := time.Now().Truncate(time.Millisecond).UTC()
-		r.rw.DoTx(ctx, 1, db.ExpBackoff{}, func(r db.Reader, w db.Writer) error {
+		_, err := r.rw.DoTx(ctx, 1, db.ExpBackoff{}, func(r db.Reader, w db.Writer) error {
 			require.NoError(t, upsertRefreshToken(ctx, w, known, targetResourceType, token))
 			return nil
 		})
+		require.NoError(t, err)
 
 		got, err := r.lookupRefreshToken(ctx, known, targetResourceType)
 		assert.NoError(t, err)

internal/clientcache/internal/cache/repository_token.go

@@ -75,7 +75,8 @@ func upsertUserAndAuthToken(ctx context.Context, reader db.Reader, writer db.Wri
 	}
 
 	var users []*user
-	if err := reader.SearchWhere(ctx, &users, "true", []any{}, db.WithLimit(-1)); err != nil {
+	// we only want users that have not been soft deleted
+	if err := reader.SearchWhere(ctx, &users, "true", []any{}, db.WithLimit(-1), db.WithTable(activeUserTableName)); err != nil {
 		return errors.Wrap(ctx, err, op)
 	}
 	if len(users) <= usersLimit {
@@ -382,6 +383,8 @@ func cleanExpiredOrOrphanedAuthTokens(ctx context.Context, writer db.Writer, idT
 	return nil
 }
 
+const activeUserTableName = "user_active" // users that have not been soft deleted
+
 // lookupUser returns a user if one is present in the repository or nil if not.
 func (r *Repository) lookupUser(ctx context.Context, id string) (*user, error) {
 	const op = "cache.(Repository).lookupUser"
@@ -390,7 +393,8 @@ func (r *Repository) lookupUser(ctx context.Context, id string) (*user, error) {
 		return nil, errors.New(ctx, errors.InvalidParameter, op, "empty id")
 	}
 	ret := &user{Id: id}
-	if err := r.rw.LookupById(ctx, ret); err != nil {
+	// we only want users that have NOT been soft deleted
+	if err := r.rw.LookupById(ctx, ret, db.WithTable(activeUserTableName)); err != nil {
 		if errors.IsNotFoundError(err) {
 			return nil, nil
 		}
@@ -403,7 +407,8 @@ func (r *Repository) lookupUser(ctx context.Context, id string) (*user, error) {
 func (r *Repository) listUsers(ctx context.Context) ([]*user, error) {
 	const op = "cache.(Repository).listUsers"
 	var ret []*user
-	if err := r.rw.SearchWhere(ctx, &ret, "true", nil); err != nil {
+	// we only want users that have NOT been soft deleted
+	if err := r.rw.SearchWhere(ctx, &ret, "true", nil, db.WithTable(activeUserTableName)); err != nil {
 		return nil, errors.Wrap(ctx, err, op)
 	}
 	return ret, nil
@@ -482,16 +487,31 @@ func deleteUser(ctx context.Context, w db.Writer, u *user) (int, error) {
 	case u.Id == "":
 		return db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, "missing id")
 	}
-	// TODO(https://github.com/go-gorm/gorm/issues/4879): Use the
-	//   writer.Delete() function once the gorm bug is fixed. Until then
-	//   the gorm driver for sqlite has an error which wont execute a
-	//   delete correctly. as a work around we manually execute the
-	//   query here.
-	n, err := w.Exec(ctx, "delete from user where id = ?", []any{u.Id})
+	const (
+		// delete the user if they don't have any refresh tokens which are
+		// newer than 20 days (the refresh token expiration time)
+		deleteStmt = "delete from user where id = ? and id not in (select user_id from refresh_token where DATETIME('now', '-20 days') < datetime(create_time) )"
+
+		// fallback to soft deleting the user
+		softDeleteStmt = "update user set deleted_at = (strftime('%Y-%m-%d %H:%M:%f','now')) where id = ?"
+	)
+	// see if we should delete the user
+	rowsAffected, err := w.Exec(ctx, deleteStmt, []any{u.Id})
+	switch {
+	case err != nil:
+		return db.NoRowsAffected, errors.Wrap(ctx, err, op)
+	case rowsAffected > 0:
+		// if we deleted the user, so we're done.
+		return rowsAffected, nil
+	}
+
+	// fallback to soft delete
+	rowsAffected, err = w.Exec(ctx, softDeleteStmt, []any{u.Id})
 	if err != nil {
-		err = errors.Wrap(ctx, err, op)
+		return db.NoRowsAffected, errors.Wrap(ctx, err, op)
 	}
-	return n, err
+
+	return rowsAffected, nil
 }
 
 // user is a gorm model for the user table.  It represents a user