mirror of
https://github.com/danny-avila/LibreChat.git
synced 2025-09-22 06:00:56 +02:00
🔧 fix: Remove Unused Duplicate Group Methods (#9091)
This commit is contained in:
Danny Avila
GitHub
parent
3547873bc4
commit
cc0cf359a2
4 changed files with 15 additions and 504 deletions
|
|
@ -1,345 +0,0 @@
|
|||
import mongoose from 'mongoose';
|
||||
import { MongoMemoryServer } from 'mongodb-memory-server';
|
||||
import { createGroupMethods } from './group';
|
||||
import groupSchema from '~/schema/group';
|
||||
import type * as t from '~/types';
|
||||
|
||||
let mongoServer: MongoMemoryServer;
|
||||
let Group: mongoose.Model<t.IGroup>;
|
||||
let methods: ReturnType<typeof createGroupMethods>;
|
||||
|
||||
beforeAll(async () => {
|
||||
mongoServer = await MongoMemoryServer.create();
|
||||
const mongoUri = mongoServer.getUri();
|
||||
Group = mongoose.models.Group || mongoose.model('Group', groupSchema);
|
||||
methods = createGroupMethods(mongoose);
|
||||
await mongoose.connect(mongoUri);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await mongoose.disconnect();
|
||||
await mongoServer.stop();
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
await mongoose.connection.dropDatabase();
|
||||
await Group.ensureIndexes();
|
||||
});
|
||||
|
||||
describe('Group Model Tests', () => {
|
||||
test('should create a new group with valid data', async () => {
|
||||
const groupData: t.Group = {
|
||||
name: 'Test Group',
|
||||
source: 'local',
|
||||
memberIds: [],
|
||||
};
|
||||
|
||||
const group = await methods.createGroup(groupData);
|
||||
|
||||
expect(group).toBeDefined();
|
||||
expect(group._id).toBeDefined();
|
||||
expect(group.name).toBe(groupData.name);
|
||||
expect(group.source).toBe(groupData.source);
|
||||
expect(group.memberIds).toEqual([]);
|
||||
});
|
||||
|
||||
test('should create a group with members', async () => {
|
||||
const userId1 = new mongoose.Types.ObjectId();
|
||||
const userId2 = new mongoose.Types.ObjectId();
|
||||
|
||||
const groupData: t.Group = {
|
||||
name: 'Test Group with Members',
|
||||
source: 'local',
|
||||
memberIds: [userId1.toString(), userId2.toString()],
|
||||
};
|
||||
|
||||
const group = await methods.createGroup(groupData);
|
||||
|
||||
expect(group).toBeDefined();
|
||||
expect(group.memberIds).toHaveLength(2);
|
||||
expect(group.memberIds[0]).toBe(userId1.toString());
|
||||
expect(group.memberIds[1]).toBe(userId2.toString());
|
||||
});
|
||||
|
||||
test('should create an Entra ID group', async () => {
|
||||
const groupData: t.Group = {
|
||||
name: 'Entra Group',
|
||||
source: 'entra',
|
||||
idOnTheSource: 'entra-id-12345',
|
||||
memberIds: [],
|
||||
};
|
||||
|
||||
const group = await methods.createGroup(groupData);
|
||||
|
||||
expect(group).toBeDefined();
|
||||
expect(group.source).toBe('entra');
|
||||
expect(group.idOnTheSource).toBe(groupData.idOnTheSource);
|
||||
});
|
||||
|
||||
test('should fail when creating an Entra group without idOnTheSource', async () => {
|
||||
const groupData = {
|
||||
name: 'Invalid Entra Group',
|
||||
source: 'entra' as const,
|
||||
memberIds: [],
|
||||
/** Missing idOnTheSource */
|
||||
};
|
||||
|
||||
await expect(methods.createGroup(groupData)).rejects.toThrow();
|
||||
});
|
||||
|
||||
test('should fail when creating a group with an invalid source', async () => {
|
||||
const groupData = {
|
||||
name: 'Invalid Source Group',
|
||||
source: 'invalid_source' as 'local',
|
||||
memberIds: [],
|
||||
};
|
||||
|
||||
await expect(methods.createGroup(groupData)).rejects.toThrow();
|
||||
});
|
||||
|
||||
test('should fail when creating a group without a name', async () => {
|
||||
const groupData = {
|
||||
source: 'local' as const,
|
||||
memberIds: [],
|
||||
/** Missing name */
|
||||
};
|
||||
|
||||
await expect(methods.createGroup(groupData)).rejects.toThrow();
|
||||
});
|
||||
|
||||
test('should enforce unique idOnTheSource for same source', async () => {
|
||||
const groupData1: t.Group = {
|
||||
name: 'First Entra Group',
|
||||
source: 'entra',
|
||||
idOnTheSource: 'duplicate-id',
|
||||
memberIds: [],
|
||||
};
|
||||
|
||||
const groupData2: t.Group = {
|
||||
name: 'Second Entra Group',
|
||||
source: 'entra',
|
||||
idOnTheSource: 'duplicate-id' /** Same as above */,
|
||||
memberIds: [],
|
||||
};
|
||||
|
||||
await methods.createGroup(groupData1);
|
||||
await expect(methods.createGroup(groupData2)).rejects.toThrow();
|
||||
});
|
||||
|
||||
test('should not enforce unique idOnTheSource across different sources', async () => {
|
||||
/** This test is hypothetical as we currently only have 'local' and 'entra' sources,
|
||||
* and 'local' doesn't require idOnTheSource
|
||||
*/
|
||||
const groupData1: t.Group = {
|
||||
name: 'Entra Group',
|
||||
source: 'entra',
|
||||
idOnTheSource: 'test-id',
|
||||
memberIds: [],
|
||||
};
|
||||
|
||||
/** Simulate a future source type */
|
||||
const groupData2: t.Group = {
|
||||
name: 'Other Source Group',
|
||||
source: 'local',
|
||||
idOnTheSource: 'test-id' /** Same as above but different source */,
|
||||
memberIds: [],
|
||||
};
|
||||
|
||||
await methods.createGroup(groupData1);
|
||||
|
||||
/** This should succeed because the uniqueness constraint includes both idOnTheSource and source */
|
||||
const group2 = await methods.createGroup(groupData2);
|
||||
expect(group2).toBeDefined();
|
||||
expect(group2.source).toBe('local');
|
||||
expect(group2.idOnTheSource).toBe(groupData2.idOnTheSource);
|
||||
});
|
||||
|
||||
describe('Group Query Methods', () => {
|
||||
let testGroup: t.IGroup;
|
||||
|
||||
beforeEach(async () => {
|
||||
testGroup = await methods.createGroup({
|
||||
name: 'Test Group',
|
||||
source: 'local',
|
||||
memberIds: ['user-123'],
|
||||
});
|
||||
});
|
||||
|
||||
test('should find group by ID', async () => {
|
||||
const group = await methods.findGroupById(testGroup._id);
|
||||
|
||||
expect(group).toBeDefined();
|
||||
expect(group?._id.toString()).toBe(testGroup._id.toString());
|
||||
expect(group?.name).toBe(testGroup.name);
|
||||
});
|
||||
|
||||
test('should return null for non-existent group ID', async () => {
|
||||
const nonExistentId = new mongoose.Types.ObjectId();
|
||||
const group = await methods.findGroupById(nonExistentId);
|
||||
expect(group).toBeNull();
|
||||
});
|
||||
|
||||
test('should find group by external ID', async () => {
|
||||
const entraGroup = await methods.createGroup({
|
||||
name: 'Entra Group',
|
||||
source: 'entra',
|
||||
idOnTheSource: 'entra-id-xyz',
|
||||
memberIds: [],
|
||||
});
|
||||
|
||||
const found = await methods.findGroupByExternalId('entra-id-xyz', 'entra');
|
||||
expect(found).toBeDefined();
|
||||
expect(found?._id.toString()).toBe(entraGroup._id.toString());
|
||||
});
|
||||
|
||||
test('should find groups by source', async () => {
|
||||
await methods.createGroup({
|
||||
name: 'Another Local Group',
|
||||
source: 'local',
|
||||
memberIds: [],
|
||||
});
|
||||
|
||||
await methods.createGroup({
|
||||
name: 'Entra Group',
|
||||
source: 'entra',
|
||||
idOnTheSource: 'entra-123',
|
||||
memberIds: [],
|
||||
});
|
||||
|
||||
const localGroups = await methods.findGroupsBySource('local');
|
||||
expect(localGroups).toHaveLength(2);
|
||||
|
||||
const entraGroups = await methods.findGroupsBySource('entra');
|
||||
expect(entraGroups).toHaveLength(1);
|
||||
});
|
||||
|
||||
test('should get all groups', async () => {
|
||||
await methods.createGroup({
|
||||
name: 'Group 2',
|
||||
source: 'local',
|
||||
memberIds: [],
|
||||
});
|
||||
|
||||
await methods.createGroup({
|
||||
name: 'Group 3',
|
||||
source: 'entra',
|
||||
idOnTheSource: 'entra-456',
|
||||
memberIds: [],
|
||||
});
|
||||
|
||||
const allGroups = await methods.getAllGroups();
|
||||
expect(allGroups).toHaveLength(3);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Group Update and Delete Methods', () => {
|
||||
let testGroup: t.IGroup;
|
||||
|
||||
beforeEach(async () => {
|
||||
testGroup = await methods.createGroup({
|
||||
name: 'Original Name',
|
||||
source: 'local',
|
||||
memberIds: [],
|
||||
});
|
||||
});
|
||||
|
||||
test('should update a group', async () => {
|
||||
const updateData = {
|
||||
name: 'Updated Name',
|
||||
description: 'New description',
|
||||
};
|
||||
|
||||
const updated = await methods.updateGroup(testGroup._id, updateData);
|
||||
|
||||
expect(updated).toBeDefined();
|
||||
expect(updated?.name).toBe(updateData.name);
|
||||
expect(updated?.description).toBe(updateData.description);
|
||||
expect(updated?.source).toBe(testGroup.source); /** Unchanged */
|
||||
});
|
||||
|
||||
test('should delete a group', async () => {
|
||||
const result = await methods.deleteGroup(testGroup._id);
|
||||
expect(result.deletedCount).toBe(1);
|
||||
|
||||
const found = await methods.findGroupById(testGroup._id);
|
||||
expect(found).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Group Member Management', () => {
|
||||
let testGroup: t.IGroup;
|
||||
|
||||
beforeEach(async () => {
|
||||
testGroup = await methods.createGroup({
|
||||
name: 'Member Test Group',
|
||||
source: 'local',
|
||||
memberIds: [],
|
||||
});
|
||||
});
|
||||
|
||||
test('should add a member to a group', async () => {
|
||||
const memberId = 'user-456';
|
||||
const updated = await methods.addMemberToGroup(testGroup._id, memberId);
|
||||
|
||||
expect(updated).toBeDefined();
|
||||
expect(updated?.memberIds).toContain(memberId);
|
||||
expect(updated?.memberIds).toHaveLength(1);
|
||||
});
|
||||
|
||||
test('should not duplicate members when adding', async () => {
|
||||
const memberId = 'user-789';
|
||||
|
||||
/** Add the same member twice */
|
||||
await methods.addMemberToGroup(testGroup._id, memberId);
|
||||
const updated = await methods.addMemberToGroup(testGroup._id, memberId);
|
||||
|
||||
expect(updated?.memberIds).toHaveLength(1);
|
||||
expect(updated?.memberIds[0]).toBe(memberId);
|
||||
});
|
||||
|
||||
test('should remove a member from a group', async () => {
|
||||
const memberId = 'user-999';
|
||||
|
||||
/** First add the member */
|
||||
await methods.addMemberToGroup(testGroup._id, memberId);
|
||||
|
||||
/** Then remove them */
|
||||
const updated = await methods.removeMemberFromGroup(testGroup._id, memberId);
|
||||
|
||||
expect(updated).toBeDefined();
|
||||
expect(updated?.memberIds).not.toContain(memberId);
|
||||
expect(updated?.memberIds).toHaveLength(0);
|
||||
});
|
||||
|
||||
test('should find groups by member ID', async () => {
|
||||
const memberId = 'shared-user-123';
|
||||
|
||||
/** Create multiple groups with the same member */
|
||||
const group1 = await methods.createGroup({
|
||||
name: 'Group 1',
|
||||
source: 'local',
|
||||
memberIds: [memberId],
|
||||
});
|
||||
|
||||
const group2 = await methods.createGroup({
|
||||
name: 'Group 2',
|
||||
source: 'local',
|
||||
memberIds: [memberId, 'other-user'],
|
||||
});
|
||||
|
||||
/** Create a group without the member */
|
||||
await methods.createGroup({
|
||||
name: 'Group 3',
|
||||
source: 'local',
|
||||
memberIds: ['different-user'],
|
||||
});
|
||||
|
||||
const memberGroups = await methods.findGroupsByMemberId(memberId);
|
||||
expect(memberGroups).toHaveLength(2);
|
||||
|
||||
const groupIds = memberGroups.map((g) => g._id.toString());
|
||||
expect(groupIds).toContain(group1._id.toString());
|
||||
expect(groupIds).toContain(group2._id.toString());
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -1,142 +0,0 @@
|
|||
import type { Model, Types, DeleteResult } from 'mongoose';
|
||||
import type { IGroup } from '~/types';
|
||||
|
||||
export function createGroupMethods(mongoose: typeof import('mongoose')) {
|
||||
/**
|
||||
* Find a group by its ID
|
||||
* @param groupId - The group ID
|
||||
* @returns The group document or null if not found
|
||||
*/
|
||||
async function findGroupById(groupId: string | Types.ObjectId): Promise<IGroup | null> {
|
||||
const Group = mongoose.models.Group as Model<IGroup>;
|
||||
return await Group.findById(groupId).lean();
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a new group
|
||||
* @param groupData - Group data including name, source, and optional fields
|
||||
* @returns The created group
|
||||
*/
|
||||
async function createGroup(groupData: Partial<IGroup>): Promise<IGroup> {
|
||||
const Group = mongoose.models.Group as Model<IGroup>;
|
||||
return await Group.create(groupData);
|
||||
}
|
||||
|
||||
/**
|
||||
* Update an existing group
|
||||
* @param groupId - The ID of the group to update
|
||||
* @param updateData - Data to update
|
||||
* @returns The updated group document or null if not found
|
||||
*/
|
||||
async function updateGroup(
|
||||
groupId: string | Types.ObjectId,
|
||||
updateData: Partial<IGroup>,
|
||||
): Promise<IGroup | null> {
|
||||
const Group = mongoose.models.Group as Model<IGroup>;
|
||||
return await Group.findByIdAndUpdate(groupId, { $set: updateData }, { new: true }).lean();
|
||||
}
|
||||
|
||||
/**
|
||||
* Delete a group
|
||||
* @param groupId - The ID of the group to delete
|
||||
* @returns The result of the delete operation
|
||||
*/
|
||||
async function deleteGroup(groupId: string | Types.ObjectId): Promise<DeleteResult> {
|
||||
const Group = mongoose.models.Group as Model<IGroup>;
|
||||
return await Group.deleteOne({ _id: groupId });
|
||||
}
|
||||
|
||||
/**
|
||||
* Find all groups
|
||||
* @returns Array of all group documents
|
||||
*/
|
||||
async function getAllGroups(): Promise<IGroup[]> {
|
||||
const Group = mongoose.models.Group as Model<IGroup>;
|
||||
return await Group.find().lean();
|
||||
}
|
||||
|
||||
/**
|
||||
* Find groups by source
|
||||
* @param source - The source ('local' or 'entra')
|
||||
* @returns Array of group documents
|
||||
*/
|
||||
async function findGroupsBySource(source: 'local' | 'entra'): Promise<IGroup[]> {
|
||||
const Group = mongoose.models.Group as Model<IGroup>;
|
||||
return await Group.find({ source }).lean();
|
||||
}
|
||||
|
||||
/**
|
||||
* Find a group by its external ID
|
||||
* @param idOnTheSource - The external ID
|
||||
* @param source - The source ('entra' or 'local')
|
||||
* @returns The group document or null if not found
|
||||
*/
|
||||
async function findGroupByExternalId(
|
||||
idOnTheSource: string,
|
||||
source: 'local' | 'entra' = 'entra',
|
||||
): Promise<IGroup | null> {
|
||||
const Group = mongoose.models.Group as Model<IGroup>;
|
||||
return await Group.findOne({ idOnTheSource, source }).lean();
|
||||
}
|
||||
|
||||
/**
|
||||
* Add a member to a group
|
||||
* @param groupId - The group ID
|
||||
* @param memberId - The member ID to add (idOnTheSource value)
|
||||
* @returns The updated group or null if not found
|
||||
*/
|
||||
async function addMemberToGroup(
|
||||
groupId: string | Types.ObjectId,
|
||||
memberId: string,
|
||||
): Promise<IGroup | null> {
|
||||
const Group = mongoose.models.Group as Model<IGroup>;
|
||||
return await Group.findByIdAndUpdate(
|
||||
groupId,
|
||||
{ $addToSet: { memberIds: memberId } },
|
||||
{ new: true },
|
||||
).lean();
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove a member from a group
|
||||
* @param groupId - The group ID
|
||||
* @param memberId - The member ID to remove (idOnTheSource value)
|
||||
* @returns The updated group or null if not found
|
||||
*/
|
||||
async function removeMemberFromGroup(
|
||||
groupId: string | Types.ObjectId,
|
||||
memberId: string,
|
||||
): Promise<IGroup | null> {
|
||||
const Group = mongoose.models.Group as Model<IGroup>;
|
||||
return await Group.findByIdAndUpdate(
|
||||
groupId,
|
||||
{ $pull: { memberIds: memberId } },
|
||||
{ new: true },
|
||||
).lean();
|
||||
}
|
||||
|
||||
/**
|
||||
* Find all groups that contain a specific member
|
||||
* @param memberId - The member ID (idOnTheSource value)
|
||||
* @returns Array of groups containing the member
|
||||
*/
|
||||
async function findGroupsByMemberId(memberId: string): Promise<IGroup[]> {
|
||||
const Group = mongoose.models.Group as Model<IGroup>;
|
||||
return await Group.find({ memberIds: memberId }).lean();
|
||||
}
|
||||
|
||||
return {
|
||||
createGroup,
|
||||
updateGroup,
|
||||
deleteGroup,
|
||||
getAllGroups,
|
||||
findGroupById,
|
||||
addMemberToGroup,
|
||||
findGroupsBySource,
|
||||
removeMemberFromGroup,
|
||||
findGroupsByMemberId,
|
||||
findGroupByExternalId,
|
||||
};
|
||||
}
|
||||
|
||||
export type GroupMethods = ReturnType<typeof createGroupMethods>;
|
||||
|
|
@ -12,13 +12,24 @@ import { createPluginAuthMethods, type PluginAuthMethods } from './pluginAuth';
|
|||
import { createAccessRoleMethods, type AccessRoleMethods } from './accessRole';
|
||||
import { createUserGroupMethods, type UserGroupMethods } from './userGroup';
|
||||
import { createAclEntryMethods, type AclEntryMethods } from './aclEntry';
|
||||
import { createGroupMethods, type GroupMethods } from './group';
|
||||
import { createShareMethods, type ShareMethods } from './share';
|
||||
|
||||
export type AllMethods = UserMethods &
|
||||
SessionMethods &
|
||||
TokenMethods &
|
||||
RoleMethods &
|
||||
MemoryMethods &
|
||||
AgentCategoryMethods &
|
||||
UserGroupMethods &
|
||||
AclEntryMethods &
|
||||
ShareMethods &
|
||||
AccessRoleMethods &
|
||||
PluginAuthMethods;
|
||||
|
||||
/**
|
||||
* Creates all database methods for all collections
|
||||
*/
|
||||
export function createMethods(mongoose: typeof import('mongoose')) {
|
||||
export function createMethods(mongoose: typeof import('mongoose')): AllMethods {
|
||||
return {
|
||||
...createUserMethods(mongoose),
|
||||
...createSessionMethods(mongoose),
|
||||
|
|
@ -29,7 +40,6 @@ export function createMethods(mongoose: typeof import('mongoose')) {
|
|||
...createAccessRoleMethods(mongoose),
|
||||
...createUserGroupMethods(mongoose),
|
||||
...createAclEntryMethods(mongoose),
|
||||
...createGroupMethods(mongoose),
|
||||
...createShareMethods(mongoose),
|
||||
...createPluginAuthMethods(mongoose),
|
||||
};
|
||||
|
|
@ -44,20 +54,7 @@ export type {
|
|||
AgentCategoryMethods,
|
||||
UserGroupMethods,
|
||||
AclEntryMethods,
|
||||
GroupMethods,
|
||||
ShareMethods,
|
||||
AccessRoleMethods,
|
||||
PluginAuthMethods,
|
||||
};
|
||||
export type AllMethods = UserMethods &
|
||||
SessionMethods &
|
||||
TokenMethods &
|
||||
RoleMethods &
|
||||
MemoryMethods &
|
||||
AgentCategoryMethods &
|
||||
UserGroupMethods &
|
||||
AclEntryMethods &
|
||||
GroupMethods &
|
||||
ShareMethods &
|
||||
AccessRoleMethods &
|
||||
PluginAuthMethods;
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import { Document } from 'mongoose';
|
||||
import type { Document, Types } from 'mongoose';
|
||||
|
||||
export interface IGroup extends Document {
|
||||
_id: Types.ObjectId;
|
||||
/** The name of the group */
|
||||
name: string;
|
||||
/** Optional description of the group */
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue