feat: add client retrieval and pagination to stores and tests

- Add GetClients method to Store interface for retrieving all clients
- Implement GetClients for in-memory and Redis stores
- Introduce tests for GetClients covering empty, small, and large client sets in both memory and Redis stores
- Add Redis tests to verify correct pagination when retrieving large numbers of clients
- Improve Redis store tests to handle missing Docker environment gracefully

Signed-off-by: appleboy <[email protected]>

Author: appleboy

pkg/core/store.go

@@ -34,6 +34,7 @@ type Store interface {
 	DeleteAuthorizationCode(ctx context.Context, clientID string) error
 
 	GetClient(ctx context.Context, clientID string) (*Client, error)
+	GetClients(ctx context.Context) ([]*Client, error)
 	CreateClient(ctx context.Context, client *Client) error
 	UpdateClient(ctx context.Context, client *Client) error
 	DeleteClient(ctx context.Context, clientID string) error

pkg/store/factory_test.go

@@ -67,19 +67,19 @@ func TestParseStoreType(t *testing.T) {
 
 func TestStoreType_String(t *testing.T) {
 	tests := []struct {
-		name     string
+		name      string
 		storeType StoreType
-		expected string
+		expected  string
 	}{
 		{
-			name:     "memory to string",
+			name:      "memory to string",
 			storeType: StoreTypeMemory,
-			expected: "memory",
+			expected:  "memory",
 		},
 		{
-			name:     "redis to string",
+			name:      "redis to string",
 			storeType: StoreTypeRedis,
-			expected: "redis",
+			expected:  "redis",
 		},
 	}
 
@@ -95,29 +95,29 @@ func TestStoreType_String(t *testing.T) {
 
 func TestStoreType_IsValid(t *testing.T) {
 	tests := []struct {
-		name     string
+		name      string
 		storeType StoreType
-		expected bool
+		expected  bool
 	}{
 		{
-			name:     "memory is valid",
+			name:      "memory is valid",
 			storeType: StoreTypeMemory,
-			expected: true,
+			expected:  true,
 		},
 		{
-			name:     "redis is valid",
+			name:      "redis is valid",
 			storeType: StoreTypeRedis,
-			expected: true,
+			expected:  true,
 		},
 		{
-			name:     "invalid type",
+			name:      "invalid type",
 			storeType: StoreType("invalid"),
-			expected: false,
+			expected:  false,
 		},
 		{
-			name:     "empty type",
+			name:      "empty type",
 			storeType: StoreType(""),
-			expected: false,
+			expected:  false,
 		},
 	}
 
@@ -167,6 +167,13 @@ func TestFactory_Create_Memory(t *testing.T) {
 }
 
 func TestFactory_Create_Redis(t *testing.T) {
+	// Recover from panic (e.g., Docker not available)
+	defer func() {
+		if r := recover(); r != nil {
+			t.Skipf("Cannot setup Redis container (Docker may not be running): %v", r)
+		}
+	}()
+
 	ctx := context.Background()
 
 	// Setup Redis container using testcontainers
@@ -192,7 +199,6 @@ func TestFactory_Create_Redis(t *testing.T) {
 	factory := NewFactory(config)
 
 	store, err := factory.Create()
-
 	// Skip test if Redis is not available
 	if err != nil {
 		t.Skipf("Redis not available, skipping test: %v", err)
@@ -231,17 +237,17 @@ func TestFactory_Create_InvalidType(t *testing.T) {
 
 func TestNewStore(t *testing.T) {
 	tests := []struct {
-		name    string
-		config  Config
-		wantErr bool
+		name     string
+		config   Config
+		wantErr  bool
 		wantType interface{}
 	}{
 		{
 			name: "create memory store",
 			config: Config{
 				Type: StoreTypeMemory,
 			},
-			wantErr: false,
+			wantErr:  false,
 			wantType: (*MemoryStore)(nil),
 		},
 		{

pkg/store/memory.go

@@ -176,3 +176,16 @@ func (m *MemoryStore) DeleteClient(ctx context.Context, clientID string) error {
 	delete(m.clients, clientID)
 	return nil
 }
+
+// GetClients retrieves all clients from memory.
+func (m *MemoryStore) GetClients(ctx context.Context) ([]*core.Client, error) {
+	m.mu.RLock()
+	defer m.mu.RUnlock()
+
+	clients := make([]*core.Client, 0, len(m.clients))
+	for _, client := range m.clients {
+		clients = append(clients, client)
+	}
+
+	return clients, nil
+}

pkg/store/memory_test.go

@@ -3,6 +3,7 @@ package store
 import (
 	"context"
 	"errors"
+	"fmt"
 	"sync"
 	"testing"
 	"time"
@@ -713,3 +714,92 @@ func TestMemoryStore_Client_Concurrent(t *testing.T) {
 
 	wg.Wait()
 }
+
+func TestMemoryStore_GetClients(t *testing.T) {
+	store := NewMemoryStore()
+	ctx := context.Background()
+
+	// 1. Test with an empty store
+	clients, err := store.GetClients(ctx)
+	if err != nil {
+		t.Fatalf("GetClients() on empty store failed: %v", err)
+	}
+	if len(clients) != 0 {
+		t.Fatalf("Expected 0 clients, got %d", len(clients))
+	}
+
+	// 2. Add some clients
+	client1 := &core.Client{ID: "client1", Secret: "secret1"}
+	client2 := &core.Client{ID: "client2", Secret: "secret2"}
+	if err := store.CreateClient(ctx, client1); err != nil {
+		t.Fatalf("Failed to create client1: %v", err)
+	}
+	if err := store.CreateClient(ctx, client2); err != nil {
+		t.Fatalf("Failed to create client2: %v", err)
+	}
+
+	// 3. Test with multiple clients
+	clients, err = store.GetClients(ctx)
+	if err != nil {
+		t.Fatalf("GetClients() with multiple clients failed: %v", err)
+	}
+	if len(clients) != 2 {
+		t.Fatalf("Expected 2 clients, got %d", len(clients))
+	}
+
+	// Check if the correct clients are returned (order is not guaranteed)
+	found1 := false
+	found2 := false
+	for _, c := range clients {
+		if c.ID == "client1" {
+			found1 = true
+		}
+		if c.ID == "client2" {
+			found2 = true
+		}
+	}
+	if !found1 || !found2 {
+		t.Errorf("Did not find all clients. Found1: %v, Found2: %v", found1, found2)
+	}
+}
+
+func TestMemoryStore_GetClients_LargeNumber(t *testing.T) {
+	store := NewMemoryStore()
+	ctx := context.Background()
+	numClients := 150
+
+	// Create a large number of clients
+	for i := 0; i < numClients; i++ {
+		client := &core.Client{
+			ID:     fmt.Sprintf("client-large-%d", i),
+			Secret: "secret",
+		}
+		if err := store.CreateClient(ctx, client); err != nil {
+			t.Fatalf("Failed to create client %d: %v", i, err)
+		}
+	}
+
+	// Get all clients
+	clients, err := store.GetClients(ctx)
+	if err != nil {
+		t.Fatalf("GetClients() with large number of clients failed: %v", err)
+	}
+
+	// Verify the number of clients retrieved
+	if len(clients) != numClients {
+		t.Errorf("Expected %d clients, but got %d", numClients, len(clients))
+	}
+
+	// Verify all clients are present
+	clientMap := make(map[string]bool)
+	for _, c := range clients {
+		clientMap[c.ID] = true
+	}
+
+	for i := 0; i < numClients; i++ {
+		clientID := fmt.Sprintf("client-large-%d", i)
+		if !clientMap[clientID] {
+			t.Errorf("Client %s was not found in the retrieved list", clientID)
+		}
+	}
+}

pkg/store/redis.go

@@ -259,3 +259,45 @@ func (r *RedisStore) DeleteClient(ctx context.Context, clientID string) error {
 
 	return nil
 }
+
+// GetClients retrieves all clients from Redis.
+// This operation can be expensive and should be used with caution in production.
+func (r *RedisStore) GetClients(ctx context.Context) ([]*core.Client, error) {
+	var clients []*core.Client
+	var cursor uint64
+
+	for {
+		scanCmd := r.client.B().Scan().Cursor(cursor).Match(clientPrefix + "*").Count(100).Build()
+		scanResult, err := r.client.Do(ctx, scanCmd).AsScanEntry()
+		if err != nil {
+			return nil, fmt.Errorf("failed to scan for clients in redis: %w", err)
+		}
+
+		if len(scanResult.Elements) > 0 {
+			mgetCmd := r.client.B().Mget().Key(scanResult.Elements...).Build()
+			values, err := r.client.Do(ctx, mgetCmd).AsStrSlice()
+			if err != nil {
+				return nil, fmt.Errorf("failed to get clients from redis: %w", err)
+			}
+
+			for _, val := range values {
+				if val == "" {
+					continue
+				}
+				var client core.Client
+				if err := json.Unmarshal([]byte(val), &client); err != nil {
+					// Log or skip corrupted data
+					continue
+				}
+				clients = append(clients, &client)
+			}
+		}
+
+		cursor = scanResult.Cursor
+		if cursor == 0 {
+			break
+		}
+	}
+
+	return clients, nil
+}

pkg/store/redis_test.go

@@ -21,8 +21,8 @@ func setupRedisContainer(ctx context.Context) (string, error) {
 		Image:        "redis:7-alpine",
 		ExposedPorts: []string{"6379/tcp"},
 		WaitingFor: wait.ForAll(
-			wait.ForLog("Ready to accept connections").WithStartupTimeout(30 * time.Second),
-			wait.ForListeningPort("6379/tcp").WithStartupTimeout(30 * time.Second),
+			wait.ForLog("Ready to accept connections").WithStartupTimeout(30*time.Second),
+			wait.ForListeningPort("6379/tcp").WithStartupTimeout(30*time.Second),
 		),
 	}
 
@@ -958,3 +958,102 @@ func TestRedisStore_GetClient_CacheInvalidation(t *testing.T) {
 
 	t.Log("Cache invalidation working correctly after client update")
 }
+
+func TestRedisStore_GetClients(t *testing.T) {
+	store, cleanup := setupRedisStore(t)
+	if store == nil {
+		return // Skip if Redis not available
+	}
+	defer cleanup()
+
+	ctx := context.Background()
+
+	// 1. Test with an empty store
+	clients, err := store.GetClients(ctx)
+	if err != nil {
+		t.Fatalf("GetClients() on empty store failed: %v", err)
+	}
+	if len(clients) != 0 {
+		t.Fatalf("Expected 0 clients, got %d", len(clients))
+	}
+
+	// 2. Add some clients
+	client1 := &core.Client{ID: "client1-redis", Secret: "secret1"}
+	client2 := &core.Client{ID: "client2-redis", Secret: "secret2"}
+	if err := store.CreateClient(ctx, client1); err != nil {
+		t.Fatalf("Failed to create client1: %v", err)
+	}
+	if err := store.CreateClient(ctx, client2); err != nil {
+		t.Fatalf("Failed to create client2: %v", err)
+	}
+
+	// 3. Test with multiple clients
+	clients, err = store.GetClients(ctx)
+	if err != nil {
+		t.Fatalf("GetClients() with multiple clients failed: %v", err)
+	}
+	if len(clients) != 2 {
+		t.Fatalf("Expected 2 clients, got %d", len(clients))
+	}
+
+	// Check if the correct clients are returned
+	found1 := false
+	found2 := false
+	for _, c := range clients {
+		if c.ID == "client1-redis" {
+			found1 = true
+		}
+		if c.ID == "client2-redis" {
+			found2 = true
+		}
+	}
+	if !found1 || !found2 {
+		t.Errorf("Did not find all clients. Found1: %v, Found2: %v", found1, found2)
+	}
+}
+
+func TestRedisStore_GetClients_Pagination(t *testing.T) {
+	store, cleanup := setupRedisStore(t)
+	if store == nil {
+		return // Skip if Redis not available
+	}
+	defer cleanup()
+
+	ctx := context.Background()
+	numClients := 150 // More than the SCAN COUNT of 100
+
+	// Create a large number of clients
+	for i := 0; i < numClients; i++ {
+		client := &core.Client{
+			ID:     fmt.Sprintf("client-pagination-%d", i),
+			Secret: "secret",
+		}
+		if err := store.CreateClient(ctx, client); err != nil {
+			t.Fatalf("Failed to create client %d: %v", i, err)
+		}
+	}
+
+	// Get all clients
+	clients, err := store.GetClients(ctx)
+	if err != nil {
+		t.Fatalf("GetClients() with pagination failed: %v", err)
+	}
+
+	// Verify the number of clients retrieved
+	if len(clients) != numClients {
+		t.Errorf("Expected %d clients, but got %d", numClients, len(clients))
+	}
+
+	// Verify all clients are present
+	clientMap := make(map[string]bool)
+	for _, c := range clients {
+		clientMap[c.ID] = true
+	}
+
+	for i := 0; i < numClients; i++ {
+		clientID := fmt.Sprintf("client-pagination-%d", i)
+		if !clientMap[clientID] {
+			t.Errorf("Client %s was not found in the retrieved list", clientID)
+		}
+	}
+}