Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -525,5 +525,46 @@ describe('implicit feedback', () => {
expect(result.size).toBe(1)
},
)

it('bounds getArtistKeysFromDb with take:5000 to prevent unbounded query', async () => {
const service = new RecommendationFeedbackService(30)
getMock.mockResolvedValue(JSON.stringify({}))
// Mock result at the take limit
const largeSet = Array.from({ length: 5000 }, (_, i) => ({
artistKey: `artist${i}`,
}))
mockUserArtistPreference.findMany.mockResolvedValue(largeSet)

const result = await service.getPreferredArtistKeys(
'guild-1',
'user-1',
)

// Verify take: 5000 was applied
expect(mockUserArtistPreference.findMany).toHaveBeenCalledWith(
expect.objectContaining({ take: 5000 }),
)
expect(result.size).toBe(5000)
})

it('bounds blocked artist keys query with take:5000', async () => {
const service = new RecommendationFeedbackService(30)
getMock.mockResolvedValue(JSON.stringify({}))
mockUserArtistPreference.findMany.mockResolvedValue(
Array.from({ length: 3000 }, (_, i) => ({
artistKey: `blocked${i}`,
})),
)

const result = await service.getBlockedArtistKeys(
'guild-1',
'user-1',
)

expect(mockUserArtistPreference.findMany).toHaveBeenCalledWith(
expect.objectContaining({ take: 5000 }),
)
expect(result.size).toBe(3000)
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -322,9 +322,12 @@ export class RecommendationFeedbackService {
): Promise<Set<string>> {
try {
const db = getPrismaClient()
// Cap at 5000 prefs per user/guild/type to prevent unbounded
// query on power users. Typical users have <100 prefs.
const prefs = await db.userArtistPreference.findMany({
where: { discordUserId: userId, guildId, preference },
select: { artistKey: true },
take: 5000,
})
return new Set(prefs.map((p) => p.artistKey))
} catch {
Expand Down
110 changes: 99 additions & 11 deletions packages/bot/src/utils/general/birthdayScheduler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ const { __mocks: dbMocks } = jest.requireMock('@lucky/shared/utils') as {

import { BirthdayScheduler } from './birthdayScheduler'

function makeClient(sentCapture: Array<{ channelId: string; payload: unknown }>) {
function makeClient(
sentCapture: Array<{ channelId: string; payload: unknown }>,
) {
const channelSend = (channelId: string) =>
jest.fn((payload: unknown) => {
sentCapture.push({ channelId, payload })
Expand All @@ -60,7 +62,14 @@ beforeEach(() => {

function makeGuildWithRole(
roleId: string,
currentMembers: Map<string, { rolesCacheHas: (id: string) => boolean; rolesAdd: jest.Mock; rolesRemove: jest.Mock }>,
currentMembers: Map<
string,
{
rolesCacheHas: (id: string) => boolean
rolesAdd: jest.Mock
rolesRemove: jest.Mock
}
>,
roleMembers: Map<string, { rolesAdd: jest.Mock; rolesRemove: jest.Mock }>,
) {
return {
Expand All @@ -85,7 +94,10 @@ function makeGuildWithRole(
if (id !== roleId) return Promise.resolve(null)
return Promise.resolve({
id: roleId,
members: roleMembers as unknown as Map<string, { roles: { remove: jest.Mock } }>,
members: roleMembers as unknown as Map<
string,
{ roles: { remove: jest.Mock } }
>,
})
}),
},
Expand Down Expand Up @@ -181,7 +193,7 @@ describe('BirthdayScheduler.tick', () => {
expect(sent).toHaveLength(2)
})

test('grants birthday role to today\'s celebrators', async () => {
test("grants birthday role to today's celebrators", async () => {
dbMocks.findMany.mockResolvedValue([{ guildId: 'g1', userId: 'u1' }])
dbMocks.settingsFindUnique.mockResolvedValue({
birthdayChannelId: 'chan-1',
Expand All @@ -196,7 +208,11 @@ describe('BirthdayScheduler.tick', () => {
rolesRemove,
}
const currentMembers = new Map([['u1', member]])
const guildWithRole = makeGuildWithRole('role-1', currentMembers, new Map())
const guildWithRole = makeGuildWithRole(
'role-1',
currentMembers,
new Map(),
)
const client = {
...makeClient(sent),
guilds: { fetch: jest.fn(() => Promise.resolve(guildWithRole)) },
Expand Down Expand Up @@ -316,9 +332,7 @@ describe('BirthdayScheduler.tick', () => {
})

test('handles missing channel gracefully', async () => {
dbMocks.findMany.mockResolvedValue([
{ guildId: 'g1', userId: 'u1' },
])
dbMocks.findMany.mockResolvedValue([{ guildId: 'g1', userId: 'u1' }])
dbMocks.settingsFindUnique.mockResolvedValue({
birthdayChannelId: 'chan-missing',
})
Expand Down Expand Up @@ -422,9 +436,7 @@ describe('BirthdayScheduler.tick', () => {
})

test('handles settings.findUnique returning null gracefully', async () => {
dbMocks.findMany.mockResolvedValue([
{ guildId: 'g1', userId: 'u1' },
])
dbMocks.findMany.mockResolvedValue([{ guildId: 'g1', userId: 'u1' }])
dbMocks.settingsFindUnique.mockResolvedValue(null)
const sent: Array<{ channelId: string; payload: unknown }> = []
const client = makeClient(sent) as never
Expand All @@ -436,4 +448,80 @@ describe('BirthdayScheduler.tick', () => {
// No channel configured, so no announcement
expect(sent).toHaveLength(0)
})

test('paginates through guilds with role in batches of 500', async () => {
dbMocks.findMany.mockResolvedValue([])
// Mock settingsFindMany to return two pages
const page1 = Array.from({ length: 500 }, (_, i) => ({
guildId: `g${i}`,
birthdayRoleId: `role-${i}`,
}))
const page2 = Array.from({ length: 100 }, (_, i) => ({
guildId: `g${500 + i}`,
birthdayRoleId: `role-${500 + i}`,
}))
dbMocks.settingsFindMany
.mockResolvedValueOnce(page1)
.mockResolvedValueOnce(page2)
const client = {
channels: { fetch: jest.fn() },
guilds: {
fetch: jest.fn(() =>
Promise.resolve({
roles: {
fetch: jest.fn(() =>
Promise.resolve({
id: 'role-1',
members: new Map(),
}),
),
},
}),
),
},
} as never
const scheduler = new BirthdayScheduler({
clock: () => new Date('2026-04-20T00:00:00Z'),
})
scheduler['client'] = client
await scheduler.tick()
expect(dbMocks.settingsFindMany).toHaveBeenCalledTimes(2)
// First call should request page 1
const call1 = dbMocks.settingsFindMany.mock.calls[0][0]
expect(call1.take).toBe(500)
expect(call1.orderBy).toEqual({ guildId: 'asc' })
})

test('stops pagination when page size is less than limit', async () => {
dbMocks.findMany.mockResolvedValue([])
const partialPage = Array.from({ length: 200 }, (_, i) => ({
guildId: `g${i}`,
birthdayRoleId: `role-${i}`,
}))
dbMocks.settingsFindMany.mockResolvedValueOnce(partialPage)
const client = {
channels: { fetch: jest.fn() },
guilds: {
fetch: jest.fn(() =>
Promise.resolve({
roles: {
fetch: jest.fn(() =>
Promise.resolve({
id: 'role-1',
members: new Map(),
}),
),
},
}),
),
},
} as never
const scheduler = new BirthdayScheduler({
clock: () => new Date('2026-04-20T00:00:00Z'),
})
scheduler['client'] = client
await scheduler.tick()
// Should only call settingsFindMany once (partial page < 500)
expect(dbMocks.settingsFindMany).toHaveBeenCalledTimes(1)
})
})
43 changes: 33 additions & 10 deletions packages/bot/src/utils/general/birthdayScheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@
import { ChannelType } from 'discord.js'
import { EmbedBuilder } from '@discordjs/builders'
import { COLOR } from '@lucky/shared/constants'
import { getPrismaClient, debugLog, errorLog, infoLog } from '@lucky/shared/utils'
import {
getPrismaClient,
debugLog,
errorLog,
infoLog,
} from '@lucky/shared/utils'

// Tick every hour by default. The scheduler tracks the last UTC date it
// announced for each guild (in-memory) so multiple ticks on the same day
Expand Down Expand Up @@ -111,7 +116,9 @@
await this.reconcileGuildsWithoutMatches(byGuild)

if (rows.length === 0) {
debugLog({ message: `birthday tick: no matches for ${todayKey}` })
debugLog({
message: `birthday tick: no matches for ${todayKey}`,
})
}
} catch (error) {
errorLog({
Expand All @@ -123,22 +130,38 @@
}
}

private async reconcileGuildsWithoutMatches(

Check failure on line 133 in packages/bot/src/utils/general/birthdayScheduler.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 16 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=LucasSantana-Dev_Lucky&issues=AZ6UNXV1sIEvOGOzIb6K&open=AZ6UNXV1sIEvOGOzIb6K&pullRequest=1214
byGuild: Map<string, Set<string>>,
): Promise<void> {
if (!this.client) return
// Find guilds that have a role configured but aren't in today's match
// set — those need role revocation from any stale holders.
// Paginate to avoid unbounded query on large deployments.
const prisma = getPrismaClient()
const guildsWithRole = (await prisma.guildSettings.findMany({
where: { birthdayRoleId: { not: null } },
select: { guildId: true, birthdayRoleId: true },
})) as Array<{ guildId: string; birthdayRoleId: string | null }>
const PAGE_SIZE = 500
let cursor: string | undefined
let hasMore = true

while (hasMore) {
const guildsWithRole = (await prisma.guildSettings.findMany({
where: { birthdayRoleId: { not: null } },
select: { guildId: true, birthdayRoleId: true },
take: PAGE_SIZE,
skip: cursor ? 1 : 0,
cursor: cursor ? { guildId: cursor } : undefined,
orderBy: { guildId: 'asc' },
})) as Array<{ guildId: string; birthdayRoleId: string | null }>

hasMore = guildsWithRole.length === PAGE_SIZE
if (guildsWithRole.length === 0) break

for (const row of guildsWithRole) {
if (byGuild.has(row.guildId)) continue // handled above
if (!row.birthdayRoleId) continue
await this.reconcileBirthdayRole(row.guildId, new Set())
}

for (const row of guildsWithRole) {
if (byGuild.has(row.guildId)) continue // handled above
if (!row.birthdayRoleId) continue
await this.reconcileBirthdayRole(row.guildId, new Set())
cursor = guildsWithRole[guildsWithRole.length - 1]?.guildId

Check warning on line 164 in packages/bot/src/utils/general/birthdayScheduler.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Prefer `.at(…)` over `[….length - index]`.

See more on https://sonarcloud.io/project/issues?id=LucasSantana-Dev_Lucky&issues=AZ6UNXV1sIEvOGOzIb6L&open=AZ6UNXV1sIEvOGOzIb6L&pullRequest=1214
}
}

Expand Down
Loading