From 1cd8a19a5f8eedd3751b380e23c03b72f35c26df Mon Sep 17 00:00:00 2001 From: axel7083 <42176370+axel7083@users.noreply.github.com> Date: Wed, 6 Mar 2024 16:14:38 +0100 Subject: [PATCH] feat(catalog): splitting logic in smaller utilities Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> --- .../src/managers/catalogManager.spec.ts | 48 +++++---- .../backend/src/managers/catalogManager.ts | 101 +++++------------- packages/backend/src/studio-api-impl.spec.ts | 44 ++++---- packages/backend/src/studio-api-impl.ts | 2 +- packages/backend/src/utils/JsonWatcher.ts | 61 +++++++++++ packages/backend/src/utils/Publisher.ts | 37 +++++++ 6 files changed, 176 insertions(+), 117 deletions(-) create mode 100644 packages/backend/src/utils/JsonWatcher.ts create mode 100644 packages/backend/src/utils/Publisher.ts diff --git a/packages/backend/src/managers/catalogManager.spec.ts b/packages/backend/src/managers/catalogManager.spec.ts index 596a1e8fb..256838626 100644 --- a/packages/backend/src/managers/catalogManager.spec.ts +++ b/packages/backend/src/managers/catalogManager.spec.ts @@ -21,7 +21,7 @@ import { beforeEach, describe, expect, test, vi } from 'vitest'; import content from '../ai-test.json'; import userContent from '../ai-user-test.json'; -import type { Webview } from '@podman-desktop/api'; +import { EventEmitter, Webview } from '@podman-desktop/api'; import { CatalogManager } from '../managers/catalogManager'; import * as fs from 'node:fs'; @@ -41,24 +41,13 @@ vi.mock('node:fs', () => { }; }); -vi.mock('@podman-desktop/api', () => { - return { - fs: { - createFileSystemWatcher: () => ({ - onDidCreate: vi.fn(), - onDidDelete: vi.fn(), - onDidChange: vi.fn(), - }), - }, - }; -}); - const mocks = vi.hoisted(() => ({ withProgressMock: vi.fn(), })); vi.mock('@podman-desktop/api', async () => { return { + EventEmitter: vi.fn(), window: { withProgress: mocks.withProgressMock, }, @@ -78,20 +67,33 @@ vi.mock('@podman-desktop/api', async () => { let catalogManager: CatalogManager; beforeEach(async () => { - const appUserDirectory = '.'; + vi.resetAllMocks(); + const appUserDirectory = '.'; // Creating CatalogManager - catalogManager = new CatalogManager(appUserDirectory, { - postMessage: vi.fn(), - } as unknown as Webview); - vi.resetAllMocks(); + catalogManager = new CatalogManager({ + postMessage: vi.fn().mockResolvedValue(undefined), + } as unknown as Webview, + appUserDirectory, ); + vi.mock('node:fs'); + + const listeners: ((value: unknown) => void)[] = []; + + vi.mocked(EventEmitter).mockReturnValue({ + event: vi.fn().mockImplementation((callback) => { + listeners.push(callback); + }), + fire: vi.fn().mockImplementation((content: unknown) => { + listeners.forEach((listener) => listener(content)); + }), + } as unknown as EventEmitter); }); describe('invalid user catalog', () => { beforeEach(async () => { vi.spyOn(fs.promises, 'readFile').mockResolvedValue('invalid json'); - await catalogManager.loadCatalog(); + catalogManager.init(); }); test('expect correct model is returned with valid id', () => { @@ -111,7 +113,9 @@ describe('invalid user catalog', () => { test('expect correct model is returned from default catalog with valid id when no user catalog exists', async () => { vi.spyOn(fs, 'existsSync').mockReturnValue(false); - await catalogManager.loadCatalog(); + catalogManager.init(); + await vi.waitUntil(() => catalogManager.getRecipes().length > 0); + const model = catalogManager.getModelById('hf.TheBloke.llama-2-7b-chat.Q5_K_S'); expect(model).toBeDefined(); expect(model.name).toEqual('TheBloke/Llama-2-7B-Chat-GGUF'); @@ -125,7 +129,9 @@ test('expect correct model is returned with valid id when the user catalog is va vi.spyOn(fs, 'existsSync').mockReturnValue(true); vi.spyOn(fs.promises, 'readFile').mockResolvedValue(JSON.stringify(userContent)); - await catalogManager.loadCatalog(); + catalogManager.init(); + await vi.waitUntil(() => catalogManager.getRecipes().length > 0); + const model = catalogManager.getModelById('model1'); expect(model).toBeDefined(); expect(model.name).toEqual('Model 1'); diff --git a/packages/backend/src/managers/catalogManager.ts b/packages/backend/src/managers/catalogManager.ts index cbbd3fc62..c8b3308ca 100644 --- a/packages/backend/src/managers/catalogManager.ts +++ b/packages/backend/src/managers/catalogManager.ts @@ -18,42 +18,58 @@ import type { Catalog } from '@shared/src/models/ICatalog'; import path from 'node:path'; -import { existsSync, promises } from 'node:fs'; import defaultCatalog from '../ai.json'; -import type { Category } from '@shared/src/models/ICategory'; import type { Recipe } from '@shared/src/models/IRecipe'; import type { ModelInfo } from '@shared/src/models/IModelInfo'; import { MSG_NEW_CATALOG_STATE } from '@shared/Messages'; import { type Disposable, type Webview, fs } from '@podman-desktop/api'; +import { JsonWatcher } from '../utils/JsonWatcher'; +import { Publisher } from '../utils/Publisher'; -export class CatalogManager implements Disposable { - private watchers: Map = new Map(); +export class CatalogManager extends Publisher implements Disposable { private catalog: Catalog; + #disposables: Disposable[]; constructor( + webview: Webview, private appUserDirectory: string, - private webview: Webview, ) { + super(webview, MSG_NEW_CATALOG_STATE, () => this.getCatalog()); // We start with an empty catalog, for the methods to work before the catalog is loaded this.catalog = { categories: [], models: [], recipes: [], }; + + this.#disposables = []; + } + + init(): void { + // Creating a json watcher + const jsonWatcher: JsonWatcher = new JsonWatcher( + path.resolve(this.appUserDirectory, 'catalog.json'), + defaultCatalog + ) + jsonWatcher.onContentUpdated((content) => this.onCatalogUpdated(content)); + jsonWatcher.init(); + + this.#disposables.push(jsonWatcher); + } + + private onCatalogUpdated(content: Catalog): void { + this.catalog = content; + this.notify(); } dispose(): void { - Array.from(this.watchers.values()).forEach(watcher => watcher.dispose()); + this.#disposables.forEach(watcher => watcher.dispose()); } public getCatalog(): Catalog { return this.catalog; } - public getCategories(): Category[] { - return this.catalog.categories; - } - public getModels(): ModelInfo[] { return this.catalog.models; } @@ -77,69 +93,4 @@ export class CatalogManager implements Disposable { } return recipe; } - - async loadCatalog() { - const catalogPath = path.resolve(this.appUserDirectory, 'catalog.json'); - - try { - this.watchCatalogFile(catalogPath); // do not await, we want to do this async - } catch (err: unknown) { - console.error(`unable to watch catalog file, changes to the catalog file won't be reflected to the UI`, err); - } - - if (!existsSync(catalogPath)) { - return this.setCatalog(defaultCatalog); - } - - try { - const cat = await this.readAndAnalyzeCatalog(catalogPath); - return this.setCatalog(cat); - } catch (err: unknown) { - console.error('unable to read catalog file, reverting to default catalog', err); - } - // If something went wrong we load the default catalog - return this.setCatalog(defaultCatalog); - } - - watchCatalogFile(path: string) { - if (this.watchers.has(path)) throw new Error(`A watcher already exist for file ${path}.`); - - const watcher = fs.createFileSystemWatcher(path); - this.watchers.set(path, watcher); - - watcher.onDidCreate(async () => { - try { - const cat = await this.readAndAnalyzeCatalog(path); - await this.setCatalog(cat); - } catch (err: unknown) { - console.error('unable to read created catalog file, continue using default catalog', err); - } - }); - watcher.onDidDelete(async () => { - console.log('user catalog file deleted, reverting to default catalog'); - await this.setCatalog(defaultCatalog); - }); - watcher.onDidChange(async () => { - try { - const cat = await this.readAndAnalyzeCatalog(path); - await this.setCatalog(cat); - } catch (err: unknown) { - console.error('unable to read modified catalog file, reverting to default catalog', err); - } - }); - } - - async readAndAnalyzeCatalog(path: string): Promise { - const data = await promises.readFile(path, 'utf-8'); - return JSON.parse(data) as Catalog; - // TODO(feloy): check version, ... - } - - async setCatalog(newCatalog: Catalog) { - this.catalog = newCatalog; - await this.webview.postMessage({ - id: MSG_NEW_CATALOG_STATE, - body: this.catalog, - }); - } } diff --git a/packages/backend/src/studio-api-impl.spec.ts b/packages/backend/src/studio-api-impl.spec.ts index b8cd36e81..7d5cf12b1 100644 --- a/packages/backend/src/studio-api-impl.spec.ts +++ b/packages/backend/src/studio-api-impl.spec.ts @@ -24,7 +24,7 @@ import userContent from './ai-user-test.json'; import type { ApplicationManager } from './managers/applicationManager'; import { StudioApiImpl } from './studio-api-impl'; import type { PlayGroundManager } from './managers/playground'; -import type { TelemetryLogger, Webview } from '@podman-desktop/api'; +import { EventEmitter, TelemetryLogger, Webview } from '@podman-desktop/api'; import { CatalogManager } from './managers/catalogManager'; import type { ModelsManager } from './managers/modelsManager'; @@ -32,6 +32,7 @@ import * as fs from 'node:fs'; import { timeout } from './utils/utils'; import type { TaskRegistry } from './registries/TaskRegistry'; import type { LocalRepositoryRegistry } from './registries/LocalRepositoryRegistry'; +import { Recipe } from '@shared/src/models/IRecipe'; vi.mock('./ai.json', () => { return { @@ -48,18 +49,6 @@ vi.mock('node:fs', () => { }; }); -vi.mock('@podman-desktop/api', () => { - return { - fs: { - createFileSystemWatcher: () => ({ - onDidCreate: vi.fn(), - onDidDelete: vi.fn(), - onDidChange: vi.fn(), - }), - }, - }; -}); - const mocks = vi.hoisted(() => ({ withProgressMock: vi.fn(), showWarningMessageMock: vi.fn(), @@ -68,6 +57,7 @@ const mocks = vi.hoisted(() => ({ vi.mock('@podman-desktop/api', async () => { return { + EventEmitter: vi.fn(), window: { withProgress: mocks.withProgressMock, showWarningMessage: mocks.showWarningMessageMock, @@ -86,15 +76,18 @@ vi.mock('@podman-desktop/api', async () => { }); let studioApiImpl: StudioApiImpl; -let catalogManager; +let catalogManager: CatalogManager; beforeEach(async () => { + vi.resetAllMocks(); + const appUserDirectory = '.'; // Creating CatalogManager - catalogManager = new CatalogManager(appUserDirectory, { - postMessage: vi.fn(), - } as unknown as Webview); + catalogManager = new CatalogManager({ + postMessage: vi.fn().mockResolvedValue(undefined), + } as unknown as Webview, + appUserDirectory); // Creating StudioApiImpl studioApiImpl = new StudioApiImpl( @@ -108,8 +101,18 @@ beforeEach(async () => { {} as LocalRepositoryRegistry, {} as unknown as TaskRegistry, ); - vi.resetAllMocks(); vi.mock('node:fs'); + + const listeners: ((value: unknown) => void)[] = []; + + vi.mocked(EventEmitter).mockReturnValue({ + event: vi.fn().mockImplementation((callback) => { + listeners.push(callback); + }), + fire: vi.fn().mockImplementation((content: unknown) => { + listeners.forEach((listener) => listener(content)); + }), + } as unknown as EventEmitter); }); test('expect pull application to call the withProgress api method', async () => { @@ -118,7 +121,8 @@ test('expect pull application to call the withProgress api method', async () => mocks.withProgressMock.mockResolvedValue(undefined); - await catalogManager.loadCatalog(); + catalogManager.init(); + await vi.waitUntil(() => catalogManager.getRecipes().length > 0); await studioApiImpl.pullApplication('recipe 1', 'model1'); expect(mocks.withProgressMock).toHaveBeenCalledOnce(); }); @@ -126,7 +130,7 @@ test('expect pull application to call the withProgress api method', async () => test('requestRemoveEnvironment should ask confirmation', async () => { vi.spyOn(catalogManager, 'getRecipeById').mockReturnValue({ name: 'Recipe 1', - }); + } as unknown as Recipe); mocks.showWarningMessageMock.mockResolvedValue('Confirm'); await studioApiImpl.requestRemoveEnvironment('recipe-id-1', 'model-id-1'); await timeout(0); diff --git a/packages/backend/src/studio-api-impl.ts b/packages/backend/src/studio-api-impl.ts index d92fd40d0..65d408d37 100644 --- a/packages/backend/src/studio-api-impl.ts +++ b/packages/backend/src/studio-api-impl.ts @@ -59,7 +59,7 @@ export class StudioApiImpl implements StudioAPI { async pullApplication(recipeId: string, modelId: string): Promise { const recipe = this.catalogManager.getRecipes().find(recipe => recipe.id === recipeId); - if (!recipe) throw new Error('Not found'); + if (!recipe) throw new Error(`recipe with if ${recipeId} not found`); const model = this.catalogManager.getModelById(modelId); diff --git a/packages/backend/src/utils/JsonWatcher.ts b/packages/backend/src/utils/JsonWatcher.ts new file mode 100644 index 000000000..4d9e322a5 --- /dev/null +++ b/packages/backend/src/utils/JsonWatcher.ts @@ -0,0 +1,61 @@ +import { type Disposable, type FileSystemWatcher, fs, EventEmitter, type Event } from '@podman-desktop/api'; +import * as nodeFs from 'fs'; +import { promises } from 'node:fs'; + +export class JsonWatcher implements Disposable { + #fileSystemWatcher: FileSystemWatcher | undefined; + + private readonly _onEvent = new EventEmitter(); + readonly onContentUpdated: Event = this._onEvent.event; + + constructor(private path: string, private defaultValue: T) {} + + init(): void { + try { + this.#fileSystemWatcher = fs.createFileSystemWatcher(this.path); + // Setup listeners + this.#fileSystemWatcher.onDidChange(this.onDidChange.bind(this)); + this.#fileSystemWatcher.onDidDelete(this.onDidDelete.bind(this)); + this.#fileSystemWatcher.onDidCreate(this.onDidCreate.bind(this)); + } catch (err: unknown) { + console.error(`unable to watch file ${this.path}, changes wont be detected.`, err); + } + this.requestUpdate(); + } + + private onDidCreate(): void { + this.requestUpdate(); + } + + private onDidDelete(): void { + this.requestUpdate(); + } + + private onDidChange(): void { + this.requestUpdate(); + } + + private requestUpdate(): void { + this.updateContent().catch((err) => { + console.error('Something went wrong in update content', err); + }); + } + + private async updateContent(): Promise { + if(!nodeFs.existsSync(this.path)) { + this._onEvent.fire(this.defaultValue); + return; + } + + try { + const data = await promises.readFile(this.path, 'utf-8'); + this._onEvent.fire(JSON.parse(data)); + } catch (err: unknown) { + console.error('Something went wrong JsonWatcher', err); + } + } + + dispose(): void { + this.#fileSystemWatcher?.dispose(); + } +} diff --git a/packages/backend/src/utils/Publisher.ts b/packages/backend/src/utils/Publisher.ts new file mode 100644 index 000000000..ca3e4d287 --- /dev/null +++ b/packages/backend/src/utils/Publisher.ts @@ -0,0 +1,37 @@ +/********************************************************************** + * Copyright (C) 2024 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + ***********************************************************************/ +import type { Webview } from '@podman-desktop/api'; + +export class Publisher { + constructor( + private webview: Webview, + private channel: string, + private getter: () => T, + ) {} + + notify(): void { + this.webview + .postMessage({ + id: this.channel, + body: this.getter(), + }) + .catch((err: unknown) => { + console.error(`Something went wrong while emitting ${this.channel}: ${String(err)}`); + }); + } +}