From f8b872645e408ef11d691d451f14df0e8e14e6d0 Mon Sep 17 00:00:00 2001 From: lstocchi Date: Tue, 11 Jul 2023 12:04:36 +0200 Subject: [PATCH] fix: clean and add tests Signed-off-by: lstocchi --- .../main/src/plugin/api/container-info.ts | 1 - packages/main/src/plugin/api/context-into.ts | 2 +- .../main/src/plugin/authentication.spec.ts | 2 + .../src/plugin/container-registry.spec.ts | 4 +- .../main/src/plugin/context-registry.spec.ts | 84 ++++++++++++++++++ packages/main/src/plugin/context-registry.ts | 26 +++--- .../main/src/plugin/extension-loader.spec.ts | 4 + packages/main/src/plugin/index.ts | 2 +- packages/main/src/plugin/types/disposable.ts | 76 ---------------- .../main/src/plugin/view-registry.spec.ts | 8 +- packages/main/src/plugin/view-registry.ts | 6 +- .../renderer/src/lib/ContainerList.svelte | 18 ++-- .../src/lib/container/ContainerInfoUI.ts | 1 - .../src/lib/container/container-utils.ts | 1 - packages/renderer/src/lib/context/context.ts | 8 +- .../renderer/src/lib/images/StatusIcon.svelte | 5 +- packages/renderer/src/stores/views.spec.ts | 88 +++++++++++++++++++ packages/renderer/src/stores/views.ts | 8 +- 18 files changed, 217 insertions(+), 127 deletions(-) create mode 100644 packages/main/src/plugin/context-registry.spec.ts create mode 100644 packages/renderer/src/stores/views.spec.ts diff --git a/packages/main/src/plugin/api/container-info.ts b/packages/main/src/plugin/api/container-info.ts index a5498e296cfff..c62f77589a3fd 100644 --- a/packages/main/src/plugin/api/container-info.ts +++ b/packages/main/src/plugin/api/container-info.ts @@ -29,7 +29,6 @@ export interface ContainerInfo extends Dockerode.ContainerInfo { status: string; engineId: string; }; - icon?: string; } export interface SimpleContainerInfo extends Dockerode.ContainerInfo { diff --git a/packages/main/src/plugin/api/context-into.ts b/packages/main/src/plugin/api/context-into.ts index 673d64fa0e805..261a3a9f64cbc 100644 --- a/packages/main/src/plugin/api/context-into.ts +++ b/packages/main/src/plugin/api/context-into.ts @@ -24,6 +24,6 @@ export interface IContext { export interface ContextInfo { readonly id: number; - readonly parent?: Context | null; + readonly parent: Context | null; readonly extension: string | null; } diff --git a/packages/main/src/plugin/authentication.spec.ts b/packages/main/src/plugin/authentication.spec.ts index 08730c08484eb..82818dc806b11 100644 --- a/packages/main/src/plugin/authentication.spec.ts +++ b/packages/main/src/plugin/authentication.spec.ts @@ -48,6 +48,7 @@ import type { IconRegistry } from './icon-registry.js'; import type { Directories } from './directories.js'; import type { CustomPickRegistry } from './custompick/custompick-registry.js'; import type { ViewRegistry } from './view-registry.js'; +import type { ContextRegistry } from './context-registry.js'; function randomNumber(n = 5) { return Math.round(Math.random() * 10 * n); @@ -256,6 +257,7 @@ suite('Authentication', () => { authentication, vi.fn() as unknown as IconRegistry, vi.fn() as unknown as Telemetry, + vi.fn() as unknown as ContextRegistry, vi.fn() as unknown as ViewRegistry, directories, ); diff --git a/packages/main/src/plugin/container-registry.spec.ts b/packages/main/src/plugin/container-registry.spec.ts index 06a8fc34a6225..7d19b5bd42a15 100644 --- a/packages/main/src/plugin/container-registry.spec.ts +++ b/packages/main/src/plugin/container-registry.spec.ts @@ -24,7 +24,6 @@ import type { Proxy } from '/@/plugin/proxy.js'; import { ImageRegistry } from '/@/plugin/image-registry.js'; import type { ApiSenderType } from '/@/plugin/api.js'; import type Dockerode from 'dockerode'; -import { ViewRegistry } from './view-registry.js'; /* eslint-disable @typescript-eslint/no-empty-function */ @@ -125,8 +124,7 @@ beforeEach(() => { } as unknown as Proxy; const imageRegistry = new ImageRegistry({} as ApiSenderType, telemetry, certificates, proxy); - const viewRegistry = new ViewRegistry(); - containerRegistry = new TestContainerProviderRegistry({} as ApiSenderType, imageRegistry, telemetry, viewRegistry); + containerRegistry = new TestContainerProviderRegistry({} as ApiSenderType, imageRegistry, telemetry); }); test('tag should reject if no provider', async () => { diff --git a/packages/main/src/plugin/context-registry.spec.ts b/packages/main/src/plugin/context-registry.spec.ts new file mode 100644 index 0000000000000..e938d5a9ea788 --- /dev/null +++ b/packages/main/src/plugin/context-registry.spec.ts @@ -0,0 +1,84 @@ +/********************************************************************** + * Copyright (C) 2023 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 { beforeAll, beforeEach, expect, expectTypeOf, test, vi } from 'vitest'; +import { ContextRegistry } from './context-registry.js'; +import type { ApiSenderType } from './api.js'; + +let contextRegistry: ContextRegistry; + +beforeAll(() => { + contextRegistry = new ContextRegistry(vi.fn() as unknown as ApiSenderType); +}); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +test('Should register extension context', async () => { + contextRegistry.registerContext('extension'); + const contexts = contextRegistry.listContextInfos(); + expectTypeOf(contexts).toBeArray(); + expect(contexts.length).toBe(1); +}); + +test('Should not register new context for an already registered extension', async () => { + contextRegistry.registerContext('extension'); + contextRegistry.registerContext('extension'); + const contexts = contextRegistry.listContextInfos(); + expect(contexts.length).toBe(1); +}); + +test('Should unregister extension context', async () => { + contextRegistry.registerContext('extension'); + let contexts = contextRegistry.listContextInfos(); + expectTypeOf(contexts).toBeArray(); + expect(contexts.length).toBe(1); + contextRegistry.unregisterContext('extension'); + contexts = contextRegistry.listContextInfos(); + expectTypeOf(contexts).toBeArray(); + expect(contexts.length).toBe(0); +}); + +test('Should return empty array if no extension has been registered', async () => { + const contexts = contextRegistry.listContextInfos(); + expectTypeOf(contexts).toBeArray(); + expect(contexts.length).toBe(0); +}); + +test('Should return array with contexts of registered extensions', async () => { + contextRegistry.registerContext('extension'); + const contexts = contextRegistry.listContextInfos(); + expectTypeOf(contexts).toBeArray(); + expect(contexts.length).toBe(1); + expect(contexts[0].id).toEqual(0); + expect(contexts[0].parent).toBeNull(); + expect(contexts[0].extension).toEqual('extension'); +}); + +test('Should throw an error if trying to get context not registered extension', async () => { + expect(() => contextRegistry.getContextInfo('unknown')).toThrowError('no context found for extension unknown'); +}); + +test('Should return the contextInfo of the registered extension', async () => { + contextRegistry.registerContext('extension'); + const context = contextRegistry.getContextInfo('extension'); + expect(context.id).toEqual(0); + expect(context.parent).toBeNull(); + expect(context.extension).toEqual('extension'); +}); diff --git a/packages/main/src/plugin/context-registry.ts b/packages/main/src/plugin/context-registry.ts index 3e6e999e34008..cdb418b19f3de 100644 --- a/packages/main/src/plugin/context-registry.ts +++ b/packages/main/src/plugin/context-registry.ts @@ -20,43 +20,43 @@ import type { ContextInfo } from './api/context-into.js'; import { Context } from './context/context.js'; export class ContextRegistry { - private contexts: Map; + private contexts: Context[]; constructor(private apiSender: ApiSenderType) { - this.contexts = new Map(); + this.contexts = []; } registerContext(extension: string): void { - if (!this.contexts.has(extension)) { - const ctx = new Context(this.contexts.size, null, extension, this.apiSender); - this.contexts.set(extension, ctx); + if (!this.contexts.find(context => context.extension === extension)) { + const ctx = new Context(this.contexts.length, null, extension, this.apiSender); + this.contexts.push(ctx); } } unregisterContext(extension: string): void { - const ctx = this.contexts.get(extension); + const ctx = this.contexts.find(context => context.extension === extension); ctx?.dispose(); - this.contexts.delete(extension); + this.contexts = this.contexts.filter(context => context.extension !== extension); } getContext(extension: string): Context | undefined { - return this.contexts.get(extension); + return this.contexts.find(context => context.extension === extension); } listContextInfos(): ContextInfo[] { - const contexts: ContextInfo[] = []; - Array.from(this.contexts.values()).forEach(value => { - contexts.push({ + const contextsInfos: ContextInfo[] = []; + this.contexts.forEach(value => { + contextsInfos.push({ extension: value.extension, id: value.id, parent: value.parent, }); }); - return contexts; + return contextsInfos; } getContextInfo(extension: string): ContextInfo { - const context = this.contexts.get(extension); + const context = this.getContext(extension); if (!context) { throw new Error(`no context found for extension ${extension}`); } diff --git a/packages/main/src/plugin/extension-loader.spec.ts b/packages/main/src/plugin/extension-loader.spec.ts index 380795ab13f54..6d94b6455fe11 100644 --- a/packages/main/src/plugin/extension-loader.spec.ts +++ b/packages/main/src/plugin/extension-loader.spec.ts @@ -46,6 +46,7 @@ import type { IconRegistry } from './icon-registry.js'; import type { Directories } from './directories.js'; import type { CustomPickRegistry } from './custompick/custompick-registry.js'; import type { ViewRegistry } from './view-registry.js'; +import type { ContextRegistry } from './context-registry.js'; class TestExtensionLoader extends ExtensionLoader { public async setupScanningDirectory(): Promise { @@ -112,6 +113,8 @@ const iconRegistry: IconRegistry = {} as unknown as IconRegistry; const telemetryTrackMock = vi.fn(); const telemetry: Telemetry = { track: telemetryTrackMock } as unknown as Telemetry; +const contextRegistry: ContextRegistry = {} as unknown as ContextRegistry; + const viewRegistry: ViewRegistry = {} as unknown as ViewRegistry; const directories = { @@ -143,6 +146,7 @@ beforeAll(() => { authenticationProviderRegistry, iconRegistry, telemetry, + contextRegistry, viewRegistry, directories, ); diff --git a/packages/main/src/plugin/index.ts b/packages/main/src/plugin/index.ts index cf20f58be5471..b69d22c9cf11e 100644 --- a/packages/main/src/plugin/index.ts +++ b/packages/main/src/plugin/index.ts @@ -368,7 +368,7 @@ export class PluginSystem { await certificates.init(); const imageRegistry = new ImageRegistry(apiSender, telemetry, certificates, proxy); const contextRegistry = new ContextRegistry(apiSender); - const viewRegistry = new ViewRegistry(apiSender); + const viewRegistry = new ViewRegistry(); const containerProviderRegistry = new ContainerProviderRegistry(apiSender, imageRegistry, telemetry); const cancellationTokenRegistry = new CancellationTokenRegistry(); const providerRegistry = new ProviderRegistry(apiSender, containerProviderRegistry, telemetry); diff --git a/packages/main/src/plugin/types/disposable.ts b/packages/main/src/plugin/types/disposable.ts index 6188c0b105a22..cfacfda29dfe8 100644 --- a/packages/main/src/plugin/types/disposable.ts +++ b/packages/main/src/plugin/types/disposable.ts @@ -53,79 +53,3 @@ export class Disposable implements IDisposable { return new Disposable(func); } } - -/** - * Manages a collection of disposable values. - * - * This is the preferred way to manage multiple disposables. A `DisposableStore` is safer to work with than an - * `IDisposable[]` as it considers edge cases, such as registering the same value multiple times or adding an item to a - * store that has already been disposed of. - */ -export class DisposableStore implements IDisposable { - static DISABLE_DISPOSED_WARNING = false; - - private readonly _toDispose = new Set(); - private _isDisposed = false; - - /** - * Dispose of all registered disposables and mark this object as disposed. - * - * Any future disposables added to this object will be disposed of on `add`. - */ - public dispose(): void { - if (this._isDisposed) { - return; - } - - this._isDisposed = true; - this.clear(); - } - - /** - * @return `true` if this object has been disposed of. - */ - public get isDisposed(): boolean { - return this._isDisposed; - } - - /** - * Dispose of all registered disposables but do not mark this object as disposed. - */ - public clear(): void { - if (this._toDispose.size === 0) { - return; - } - - try { - this._toDispose.forEach(d => d.dispose()); - } finally { - this._toDispose.clear(); - } - } - - /** - * Add a new {@link IDisposable disposable} to the collection. - */ - public add(o: T): T { - if (!o) { - return o; - } - if ((o as unknown as DisposableStore) === this) { - throw new Error('Cannot register a disposable on itself!'); - } - - if (this._isDisposed) { - if (!DisposableStore.DISABLE_DISPOSED_WARNING) { - console.warn( - new Error( - 'Trying to add a disposable to a DisposableStore that has already been disposed of. The added object will be leaked!', - ).stack, - ); - } - } else { - this._toDispose.add(o); - } - - return o; - } -} diff --git a/packages/main/src/plugin/view-registry.spec.ts b/packages/main/src/plugin/view-registry.spec.ts index 5957edab3a1df..31900e3c37349 100644 --- a/packages/main/src/plugin/view-registry.spec.ts +++ b/packages/main/src/plugin/view-registry.spec.ts @@ -29,7 +29,6 @@ beforeAll(() => { views: { 'icons/containersList': [ { - id: 'kind_icon', when: 'io.x-k8s.kind.cluster in containerLabelKeys', icon: '${kind-icon}', }, @@ -37,7 +36,7 @@ beforeAll(() => { }, }, }; - viewRegistry.registerViewContribution('extension', manifest.contributes.views); + viewRegistry.registerViews('extension', manifest.contributes.views); }); beforeEach(() => { @@ -45,18 +44,17 @@ beforeEach(() => { }); test('Should return empty array for unknown view', async () => { - const views = viewRegistry.getViewContribution('unknownView'); + const views = viewRegistry.fetchViewsContributions('unknown'); expect(views).toBeDefined(); expectTypeOf(views).toBeArray(); expect(views.length).toBe(0); }); test('View context should have a single entry', async () => { - const views = viewRegistry.getViewContribution('icons/containersList'); + const views = viewRegistry.fetchViewsContributions('extension'); expect(views).toBeDefined(); expectTypeOf(views).toBeArray(); expect(views.length).toBe(1); - expect(views[0].id).toBe('kind_icon'); expect(views[0].when).toBe('io.x-k8s.kind.cluster in containerLabelKeys'); expect(views[0].icon).toBe('${kind-icon}'); }); diff --git a/packages/main/src/plugin/view-registry.ts b/packages/main/src/plugin/view-registry.ts index 0b54f105575d4..c3042b15927d9 100644 --- a/packages/main/src/plugin/view-registry.ts +++ b/packages/main/src/plugin/view-registry.ts @@ -15,13 +15,12 @@ * * SPDX-License-Identifier: Apache-2.0 ***********************************************************************/ -import type { ApiSenderType } from './api.js'; import type { ViewContribution, ViewInfoUI } from './api/view-info.js'; export class ViewRegistry { private extViewContribution: Map>; - constructor(private apiSender: ApiSenderType) { + constructor() { this.extViewContribution = new Map(); } @@ -44,7 +43,6 @@ export class ViewRegistry { this.registerView(extensionId, viewId, viewContribution); }); }); - this.apiSender.send('extension-views-contribs-added', extensionId); } unregisterViews(extensionId: string): void { @@ -67,7 +65,7 @@ export class ViewRegistry { return listViewInfoUI; } - fetchViewsContributions(extensionId: string) { + fetchViewsContributions(extensionId: string): ViewInfoUI[] { const listViewInfoUI: ViewInfoUI[] = []; const viewContributions = this.extViewContribution.get(extensionId); if (!viewContributions) { diff --git a/packages/renderer/src/lib/ContainerList.svelte b/packages/renderer/src/lib/ContainerList.svelte index 2ba9e2982f6f7..642bae87f542b 100644 --- a/packages/renderer/src/lib/ContainerList.svelte +++ b/packages/renderer/src/lib/ContainerList.svelte @@ -272,7 +272,6 @@ onMount(async () => { container.actionError = matchingContainer.actionError; container.selected = matchingContainer.selected; } - // if matching when update icon }); } }); @@ -374,23 +373,22 @@ function errorCallback(container: ContainerInfoUI, errorMessage: string): void { } function iconClass(container: ContainerInfoUI): string | undefined { - // handle ${} in icon class - // and interpret the value and replace with the class-name let icon; + // loop over all contribution for this view for (const contribution of viewContributions) { - //get extension context + // retrieve the extension from the contribution and fetch its context const extensionContext: ContextUI = extensionsContext.find(ctx => { - console.log(ctx.extension); - console.log(contribution.extensionId); return ctx.extension === contribution.extensionId; }); - console.log(extensionContext); if (extensionContext) { - console.log('dentro'); + // adapt the context to work with containers (e.g save container labels into the context) adaptContextOnContainer(extensionContext, container); + // deserialize the when clause const whenDeserialized = ContextKeyExpr.deserialize(contribution.when); - //if contribution has to be applied to this container + // if the when clause has to be applied to this container if (whenDeserialized?.evaluate(extensionContext)) { + // handle ${} in icon class + // and interpret the value and replace with the class-name const match = contribution.icon.match(/\$\{(.*)\}/); if (match !== null && match.length === 2) { const className = match[1]; @@ -576,7 +574,7 @@ function iconClass(container: ContainerInfoUI): string | undefined {
{#if iconClass(container)} - + {:else} {/if} diff --git a/packages/renderer/src/lib/container/ContainerInfoUI.ts b/packages/renderer/src/lib/container/ContainerInfoUI.ts index 3dd158b4e289b..f379192394b1b 100644 --- a/packages/renderer/src/lib/container/ContainerInfoUI.ts +++ b/packages/renderer/src/lib/container/ContainerInfoUI.ts @@ -64,7 +64,6 @@ export interface ContainerInfoUI { created: number; actionInProgress?: boolean; actionError?: string; - icon?: string; labels: { [label: string]: string }; } diff --git a/packages/renderer/src/lib/container/container-utils.ts b/packages/renderer/src/lib/container/container-utils.ts index f9fd4ee3a5594..f0c57b541dca4 100644 --- a/packages/renderer/src/lib/container/container-utils.ts +++ b/packages/renderer/src/lib/container/container-utils.ts @@ -135,7 +135,6 @@ export class ContainerUtils { groupInfo: this.getContainerGroup(containerInfo), selected: false, created: containerInfo.Created, - icon: containerInfo.icon, labels: containerInfo.Labels, }; } diff --git a/packages/renderer/src/lib/context/context.ts b/packages/renderer/src/lib/context/context.ts index b0786a5b5d5a8..4c21df1f73b41 100644 --- a/packages/renderer/src/lib/context/context.ts +++ b/packages/renderer/src/lib/context/context.ts @@ -59,11 +59,7 @@ export class ContextUI implements IContext { this._parent = null; } - static adaptContext(ctx: ContextInfo, parent?: ContextUI): ContextUI { - if (parent) { - return new ContextUI(ctx.id, parent, ctx.extension); - } else { - return new ContextUI(ctx.id, null, ctx.extension); - } + static adaptContext(ctx: ContextInfo, parent: ContextUI | null): ContextUI { + return new ContextUI(ctx.id, parent, ctx.extension); } } diff --git a/packages/renderer/src/lib/images/StatusIcon.svelte b/packages/renderer/src/lib/images/StatusIcon.svelte index aeb38f0631d07..c5b3fbbf41949 100644 --- a/packages/renderer/src/lib/images/StatusIcon.svelte +++ b/packages/renderer/src/lib/images/StatusIcon.svelte @@ -5,7 +5,6 @@ import StarIcon from './StarIcon.svelte'; // any other status will result in a standard outlined box export let status = ''; export let icon: any = undefined; -export let iconClass: string = undefined; $: solid = status === 'RUNNING' || status === 'STARTING' || status === 'USED' || status === 'DEGRADED'; @@ -20,8 +19,8 @@ $: solid = status === 'RUNNING' || status === 'STARTING' || status === 'USED' || class:border-gray-700="{!solid}" class:text-gray-700="{!solid}" title="{status}"> - {#if iconClass} - + {#if typeof icon === 'string'} + {:else} {/if} diff --git a/packages/renderer/src/stores/views.spec.ts b/packages/renderer/src/stores/views.spec.ts new file mode 100644 index 0000000000000..cea20544d3c18 --- /dev/null +++ b/packages/renderer/src/stores/views.spec.ts @@ -0,0 +1,88 @@ +/********************************************************************** + * Copyright (C) 2023 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 + ***********************************************************************/ + +/* eslint-disable @typescript-eslint/no-explicit-any */ + +import { get } from 'svelte/store'; +import type { Mock } from 'vitest'; +import { expect, test, vi } from 'vitest'; +import { fetchViews, viewsEventStore, viewsContributions } from './views'; +import type { ViewInfoUI } from '../../../main/src/plugin/api/view-info'; + +// first, path window object +const callbacks = new Map(); +const eventEmitter = { + receive: (message: string, callback: any) => { + callbacks.set(message, callback); + }, +}; + +const listViewsMock: Mock> = vi.fn(); + +Object.defineProperty(global, 'window', { + value: { + listViewsContributions: listViewsMock, + events: { + receive: eventEmitter.receive, + }, + addEventListener: eventEmitter.receive, + }, + writable: true, +}); + +beforeAll(() => { + vi.clearAllMocks(); +}); + +test('views should be updated in case of an extension is stopped', async () => { + // initial view + listViewsMock.mockResolvedValue([ + { + extensionId: 'extension', + viewId: 'view', + when: 'when', + icon: 'icon', + } as unknown as ViewInfoUI, + ]); + viewsEventStore.setup(); + + const callback = callbacks.get('extensions-already-started'); + // send 'extensions-already-started' event + expect(callback).toBeDefined(); + await callback(); + + // now ready to fetch volumes + await fetchViews(); + + // now get list + const views = get(viewsContributions); + expect(views.length).toBe(1); + expect(views[0].extensionId).toEqual('extension'); + + // ok now mock the listVolumes function to return an empty list + listViewsMock.mockResolvedValue([]); + + // call 'container-removed-event' event + const extensionStoppedCallback = callbacks.get('extension-stopped'); + expect(extensionStoppedCallback).toBeDefined(); + await extensionStoppedCallback(); + + // check if the volumes are updated + const views2 = get(viewsContributions); + expect(views2.length).toBe(0); +}); diff --git a/packages/renderer/src/stores/views.ts b/packages/renderer/src/stores/views.ts index 48f8c19ce7093..6ccdb357adfcf 100644 --- a/packages/renderer/src/stores/views.ts +++ b/packages/renderer/src/stores/views.ts @@ -41,7 +41,7 @@ const listViewsContributions = (): Promise => { return window.listViewsContributions(); }; -const viewsEventStore = new EventStore( +export const viewsEventStore = new EventStore( 'views', viewsContributions, checkForUpdate, @@ -49,4 +49,8 @@ const viewsEventStore = new EventStore( windowListeners, listViewsContributions, ); -viewsEventStore.setup(); +const viewsEventStoreInfo = viewsEventStore.setup(); + +export const fetchViews = async () => { + await viewsEventStoreInfo.fetch(); +};