Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: #3476 - Mismatch id between model json and path #3645

Merged
merged 3 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -26,7 +26,7 @@
const downloadFile: (downloadRequest: DownloadRequest, network?: NetworkConfig) => Promise<any> = (
downloadRequest,
network
) => globalThis.core?.api?.downloadFile(downloadRequest, network)

Check warning on line 29 in core/src/browser/core.ts

View workflow job for this annotation

GitHub Actions / coverage-check

29 line is not covered with tests

/**
* Get unit in bytes for a remote file.
Expand Down Expand Up @@ -68,6 +68,13 @@
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 @@ -103,7 +110,7 @@
* @param message - Message to log.
*/
const log: (message: string, fileName?: string) => void = (message, fileName) =>
globalThis.core.api?.log(message, fileName)

Check warning on line 113 in core/src/browser/core.ts

View workflow job for this annotation

GitHub Actions / coverage-check

113 line is not covered with tests

/**
* Check whether the path is a subdirectory of another path.
Expand All @@ -114,14 +121,14 @@
* @returns {Promise<boolean>} - A promise that resolves with a boolean indicating whether the path is a subdirectory.
*/
const isSubdirectory: (from: string, to: string) => Promise<boolean> = (from: string, to: string) =>
globalThis.core.api?.isSubdirectory(from, to)

Check warning on line 124 in core/src/browser/core.ts

View workflow job for this annotation

GitHub Actions / coverage-check

124 line is not covered with tests

/**
* Get system information
* @returns {Promise<any>} - A promise that resolves with the system information.
*/
const systemInformation: () => Promise<SystemInformation> = () =>
globalThis.core.api?.systemInformation()

Check warning on line 131 in core/src/browser/core.ts

View workflow job for this annotation

GitHub Actions / coverage-check

131 line is not covered with tests

/**
* Show toast message from browser processes.
Expand All @@ -130,7 +137,7 @@
* @returns
*/
const showToast: (title: string, message: string) => void = (title, message) =>
globalThis.core.api?.showToast(title, message)

Check warning on line 140 in core/src/browser/core.ts

View workflow job for this annotation

GitHub Actions / coverage-check

140 line is not covered with tests

/**
* Register extension point function type definition
Expand Down Expand Up @@ -161,5 +168,6 @@
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 { 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 @@ -19,21 +19,21 @@
* On extension load, subscribe to events.
*/
override onLoad() {
this.registerEngine()

Check warning on line 22 in core/src/browser/extensions/engines/AIEngine.ts

View workflow job for this annotation

GitHub Actions / coverage-check

22 line is not covered with tests

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))

Check warning on line 25 in core/src/browser/extensions/engines/AIEngine.ts

View workflow job for this annotation

GitHub Actions / coverage-check

24-25 lines are not covered with tests
}

/**
* Registers AI Engines
*/
registerEngine() {
EngineManager.instance().register(this)

Check warning on line 32 in core/src/browser/extensions/engines/AIEngine.ts

View workflow job for this annotation

GitHub Actions / coverage-check

32 line is not covered with tests
}

async registerModels(models: Model[]): Promise<void> {
const modelFolderPath = await joinPath([await getJanDataFolderPath(), AIEngine.modelsFolder])

Check warning on line 36 in core/src/browser/extensions/engines/AIEngine.ts

View workflow job for this annotation

GitHub Actions / coverage-check

36 line is not covered with tests

let shouldNotifyModelUpdate = false
for (const model of models) {
Expand Down Expand Up @@ -78,7 +78,7 @@
/**
* 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 @@
HuggingFaceRepoData,
ImportingModel,
Model,
ModelFile,
ModelInterface,
OptionType,
} from '../../types'
Expand All @@ -16,7 +17,7 @@
* Model extension type.
*/
type(): ExtensionTypeEnum | undefined {
return ExtensionTypeEnum.Model

Check warning on line 20 in core/src/browser/extensions/model.ts

View workflow job for this annotation

GitHub Actions / coverage-check

20 line is not covered with tests
}

abstract downloadModel(
Expand All @@ -25,12 +26,11 @@
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
Loading