diff --git a/src/services/dataService.ts b/src/services/dataService.ts index 0905008..b41ec77 100644 --- a/src/services/dataService.ts +++ b/src/services/dataService.ts @@ -22,7 +22,19 @@ export class DataServiceImpl implements DataService { } mergeSettings(all: McpSettings, newSettings: McpSettings, _user?: IUser): McpSettings { - return newSettings; + // Merge all fields from newSettings into all, preserving fields not present in newSettings + return { + ...all, + ...newSettings, + // Ensure arrays and objects are properly handled + users: newSettings.users !== undefined ? newSettings.users : all.users, + mcpServers: newSettings.mcpServers !== undefined ? newSettings.mcpServers : all.mcpServers, + groups: newSettings.groups !== undefined ? newSettings.groups : all.groups, + systemConfig: + newSettings.systemConfig !== undefined ? newSettings.systemConfig : all.systemConfig, + userConfigs: + newSettings.userConfigs !== undefined ? newSettings.userConfigs : all.userConfigs, + }; } getPermissions(_user: IUser): string[] { diff --git a/src/services/dataServicex.ts b/src/services/dataServicex.ts index 0dfd5f7..a0a3984 100644 --- a/src/services/dataServicex.ts +++ b/src/services/dataServicex.ts @@ -36,11 +36,17 @@ export class DataServicex implements DataService { // Use passed user parameter if available, otherwise fall back to context const currentUser = user || UserContextService.getInstance().getCurrentUser(); if (!currentUser || currentUser.isAdmin) { + // Admin users can modify all settings const result = { ...all }; - result.users = newSettings.users; - result.systemConfig = newSettings.systemConfig; + // Merge all fields, using newSettings values when present + if (newSettings.users !== undefined) result.users = newSettings.users; + if (newSettings.mcpServers !== undefined) result.mcpServers = newSettings.mcpServers; + if (newSettings.groups !== undefined) result.groups = newSettings.groups; + if (newSettings.systemConfig !== undefined) result.systemConfig = newSettings.systemConfig; + if (newSettings.userConfigs !== undefined) result.userConfigs = newSettings.userConfigs; return result; } else { + // Non-admin users can only modify their own userConfig const result = JSON.parse(JSON.stringify(all)); if (!result.userConfigs) { result.userConfigs = {}; diff --git a/tests/services/dataServiceMerge.test.ts b/tests/services/dataServiceMerge.test.ts new file mode 100644 index 0000000..bfedd2e --- /dev/null +++ b/tests/services/dataServiceMerge.test.ts @@ -0,0 +1,225 @@ +import { DataServiceImpl } from '../../src/services/dataService.js'; +import { DataServicex } from '../../src/services/dataServicex.js'; +import { McpSettings, IUser } from '../../src/types/index.js'; + +describe('DataService mergeSettings', () => { + describe('DataServiceImpl', () => { + let service: DataServiceImpl; + + beforeEach(() => { + service = new DataServiceImpl(); + }); + + it('should merge all fields from newSettings into existing settings', () => { + const all: McpSettings = { + users: [ + { username: 'admin', password: 'hash1', isAdmin: true }, + { username: 'user1', password: 'hash2', isAdmin: false }, + ], + mcpServers: { + 'server1': { command: 'cmd1', args: [] }, + 'server2': { command: 'cmd2', args: [] }, + }, + groups: [ + { id: '1', name: 'group1', servers: [], owner: 'admin' }, + ], + systemConfig: { + routing: { enableGlobalRoute: true, enableGroupNameRoute: true }, + }, + userConfigs: { + user1: { routing: { enableGlobalRoute: false, enableGroupNameRoute: false } }, + }, + }; + + const newSettings: McpSettings = { + mcpServers: {}, + groups: [ + { id: '1', name: 'group1', servers: [], owner: 'admin' }, + { id: '2', name: 'group2', servers: [], owner: 'admin' }, + ], + }; + + const result = service.mergeSettings(all, newSettings); + + // New groups should be present + expect(result.groups).toHaveLength(2); + expect(result.groups).toEqual(newSettings.groups); + + // Other fields from 'all' should be preserved when not in newSettings + expect(result.users).toEqual(all.users); + expect(result.systemConfig).toEqual(all.systemConfig); + expect(result.userConfigs).toEqual(all.userConfigs); + }); + + it('should preserve fields not present in newSettings', () => { + const all: McpSettings = { + users: [{ username: 'admin', password: 'hash', isAdmin: true }], + mcpServers: { + 'server1': { command: 'cmd1', args: [] }, + }, + groups: [{ id: '1', name: 'group1', servers: [], owner: 'admin' }], + systemConfig: { routing: { enableGlobalRoute: true, enableGroupNameRoute: true } }, + }; + + const newSettings: McpSettings = { + mcpServers: {}, + groups: [ + { id: '1', name: 'group1', servers: [], owner: 'admin' }, + { id: '2', name: 'group2', servers: [], owner: 'admin' }, + ], + }; + + const result = service.mergeSettings(all, newSettings); + + // Groups from newSettings should be present + expect(result.groups).toEqual(newSettings.groups); + + // Other fields should be preserved from 'all' + expect(result.users).toEqual(all.users); + expect(result.systemConfig).toEqual(all.systemConfig); + }); + + it('should handle undefined fields in newSettings', () => { + const all: McpSettings = { + users: [{ username: 'admin', password: 'hash', isAdmin: true }], + mcpServers: { 'server1': { command: 'cmd1', args: [] } }, + groups: [{ id: '1', name: 'group1', servers: [], owner: 'admin' }], + }; + + const newSettings: McpSettings = { + mcpServers: {}, + // groups is undefined + }; + + const result = service.mergeSettings(all, newSettings); + + // Groups from 'all' should be preserved since newSettings.groups is undefined + expect(result.groups).toEqual(all.groups); + expect(result.users).toEqual(all.users); + }); + }); + + describe('DataServicex', () => { + let service: DataServicex; + + beforeEach(() => { + service = new DataServicex(); + }); + + it('should merge all fields for admin users', () => { + const adminUser: IUser = { username: 'admin', password: 'hash', isAdmin: true }; + + const all: McpSettings = { + users: [adminUser], + mcpServers: { + 'server1': { command: 'cmd1', args: [] }, + }, + groups: [{ id: '1', name: 'group1', servers: [], owner: 'admin' }], + systemConfig: { routing: { enableGlobalRoute: true, enableGroupNameRoute: true } }, + }; + + const newSettings: McpSettings = { + mcpServers: {}, + groups: [ + { id: '1', name: 'group1', servers: [], owner: 'admin' }, + { id: '2', name: 'group2', servers: [], owner: 'admin' }, + ], + systemConfig: { routing: { enableGlobalRoute: false, enableGroupNameRoute: false } }, + }; + + const result = service.mergeSettings(all, newSettings, adminUser); + + // All fields from newSettings should be merged + expect(result.groups).toEqual(newSettings.groups); + expect(result.systemConfig).toEqual(newSettings.systemConfig); + + // Users should be preserved from 'all' since not in newSettings + expect(result.users).toEqual(all.users); + }); + + it('should preserve groups for admin users when adding new groups', () => { + const adminUser: IUser = { username: 'admin', password: 'hash', isAdmin: true }; + + const all: McpSettings = { + users: [adminUser], + mcpServers: { 'server1': { command: 'cmd1', args: [] } }, + groups: [{ id: '1', name: 'group1', servers: [], owner: 'admin' }], + }; + + const newSettings: McpSettings = { + mcpServers: {}, + groups: [ + { id: '1', name: 'group1', servers: [], owner: 'admin' }, + { id: '2', name: 'group2', servers: [], owner: 'admin' }, + ], + }; + + const result = service.mergeSettings(all, newSettings, adminUser); + + // New groups should be present + expect(result.groups).toHaveLength(2); + expect(result.groups).toEqual(newSettings.groups); + }); + + it('should handle non-admin users correctly', () => { + const regularUser: IUser = { username: 'user1', password: 'hash', isAdmin: false }; + + const all: McpSettings = { + users: [regularUser], + mcpServers: { 'server1': { command: 'cmd1', args: [] } }, + groups: [{ id: '1', name: 'group1', servers: [], owner: 'admin' }], + userConfigs: {}, + }; + + const newSettings: McpSettings = { + mcpServers: {}, + systemConfig: { + routing: { + enableGlobalRoute: false, + enableGroupNameRoute: false, + enableBearerAuth: true, + bearerAuthKey: 'test-key', + }, + }, + }; + + const result = service.mergeSettings(all, newSettings, regularUser); + + // For non-admin users, groups should not change + expect(result.groups).toEqual(all.groups); + + // User config should be updated + expect(result.userConfigs).toBeDefined(); + expect(result.userConfigs?.['user1']).toBeDefined(); + expect(result.userConfigs?.['user1'].routing).toEqual(newSettings.systemConfig?.routing); + }); + + it('should preserve all fields from original when only updating systemConfig', () => { + const adminUser: IUser = { username: 'admin', password: 'hash', isAdmin: true }; + + const all: McpSettings = { + users: [adminUser], + mcpServers: { 'server1': { command: 'cmd1', args: [] } }, + groups: [{ id: '1', name: 'group1', servers: [], owner: 'admin' }], + systemConfig: { routing: { enableGlobalRoute: true, enableGroupNameRoute: true } }, + }; + + const newSettings: McpSettings = { + mcpServers: {}, + systemConfig: { routing: { enableGlobalRoute: false, enableGroupNameRoute: false } }, + }; + + const result = service.mergeSettings(all, newSettings, adminUser); + + // Groups should be preserved from 'all' since not in newSettings + expect(result.groups).toEqual(all.groups); + // SystemConfig should be updated from newSettings + expect(result.systemConfig).toEqual(newSettings.systemConfig); + // Users should be preserved from 'all' since not in newSettings + expect(result.users).toEqual(all.users); + // mcpServers should be updated from newSettings (empty in this case) + // This is expected behavior - when mcpServers is explicitly provided, it replaces the old value + expect(result.mcpServers).toEqual(newSettings.mcpServers); + }); + }); +}); diff --git a/tests/services/groupService.test.ts b/tests/services/groupService.test.ts new file mode 100644 index 0000000..d7d6451 --- /dev/null +++ b/tests/services/groupService.test.ts @@ -0,0 +1,262 @@ +import { createGroup, getAllGroups, deleteGroup } from '../../src/services/groupService.js'; +import * as config from '../../src/config/index.js'; +import { McpSettings } from '../../src/types/index.js'; + +// Mock the config module +jest.mock('../../src/config/index.js', () => { + let mockSettings: McpSettings = { + mcpServers: { + 'test-server': { + command: 'test', + args: [], + }, + }, + groups: [], + users: [], + }; + + return { + loadSettings: jest.fn(() => mockSettings), + saveSettings: jest.fn((settings: McpSettings) => { + mockSettings = settings; + return true; + }), + clearSettingsCache: jest.fn(), + }; +}); + +// Mock the mcpService +jest.mock('../../src/services/mcpService.js', () => ({ + notifyToolChanged: jest.fn(), +})); + +// Mock the dataService +jest.mock('../../src/services/services.js', () => ({ + getDataService: jest.fn(() => ({ + filterData: (data: any[]) => data, + filterSettings: (settings: any) => settings, + mergeSettings: (all: any, newSettings: any) => newSettings, + getPermissions: () => ['*'], + })), +})); + +describe('Group Service', () => { + beforeEach(() => { + // Reset mocks before each test + jest.clearAllMocks(); + + // Reset the mock settings to initial state + const mockSettings: McpSettings = { + mcpServers: { + 'test-server': { + command: 'test', + args: [], + }, + 'test-server-2': { + command: 'test2', + args: [], + }, + }, + groups: [], + users: [], + }; + + (config.loadSettings as jest.Mock).mockReturnValue(mockSettings); + (config.saveSettings as jest.Mock).mockImplementation((settings: McpSettings) => { + mockSettings.groups = settings.groups; + return true; + }); + }); + + describe('createGroup', () => { + it('should create a new group and persist it', () => { + const groupName = 'test-group'; + const description = 'Test group description'; + const servers = ['test-server']; + + const newGroup = createGroup(groupName, description, servers); + + expect(newGroup).not.toBeNull(); + expect(newGroup?.name).toBe(groupName); + expect(newGroup?.description).toBe(description); + expect(newGroup?.servers).toHaveLength(1); + expect(newGroup?.servers[0]).toEqual({ name: 'test-server', tools: 'all' }); + + // Verify saveSettings was called + expect(config.saveSettings).toHaveBeenCalled(); + + // Verify the settings passed to saveSettings include the new group + const savedSettings = (config.saveSettings as jest.Mock).mock.calls[0][0]; + expect(savedSettings.groups).toHaveLength(1); + expect(savedSettings.groups[0].name).toBe(groupName); + }); + + it('should create a group with multiple servers', () => { + const groupName = 'multi-server-group'; + const servers = ['test-server', 'test-server-2']; + + const newGroup = createGroup(groupName, undefined, servers); + + expect(newGroup).not.toBeNull(); + expect(newGroup?.servers).toHaveLength(2); + expect(newGroup?.servers[0]).toEqual({ name: 'test-server', tools: 'all' }); + expect(newGroup?.servers[1]).toEqual({ name: 'test-server-2', tools: 'all' }); + }); + + it('should create a group with server configuration objects', () => { + const groupName = 'config-group'; + const servers = [ + { name: 'test-server', tools: 'all' }, + { name: 'test-server-2', tools: ['tool1', 'tool2'] }, + ]; + + const newGroup = createGroup(groupName, undefined, servers); + + expect(newGroup).not.toBeNull(); + expect(newGroup?.servers).toHaveLength(2); + expect(newGroup?.servers[0]).toEqual({ name: 'test-server', tools: 'all' }); + expect(newGroup?.servers[1]).toEqual({ name: 'test-server-2', tools: ['tool1', 'tool2'] }); + }); + + it('should filter out non-existent servers', () => { + const groupName = 'filtered-group'; + const servers = ['test-server', 'non-existent-server']; + + const newGroup = createGroup(groupName, undefined, servers); + + expect(newGroup).not.toBeNull(); + expect(newGroup?.servers).toHaveLength(1); + expect(newGroup?.servers[0]).toEqual({ name: 'test-server', tools: 'all' }); + }); + + it('should not create a group with duplicate name', () => { + const groupName = 'duplicate-group'; + + // Create first group + const firstGroup = createGroup(groupName, 'First group'); + expect(firstGroup).not.toBeNull(); + + // Update the mock to include the first group + const mockSettings: McpSettings = { + mcpServers: { + 'test-server': { + command: 'test', + args: [], + }, + }, + groups: [firstGroup!], + users: [], + }; + (config.loadSettings as jest.Mock).mockReturnValue(mockSettings); + + // Try to create second group with same name + const secondGroup = createGroup(groupName, 'Second group'); + expect(secondGroup).toBeNull(); + }); + + it('should set owner to admin by default', () => { + const groupName = 'owned-group'; + + const newGroup = createGroup(groupName); + + expect(newGroup).not.toBeNull(); + expect(newGroup?.owner).toBe('admin'); + }); + + it('should set custom owner when provided', () => { + const groupName = 'custom-owned-group'; + const owner = 'testuser'; + + const newGroup = createGroup(groupName, undefined, [], owner); + + expect(newGroup).not.toBeNull(); + expect(newGroup?.owner).toBe(owner); + }); + }); + + describe('getAllGroups', () => { + it('should return all groups', () => { + const mockSettings: McpSettings = { + mcpServers: {}, + groups: [ + { + id: '1', + name: 'group1', + servers: [], + owner: 'admin', + }, + { + id: '2', + name: 'group2', + servers: [], + owner: 'admin', + }, + ], + users: [], + }; + (config.loadSettings as jest.Mock).mockReturnValue(mockSettings); + + const groups = getAllGroups(); + + expect(groups).toHaveLength(2); + expect(groups[0].name).toBe('group1'); + expect(groups[1].name).toBe('group2'); + }); + + it('should return empty array when no groups exist', () => { + const mockSettings: McpSettings = { + mcpServers: {}, + users: [], + }; + (config.loadSettings as jest.Mock).mockReturnValue(mockSettings); + + const groups = getAllGroups(); + + expect(groups).toEqual([]); + }); + }); + + describe('deleteGroup', () => { + it('should delete a group by id', () => { + const mockSettings: McpSettings = { + mcpServers: {}, + groups: [ + { + id: 'group-to-delete', + name: 'Delete Me', + servers: [], + owner: 'admin', + }, + ], + users: [], + }; + (config.loadSettings as jest.Mock).mockReturnValue(mockSettings); + (config.saveSettings as jest.Mock).mockImplementation((settings: McpSettings) => { + mockSettings.groups = settings.groups; + return true; + }); + + const result = deleteGroup('group-to-delete'); + + expect(result).toBe(true); + expect(config.saveSettings).toHaveBeenCalled(); + + // Verify the settings passed to saveSettings have the group removed + const savedSettings = (config.saveSettings as jest.Mock).mock.calls[0][0]; + expect(savedSettings.groups).toHaveLength(0); + }); + + it('should return false when group does not exist', () => { + const mockSettings: McpSettings = { + mcpServers: {}, + groups: [], + users: [], + }; + (config.loadSettings as jest.Mock).mockReturnValue(mockSettings); + + const result = deleteGroup('non-existent-id'); + + expect(result).toBe(false); + }); + }); +});