From 67db0c1cb36352d106ee4258950078d075b369c4 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 16 Mar 2026 09:07:30 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=97=91=EF=B8=8F=20chore:=20Remove=20Actio?= =?UTF-8?q?n=20Test=20Suite=20and=20Update=20Mock=20Implementations=20(#12?= =?UTF-8?q?268)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Deleted the Action test suite located in `api/models/Action.spec.js` to streamline the codebase. - Updated various test files to reflect changes in model mocks, consolidating mock implementations for user-related actions and enhancing clarity. - Improved consistency in test setups by aligning with the latest model updates and removing redundant mock definitions. --- api/models/Action.spec.js | 250 ------------------ .../controllers/__tests__/deleteUser.spec.js | 46 ++-- .../agents/filterAuthorizedTools.spec.js | 31 +-- api/server/controllers/agents/v1.js | 4 +- .../__test-utils__/convos-route-mocks.js | 5 + .../convos-duplicate-ratelimit.spec.js | 8 +- .../routes/__tests__/messages-delete.spec.js | 7 +- 7 files changed, 40 insertions(+), 311 deletions(-) delete mode 100644 api/models/Action.spec.js diff --git a/api/models/Action.spec.js b/api/models/Action.spec.js deleted file mode 100644 index 61a3b10f0f..0000000000 --- a/api/models/Action.spec.js +++ /dev/null @@ -1,250 +0,0 @@ -const mongoose = require('mongoose'); -const { MongoMemoryServer } = require('mongodb-memory-server'); -const { actionSchema } = require('@librechat/data-schemas'); -const { updateAction, getActions, deleteAction } = require('./Action'); - -let mongoServer; - -beforeAll(async () => { - mongoServer = await MongoMemoryServer.create(); - const mongoUri = mongoServer.getUri(); - if (!mongoose.models.Action) { - mongoose.model('Action', actionSchema); - } - await mongoose.connect(mongoUri); -}, 20000); - -afterAll(async () => { - await mongoose.disconnect(); - await mongoServer.stop(); -}); - -beforeEach(async () => { - await mongoose.models.Action.deleteMany({}); -}); - -const userId = new mongoose.Types.ObjectId(); - -describe('Action ownership scoping', () => { - describe('updateAction', () => { - it('updates when action_id and agent_id both match', async () => { - await mongoose.models.Action.create({ - user: userId, - action_id: 'act_1', - agent_id: 'agent_A', - metadata: { domain: 'example.com' }, - }); - - const result = await updateAction( - { action_id: 'act_1', agent_id: 'agent_A' }, - { metadata: { domain: 'updated.com' } }, - ); - - expect(result).not.toBeNull(); - expect(result.metadata.domain).toBe('updated.com'); - expect(result.agent_id).toBe('agent_A'); - }); - - it('does not update when agent_id does not match (creates a new doc via upsert)', async () => { - await mongoose.models.Action.create({ - user: userId, - action_id: 'act_1', - agent_id: 'agent_B', - metadata: { domain: 'victim.com', api_key: 'secret' }, - }); - - const result = await updateAction( - { action_id: 'act_1', agent_id: 'agent_A' }, - { user: userId, metadata: { domain: 'attacker.com' } }, - ); - - expect(result.metadata.domain).toBe('attacker.com'); - - const original = await mongoose.models.Action.findOne({ - action_id: 'act_1', - agent_id: 'agent_B', - }).lean(); - expect(original).not.toBeNull(); - expect(original.metadata.domain).toBe('victim.com'); - expect(original.metadata.api_key).toBe('secret'); - }); - - it('updates when action_id and assistant_id both match', async () => { - await mongoose.models.Action.create({ - user: userId, - action_id: 'act_2', - assistant_id: 'asst_X', - metadata: { domain: 'example.com' }, - }); - - const result = await updateAction( - { action_id: 'act_2', assistant_id: 'asst_X' }, - { metadata: { domain: 'updated.com' } }, - ); - - expect(result).not.toBeNull(); - expect(result.metadata.domain).toBe('updated.com'); - }); - - it('does not overwrite when assistant_id does not match', async () => { - await mongoose.models.Action.create({ - user: userId, - action_id: 'act_2', - assistant_id: 'asst_victim', - metadata: { domain: 'victim.com', api_key: 'secret' }, - }); - - await updateAction( - { action_id: 'act_2', assistant_id: 'asst_attacker' }, - { user: userId, metadata: { domain: 'attacker.com' } }, - ); - - const original = await mongoose.models.Action.findOne({ - action_id: 'act_2', - assistant_id: 'asst_victim', - }).lean(); - expect(original).not.toBeNull(); - expect(original.metadata.domain).toBe('victim.com'); - expect(original.metadata.api_key).toBe('secret'); - }); - }); - - describe('deleteAction', () => { - it('deletes when action_id and agent_id both match', async () => { - await mongoose.models.Action.create({ - user: userId, - action_id: 'act_del', - agent_id: 'agent_A', - metadata: { domain: 'example.com' }, - }); - - const result = await deleteAction({ action_id: 'act_del', agent_id: 'agent_A' }); - expect(result).not.toBeNull(); - expect(result.action_id).toBe('act_del'); - - const remaining = await mongoose.models.Action.countDocuments(); - expect(remaining).toBe(0); - }); - - it('returns null and preserves the document when agent_id does not match', async () => { - await mongoose.models.Action.create({ - user: userId, - action_id: 'act_del', - agent_id: 'agent_B', - metadata: { domain: 'victim.com' }, - }); - - const result = await deleteAction({ action_id: 'act_del', agent_id: 'agent_A' }); - expect(result).toBeNull(); - - const remaining = await mongoose.models.Action.countDocuments(); - expect(remaining).toBe(1); - }); - - it('deletes when action_id and assistant_id both match', async () => { - await mongoose.models.Action.create({ - user: userId, - action_id: 'act_del_asst', - assistant_id: 'asst_X', - metadata: { domain: 'example.com' }, - }); - - const result = await deleteAction({ action_id: 'act_del_asst', assistant_id: 'asst_X' }); - expect(result).not.toBeNull(); - - const remaining = await mongoose.models.Action.countDocuments(); - expect(remaining).toBe(0); - }); - - it('returns null and preserves the document when assistant_id does not match', async () => { - await mongoose.models.Action.create({ - user: userId, - action_id: 'act_del_asst', - assistant_id: 'asst_victim', - metadata: { domain: 'victim.com' }, - }); - - const result = await deleteAction({ - action_id: 'act_del_asst', - assistant_id: 'asst_attacker', - }); - expect(result).toBeNull(); - - const remaining = await mongoose.models.Action.countDocuments(); - expect(remaining).toBe(1); - }); - }); - - describe('getActions (unscoped baseline)', () => { - it('returns actions by action_id regardless of agent_id', async () => { - await mongoose.models.Action.create({ - user: userId, - action_id: 'act_shared', - agent_id: 'agent_B', - metadata: { domain: 'example.com' }, - }); - - const results = await getActions({ action_id: 'act_shared' }, true); - expect(results).toHaveLength(1); - expect(results[0].agent_id).toBe('agent_B'); - }); - - it('returns actions scoped by agent_id when provided', async () => { - await mongoose.models.Action.create({ - user: userId, - action_id: 'act_scoped', - agent_id: 'agent_A', - metadata: { domain: 'a.com' }, - }); - await mongoose.models.Action.create({ - user: userId, - action_id: 'act_other', - agent_id: 'agent_B', - metadata: { domain: 'b.com' }, - }); - - const results = await getActions({ agent_id: 'agent_A' }); - expect(results).toHaveLength(1); - expect(results[0].action_id).toBe('act_scoped'); - }); - }); - - describe('cross-type protection', () => { - it('updateAction with agent_id filter does not overwrite assistant-owned action', async () => { - await mongoose.models.Action.create({ - user: userId, - action_id: 'act_cross', - assistant_id: 'asst_victim', - metadata: { domain: 'victim.com', api_key: 'secret' }, - }); - - await updateAction( - { action_id: 'act_cross', agent_id: 'agent_attacker' }, - { user: userId, metadata: { domain: 'evil.com' } }, - ); - - const original = await mongoose.models.Action.findOne({ - action_id: 'act_cross', - assistant_id: 'asst_victim', - }).lean(); - expect(original).not.toBeNull(); - expect(original.metadata.domain).toBe('victim.com'); - expect(original.metadata.api_key).toBe('secret'); - }); - - it('deleteAction with agent_id filter does not delete assistant-owned action', async () => { - await mongoose.models.Action.create({ - user: userId, - action_id: 'act_cross_del', - assistant_id: 'asst_victim', - metadata: { domain: 'victim.com' }, - }); - - const result = await deleteAction({ action_id: 'act_cross_del', agent_id: 'agent_attacker' }); - expect(result).toBeNull(); - - const remaining = await mongoose.models.Action.countDocuments(); - expect(remaining).toBe(1); - }); - }); -}); diff --git a/api/server/controllers/__tests__/deleteUser.spec.js b/api/server/controllers/__tests__/deleteUser.spec.js index 6382cd1d8e..8dcd217657 100644 --- a/api/server/controllers/__tests__/deleteUser.spec.js +++ b/api/server/controllers/__tests__/deleteUser.spec.js @@ -35,6 +35,8 @@ jest.mock('@librechat/api', () => ({ MCPTokenStorage: {}, normalizeHttpError: jest.fn(), extractWebSearchEnvVars: jest.fn(), + needsRefresh: jest.fn(), + getNewS3URL: jest.fn(), })); jest.mock('~/models', () => ({ @@ -51,20 +53,19 @@ jest.mock('~/models', () => ({ updateUser: (...args) => mockUpdateUser(...args), findToken: (...args) => mockFindToken(...args), getFiles: (...args) => mockGetFiles(...args), -})); - -jest.mock('~/db/models', () => ({ - ConversationTag: { deleteMany: jest.fn() }, - AgentApiKey: { deleteMany: jest.fn() }, - Transaction: { deleteMany: jest.fn() }, - MemoryEntry: { deleteMany: jest.fn() }, - Assistant: { deleteMany: jest.fn() }, - AclEntry: { deleteMany: jest.fn() }, - Balance: { deleteMany: jest.fn() }, - Action: { deleteMany: jest.fn() }, - Group: { updateMany: jest.fn() }, - Token: { deleteMany: jest.fn() }, - User: {}, + deleteToolCalls: (...args) => mockDeleteToolCalls(...args), + deleteUserAgents: (...args) => mockDeleteUserAgents(...args), + deleteUserPrompts: (...args) => mockDeleteUserPrompts(...args), + deleteTransactions: jest.fn(), + deleteBalances: jest.fn(), + deleteAllAgentApiKeys: jest.fn(), + deleteAssistants: jest.fn(), + deleteConversationTags: jest.fn(), + deleteAllUserMemories: jest.fn(), + deleteActions: jest.fn(), + deleteTokens: jest.fn(), + removeUserFromAllGroups: jest.fn(), + deleteAclEntries: jest.fn(), })); jest.mock('~/server/services/PluginService', () => ({ @@ -91,11 +92,6 @@ jest.mock('~/server/services/Config/getCachedTools', () => ({ invalidateCachedTools: jest.fn(), })); -jest.mock('~/server/services/Files/S3/crud', () => ({ - needsRefresh: jest.fn(), - getNewS3URL: jest.fn(), -})); - jest.mock('~/server/services/Files/process', () => ({ processDeleteRequest: (...args) => mockProcessDeleteRequest(...args), })); @@ -108,18 +104,6 @@ jest.mock('~/server/services/PermissionService', () => ({ getSoleOwnedResourceIds: jest.fn().mockResolvedValue([]), })); -jest.mock('~/models/ToolCall', () => ({ - deleteToolCalls: (...args) => mockDeleteToolCalls(...args), -})); - -jest.mock('~/models/Prompt', () => ({ - deleteUserPrompts: (...args) => mockDeleteUserPrompts(...args), -})); - -jest.mock('~/models/Agent', () => ({ - deleteUserAgents: (...args) => mockDeleteUserAgents(...args), -})); - jest.mock('~/cache', () => ({ getLogStores: jest.fn(), })); diff --git a/api/server/controllers/agents/filterAuthorizedTools.spec.js b/api/server/controllers/agents/filterAuthorizedTools.spec.js index 259e41fb0d..e215fdc1fc 100644 --- a/api/server/controllers/agents/filterAuthorizedTools.spec.js +++ b/api/server/controllers/agents/filterAuthorizedTools.spec.js @@ -22,10 +22,6 @@ jest.mock('~/config', () => ({ })), })); -jest.mock('~/models/Project', () => ({ - getProjectByName: jest.fn().mockResolvedValue(null), -})); - jest.mock('~/server/services/Files/strategies', () => ({ getStrategyFunctions: jest.fn(), })); @@ -34,23 +30,10 @@ jest.mock('~/server/services/Files/images/avatar', () => ({ resizeAvatar: jest.fn(), })); -jest.mock('~/server/services/Files/S3/crud', () => ({ - refreshS3Url: jest.fn(), -})); - jest.mock('~/server/services/Files/process', () => ({ filterFile: jest.fn(), })); -jest.mock('~/models/Action', () => ({ - updateAction: jest.fn(), - getActions: jest.fn().mockResolvedValue([]), -})); - -jest.mock('~/models/File', () => ({ - deleteFileByFilter: jest.fn(), -})); - jest.mock('~/server/services/PermissionService', () => ({ findAccessibleResources: jest.fn().mockResolvedValue([]), findPubliclyAccessibleResources: jest.fn().mockResolvedValue([]), @@ -59,9 +42,17 @@ jest.mock('~/server/services/PermissionService', () => ({ checkPermission: jest.fn().mockResolvedValue(true), })); -jest.mock('~/models', () => ({ - getCategoriesWithCounts: jest.fn(), -})); +jest.mock('~/models', () => { + const mongoose = require('mongoose'); + const { createModels, createMethods } = require('@librechat/data-schemas'); + createModels(mongoose); + const methods = createMethods(mongoose); + return { + ...methods, + getCategoriesWithCounts: jest.fn(), + deleteFileByFilter: jest.fn(), + }; +}); jest.mock('~/cache', () => ({ getLogStores: jest.fn(() => ({ diff --git a/api/server/controllers/agents/v1.js b/api/server/controllers/agents/v1.js index b6eb4fc22c..17985f97ce 100644 --- a/api/server/controllers/agents/v1.js +++ b/api/server/controllers/agents/v1.js @@ -66,9 +66,7 @@ const validateEdgeAgentAccess = async (edges, userId, userRole) => { return []; } - const agents = (await Promise.all([...edgeAgentIds].map((id) => getAgent({ id })))).filter( - Boolean, - ); + const agents = await db.getAgents({ id: { $in: [...edgeAgentIds] } }); if (agents.length === 0) { return []; diff --git a/api/server/routes/__test-utils__/convos-route-mocks.js b/api/server/routes/__test-utils__/convos-route-mocks.js index f89b77db3f..0929e0759d 100644 --- a/api/server/routes/__test-utils__/convos-route-mocks.js +++ b/api/server/routes/__test-utils__/convos-route-mocks.js @@ -48,8 +48,13 @@ module.exports = { toolCallModel: () => ({ deleteToolCalls: jest.fn() }), sharedModels: () => ({ + getConvosByCursor: jest.fn(), + getConvo: jest.fn(), + deleteConvos: jest.fn(), + saveConvo: jest.fn(), deleteAllSharedLinks: jest.fn(), deleteConvoSharedLink: jest.fn(), + deleteToolCalls: jest.fn(), }), requireJwtAuth: () => (req, res, next) => next(), diff --git a/api/server/routes/__tests__/convos-duplicate-ratelimit.spec.js b/api/server/routes/__tests__/convos-duplicate-ratelimit.spec.js index 788119a569..a75c11ccba 100644 --- a/api/server/routes/__tests__/convos-duplicate-ratelimit.spec.js +++ b/api/server/routes/__tests__/convos-duplicate-ratelimit.spec.js @@ -12,9 +12,11 @@ jest.mock('librechat-data-provider', () => jest.mock('~/cache/logViolation', () => jest.fn().mockResolvedValue(undefined)); jest.mock('~/cache/getLogStores', () => require(MOCKS).logStores()); -jest.mock('~/models/Conversation', () => require(MOCKS).conversationModel()); -jest.mock('~/models/ToolCall', () => require(MOCKS).toolCallModel()); -jest.mock('~/models', () => require(MOCKS).sharedModels()); +jest.mock('~/models', () => ({ + ...require(MOCKS).sharedModels(), + ...require(MOCKS).conversationModel(), + ...require(MOCKS).toolCallModel(), +})); jest.mock('~/server/middleware/requireJwtAuth', () => require(MOCKS).requireJwtAuth()); jest.mock('~/server/middleware', () => { diff --git a/api/server/routes/__tests__/messages-delete.spec.js b/api/server/routes/__tests__/messages-delete.spec.js index e134eecfd0..714d497719 100644 --- a/api/server/routes/__tests__/messages-delete.spec.js +++ b/api/server/routes/__tests__/messages-delete.spec.js @@ -34,6 +34,9 @@ jest.mock('~/models', () => ({ getMessages: jest.fn(), updateMessage: jest.fn(), deleteMessages: jest.fn(), + getConvosQueried: jest.fn(), + searchMessages: jest.fn(), + getMessagesByCursor: jest.fn(), })); jest.mock('~/server/services/Artifacts/update', () => ({ @@ -48,10 +51,6 @@ jest.mock('~/server/middleware', () => ({ validateMessageReq: (req, res, next) => next(), })); -jest.mock('~/models/Conversation', () => ({ - getConvosQueried: jest.fn(), -})); - jest.mock('~/db/models', () => ({ Message: { findOne: jest.fn(),