From e7ee7fe2b0765c25de5cf3aeb281e3b0a1ec4fa6 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Tue, 19 Jan 2021 14:49:15 -0500 Subject: [PATCH] Auto-clean extension handlers on deactivation - remove handlers when an extension is deactivated or removed - make sure that the found extension when routing a request is currently enabled (as a backup) - added documentation about the above behaviour to the guide - tweaked the naming convension so that it is clearer that the router uses extension names as not IDs (which currently are folder paths) Signed-off-by: Sebastian Malton --- docs/extensions/guides/protocol-handlers.md | 6 +++++ src/common/protocol-handler/router.ts | 25 ++++++++++----------- src/extensions/extension-loader.ts | 1 - src/extensions/extensions-store.ts | 6 ++++- src/extensions/lens-main-extension.ts | 8 +++++++ src/extensions/lens-renderer-extension.ts | 8 +++++++ src/main/protocol-handler/router.ts | 7 +++--- src/renderer/protocol-handler/router.ts | 20 ++++++++--------- 8 files changed, 53 insertions(+), 28 deletions(-) diff --git a/docs/extensions/guides/protocol-handlers.md b/docs/extensions/guides/protocol-handlers.md index ff8a727796..b3d8bcdb5c 100644 --- a/docs/extensions/guides/protocol-handlers.md +++ b/docs/extensions/guides/protocol-handlers.md @@ -45,3 +45,9 @@ On the other hand, the subpath `"/display/notification"` would be routed to #3. The URI is routed to the most specific matching `pathSchema`. This way the `"/"` (root) `pathSchema` acts as a sort of catch all or default route if no other route matches. + +### Cleaning Up + +Currently there is not way to remove a protocol handler once it has been registered. +Handlers will not be called if the extension is deactivated or uninstalled. +This means that the handlers should be added (or re-added as the case may be) on every activation of an extension instance. diff --git a/src/common/protocol-handler/router.ts b/src/common/protocol-handler/router.ts index eaba3e780f..34bc7b9ece 100644 --- a/src/common/protocol-handler/router.ts +++ b/src/common/protocol-handler/router.ts @@ -1,4 +1,3 @@ -import { LensExtensionId } from "../../extensions/lens-extension"; import { hasOwnProperties, hasOwnProperty, Singleton } from "../utils"; const ProtocolHandlerIpcPrefix = "protocol-handler"; @@ -22,7 +21,7 @@ export enum HandlerType { interface ExtensionParams { handlerType: HandlerType.EXTENSION, - extensionId: string, + extensionName: string, } interface InternalParams { @@ -37,7 +36,7 @@ export type RegisterParams = BaseParams & { }; export interface DeregisterParams { - extensionId: string, + extensionName: string, } export type BackChannelParams = BaseParams & { @@ -49,8 +48,8 @@ export abstract class LensProtocolRouter extends Singleton { public static readonly LoggingPrefix = "[PROTOCOL ROUTER]"; public abstract on(urlSchema: string, handler: RouteHandler): void; - public abstract extensionOn(id: LensExtensionId, urlSchema: string, handler: RouteHandler): void; - public abstract removeExtensionHandlers(id: LensExtensionId): void; + public abstract extensionOn(extName: string, urlSchema: string, handler: RouteHandler): void; + public abstract removeExtensionHandlers(extName: string): void; } /** @@ -75,15 +74,15 @@ function validateBaseParams(args: unknown): args is BaseParams { } if (handlerType === HandlerType.EXTENSION) { - if (!hasOwnProperty(args, "extensionId")) { + if (!hasOwnProperty(args, "extensionName")) { return false; } // or handlerType must be HandlerType.EXTENSION - const { extensionId } = args; + const { extensionName } = args; - // but if for an extension then the extensionId is required, must be a stirng, and must be non-empty - return Boolean(extensionId && typeof extensionId === "string"); + // but if for an extension then the extensionName is required, must be a stirng, and must be non-empty + return Boolean(extensionName && typeof extensionName === "string"); } // reject all other values of handlerType @@ -121,16 +120,16 @@ export function validateRegisterParams(args: unknown): args is RegisterParams { * @param args a deserialized value */ export function validateDeregisterParams(args: unknown): args is DeregisterParams { - if (args == null || typeof args !== "object") { + if (!args || typeof args !== "object") { // it must be an object return false; } - if (!hasOwnProperties(args, "extensionId")) { + if (!hasOwnProperties(args, "extensionName")) { return false; } - if (typeof args.extensionId !== "string" || args.extensionId.length === 0) { + if (typeof args.extensionName !== "string" || args.extensionName.length === 0) { // ipcChannel is required, must be a string, must be non-empty return false; } @@ -143,7 +142,7 @@ export function validateDeregisterParams(args: unknown): args is DeregisterParam * @param args a deserialized value */ export function validateRouteParams(args: unknown): args is RouteParams { - if (args == null || typeof args !== "object") { + if (!args || typeof args !== "object") { // it must be an object return false; } diff --git a/src/extensions/extension-loader.ts b/src/extensions/extension-loader.ts index 98697d252c..855d902c00 100644 --- a/src/extensions/extension-loader.ts +++ b/src/extensions/extension-loader.ts @@ -102,7 +102,6 @@ export class ExtensionLoader { } catch (error) { logger.error(`${logModule}: deactivation extension error`, { lensExtensionId, error }); } - } removeExtension(lensExtensionId: LensExtensionId) { diff --git a/src/extensions/extensions-store.ts b/src/extensions/extensions-store.ts index 0885bbb730..41602be6df 100644 --- a/src/extensions/extensions-store.ts +++ b/src/extensions/extensions-store.ts @@ -27,7 +27,7 @@ export class ExtensionsStore extends BaseStore { protected state = observable.map(); - isEnabled(extId: LensExtensionId) { + isEnabled(extId: LensExtensionId): boolean { const state = this.state.get(extId); // By default false, so that copied extensions are disabled by default. @@ -35,6 +35,10 @@ export class ExtensionsStore extends BaseStore { return Boolean(state?.enabled); } + isEnabledByName(extName: string): boolean { + return this.enabledExtensions.includes(extName); + } + @action mergeState(extensionsState: Record) { this.state.merge(extensionsState); diff --git a/src/extensions/lens-main-extension.ts b/src/extensions/lens-main-extension.ts index 98dc70e6ec..4f2b0a485b 100644 --- a/src/extensions/lens-main-extension.ts +++ b/src/extensions/lens-main-extension.ts @@ -19,6 +19,14 @@ export class LensMainExtension extends LensExtension { await windowManager.navigate(pageUrl, frameId); } + async disable() { + const lprm = LensProtocolRouterMain.getInstance(); + + lprm.removeExtensionHandlers(this.name); + + return super.disable(); + } + /** * Registers a handler to be called when a `lens://` link is called. * @param pathSchema The path schema for the route. diff --git a/src/extensions/lens-renderer-extension.ts b/src/extensions/lens-renderer-extension.ts index 1e54bc0297..4dd9f0f4da 100644 --- a/src/extensions/lens-renderer-extension.ts +++ b/src/extensions/lens-renderer-extension.ts @@ -30,6 +30,14 @@ export class LensRendererExtension extends LensExtension { navigate(pageUrl); } + async disable() { + const lprm = LensProtocolRouterRenderer.getInstance(); + + lprm.removeExtensionHandlers(this.name); + + return super.disable(); + } + /** * Defines if extension is enabled for a given cluster. Defaults to `true`. */ diff --git a/src/main/protocol-handler/router.ts b/src/main/protocol-handler/router.ts index 91635c00bd..a26ae554b6 100644 --- a/src/main/protocol-handler/router.ts +++ b/src/main/protocol-handler/router.ts @@ -7,6 +7,7 @@ import * as proto from "../../common/protocol-handler"; import { LensExtensionId } from "../../extensions/lens-extension"; import { ipcMain } from "electron"; import { WindowManager } from "../window-manager"; +import { extensionsStore } from "../../extensions/extensions-store"; const EXTENSION_PUBLISHER_MATCH = "LENS_INTERNAL_EXTENSION_PUBLISHER_MATCH"; const EXTENSION_NAME_MATCH = "LENS_INTERNAL_EXTENSION_NAME_MATCH"; @@ -68,7 +69,7 @@ function registerIpcHandler(event: Electron.IpcMainEvent, ...ipcArgs: unknown[]) case proto.HandlerType.INTERNAL: return lprm.on(args.pathSchema, produceNotifyRenderer(args.handlerId)); case proto.HandlerType.EXTENSION: - return lprm.extensionOn(args.extensionId, args.pathSchema, produceNotifyRenderer(args.handlerId)); + return lprm.extensionOn(args.extensionName, args.pathSchema, produceNotifyRenderer(args.handlerId)); } } @@ -79,7 +80,7 @@ function deregisterIpcHandler(event: Electron.IpcMainEvent, ...ipcArgs: unknown[ return void logger.warn(`${proto.LensProtocolRouter.LoggingPrefix}: ipc call to "${proto.ProtocolHandlerDeregister}" invalid arguments`, { ipcArgs }); } - LensProtocolRouterMain.getInstance().removeExtensionHandlers(args.extensionId); + LensProtocolRouterMain.getInstance().removeExtensionHandlers(args.extensionName); } export class LensProtocolRouterMain extends proto.LensProtocolRouter { @@ -128,7 +129,7 @@ export class LensProtocolRouterMain extends proto.LensProtocolRouter { let routes = this.extentionRoutes.get(name); - if (!routes) { + if (!routes || !extensionsStore.isEnabledByName(name)) { if (this.missingExtensionHandlers.length === 0) { throw new proto.RoutingError(proto.RoutingErrorType.MISSING_EXTENSION, url); } diff --git a/src/renderer/protocol-handler/router.ts b/src/renderer/protocol-handler/router.ts index 310a30069b..8a42b7cdb9 100644 --- a/src/renderer/protocol-handler/router.ts +++ b/src/renderer/protocol-handler/router.ts @@ -4,7 +4,7 @@ import { autobind } from "../utils"; import * as uuid from "uuid"; import logger from "../../main/logger"; export class LensProtocolRouterRenderer extends proto.LensProtocolRouter { - // Map between extension IDs and a Map betweeen generated UUIDs and the handlers + // Map between extension names and a Map betweeen generated UUIDs and the handlers private extensionHandlers = new Map>(); // Map between generated UUIDs and the handlers private internalHandlers = new Map(); @@ -37,8 +37,8 @@ export class LensProtocolRouterRenderer extends proto.LensProtocolRouter { } case proto.HandlerType.EXTENSION: { - const { handlerId, params, extensionId } = args; - const handler = this.extensionHandlers.get(handlerId)?.get(extensionId); + const { handlerId, params, extensionName } = args; + const handler = this.extensionHandlers.get(handlerId)?.get(extensionName); if (!handler) { return void logger.error(`${proto.LensProtocolRouter.LoggingPrefix}: ipc call to "${proto.ProtocolHandlerBackChannel}" unknown handlerId or unknown extensionId`, { args }); @@ -63,30 +63,30 @@ export class LensProtocolRouterRenderer extends proto.LensProtocolRouter { ipcRenderer.send(proto.ProtocolHandlerRegister, args); } - public extensionOn(extensionId: string, pathSchema: string, handler: proto.RouteHandler): void { + public extensionOn(extensionName: string, pathSchema: string, handler: proto.RouteHandler): void { const handlerId = uuid.v4(); const args: proto.RegisterParams = { handlerType: proto.HandlerType.EXTENSION, - extensionId, + extensionName, pathSchema, handlerId, }; this.extensionHandlers - .set(extensionId, this.extensionHandlers.get(extensionId) ?? new Map()) - .get(extensionId) + .set(extensionName, this.extensionHandlers.get(extensionName) ?? new Map()) + .get(extensionName) .set(handlerId, handler); ipcRenderer.send(proto.ProtocolHandlerRegister, args); } - public removeExtensionHandlers(extensionId: string): void { + public removeExtensionHandlers(extensionName: string): void { const args: proto.DeregisterParams = { - extensionId, + extensionName, }; ipcRenderer.send(proto.ProtocolHandlerDeregister, args); - this.extensionHandlers.delete(extensionId); + this.extensionHandlers.delete(extensionName); } }