From c9326fd5f88b9c830014d1ba377b44dcdf0a2dd4 Mon Sep 17 00:00:00 2001 From: Salah Benmoussati <51402489+sbenmoussati@users.noreply.github.com> Date: Wed, 22 Jan 2025 12:18:38 +0100 Subject: [PATCH] SDA-4770 Adaptations on openfin integration (#2268) * SDA-4770 Adaptations on openfin integration * SDA-4770 Adaptations on openfin integration --- config/Symphony.config | 4 +- installer/mac/postinstall.sh | 2 + spec/mainApiHandler.spec.ts | 2 + spec/openfinHandler.spec.ts | 46 +++++++++++- spec/plistHandler.spec.ts | 2 + src/app/config-handler.ts | 2 + src/app/main-api-handler.ts | 10 ++- src/app/openfin-handler.ts | 127 +++++++++++++++++++++++---------- src/app/plist-handler.ts | 2 + src/common/config-interface.ts | 2 + src/renderer/ssf-api.ts | 61 +++++++++++----- 11 files changed, 197 insertions(+), 63 deletions(-) diff --git a/config/Symphony.config b/config/Symphony.config index df45dae1..ad64ab40 100644 --- a/config/Symphony.config +++ b/config/Symphony.config @@ -52,6 +52,8 @@ "uuid": "", "licenseKey": "", "runtimeVersion": "", - "autoConnect": false + "autoConnect": false, + "channelName": "", + "connectionTimeout": 10000 } } diff --git a/installer/mac/postinstall.sh b/installer/mac/postinstall.sh index 77e904c6..0732904e 100755 --- a/installer/mac/postinstall.sh +++ b/installer/mac/postinstall.sh @@ -126,6 +126,7 @@ if [ "$EUID" -ne 0 ]; then defaults write "$plistFilePath" uuid -string "" defaults write "$plistFilePath" licenseKey -string "" defaults write "$plistFilePath" runtimeVersion -string "" + defaults write "$plistFilePath" channelName -string "" defaults write "$plistFilePath" autoConnect -bool false else sudo -u "$userName" defaults write "$plistFilePath" url -string "$pod_url" @@ -175,6 +176,7 @@ else sudo -u "$userName" defaults write "$plistFilePath" uuid -string "" sudo -u "$userName" defaults write "$plistFilePath" licenseKey -string "" sudo -u "$userName" defaults write "$plistFilePath" runtimeVersion -string "" + sudo -u "$userName" defaults write "$plistFilePath" channelName -string "" sudo -u "$userName" defaults write "$plistFilePath" autoConnect -bool false fi diff --git a/spec/mainApiHandler.spec.ts b/spec/mainApiHandler.spec.ts index ca3bd542..a79be1d9 100644 --- a/spec/mainApiHandler.spec.ts +++ b/spec/mainApiHandler.spec.ts @@ -54,6 +54,8 @@ jest.mock('../src/app/config-handler', () => { uuid: 'some-uuid', licenseKey: 'some-license-key', runtimeVersion: 'some-runtime-version', + channelName: 'some-channel-name', + connectionTimeout: '1000', }, }; }), diff --git a/spec/openfinHandler.spec.ts b/spec/openfinHandler.spec.ts index 9ae517dc..d6a35a9f 100644 --- a/spec/openfinHandler.spec.ts +++ b/spec/openfinHandler.spec.ts @@ -25,6 +25,8 @@ jest.mock('../src/app/config-handler', () => ({ uuid: 'mock-uuid', licenseKey: 'mock-license', runtimeVersion: 'mock-version', + channelName: 'mock-channel', + connectionTimeout: '10000', }, })), }, @@ -43,13 +45,16 @@ describe('Openfin', () => { beforeAll(async () => { connectMock = await connect({} as any); }); + beforeEach(() => { + jest.clearAllMocks(); + }); it('should not be connected', () => { const info = openfinHandler.getInfo(); - const isConnected = openfinHandler.getConnectionStatus(); + const connectionStatus = openfinHandler.getConnectionStatus(); expect(info.isConnected).toBeFalsy(); - expect(isConnected).toBeFalsy(); + expect(connectionStatus.isConnected).toBeFalsy(); }); it('should connect', async () => { @@ -65,6 +70,43 @@ describe('Openfin', () => { expect(isConnected).toBeTruthy(); }); + it('should reject and return false if connection times out', async () => { + jest.useFakeTimers(); + const connectSyncSpy = jest + .spyOn(connectMock.Interop, 'connectSync') + .mockImplementationOnce((_channelName) => { + return new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, 12000); + }); + }); + + const connectionTimeoutSpy = jest.spyOn(global, 'setTimeout'); + + let connectionStatus; + + const connectPromise = openfinHandler.connect(); + const resultPromise = connectPromise.then((res) => { + connectionStatus = res; + }); + + jest.advanceTimersByTime(10000); + + expect(connectionStatus).toBeUndefined(); + + await resultPromise; + + expect(connectionStatus.isConnected).toBe(false); + + expect(connectionTimeoutSpy).toHaveBeenCalledTimes(2); + expect(connectionTimeoutSpy.mock.calls[0][1]).toBeGreaterThanOrEqual(10000); + + expect(connectSyncSpy).toHaveBeenCalledTimes(1); + + jest.useRealTimers(); + }); + it('should fire an intent', async () => { const connectSyncMock = await connectMock.Interop.connectSync(); const fireIntentSpy = jest.spyOn(connectSyncMock, 'fireIntent'); diff --git a/spec/plistHandler.spec.ts b/spec/plistHandler.spec.ts index ab30bbd1..6ef254a2 100644 --- a/spec/plistHandler.spec.ts +++ b/spec/plistHandler.spec.ts @@ -95,7 +95,9 @@ describe('Plist Handler', () => { autoConnect: undefined, licenseKey: undefined, runtimeVersion: undefined, + channelName: undefined, uuid: undefined, + connectionTimeout: undefined, }, }); }); diff --git a/src/app/config-handler.ts b/src/app/config-handler.ts index 864c3f2d..6c142d9c 100644 --- a/src/app/config-handler.ts +++ b/src/app/config-handler.ts @@ -168,7 +168,9 @@ export interface IOpenfin { uuid: string; licenseKey: string; runtimeVersion: string; + channelName: string; autoConnect: boolean; + connectionTimeout: string; } class Config { diff --git a/src/app/main-api-handler.ts b/src/app/main-api-handler.ts index 18d5df80..42d67374 100644 --- a/src/app/main-api-handler.ts +++ b/src/app/main-api-handler.ts @@ -560,18 +560,12 @@ ipcMain.on( helpMenu.setValue(helpCenter); break; - case apiCmds.openfinConnect: - openfinHandler.connect(); - break; case apiCmds.openfinFireIntent: openfinHandler.fireIntent(arg.intent); break; case apiCmds.openfinJoinContextGroup: openfinHandler.joinContextGroup(arg.contextGroupId, arg.target); break; - case apiCmds.openfinRegisterIntentHandler: - openfinHandler.registerIntentHandler(arg.intentName); - break; case apiCmds.openfinUnregisterIntentHandler: openfinHandler.unregisterIntentHandler(arg.intentName); break; @@ -643,6 +637,10 @@ ipcMain.handle( return getContentWindowHandle(windowHandle); } break; + case apiCmds.openfinConnect: + return openfinHandler.connect(); + case apiCmds.openfinRegisterIntentHandler: + return openfinHandler.registerIntentHandler(arg.intentName); case apiCmds.openfinGetConnectionStatus: return openfinHandler.getConnectionStatus(); case apiCmds.openfinGetInfo: diff --git a/src/app/openfin-handler.ts b/src/app/openfin-handler.ts index 52af092e..8131d8d2 100644 --- a/src/app/openfin-handler.ts +++ b/src/app/openfin-handler.ts @@ -1,43 +1,95 @@ import { connect } from '@openfin/node-adapter'; +import { randomUUID, UUID } from 'crypto'; import { logger } from '../common/openfin-logger'; import { config, IConfig } from './config-handler'; import { windowHandler } from './window-handler'; +const OPENFIN_PROVIDER = 'Openfin'; +const TIMEOUT_THRESHOLD = 10000; + export class OpenfinHandler { private interopClient; - private intentHandlerSubscriptions = new Map(); + private intentHandlerSubscriptions: Map = new Map(); private isConnected: boolean = false; + private fin: any; /** * Connection to interop brocker */ public async connect() { - logger.info('openfin-handler: connecting'); const { openfin }: IConfig = config.getConfigFields(['openfin']); - if (openfin) { - const fin = await connect({ - uuid: openfin.uuid, - licenseKey: openfin.licenseKey, - runtime: { - version: openfin.runtimeVersion, - }, - }); - logger.info('openfin-handler: connected'); - logger.info('openfin-handler: connecting to interop broker'); - this.interopClient = fin.Interop.connectSync( - 'workspace-platform-starter', - ); - this.isConnected = true; - this.interopClient.onDisconnection((event) => { - const { brokerName } = event; - logger.warn( - `openfin-handler: Disconnected from Interop Broker ${brokerName} `, - ); - this.clearSubscriptions(); - }); - return; + if (!openfin) { + logger.error('openfin-handler: missing openfin params to connect.'); + return { isConnected: false }; + } + logger.info('openfin-handler: connecting'); + const parsedTimeoutValue = parseInt(openfin.connectionTimeout, 10); + const timeoutValue = isNaN(parsedTimeoutValue) + ? TIMEOUT_THRESHOLD + : parsedTimeoutValue; + const connectionTimeoutPromise = new Promise((_, reject) => + setTimeout(() => { + logger.error( + `openfin-handler: Connection timeout after ${ + timeoutValue / 1000 + } seconds`, + ); + return reject( + new Error(`Connection timeout after ${timeoutValue / 1000} seconds`), + ); + }, timeoutValue), + ); + + const connectionPromise = (async () => { + try { + if (!this.fin) { + this.fin = await connect({ + uuid: openfin.uuid, + licenseKey: openfin.licenseKey, + runtime: { + version: openfin.runtimeVersion, + }, + }); + } + + logger.info( + 'openfin-handler: connection established to Openfin runtime', + ); + logger.info( + `openfin-handler: starting connection to interop broker using channel ${openfin.channelName}`, + ); + + this.interopClient = this.fin.Interop.connectSync(openfin.channelName); + this.isConnected = true; + + this.interopClient.onDisconnection((event) => { + const { brokerName } = event; + logger.warn( + `openfin-handler: Disconnected from Interop Broker ${brokerName}`, + ); + this.clearSubscriptions(); + }); + + return true; + } catch (error) { + logger.error('openfin-handler: error while connecting: ', error); + return false; + } + })(); + + try { + const isConnected = await Promise.race([ + connectionPromise, + connectionTimeoutPromise, + ]); + return { isConnected }; + } catch (error) { + logger.error( + 'openfin-handler: error or timeout while connecting: ', + error, + ); + return { isConnected: false }; } - logger.error('openfin-handler: missing openfin params to connect.'); } /** @@ -50,23 +102,24 @@ export class OpenfinHandler { /** * Adds an intent handler for incoming intents */ - public async registerIntentHandler(intentName: string) { + public async registerIntentHandler(intentName: string): Promise { const unsubscriptionCallback = await this.interopClient.registerIntentHandler( this.intentHandler, intentName, ); - this.intentHandlerSubscriptions.set(intentName, unsubscriptionCallback); + const uuid = randomUUID(); + this.intentHandlerSubscriptions.set(uuid, unsubscriptionCallback); + return uuid; } /** * Removes an intent handler for a given intent */ - public unregisterIntentHandler(intentName) { - const unsubscriptionCallback = - this.intentHandlerSubscriptions.get(intentName); + public unregisterIntentHandler(uuid: UUID) { + const unsubscriptionCallback = this.intentHandlerSubscriptions.get(uuid); unsubscriptionCallback.unsubscribe(); - this.intentHandlerSubscriptions.delete(intentName); + this.intentHandlerSubscriptions.delete(uuid); } /** @@ -114,8 +167,10 @@ export class OpenfinHandler { /** * Returns openfin connection status */ - public getConnectionStatus(): boolean { - return this.isConnected; + public getConnectionStatus() { + return { + isConnected: this.isConnected, + }; } /** @@ -123,15 +178,15 @@ export class OpenfinHandler { */ public getInfo() { return { - provider: 'Openfin', - isConnected: this.getConnectionStatus(), + provider: OPENFIN_PROVIDER, + isConnected: this.getConnectionStatus().isConnected, }; } private intentHandler = (intent: any) => { logger.info('openfin-handler: intent received - ', intent); const mainWebContents = windowHandler.getMainWebContents(); - mainWebContents?.send('intent-received', intent.name); + mainWebContents?.send('intent-received', intent); }; } diff --git a/src/app/plist-handler.ts b/src/app/plist-handler.ts index f3c2ac04..aea532a7 100644 --- a/src/app/plist-handler.ts +++ b/src/app/plist-handler.ts @@ -69,7 +69,9 @@ const OPENFIN = { uuid: 'string', licenseKey: 'string', runtimeVersion: 'string', + channelName: 'string', autoConnect: 'boolean', + connectionTimeout: 'string', }; export const getAllUserDefaults = (): IConfig => { diff --git a/src/common/config-interface.ts b/src/common/config-interface.ts index d1686c7a..ac56fb57 100644 --- a/src/common/config-interface.ts +++ b/src/common/config-interface.ts @@ -12,6 +12,8 @@ export const ConfigFieldsDefaultValues: Partial = { uuid: '', licenseKey: '', runtimeVersion: '', + channelName: '', autoConnect: false, + connectionTimeout: '10000', }, }; diff --git a/src/renderer/ssf-api.ts b/src/renderer/ssf-api.ts index 7c568d6b..05550dbc 100644 --- a/src/renderer/ssf-api.ts +++ b/src/renderer/ssf-api.ts @@ -1,3 +1,4 @@ +import { UUID } from 'crypto'; import { ipcRenderer, webFrame } from 'electron'; import { buildNumber, @@ -66,14 +67,14 @@ export interface ILocalObject { c9MessageCallback?: (status: IShellStatus) => void; updateMyPresenceCallback?: (presence: EPresenceStatusCategory) => void; phoneNumberCallback?: (arg: string) => void; - intentsCallbacks: {}; + intentsCallbacks: Map>; writeImageToClipboard?: (blob: string) => void; getHelpInfo?: () => Promise; } const local: ILocalObject = { ipcRenderer, - intentsCallbacks: {}, + intentsCallbacks: new Map(), }; const notificationActionCallbacks = new Map< @@ -957,10 +958,14 @@ export class SSFApi { /** * Openfin Interop client initialization */ - public openfinInit(): void { - local.ipcRenderer.send(apiName.symphonyApi, { - cmd: apiCmds.openfinConnect, - }); + public async openfinInit(): Promise { + const connectionStatus = await local.ipcRenderer.invoke( + apiName.symphonyApi, + { + cmd: apiCmds.openfinConnect, + }, + ); + return connectionStatus; } /** @@ -1000,25 +1005,39 @@ export class SSFApi { /** * Registers a handler for a given intent */ - public openfinRegisterIntentHandler( + public async openfinRegisterIntentHandler( intentHandler: any, intentName: any, - ): void { - local.intentsCallbacks[intentName] = intentHandler; - local.ipcRenderer.send(apiName.symphonyApi, { + ): Promise { + const uuid: UUID = await local.ipcRenderer.invoke(apiName.symphonyApi, { cmd: apiCmds.openfinRegisterIntentHandler, intentName, }); + if (local.intentsCallbacks.has(intentName)) { + local.intentsCallbacks.get(intentName)?.set(uuid, intentHandler); + } else { + const innerMap = new Map(); + innerMap.set(uuid, intentHandler); + local.intentsCallbacks.set(intentName, innerMap); + } + return uuid; } /** - * Unregisters a handler based on a given intent name - * @param intentName + * Unregisters a handler based on a given intent handler callback id + * @param UUID */ - public openfinUnregisterIntentHandler(intentName: string): void { + public openfinUnregisterIntentHandler(callbackId: UUID): void { + for (const innerMap of local.intentsCallbacks.values()) { + if (innerMap.has(callbackId)) { + innerMap.delete(callbackId); + break; + } + } + local.ipcRenderer.send(apiName.symphonyApi, { cmd: apiCmds.openfinUnregisterIntentHandler, - intentName, + callbackId, }); } @@ -1393,9 +1412,15 @@ local.ipcRenderer.on( }, ); -local.ipcRenderer.on('intent-received', (_event: Event, intentName: string) => { - if (typeof intentName === 'string' && local.intentsCallbacks[intentName]) { - local.intentsCallbacks[intentName](); +local.ipcRenderer.on('intent-received', (_event: Event, intent: any) => { + if ( + typeof intent.name === 'string' && + local.intentsCallbacks.has(intent.name) + ) { + const uuidCallbacks = local.intentsCallbacks.get(intent.name); + uuidCallbacks?.forEach((callbacks, _uuid) => { + callbacks(intent.context); + }); } }); @@ -1407,7 +1432,7 @@ const sanitize = (): void => { windowName: window.name, }); } - local.intentsCallbacks = {}; + local.intentsCallbacks = new Map(); }; // listens for the online/offline events and updates the main process