From ca392749e3a5ea9723f2d36ca7014e5ceea5bf73 Mon Sep 17 00:00:00 2001 From: syntaxbullet Date: Wed, 7 Jan 2026 11:04:34 +0100 Subject: [PATCH] refactor: replace cleanup service with focused temp role service and fix daily streaks --- src/commands/admin/cleanup.ts | 73 ------------ src/lib/config.ts | 19 +-- src/modules/economy/economy.service.test.ts | 9 ++ src/modules/economy/economy.service.ts | 12 +- src/modules/system/cleanup.service.test.ts | 111 ----------------- src/modules/system/cleanup.service.ts | 119 ------------------- src/modules/system/scheduler.ts | 23 +--- src/modules/system/temp-role.service.test.ts | 114 ++++++++++++++++++ src/modules/system/temp-role.service.ts | 67 +++++++++++ 9 files changed, 206 insertions(+), 341 deletions(-) delete mode 100644 src/commands/admin/cleanup.ts delete mode 100644 src/modules/system/cleanup.service.test.ts delete mode 100644 src/modules/system/cleanup.service.ts create mode 100644 src/modules/system/temp-role.service.test.ts create mode 100644 src/modules/system/temp-role.service.ts diff --git a/src/commands/admin/cleanup.ts b/src/commands/admin/cleanup.ts deleted file mode 100644 index a02914b..0000000 --- a/src/commands/admin/cleanup.ts +++ /dev/null @@ -1,73 +0,0 @@ -import { createCommand } from "@lib/utils"; -import { SlashCommandBuilder, PermissionFlagsBits } from "discord.js"; -import { lootdropService } from "@/modules/economy/lootdrop.service"; -import { createBaseEmbed } from "@lib/embeds"; - -export const cleanup = createCommand({ - data: new SlashCommandBuilder() - .setName("cleanup") - .setDescription("Manually trigger cleanup tasks") - .setDefaultMemberPermissions(PermissionFlagsBits.Administrator) - .addStringOption(option => - option.setName("type") - .setDescription("The type of cleanup to perform") - .setRequired(true) - .addChoices( - { name: 'Lootdrops', value: 'lootdrops' }, - { name: 'Timers (Expired)', value: 'timers' }, - { name: 'Quests (Old Completed)', value: 'quests' }, - { name: 'All', value: 'all' } - ) - ) - .addBooleanOption(option => - option.setName("include_claimed") - .setDescription("Whether to cleanup claimed lootdrops as well (only for lootdrops/all)") - .setRequired(false) - ), - execute: async (interaction) => { - await interaction.deferReply({ ephemeral: true }); - - const type = interaction.options.getString("type", true); - const includeClaimed = interaction.options.getBoolean("include_claimed") || false; - - try { - let stats = { - lootdrops: 0, - timers: 0, - quests: 0 - }; - - const runLootdrops = type === 'lootdrops' || type === 'all'; - const runTimers = type === 'timers' || type === 'all'; - const runQuests = type === 'quests' || type === 'all'; - - const messages: string[] = []; - - if (runLootdrops) { - stats.lootdrops = await lootdropService.cleanupExpiredLootdrops(includeClaimed); - messages.push(`- **Lootdrops**: ${stats.lootdrops} removed ${includeClaimed ? "(including claimed)" : ""}`); - } - - if (runTimers) { - // Import dynamically to avoid circular deps if any, or just standard import - const { cleanupService } = await import("@/modules/system/cleanup.service"); - stats.timers = await cleanupService.cleanupTimers(); - messages.push(`- **Timers**: ${stats.timers} expired timers processing/removed`); - } - - if (runQuests) { - const { cleanupService } = await import("@/modules/system/cleanup.service"); - stats.quests = await cleanupService.cleanupQuests(); - messages.push(`- **Quests**: ${stats.quests} archived/removed`); - } - - const embed = createBaseEmbed("Cleanup Complete") - .setDescription(`successfully executed cleanup for **${type}**.\n\n${messages.join("\n")}`); - - await interaction.editReply({ embeds: [embed] }); - } catch (error) { - console.error("Cleanup failed:", error); - await interaction.editReply({ content: "โŒ An error occurred while performing cleanup." }); - } - } -}); diff --git a/src/lib/config.ts b/src/lib/config.ts index d6b65f4..f5e678d 100644 --- a/src/lib/config.ts +++ b/src/lib/config.ts @@ -69,12 +69,7 @@ export interface GameConfigType { autoTimeoutThreshold?: number; }; }; - system: { - cleanup: { - intervalMs: number; - questArchiveDays: number; - }; - }; + system: Record; } // Initial default config state @@ -167,17 +162,7 @@ const configSchema = z.object({ dmOnWarn: true } }), - system: z.object({ - cleanup: z.object({ - intervalMs: z.number().default(24 * 60 * 60 * 1000), // Daily - questArchiveDays: z.number().default(30) - }) - }).default({ - cleanup: { - intervalMs: 24 * 60 * 60 * 1000, - questArchiveDays: 30 - } - }) + system: z.record(z.string(), z.any()).default({}), }); export function reloadConfig() { diff --git a/src/modules/economy/economy.service.test.ts b/src/modules/economy/economy.service.test.ts index 8f21bde..537c11d 100644 --- a/src/modules/economy/economy.service.test.ts +++ b/src/modules/economy/economy.service.test.ts @@ -209,6 +209,15 @@ describe("economyService", () => { expect(result.streak).toBe(1); }); + it("should preserve streak if cooldown is missing but user has a streak", async () => { + mockFindFirst + .mockResolvedValueOnce(undefined) // No cooldown + .mockResolvedValueOnce({ id: 1n, dailyStreak: 10 }); + + const result = await economyService.claimDaily("1"); + expect(result.streak).toBe(11); + }); + it("should prevent weekly bonus exploit by resetting streak", async () => { // Mock user at streak 7. // Mock time as 24h + 1m after expiry. diff --git a/src/modules/economy/economy.service.ts b/src/modules/economy/economy.service.ts index 8848ef9..57288a6 100644 --- a/src/modules/economy/economy.service.ts +++ b/src/modules/economy/economy.service.ts @@ -68,8 +68,6 @@ export const economyService = { claimDaily: async (userId: string, tx?: Transaction) => { return await withTransaction(async (txFn) => { const now = new Date(); - const startOfDay = new Date(now); - startOfDay.setHours(0, 0, 0, 0); // Check cooldown const cooldown = await txFn.query.userTimers.findFirst({ @@ -90,17 +88,23 @@ export const economyService = { }); if (!user) { - throw new Error("User not found"); // This might be system error because user should exist if authenticated, but keeping simple for now + throw new Error("User not found"); } let streak = (user.dailyStreak || 0) + 1; - // If previous cooldown exists and expired more than 24h ago (meaning >48h since last claim), reduce streak by one for each day passed minimum 1 + // Check if streak should be reset due to missing a day if (cooldown) { const timeSinceReady = now.getTime() - cooldown.expiresAt.getTime(); + // If more than 24h passed since it became ready, they missed a full calendar day if (timeSinceReady > 24 * 60 * 60 * 1000) { streak = 1; } + } else if ((user.dailyStreak || 0) > 0) { + // If no cooldown record exists but user has a streak, + // we'll allow one "free" increment to restore the timer state. + // This prevents unfair resets if timers were cleared/lost. + streak = (user.dailyStreak || 0) + 1; } else { streak = 1; } diff --git a/src/modules/system/cleanup.service.test.ts b/src/modules/system/cleanup.service.test.ts deleted file mode 100644 index f304169..0000000 --- a/src/modules/system/cleanup.service.test.ts +++ /dev/null @@ -1,111 +0,0 @@ -import { describe, it, expect, mock, beforeEach, spyOn } from "bun:test"; -import { cleanupService } from "./cleanup.service"; -import { lootdrops, userTimers, userQuests } from "@/db/schema"; -import { config } from "@/lib/config"; - -const mockDelete = mock(); -const mockReturning = mock(); -const mockWhere = mock(); -const mockFindMany = mock(); - -mockDelete.mockReturnValue({ where: mockWhere }); -mockWhere.mockReturnValue({ returning: mockReturning }); - -// Mock DrizzleClient -mock.module("@/lib/DrizzleClient", () => ({ - DrizzleClient: { - delete: mockDelete, - query: { - userTimers: { - findMany: mockFindMany - } - } - } -})); - -// Mock AuroraClient -mock.module("@/lib/BotClient", () => ({ - AuroraClient: { - guilds: { - fetch: mock().mockResolvedValue({ - members: { - fetch: mock().mockResolvedValue({ - user: { tag: "TestUser#1234" }, - roles: { remove: mock().mockResolvedValue({}) } - }) - } - }) - } - } -})); - -// Mock Config -mock.module("@/lib/config", () => ({ - config: { - system: { - cleanup: { - intervalMs: 86400000, - questArchiveDays: 30 - } - } - } -})); - -describe("cleanupService", () => { - beforeEach(() => { - mockDelete.mockClear(); - mockWhere.mockClear(); - mockReturning.mockClear(); - mockFindMany.mockClear(); - }); - - it("cleanupLootdrops should delete expired unclaimed lootdrops", async () => { - mockReturning.mockResolvedValue([{ id: "msg1" }, { id: "msg2" }]); - - const count = await cleanupService.cleanupLootdrops(); - - expect(count).toBe(2); - expect(mockDelete).toHaveBeenCalledWith(lootdrops); - }); - - it("cleanupTimers should delete expired timers and handle roles", async () => { - // Mock findMany for expired ACCESS timers - mockFindMany.mockResolvedValue([ - { userId: 123n, type: 'ACCESS', key: 'role_456', metadata: { roleId: '456' } } - ]); - - // Mock returning for bulk delete - mockReturning.mockResolvedValue([{ userId: 789n }]); // One other timer - - const count = await cleanupService.cleanupTimers(); - - // 1 from findMany + 1 from bulk delete (simplified mock behavior) - expect(count).toBe(2); - expect(mockDelete).toHaveBeenCalledWith(userTimers); - }); - - it("cleanupQuests should delete old completed quests", async () => { - mockReturning.mockResolvedValue([{ userId: 123n }]); - - const count = await cleanupService.cleanupQuests(); - - expect(count).toBe(1); - expect(mockDelete).toHaveBeenCalledWith(userQuests); - }); - - it("runAll should run all cleanup tasks and log stats", async () => { - const spyLootdrops = spyOn(cleanupService, 'cleanupLootdrops').mockResolvedValue(1); - const spyTimers = spyOn(cleanupService, 'cleanupTimers').mockResolvedValue(2); - const spyQuests = spyOn(cleanupService, 'cleanupQuests').mockResolvedValue(3); - - await cleanupService.runAll(); - - expect(spyLootdrops).toHaveBeenCalled(); - expect(spyTimers).toHaveBeenCalled(); - expect(spyQuests).toHaveBeenCalled(); - - spyLootdrops.mockRestore(); - spyTimers.mockRestore(); - spyQuests.mockRestore(); - }); -}); diff --git a/src/modules/system/cleanup.service.ts b/src/modules/system/cleanup.service.ts deleted file mode 100644 index 43d8496..0000000 --- a/src/modules/system/cleanup.service.ts +++ /dev/null @@ -1,119 +0,0 @@ -import { lootdrops, userTimers, userQuests } from "@/db/schema"; -import { eq, and, lt, isNull, sql } from "drizzle-orm"; -import { DrizzleClient } from "@/lib/DrizzleClient"; -import { AuroraClient } from "@/lib/BotClient"; -import { env } from "@/lib/env"; -import { config } from "@/lib/config"; -import { TimerType } from "@/lib/constants"; - -export const cleanupService = { - /** - * Runs all cleanup tasks - */ - runAll: async () => { - console.log("๐Ÿงน Starting system cleanup..."); - const stats = { - lootdrops: 0, - timers: 0, - quests: 0 - }; - - try { - stats.lootdrops = await cleanupService.cleanupLootdrops(); - stats.timers = await cleanupService.cleanupTimers(); - stats.quests = await cleanupService.cleanupQuests(); - - console.log(`โœ… Cleanup finished. Stats: -- Lootdrops: ${stats.lootdrops} removed -- Timers: ${stats.timers} removed -- Quests: ${stats.quests} cleaned`); - } catch (error) { - console.error("โŒ Error during cleanup:", error); - } - }, - - /** - * Deletes unclaimed expired lootdrops - */ - cleanupLootdrops: async (): Promise => { - const now = new Date(); - const result = await DrizzleClient.delete(lootdrops) - .where(and( - lt(lootdrops.expiresAt, now), - isNull(lootdrops.claimedBy) - )) - .returning({ id: lootdrops.messageId }); - - return result.length; - }, - - /** - * Cleans up expired user timers and handles side effects (like role removal) - */ - cleanupTimers: async (): Promise => { - const now = new Date(); - let deletedCount = 0; - - // 1. Specific handling for ACCESS timers (revoking roles etc) - // This is migrated from scheduler.ts - const expiredAccess = await DrizzleClient.query.userTimers.findMany({ - where: and( - eq(userTimers.type, TimerType.ACCESS), - lt(userTimers.expiresAt, now) - ) - }); - - for (const timer of expiredAccess) { - const meta = timer.metadata as any; - const userIdStr = timer.userId.toString(); - - if (timer.key.startsWith('role_')) { - try { - const roleId = meta?.roleId || timer.key.replace('role_', ''); - const guildId = env.DISCORD_GUILD_ID; - - if (guildId) { - const guild = await AuroraClient.guilds.fetch(guildId); - const member = await guild.members.fetch(userIdStr); - await member.roles.remove(roleId); - console.log(`๐Ÿ‘‹ Removed temporary role ${roleId} from ${member.user.tag}`); - } - } catch (err) { - console.error(`Failed to remove role for user ${userIdStr}:`, err); - } - } - - // Delete specifically this one - await DrizzleClient.delete(userTimers) - .where(and( - eq(userTimers.userId, timer.userId), - eq(userTimers.type, timer.type), - eq(userTimers.key, timer.key) - )); - deletedCount++; - } - - // 2. Bulk delete all other expired timers (COOLDOWN, EFFECT, etc) - const result = await DrizzleClient.delete(userTimers) - .where(lt(userTimers.expiresAt, now)) - .returning({ userId: userTimers.userId }); - - deletedCount += result.length; - return deletedCount; - }, - - /** - * Deletes or archives old completed quests - */ - cleanupQuests: async (): Promise => { - const archiveDays = config.system.cleanup.questArchiveDays; - const threshold = new Date(); - threshold.setDate(threshold.getDate() - archiveDays); - - const result = await DrizzleClient.delete(userQuests) - .where(lt(userQuests.completedAt, threshold)) - .returning({ userId: userQuests.userId }); - - return result.length; - } -}; diff --git a/src/modules/system/scheduler.ts b/src/modules/system/scheduler.ts index 9dfc159..fda1215 100644 --- a/src/modules/system/scheduler.ts +++ b/src/modules/system/scheduler.ts @@ -1,32 +1,21 @@ -import { cleanupService } from "./cleanup.service"; -import { config } from "@/lib/config"; +import { temporaryRoleService } from "./temp-role.service"; -/** - * The Scheduler responsible for periodic tasks and system maintenance. - */ export const schedulerService = { start: () => { console.log("๐Ÿ•’ Scheduler started: Maintenance loops initialized."); - // 1. High-frequency timer cleanup (every 60s) - // This handles role revocations and cooldown expirations + // 1. Temporary Role Revocation (every 60s) setInterval(() => { - cleanupService.cleanupTimers(); + temporaryRoleService.processExpiredRoles(); }, 60 * 1000); - // 2. Scheduled system cleanup (configurable, default daily) - // This handles lootdrops, quests, etc. - setInterval(() => { - cleanupService.runAll(); - }, config.system.cleanup.intervalMs); - - // 3. Terminal Update Loop (every 60s) + // 2. Terminal Update Loop (every 60s) const { terminalService } = require("@/modules/terminal/terminal.service"); setInterval(() => { terminalService.update(); }, 60 * 1000); - // Run an initial cleanup on start for good measure - cleanupService.runAll(); + // Run an initial check on start + temporaryRoleService.processExpiredRoles(); } }; diff --git a/src/modules/system/temp-role.service.test.ts b/src/modules/system/temp-role.service.test.ts new file mode 100644 index 0000000..9d9e03a --- /dev/null +++ b/src/modules/system/temp-role.service.test.ts @@ -0,0 +1,114 @@ +import { describe, it, expect, mock, beforeEach } from "bun:test"; +import { temporaryRoleService } from "./temp-role.service"; +import { userTimers } from "@/db/schema"; +import { TimerType } from "@/lib/constants"; + +const mockDelete = mock(); +const mockWhere = mock(); +const mockFindMany = mock(); + +mockDelete.mockReturnValue({ where: mockWhere }); + +// Mock DrizzleClient +mock.module("@/lib/DrizzleClient", () => ({ + DrizzleClient: { + delete: mockDelete, + query: { + userTimers: { + findMany: mockFindMany + } + } + } +})); + +// Mock AuroraClient +const mockRemoveRole = mock(); +const mockFetchMember = mock(); +const mockFetchGuild = mock(); + +mock.module("@/lib/BotClient", () => ({ + AuroraClient: { + guilds: { + fetch: mockFetchGuild + } + } +})); + +mock.module("@/lib/env", () => ({ + env: { + DISCORD_GUILD_ID: "guild123" + } +})); + +describe("temporaryRoleService", () => { + beforeEach(() => { + mockDelete.mockClear(); + mockWhere.mockClear(); + mockFindMany.mockClear(); + mockRemoveRole.mockClear(); + mockFetchMember.mockClear(); + mockFetchGuild.mockClear(); + + mockFetchGuild.mockResolvedValue({ + members: { + fetch: mockFetchMember + } + }); + + mockFetchMember.mockResolvedValue({ + user: { tag: "TestUser#1234" }, + roles: { remove: mockRemoveRole } + }); + }); + + it("should revoke expired roles and delete timers", async () => { + // Mock findMany to return an expired role timer + mockFindMany.mockResolvedValue([ + { + userId: 123n, + type: TimerType.ACCESS, + key: 'role_456', + expiresAt: new Date(Date.now() - 1000), + metadata: { roleId: '456' } + } + ]); + + const count = await temporaryRoleService.processExpiredRoles(); + + expect(count).toBe(1); + expect(mockFetchGuild).toHaveBeenCalledWith("guild123"); + expect(mockFetchMember).toHaveBeenCalledWith("123"); + expect(mockRemoveRole).toHaveBeenCalledWith("456"); + expect(mockDelete).toHaveBeenCalledWith(userTimers); + }); + + it("should still delete the timer even if member is not found", async () => { + mockFindMany.mockResolvedValue([ + { + userId: 999n, + type: TimerType.ACCESS, + key: 'role_789', + expiresAt: new Date(Date.now() - 1000), + metadata: {} + } + ]); + + // Mock member fetch failure + mockFetchMember.mockRejectedValue(new Error("Member not found")); + + const count = await temporaryRoleService.processExpiredRoles(); + + expect(count).toBe(1); + expect(mockRemoveRole).not.toHaveBeenCalled(); + expect(mockDelete).toHaveBeenCalledWith(userTimers); + }); + + it("should return 0 if no expired timers exist", async () => { + mockFindMany.mockResolvedValue([]); + + const count = await temporaryRoleService.processExpiredRoles(); + + expect(count).toBe(0); + expect(mockDelete).not.toHaveBeenCalled(); + }); +}); diff --git a/src/modules/system/temp-role.service.ts b/src/modules/system/temp-role.service.ts new file mode 100644 index 0000000..9efa5b8 --- /dev/null +++ b/src/modules/system/temp-role.service.ts @@ -0,0 +1,67 @@ +import { userTimers } from "@/db/schema"; +import { eq, and, lt } from "drizzle-orm"; +import { DrizzleClient } from "@/lib/DrizzleClient"; +import { AuroraClient } from "@/lib/BotClient"; +import { env } from "@/lib/env"; +import { TimerType } from "@/lib/constants"; + +export const temporaryRoleService = { + /** + * Checks for and revokes expired temporary roles. + * This is intended to run as a high-frequency maintenance task. + */ + processExpiredRoles: async (): Promise => { + const now = new Date(); + let revokedCount = 0; + + // Find all expired ACCESS (temporary role) timers + const expiredTimers = await DrizzleClient.query.userTimers.findMany({ + where: and( + eq(userTimers.type, TimerType.ACCESS), + lt(userTimers.expiresAt, now) + ) + }); + + if (expiredTimers.length === 0) return 0; + + for (const timer of expiredTimers) { + const userIdStr = timer.userId.toString(); + const meta = timer.metadata as any; + + // We only handle keys that indicate role management + if (timer.key.startsWith('role_')) { + try { + const roleId = meta?.roleId || timer.key.replace('role_', ''); + const guildId = env.DISCORD_GUILD_ID; + + if (guildId) { + const guild = await AuroraClient.guilds.fetch(guildId); + const member = await guild.members.fetch(userIdStr).catch(() => null); + + if (member) { + await member.roles.remove(roleId); + console.log(`๐Ÿ‘‹ Temporary role ${roleId} revoked from ${member.user.tag} (Expired)`); + } else { + console.log(`โš ๏ธ Could not find member ${userIdStr} to revoke role ${roleId}.`); + } + } + } catch (err) { + console.error(`โŒ Failed to revoke role for user ${userIdStr}:`, err); + } + } + + // Always delete the timer record after trying to revoke (or if it's not a role key) + // to prevent repeated failed attempts. + await DrizzleClient.delete(userTimers) + .where(and( + eq(userTimers.userId, timer.userId), + eq(userTimers.type, timer.type), + eq(userTimers.key, timer.key) + )); + + revokedCount++; + } + + return revokedCount; + } +};