fix: tenant pagination

internal/models/entity.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"github.com/hookdeck/outpost/internal/cursor"
+	"github.com/hookdeck/outpost/internal/pagination"
 	"github.com/hookdeck/outpost/internal/redis"
 )
 
@@ -358,99 +359,38 @@ func (s *entityStoreImpl) ListTenant(ctx context.Context, req ListTenantRequest)
 		return nil, ErrInvalidOrder
 	}
 
-	// Determine sort direction
-	sortDir := "DESC"
-	if order == "asc" {
-		sortDir = "ASC"
-	}
-
-	// Parse cursor timestamp (keyset pagination)
-	// Cursor contains the timestamp boundary for the next/prev page
-	var cursorTimestamp int64
-	isNextCursor := false
-	isPrevCursor := false
-	if req.Next != "" {
-		data, err := cursor.Decode(req.Next, "tnt", 1)
-		if err != nil {
-			return nil, fmt.Errorf("%w: %v", ErrInvalidCursor, err)
-		}
-		cursorTimestamp, err = strconv.ParseInt(data, 10, 64)
-		if err != nil {
-			return nil, fmt.Errorf("%w: invalid timestamp", ErrInvalidCursor)
-		}
-		isNextCursor = true
-	} else if req.Prev != "" {
-		data, err := cursor.Decode(req.Prev, "tnt", 1)
-		if err != nil {
-			return nil, fmt.Errorf("%w: %v", ErrInvalidCursor, err)
-		}
-		cursorTimestamp, err = strconv.ParseInt(data, 10, 64)
-		if err != nil {
-			return nil, fmt.Errorf("%w: invalid timestamp", ErrInvalidCursor)
-		}
-		isPrevCursor = true
-	}
-
-	// Build FT.SEARCH query with timestamp filter (keyset pagination)
-	// This avoids offset-based pagination which has issues on Dragonfly
-	// We use inclusive ranges with cursor-1/cursor+1 for better compatibility
+	// Base filter for tenant search
 	// Filter: @entity:{tenant} ensures only tenant records (not destinations)
 	// Filter: -@deleted_at:[1 +inf] excludes deleted tenants
-	// (documents without deleted_at field won't match the positive query, so negation includes them)
 	baseFilter := "@entity:{tenant} -@deleted_at:[1 +inf]"
-	var query string
-	querySortDir := sortDir
-
-	if !isNextCursor && !isPrevCursor {
-		// First page - only filter out deleted
-		query = baseFilter
-	} else if isNextCursor {
-		// Next cursor: get older (DESC) or newer (ASC) records
-		if order == "desc" {
-			// DESC: get records with created_at < cursor (older)
-			query = fmt.Sprintf("(@created_at:[0 %d]) %s", cursorTimestamp-1, baseFilter)
-		} else {
-			// ASC: get records with created_at > cursor (newer)
-			query = fmt.Sprintf("(@created_at:[%d +inf]) %s", cursorTimestamp+1, baseFilter)
-		}
-	} else {
-		// Prev cursor: go back to previous page
-		// We reverse the sort direction to get items closest to the cursor,
-		// then reverse the results to maintain the user's expected order.
-		if order == "desc" {
-			// DESC going back: get records with created_at > cursor (newer), sorted ASC
-			query = fmt.Sprintf("(@created_at:[%d +inf]) %s", cursorTimestamp+1, baseFilter)
-			querySortDir = "ASC"
-		} else {
-			// ASC going back: get records with created_at < cursor (older), sorted DESC
-			query = fmt.Sprintf("(@created_at:[0 %d]) %s", cursorTimestamp-1, baseFilter)
-			querySortDir = "DESC"
-		}
-	}
-
-	// Execute FT.SEARCH query with LIMIT 0 count (no offset)
-	result, err := s.doCmd(ctx, "FT.SEARCH", s.tenantIndexName(),
-		query,
-		"SORTBY", "created_at", querySortDir,
-		"LIMIT", 0, limit,
-	).Result()
-	if err != nil {
-		return nil, fmt.Errorf("failed to search tenants: %w", err)
-	}
 
-	// Parse FT.SEARCH result
-	tenants, _, err := s.parseSearchResult(ctx, result)
+	// Use pagination package for cursor-based pagination with n+1 pattern
+	result, err := pagination.Run(ctx, pagination.Config[Tenant]{
+		Limit: limit,
+		Order: order,
+		Next:  req.Next,
+		Prev:  req.Prev,
+		Cursor: pagination.Cursor[Tenant]{
+			Encode: func(t Tenant) string {
+				return cursor.Encode("tnt", 1, strconv.FormatInt(t.CreatedAt.UnixMilli(), 10))
+			},
+			Decode: func(c string) (string, error) {
+				data, err := cursor.Decode(c, "tnt", 1)
+				if err != nil {
+					return "", fmt.Errorf("%w: %v", ErrInvalidCursor, err)
+				}
+				return data, nil
+			},
+		},
+		Fetch: func(ctx context.Context, q pagination.QueryInput) ([]Tenant, error) {
+			return s.fetchTenants(ctx, baseFilter, q)
+		},
+	})
 	if err != nil {
 		return nil, err
 	}
 
-	// For prev cursor, we fetched in reverse order to get items closest to cursor.
-	// Now reverse the results to restore the user's expected order.
-	if isPrevCursor {
-		for i, j := 0, len(tenants)-1; i < j; i, j = i+1, j-1 {
-			tenants[i], tenants[j] = tenants[j], tenants[i]
-		}
-	}
+	tenants := result.Items
 
 	// Batch fetch destination summaries for all tenants in a single Redis round-trip
 	if len(tenants) > 0 {
@@ -484,30 +424,61 @@ func (s *entityStoreImpl) ListTenant(ctx context.Context, req ListTenantRequest)
 		_, totalCount, _ = s.parseSearchResult(ctx, countResult)
 	}
 
-	// Build response with cursors
-	resp := &ListTenantResponse{
+	return &ListTenantResponse{
 		Data:  tenants,
 		Count: totalCount,
-	}
+		Next:  result.Next,
+		Prev:  result.Prev,
+	}, nil
+}
 
-	// Always set next/prev cursors based on first/last item timestamps.
-	// Client discovers "no more pages" when they use the cursor and get empty result.
-	// This avoids expensive probe queries.
-	if len(tenants) > 0 {
-		lastTenant := tenants[len(tenants)-1]
-		firstTenant := tenants[0]
+// fetchTenants builds and executes the FT.SEARCH query for tenant pagination.
+func (s *entityStoreImpl) fetchTenants(ctx context.Context, baseFilter string, q pagination.QueryInput) ([]Tenant, error) {
+	// Build FT.SEARCH query with timestamp filter (keyset pagination)
+	var query string
+	sortDir := "DESC"
+	if q.SortDir == "asc" {
+		sortDir = "ASC"
+	}
 
-		// Next cursor: points to last item (for continuing in same direction)
-		resp.Next = cursor.Encode("tnt", 1, strconv.FormatInt(lastTenant.CreatedAt.UnixMilli(), 10))
+	if q.CursorPos == "" {
+		// First page - only filter out deleted
+		query = baseFilter
+	} else {
+		// Parse cursor timestamp
+		cursorTimestamp, err := strconv.ParseInt(q.CursorPos, 10, 64)
+		if err != nil {
+			return nil, fmt.Errorf("%w: invalid timestamp", ErrInvalidCursor)
+		}
 
-		// Prev cursor: points to first item (for going back)
-		// Only set if we navigated here via a cursor (not on first page)
-		if isNextCursor || isPrevCursor {
-			resp.Prev = cursor.Encode("tnt", 1, strconv.FormatInt(firstTenant.CreatedAt.UnixMilli(), 10))
+		// Build cursor condition based on compare operator
+		// Use inclusive ranges with cursor-1/cursor+1 for better compatibility
+		if q.Compare == "<" {
+			// Get records with created_at < cursor
+			query = fmt.Sprintf("(@created_at:[0 %d]) %s", cursorTimestamp-1, baseFilter)
+		} else {
+			// Get records with created_at > cursor
+			query = fmt.Sprintf("(@created_at:[%d +inf]) %s", cursorTimestamp+1, baseFilter)
 		}
 	}
 
-	return resp, nil
+	// Execute FT.SEARCH query
+	result, err := s.doCmd(ctx, "FT.SEARCH", s.tenantIndexName(),
+		query,
+		"SORTBY", "created_at", sortDir,
+		"LIMIT", 0, q.Limit,
+	).Result()
+	if err != nil {
+		return nil, fmt.Errorf("failed to search tenants: %w", err)
+	}
+
+	// Parse FT.SEARCH result
+	tenants, _, err := s.parseSearchResult(ctx, result)
+	if err != nil {
+		return nil, err
+	}
+
+	return tenants, nil
 }
 
 // parseSearchResult parses the FT.SEARCH result into a list of tenants.

internal/models/entity_test.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/hookdeck/outpost/internal/idgen"
 	"github.com/hookdeck/outpost/internal/models"
+	"github.com/hookdeck/outpost/internal/pagination/paginationtest"
 	"github.com/hookdeck/outpost/internal/redis"
 	"github.com/hookdeck/outpost/internal/util/testinfra"
 	"github.com/hookdeck/outpost/internal/util/testutil"
@@ -165,6 +166,113 @@ func TestListTenant_Dragonfly_WithDeploymentID(t *testing.T) {
 	})
 }
 
+// =============================================================================
+// ListTenant Pagination Suite - Tests using paginationtest.Suite
+// These tests verify correct cursor-based pagination behavior including:
+// - Forward/backward traversal
+// - n+1 pattern (hasMore detection)
+// - First page has no Prev, last page has no Next
+// =============================================================================
+
+func TestListTenantPagination(t *testing.T) {
+	t.Parallel()
+	runListTenantPaginationSuite(t, dragonflyStackClientFactory, "")
+}
+
+func TestListTenantPagination_WithDeploymentID(t *testing.T) {
+	t.Parallel()
+	runListTenantPaginationSuite(t, dragonflyStackClientFactory, "dp_pagination_test")
+}
+
+func TestListTenantPagination_Compat_RedisStack(t *testing.T) {
+	t.Parallel()
+	runListTenantPaginationSuite(t, redisStackClientFactory, "")
+}
+
+func TestListTenantPagination_Compat_RedisStack_WithDeploymentID(t *testing.T) {
+	t.Parallel()
+	runListTenantPaginationSuite(t, redisStackClientFactory, "dp_pagination_test")
+}
+
+func runListTenantPaginationSuite(t *testing.T, factory RedisClientFactory, deploymentID string) {
+	ctx := context.Background()
+	redisClient := factory(t)
+
+	// Add unique suffix to deployment ID to isolate test data between parallel runs
+	if deploymentID != "" {
+		deploymentID = fmt.Sprintf("%s_%d", deploymentID, time.Now().UnixNano())
+	} else {
+		deploymentID = fmt.Sprintf("pagination_test_%d", time.Now().UnixNano())
+	}
+
+	entityStore := models.NewEntityStore(redisClient,
+		models.WithCipher(models.NewAESCipher("secret")),
+		models.WithAvailableTopics(testutil.TestTopics),
+		models.WithDeploymentID(deploymentID),
+	)
+
+	// Initialize entity store (probes for RediSearch)
+	err := entityStore.Init(ctx)
+	require.NoError(t, err)
+
+	// Track created tenant IDs for cleanup between subtests
+	var createdTenantIDs []string
+	baseTime := time.Now()
+
+	paginationSuite := paginationtest.Suite[models.Tenant]{
+		Name: "entitystore_ListTenant",
+
+		NewItem: func(index int) models.Tenant {
+			return models.Tenant{
+				ID:        fmt.Sprintf("tenant_pagination_%d_%d", time.Now().UnixNano(), index),
+				CreatedAt: baseTime.Add(time.Duration(index) * time.Second),
+				UpdatedAt: baseTime.Add(time.Duration(index) * time.Second),
+			}
+		},
+
+		InsertMany: func(ctx context.Context, items []models.Tenant) error {
+			for _, item := range items {
+				if err := entityStore.UpsertTenant(ctx, item); err != nil {
+					return err
+				}
+				createdTenantIDs = append(createdTenantIDs, item.ID)
+			}
+			return nil
+		},
+
+		List: func(ctx context.Context, opts paginationtest.ListOpts) (paginationtest.ListResult[models.Tenant], error) {
+			resp, err := entityStore.ListTenant(ctx, models.ListTenantRequest{
+				Limit: opts.Limit,
+				Order: opts.Order,
+				Next:  opts.Next,
+				Prev:  opts.Prev,
+			})
+			if err != nil {
+				return paginationtest.ListResult[models.Tenant]{}, err
+			}
+			return paginationtest.ListResult[models.Tenant]{
+				Items: resp.Data,
+				Next:  resp.Next,
+				Prev:  resp.Prev,
+			}, nil
+		},
+
+		GetID: func(t models.Tenant) string {
+			return t.ID
+		},
+
+		Cleanup: func(ctx context.Context) error {
+			for _, id := range createdTenantIDs {
+				_ = entityStore.DeleteTenant(ctx, id)
+			}
+			createdTenantIDs = nil
+			return nil
+		},
+	}
+
+	paginationSuite.Run(t)
+}
+
 // TestDestinationCredentialsEncryption verifies that credentials and delivery_metadata
 // are properly encrypted when stored in Redis.
 //