From ecdff4aef5e85741252203ae04fdba90bd3ac568 Mon Sep 17 00:00:00 2001 From: RUKAYAT-CODER Date: Wed, 5 Aug 2026 13:24:39 +0000 Subject: [PATCH] feat(gamification): cache leaderboard results and clamp max limit (#1159) --- .../leaderboards/leaderboards.service.spec.ts | 286 ++++++++++++++++++ .../leaderboards/leaderboards.service.ts | 108 ++++++- 2 files changed, 388 insertions(+), 6 deletions(-) create mode 100644 src/gamification/leaderboards/leaderboards.service.spec.ts diff --git a/src/gamification/leaderboards/leaderboards.service.spec.ts b/src/gamification/leaderboards/leaderboards.service.spec.ts new file mode 100644 index 00000000..f2206f28 --- /dev/null +++ b/src/gamification/leaderboards/leaderboards.service.spec.ts @@ -0,0 +1,286 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { getRepositoryToken } from '@nestjs/typeorm'; +import { + LeaderboardService, + MAX_LEADERBOARD_LIMIT, + LEADERBOARD_CACHE_TTL_SECONDS, +} from './leaderboards.service'; +import { UserProgress } from '../entities/user-progress.entity'; +import { UserBadge } from '../entities/user-badge.entity'; +import { BadgeCategory } from '../enums/badge-category.enum'; +import { REDIS_CLIENT } from '../../common/redis/redis.constants'; +import { createMockRedisClient } from '../../../test/utils/mock-factories'; + +/** + * Builds a chained TypeORM QueryBuilder mock. Every chainable method returns + * `this` so call order in the service is preserved for assertions on the + * terminal methods (`take`, `limit`, `getMany`, `getRawMany`). + */ +const createQueryBuilderMock = (overrides: Record = {}) => ({ + innerJoin: jest.fn().mockReturnThis(), + innerJoinAndSelect: jest.fn().mockReturnThis(), + select: jest.fn().mockReturnThis(), + addSelect: jest.fn().mockReturnThis(), + groupBy: jest.fn().mockReturnThis(), + addGroupBy: jest.fn().mockReturnThis(), + orderBy: jest.fn().mockReturnThis(), + where: jest.fn().mockReturnThis(), + having: jest.fn().mockReturnThis(), + andWhere: jest.fn().mockReturnThis(), + take: jest.fn().mockReturnThis(), + limit: jest.fn().mockReturnThis(), + getMany: jest.fn(), + getRawMany: jest.fn(), + getCount: jest.fn(), + ...overrides, +}); + +describe('LeaderboardService', () => { + let service: LeaderboardService; + let progressRepo: { createQueryBuilder: jest.Mock }; + let badgeRepo: { createQueryBuilder: jest.Mock }; + let redis: ReturnType; + + beforeEach(async () => { + progressRepo = { createQueryBuilder: jest.fn() }; + badgeRepo = { createQueryBuilder: jest.fn() }; + redis = createMockRedisClient(); + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + LeaderboardService, + { provide: getRepositoryToken(UserProgress), useValue: progressRepo }, + { provide: getRepositoryToken(UserBadge), useValue: badgeRepo }, + { provide: REDIS_CLIENT, useValue: redis }, + ], + }).compile(); + + service = module.get(LeaderboardService); + }); + + // ───────────────────────────────────────────────────────────────────────── + // getTopPlayers — limit clamp + // ───────────────────────────────────────────────────────────────────────── + + describe('getTopPlayers — limit clamp (issue #1159)', () => { + it('uses the default limit of 10 when none is supplied', async () => { + const qb = createQueryBuilderMock({ getMany: jest.fn().mockResolvedValue([]) }); + progressRepo.createQueryBuilder.mockReturnValue(qb); + (redis.get as jest.Mock).mockResolvedValue(null); + + await service.getTopPlayers(); + + expect(qb.take).toHaveBeenCalledWith(10); + }); + + it('clamps an oversized limit to MAX_LEADERBOARD_LIMIT', async () => { + const qb = createQueryBuilderMock({ getMany: jest.fn().mockResolvedValue([]) }); + progressRepo.createQueryBuilder.mockReturnValue(qb); + (redis.get as jest.Mock).mockResolvedValue(null); + + await service.getTopPlayers(1000000); + + expect(qb.take).toHaveBeenCalledWith(MAX_LEADERBOARD_LIMIT); + }); + + it('coerces a non-positive limit up to 1', async () => { + const qb = createQueryBuilderMock({ getMany: jest.fn().mockResolvedValue([]) }); + progressRepo.createQueryBuilder.mockReturnValue(qb); + (redis.get as jest.Mock).mockResolvedValue(null); + + await service.getTopPlayers(0); + + expect(qb.take).toHaveBeenCalledWith(1); + }); + }); + + // ───────────────────────────────────────────────────────────────────────── + // getTopPlayers — caching + // ───────────────────────────────────────────────────────────────────────── + + describe('getTopPlayers — caching (issue #1159)', () => { + it('serves a cache hit without re-running the sort query', async () => { + const cached = [ + { rank: 1, userId: 'u1', username: 'alice', totalPoints: 500, level: 2, badgeCount: 1 }, + ]; + (redis.get as jest.Mock).mockResolvedValue(JSON.stringify(cached)); + + const result = await service.getTopPlayers(10); + + expect(result).toEqual(cached); + expect(progressRepo.createQueryBuilder).not.toHaveBeenCalled(); + }); + + it('runs the query on a cache miss and caches the result with the leaderboard TTL', async () => { + (redis.get as jest.Mock).mockResolvedValue(null); + const qb = createQueryBuilderMock({ + getMany: jest.fn().mockResolvedValue([ + { + user: { id: 'u1', username: 'alice', email: 'alice@test.com' }, + totalPoints: 500, + level: 2, + }, + ]), + }); + progressRepo.createQueryBuilder.mockReturnValue(qb); + + const result = await service.getTopPlayers(10); + + expect(result).toHaveLength(1); + expect(result[0]).toMatchObject({ + rank: 1, + userId: 'u1', + username: 'alice', + totalPoints: 500, + }); + expect(redis.setex).toHaveBeenCalledWith( + 'cache:leaderboard:points:10', + LEADERBOARD_CACHE_TTL_SECONDS, + expect.any(String), + ); + }); + + it('uses the clamped limit in the cache key', async () => { + (redis.get as jest.Mock).mockResolvedValue(null); + const qb = createQueryBuilderMock({ getMany: jest.fn().mockResolvedValue([]) }); + progressRepo.createQueryBuilder.mockReturnValue(qb); + + await service.getTopPlayers(1000000); + + expect(redis.setex).toHaveBeenCalledWith( + `cache:leaderboard:points:${MAX_LEADERBOARD_LIMIT}`, + LEADERBOARD_CACHE_TTL_SECONDS, + expect.any(String), + ); + }); + + it('falls back to the database when the Redis read fails', async () => { + (redis.get as jest.Mock).mockRejectedValue(new Error('redis down')); + const qb = createQueryBuilderMock({ + getMany: jest + .fn() + .mockResolvedValue([ + { user: { id: 'u1', username: 'alice' }, totalPoints: 500, level: 2 }, + ]), + }); + progressRepo.createQueryBuilder.mockReturnValue(qb); + + const result = await service.getTopPlayers(10); + + expect(result).toHaveLength(1); + expect(qb.take).toHaveBeenCalledWith(10); + }); + + it('works without a Redis client (optional injection)', async () => { + const module: TestingModule = await Test.createTestingModule({ + providers: [ + LeaderboardService, + { provide: getRepositoryToken(UserProgress), useValue: progressRepo }, + { provide: getRepositoryToken(UserBadge), useValue: badgeRepo }, + ], + }).compile(); + + const noRedisService = module.get(LeaderboardService); + const qb = createQueryBuilderMock({ + getMany: jest + .fn() + .mockResolvedValue([ + { user: { id: 'u1', username: 'alice' }, totalPoints: 500, level: 2 }, + ]), + }); + progressRepo.createQueryBuilder.mockReturnValue(qb); + + const result = await noRedisService.getTopPlayers(10); + + expect(result).toHaveLength(1); + expect(redis.setex).not.toHaveBeenCalled(); + }); + }); + + // ───────────────────────────────────────────────────────────────────────── + // getBadgeLeaderboard — limit clamp and caching + // ───────────────────────────────────────────────────────────────────────── + + describe('getBadgeLeaderboard — limit clamp (issue #1159)', () => { + it('clamps an oversized limit to MAX_LEADERBOARD_LIMIT', async () => { + const qb = createQueryBuilderMock({ getRawMany: jest.fn().mockResolvedValue([]) }); + badgeRepo.createQueryBuilder.mockReturnValue(qb); + (redis.get as jest.Mock).mockResolvedValue(null); + + await service.getBadgeLeaderboard(5000); + + expect(qb.limit).toHaveBeenCalledWith(MAX_LEADERBOARD_LIMIT); + }); + }); + + describe('getBadgeLeaderboard — caching (issue #1159)', () => { + it('serves a cache hit without re-running the aggregate query', async () => { + const cached = [ + { + rank: 1, + userId: 'u1', + username: 'bob', + badgeCount: 3, + category: BadgeCategory.ACHIEVEMENT, + totalPoints: 0, + level: 0, + tier: 'BRONZE', + }, + ]; + (redis.get as jest.Mock).mockResolvedValue(JSON.stringify(cached)); + + const result = await service.getBadgeLeaderboard(10, BadgeCategory.ACHIEVEMENT); + + expect(result).toEqual(cached); + expect(badgeRepo.createQueryBuilder).not.toHaveBeenCalled(); + }); + + it('runs the aggregate query on a cache miss and caches the result', async () => { + (redis.get as jest.Mock).mockResolvedValue(null); + const qb = createQueryBuilderMock({ + getRawMany: jest + .fn() + .mockResolvedValue([ + { userId: 'u1', username: 'bob', email: 'bob@test.com', badgeCount: '3' }, + ]), + }); + badgeRepo.createQueryBuilder.mockReturnValue(qb); + + const result = await service.getBadgeLeaderboard(10, BadgeCategory.ACHIEVEMENT); + + expect(result).toHaveLength(1); + expect(result[0]).toMatchObject({ rank: 1, userId: 'u1', username: 'bob', badgeCount: 3 }); + expect(redis.setex).toHaveBeenCalledWith( + `cache:leaderboard:badges:${BadgeCategory.ACHIEVEMENT}:10`, + LEADERBOARD_CACHE_TTL_SECONDS, + expect.any(String), + ); + }); + + it('uses the clamped limit in the badge cache key', async () => { + (redis.get as jest.Mock).mockResolvedValue(null); + const qb = createQueryBuilderMock({ getRawMany: jest.fn().mockResolvedValue([]) }); + badgeRepo.createQueryBuilder.mockReturnValue(qb); + + await service.getBadgeLeaderboard(9999); + + expect(redis.setex).toHaveBeenCalledWith( + `cache:leaderboard:badges:all:${MAX_LEADERBOARD_LIMIT}`, + LEADERBOARD_CACHE_TTL_SECONDS, + expect.any(String), + ); + }); + + it('filters by category on a cache miss', async () => { + (redis.get as jest.Mock).mockResolvedValue(null); + const qb = createQueryBuilderMock({ getRawMany: jest.fn().mockResolvedValue([]) }); + badgeRepo.createQueryBuilder.mockReturnValue(qb); + + await service.getBadgeLeaderboard(10, BadgeCategory.SOCIAL); + + expect(qb.andWhere).toHaveBeenCalledWith('badge.category = :category', { + category: BadgeCategory.SOCIAL, + }); + }); + }); +}); diff --git a/src/gamification/leaderboards/leaderboards.service.ts b/src/gamification/leaderboards/leaderboards.service.ts index 14b338e2..166cf54d 100644 --- a/src/gamification/leaderboards/leaderboards.service.ts +++ b/src/gamification/leaderboards/leaderboards.service.ts @@ -1,10 +1,12 @@ -import { Injectable } from '@nestjs/common'; +import { Inject, Injectable, Logger, Optional } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; import { Repository } from 'typeorm'; +import Redis from 'ioredis'; import { UserProgress } from '../entities/user-progress.entity'; import { UserBadge } from '../entities/user-badge.entity'; import { BadgeCategory } from '../enums/badge-category.enum'; import { Tier } from '../enums/tier.enum'; +import { REDIS_CLIENT } from '../../common/redis/redis.constants'; export interface LeaderboardEntry { rank: number; @@ -33,26 +35,60 @@ export interface PaginatedLeaderboard { pageSize: number; } +/** + * Maximum number of rows a caller may request from a leaderboard. + * + * Issue #1159 — a caller-supplied `limit` (e.g. `?limit=1000000`) used to be + * passed straight into `take()`/`limit()`, forcing a full-table sort and a + * large response. Any larger value is coerced down to this documented cap. + */ +export const MAX_LEADERBOARD_LIMIT = 100; + +/** + * How long (seconds) a computed top-N leaderboard snapshot is served from + * Redis before the ordering query is re-run. + * + * Issue #1159 — leaderboards are read-heavy and change slowly, so caching the + * result for a short TTL lets repeated reads skip the sort entirely while + * keeping staleness bounded. Entries naturally expire on point/badge changes. + */ +export const LEADERBOARD_CACHE_TTL_SECONDS = 60; + +const pointsCacheKey = (limit: number) => `cache:leaderboard:points:${limit}`; +const badgeCacheKey = (limit: number, category?: BadgeCategory) => + `cache:leaderboard:badges:${category ?? 'all'}:${limit}`; + @Injectable() export class LeaderboardService { + private readonly logger = new Logger(LeaderboardService.name); + constructor( @InjectRepository(UserProgress) private userProgressRepository: Repository, @InjectRepository(UserBadge) private userBadgeRepository: Repository, + @Optional() @Inject(REDIS_CLIENT) private readonly redis?: Redis, ) {} // ─── Points Leaderboard ─────────────────────────────────────────────────── async getTopPlayers(limit: number = 10): Promise { + const clampedLimit = this.clampLimit(limit); + const cacheKey = pointsCacheKey(clampedLimit); + + const cached = await this.getCached(cacheKey); + if (cached) { + return cached; + } + const rows = await this.userProgressRepository .createQueryBuilder('up') .innerJoinAndSelect('up.user', 'user') .orderBy('up.totalPoints', 'DESC') - .take(limit) + .take(clampedLimit) .getMany(); - return rows.map((up, index) => ({ + const entries: LeaderboardEntry[] = rows.map((up, index) => ({ rank: index + 1, userId: up.user.id, username: up.user.username ?? up.user.email, @@ -60,10 +96,13 @@ export class LeaderboardService { level: up.level, badgeCount: 0, // enriched below if needed })); + + await this.setCached(cacheKey, entries); + return entries; } async getLeaderboard(page = 1, pageSize = 20): Promise { - const clampedSize = Math.min(pageSize, 100); + const clampedSize = Math.min(pageSize, MAX_LEADERBOARD_LIMIT); const offset = (page - 1) * clampedSize; const [rows, total] = await this.userProgressRepository.findAndCount({ @@ -103,6 +142,14 @@ export class LeaderboardService { limit: number = 10, category?: BadgeCategory, ): Promise { + const clampedLimit = this.clampLimit(limit); + const cacheKey = badgeCacheKey(clampedLimit, category); + + const cached = await this.getCached(cacheKey); + if (cached) { + return cached; + } + const qb = this.userBadgeRepository .createQueryBuilder('ub') .innerJoin('ub.user', 'user') @@ -114,14 +161,14 @@ export class LeaderboardService { .addGroupBy('user.username') .addGroupBy('user.email') .orderBy('badgeCount', 'DESC') - .limit(limit); + .limit(clampedLimit); if (category) { qb.innerJoin('ub.badge', 'badge').andWhere('badge.category = :category', { category }); } const rows = await qb.getRawMany(); - return rows.map((row, index) => ({ + const entries: BadgeLeaderboardEntry[] = rows.map((row, index) => ({ rank: index + 1, userId: row.userId, username: row.username ?? row.email, @@ -131,6 +178,9 @@ export class LeaderboardService { level: 0, tier: 'BRONZE' as any, })); + + await this.setCached(cacheKey, entries); + return entries; } async getUserBadgeRank(userId: string, category?: BadgeCategory): Promise { @@ -153,4 +203,50 @@ export class LeaderboardService { const ahead = await qb.getRawMany(); return ahead.length + 1; } + + // ─── Cache helpers ──────────────────────────────────────────────────────── + + /** + * Coerces a caller-supplied `limit` into the safe range [1, MAX_LEADERBOARD_LIMIT]. + * + * Non-finite values fall back to the documented maximum so a malformed + * request can never expand the query into an unbounded full-table sort. + */ + private clampLimit(limit: number): number { + if (!Number.isFinite(limit)) { + return MAX_LEADERBOARD_LIMIT; + } + return Math.min(Math.max(Math.floor(limit), 1), MAX_LEADERBOARD_LIMIT); + } + + /** Reads a JSON-serialized value from Redis. Falls back to a cache miss on any error. */ + private async getCached(key: string): Promise { + if (!this.redis) { + return undefined; + } + try { + const raw = await this.redis.get(key); + if (!raw) { + return undefined; + } + return JSON.parse(raw) as T; + } catch (error) { + // A Redis outage must never break leaderboard reads — fall back to the DB. + this.logger.warn(`Leaderboard cache read failed for ${key}: ${(error as Error).message}`); + return undefined; + } + } + + /** Stores a JSON-serialized value in Redis with the short leaderboard TTL. */ + private async setCached(key: string, value: T): Promise { + if (!this.redis) { + return; + } + try { + await this.redis.setex(key, LEADERBOARD_CACHE_TTL_SECONDS, JSON.stringify(value)); + } catch (error) { + // Failing to populate the cache is non-fatal — the DB result is still returned. + this.logger.warn(`Leaderboard cache write failed for ${key}: ${(error as Error).message}`); + } + } }