Skip to content

Commit

Permalink
fix: #3476 - Mismatch id between model json and path (#3645)
Browse files Browse the repository at this point in the history
* fix: mismatch between model json and path

* chore: revert preserve model settings

* test: add tests
  • Loading branch information
louis-jan committed Sep 17, 2024
1 parent c3cb192 commit 8e603bd
Show file tree
Hide file tree
Showing 35 changed files with 878 additions and 338 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,4 @@ core/test_results.html
coverage
.yarn
.yarnrc
*.tsbuildinfo
177 changes: 94 additions & 83 deletions core/src/browser/core.test.ts
Original file line number Diff line number Diff line change
@@ -1,98 +1,109 @@
import { openExternalUrl } from './core';
import { joinPath } from './core';
import { openFileExplorer } from './core';
import { getJanDataFolderPath } from './core';
import { abortDownload } from './core';
import { getFileSize } from './core';
import { executeOnMain } from './core';
import { openExternalUrl } from './core'
import { joinPath } from './core'
import { openFileExplorer } from './core'
import { getJanDataFolderPath } from './core'
import { abortDownload } from './core'
import { getFileSize } from './core'
import { executeOnMain } from './core'

it('should open external url', async () => {
const url = 'http://example.com';
globalThis.core = {
api: {
openExternalUrl: jest.fn().mockResolvedValue('opened')
describe('test core apis', () => {
it('should open external url', async () => {
const url = 'http://example.com'
globalThis.core = {
api: {
openExternalUrl: jest.fn().mockResolvedValue('opened'),
},
}
};
const result = await openExternalUrl(url);
expect(globalThis.core.api.openExternalUrl).toHaveBeenCalledWith(url);
expect(result).toBe('opened');
});
const result = await openExternalUrl(url)
expect(globalThis.core.api.openExternalUrl).toHaveBeenCalledWith(url)
expect(result).toBe('opened')
})


it('should join paths', async () => {
const paths = ['/path/one', '/path/two'];
globalThis.core = {
api: {
joinPath: jest.fn().mockResolvedValue('/path/one/path/two')
it('should join paths', async () => {
const paths = ['/path/one', '/path/two']
globalThis.core = {
api: {
joinPath: jest.fn().mockResolvedValue('/path/one/path/two'),
},
}
};
const result = await joinPath(paths);
expect(globalThis.core.api.joinPath).toHaveBeenCalledWith(paths);
expect(result).toBe('/path/one/path/two');
});

const result = await joinPath(paths)
expect(globalThis.core.api.joinPath).toHaveBeenCalledWith(paths)
expect(result).toBe('/path/one/path/two')
})

it('should open file explorer', async () => {
const path = '/path/to/open';
globalThis.core = {
api: {
openFileExplorer: jest.fn().mockResolvedValue('opened')
it('should open file explorer', async () => {
const path = '/path/to/open'
globalThis.core = {
api: {
openFileExplorer: jest.fn().mockResolvedValue('opened'),
},
}
};
const result = await openFileExplorer(path);
expect(globalThis.core.api.openFileExplorer).toHaveBeenCalledWith(path);
expect(result).toBe('opened');
});

const result = await openFileExplorer(path)
expect(globalThis.core.api.openFileExplorer).toHaveBeenCalledWith(path)
expect(result).toBe('opened')
})

it('should get jan data folder path', async () => {
globalThis.core = {
api: {
getJanDataFolderPath: jest.fn().mockResolvedValue('/path/to/jan/data')
it('should get jan data folder path', async () => {
globalThis.core = {
api: {
getJanDataFolderPath: jest.fn().mockResolvedValue('/path/to/jan/data'),
},
}
};
const result = await getJanDataFolderPath();
expect(globalThis.core.api.getJanDataFolderPath).toHaveBeenCalled();
expect(result).toBe('/path/to/jan/data');
});
const result = await getJanDataFolderPath()
expect(globalThis.core.api.getJanDataFolderPath).toHaveBeenCalled()
expect(result).toBe('/path/to/jan/data')
})


it('should abort download', async () => {
const fileName = 'testFile';
globalThis.core = {
api: {
abortDownload: jest.fn().mockResolvedValue('aborted')
it('should abort download', async () => {
const fileName = 'testFile'
globalThis.core = {
api: {
abortDownload: jest.fn().mockResolvedValue('aborted'),
},
}
};
const result = await abortDownload(fileName);
expect(globalThis.core.api.abortDownload).toHaveBeenCalledWith(fileName);
expect(result).toBe('aborted');
});

const result = await abortDownload(fileName)
expect(globalThis.core.api.abortDownload).toHaveBeenCalledWith(fileName)
expect(result).toBe('aborted')
})

it('should get file size', async () => {
const url = 'http://example.com/file';
globalThis.core = {
api: {
getFileSize: jest.fn().mockResolvedValue(1024)
it('should get file size', async () => {
const url = 'http://example.com/file'
globalThis.core = {
api: {
getFileSize: jest.fn().mockResolvedValue(1024),
},
}
};
const result = await getFileSize(url);
expect(globalThis.core.api.getFileSize).toHaveBeenCalledWith(url);
expect(result).toBe(1024);
});
const result = await getFileSize(url)
expect(globalThis.core.api.getFileSize).toHaveBeenCalledWith(url)
expect(result).toBe(1024)
})

it('should execute function on main process', async () => {
const extension = 'testExtension'
const method = 'testMethod'
const args = ['arg1', 'arg2']
globalThis.core = {
api: {
invokeExtensionFunc: jest.fn().mockResolvedValue('result'),
},
}
const result = await executeOnMain(extension, method, ...args)
expect(globalThis.core.api.invokeExtensionFunc).toHaveBeenCalledWith(extension, method, ...args)
expect(result).toBe('result')
})
})

it('should execute function on main process', async () => {
const extension = 'testExtension';
const method = 'testMethod';
const args = ['arg1', 'arg2'];
globalThis.core = {
api: {
invokeExtensionFunc: jest.fn().mockResolvedValue('result')
describe('dirName - just a pass thru api', () => {
it('should retrieve the directory name from a file path', async () => {
const mockDirName = jest.fn()
globalThis.core = {
api: {
dirName: mockDirName.mockResolvedValue('/path/to'),
},
}
};
const result = await executeOnMain(extension, method, ...args);
expect(globalThis.core.api.invokeExtensionFunc).toHaveBeenCalledWith(extension, method, ...args);
expect(result).toBe('result');
});
// Normal file path with extension
const path = '/path/to/file.txt'
await globalThis.core.api.dirName(path)
expect(mockDirName).toHaveBeenCalledWith(path)
})
})
8 changes: 8 additions & 0 deletions core/src/browser/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,13 @@ const openFileExplorer: (path: string) => Promise<any> = (path) =>
const joinPath: (paths: string[]) => Promise<string> = (paths) =>
globalThis.core.api?.joinPath(paths)

/**
* Get dirname of a file path.
* @param path - The file path to retrieve dirname.
* @returns {Promise<string>} A promise that resolves the dirname.
*/
const dirName: (path: string) => Promise<string> = (path) => globalThis.core.api?.dirName(path)

/**
* Retrieve the basename from an url.
* @param path - The path to retrieve.
Expand Down Expand Up @@ -161,5 +168,6 @@ export {
systemInformation,
showToast,
getFileSize,
dirName,
FileStat,
}
6 changes: 3 additions & 3 deletions core/src/browser/extensions/engines/AIEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { getJanDataFolderPath, joinPath } from '../../core'
import { events } from '../../events'
import { BaseExtension } from '../../extension'
import { fs } from '../../fs'
import { MessageRequest, Model, ModelEvent } from '../../../types'
import { MessageRequest, Model, ModelEvent, ModelFile } from '../../../types'
import { EngineManager } from './EngineManager'

/**
Expand All @@ -21,7 +21,7 @@ export abstract class AIEngine extends BaseExtension {
override onLoad() {
this.registerEngine()

events.on(ModelEvent.OnModelInit, (model: Model) => this.loadModel(model))
events.on(ModelEvent.OnModelInit, (model: ModelFile) => this.loadModel(model))
events.on(ModelEvent.OnModelStop, (model: Model) => this.unloadModel(model))
}

Expand Down Expand Up @@ -78,7 +78,7 @@ export abstract class AIEngine extends BaseExtension {
/**
* Loads the model.
*/
async loadModel(model: Model): Promise<any> {
async loadModel(model: ModelFile): Promise<any> {
if (model.engine.toString() !== this.provider) return Promise.resolve()
events.emit(ModelEvent.OnModelReady, model)
return Promise.resolve()
Expand Down
16 changes: 9 additions & 7 deletions core/src/browser/extensions/engines/LocalOAIEngine.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { executeOnMain, getJanDataFolderPath, joinPath, systemInformation } from '../../core'
import { executeOnMain, systemInformation, dirName } from '../../core'
import { events } from '../../events'
import { Model, ModelEvent } from '../../../types'
import { Model, ModelEvent, ModelFile } from '../../../types'
import { OAIEngine } from './OAIEngine'

/**
Expand All @@ -14,22 +14,24 @@ export abstract class LocalOAIEngine extends OAIEngine {
unloadModelFunctionName: string = 'unloadModel'

/**
* On extension load, subscribe to events.
* This class represents a base for local inference providers in the OpenAI architecture.
* It extends the OAIEngine class and provides the implementation of loading and unloading models locally.
* The loadModel function subscribes to the ModelEvent.OnModelInit event, loading models when initiated.
* The unloadModel function subscribes to the ModelEvent.OnModelStop event, unloading models when stopped.
*/
override onLoad() {
super.onLoad()
// These events are applicable to local inference providers
events.on(ModelEvent.OnModelInit, (model: Model) => this.loadModel(model))
events.on(ModelEvent.OnModelInit, (model: ModelFile) => this.loadModel(model))
events.on(ModelEvent.OnModelStop, (model: Model) => this.unloadModel(model))
}

/**
* Load the model.
*/
override async loadModel(model: Model): Promise<void> {
override async loadModel(model: ModelFile): Promise<void> {
if (model.engine.toString() !== this.provider) return
const modelFolderName = 'models'
const modelFolder = await joinPath([await getJanDataFolderPath(), modelFolderName, model.id])
const modelFolder = await dirName(model.file_path)
const systemInfo = await systemInformation()
const res = await executeOnMain(
this.nodeModule,
Expand Down
10 changes: 5 additions & 5 deletions core/src/browser/extensions/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
HuggingFaceRepoData,
ImportingModel,
Model,
ModelFile,
ModelInterface,
OptionType,
} from '../../types'
Expand All @@ -25,12 +26,11 @@ export abstract class ModelExtension extends BaseExtension implements ModelInter
network?: { proxy: string; ignoreSSL?: boolean }
): Promise<void>
abstract cancelModelDownload(modelId: string): Promise<void>
abstract deleteModel(modelId: string): Promise<void>
abstract saveModel(model: Model): Promise<void>
abstract getDownloadedModels(): Promise<Model[]>
abstract getConfiguredModels(): Promise<Model[]>
abstract deleteModel(model: ModelFile): Promise<void>
abstract getDownloadedModels(): Promise<ModelFile[]>
abstract getConfiguredModels(): Promise<ModelFile[]>
abstract importModels(models: ImportingModel[], optionType: OptionType): Promise<void>
abstract updateModelInfo(modelInfo: Partial<Model>): Promise<Model>
abstract updateModelInfo(modelInfo: Partial<ModelFile>): Promise<ModelFile>
abstract fetchHuggingFaceRepoData(repoId: string): Promise<HuggingFaceRepoData>
abstract getDefaultModel(): Promise<Model>
}
75 changes: 46 additions & 29 deletions core/src/node/api/processors/app.test.ts
Original file line number Diff line number Diff line change
@@ -1,40 +1,57 @@
import { App } from './app';
jest.mock('../../helper', () => ({
...jest.requireActual('../../helper'),
getJanDataFolderPath: () => './app',
}))
import { dirname } from 'path'
import { App } from './app'

it('should call stopServer', () => {
const app = new App();
const stopServerMock = jest.fn().mockResolvedValue('Server stopped');
const app = new App()
const stopServerMock = jest.fn().mockResolvedValue('Server stopped')
jest.mock('@janhq/server', () => ({
stopServer: stopServerMock
}));
const result = app.stopServer();
expect(stopServerMock).toHaveBeenCalled();
});
stopServer: stopServerMock,
}))
app.stopServer()
expect(stopServerMock).toHaveBeenCalled()
})

it('should correctly retrieve basename', () => {
const app = new App();
const result = app.baseName('/path/to/file.txt');
expect(result).toBe('file.txt');
});
const app = new App()
const result = app.baseName('/path/to/file.txt')
expect(result).toBe('file.txt')
})

it('should correctly identify subdirectories', () => {
const app = new App();
const basePath = process.platform === 'win32' ? 'C:\\path\\to' : '/path/to';
const subPath = process.platform === 'win32' ? 'C:\\path\\to\\subdir' : '/path/to/subdir';
const result = app.isSubdirectory(basePath, subPath);
expect(result).toBe(true);
});
const app = new App()
const basePath = process.platform === 'win32' ? 'C:\\path\\to' : '/path/to'
const subPath = process.platform === 'win32' ? 'C:\\path\\to\\subdir' : '/path/to/subdir'
const result = app.isSubdirectory(basePath, subPath)
expect(result).toBe(true)
})

it('should correctly join multiple paths', () => {
const app = new App();
const result = app.joinPath(['path', 'to', 'file']);
const expectedPath = process.platform === 'win32' ? 'path\\to\\file' : 'path/to/file';
expect(result).toBe(expectedPath);
});
const app = new App()
const result = app.joinPath(['path', 'to', 'file'])
const expectedPath = process.platform === 'win32' ? 'path\\to\\file' : 'path/to/file'
expect(result).toBe(expectedPath)
})

it('should call correct function with provided arguments using process method', () => {
const app = new App();
const mockFunc = jest.fn();
app.joinPath = mockFunc;
app.process('joinPath', ['path1', 'path2']);
expect(mockFunc).toHaveBeenCalledWith(['path1', 'path2']);
});
const app = new App()
const mockFunc = jest.fn()
app.joinPath = mockFunc
app.process('joinPath', ['path1', 'path2'])
expect(mockFunc).toHaveBeenCalledWith(['path1', 'path2'])
})

it('should retrieve the directory name from a file path (Unix/Windows)', async () => {
const app = new App()
const path = 'C:/Users/John Doe/Desktop/file.txt'
expect(await app.dirName(path)).toBe('C:/Users/John Doe/Desktop')
})

it('should retrieve the directory name when using file protocol', async () => {
const app = new App()
const path = 'file:/models/file.txt'
expect(await app.dirName(path)).toBe(process.platform === 'win32' ? 'app\\models' : 'app/models')
})
Loading

0 comments on commit 8e603bd

Please sign in to comment.