diff --git a/packages/app-cli/app/services/plugins/PluginRunner.ts b/packages/app-cli/app/services/plugins/PluginRunner.ts index 51c320d74..c2cbb6770 100644 --- a/packages/app-cli/app/services/plugins/PluginRunner.ts +++ b/packages/app-cli/app/services/plugins/PluginRunner.ts @@ -5,6 +5,8 @@ import executeSandboxCall from '@joplin/lib/services/plugins/utils/executeSandbo import Global from '@joplin/lib/services/plugins/api/Global'; import mapEventHandlersToIds, { EventHandlers } from '@joplin/lib/services/plugins/utils/mapEventHandlersToIds'; import uuid from '@joplin/lib/uuid'; +import Joplin from '@joplin/lib/services/plugins/api/Joplin'; +import { Console } from 'console'; const sandboxProxy = require('@joplin/lib/services/plugins/sandboxProxy'); function createConsoleWrapper(pluginId: string) { @@ -33,11 +35,18 @@ function createConsoleWrapper(pluginId: string) { // For example, all plugin calls go through a proxy, however they could made directly since // the plugin script is running within the same process as the main app. +interface SandboxProxy { + joplin: Joplin; + console: typeof Console; + stop: ()=> void; +} + export default class PluginRunner extends BasePluginRunner { private eventHandlers_: EventHandlers = {}; // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied private activeSandboxCalls_: any = {}; + private sandboxProxies: Map = new Map(); public constructor() { super(); @@ -52,8 +61,14 @@ export default class PluginRunner extends BasePluginRunner { } private newSandboxProxy(pluginId: string, sandbox: Global) { + let stopped = false; + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied const target = async (path: string, args: any[]) => { + if (stopped) { + throw new Error(`Plugin with ID ${pluginId} has been stopped. Cannot execute sandbox call.`); + } + const callId = `${pluginId}::${path}::${uuid.createNano()}`; this.activeSandboxCalls_[callId] = true; const promise = executeSandboxCall(pluginId, sandbox, `joplin.${path}`, mapEventHandlersToIds(args, this.eventHandlers_), this.eventHandler); @@ -64,10 +79,15 @@ export default class PluginRunner extends BasePluginRunner { return promise; }; - return { + const proxy = { joplin: sandboxProxy(target), console: createConsoleWrapper(pluginId), + stop: () => { + stopped = true; + }, }; + this.sandboxProxies.set(pluginId, proxy); + return proxy; } public async run(plugin: Plugin, sandbox: Global): Promise { @@ -90,6 +110,13 @@ export default class PluginRunner extends BasePluginRunner { }); } + public async stop(plugin: Plugin): Promise { + // TODO: Node VM doesn't support stopping plugins without running them in a child process. + // For now, we stop the plugin by making interactions with the Joplin API throw Errors. + const proxy = this.sandboxProxies.get(plugin.id); + proxy?.stop(); + } + public async waitForSandboxCalls(): Promise { const startTime = Date.now(); // eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied diff --git a/packages/app-cli/tests/services/plugins/PluginService.ts b/packages/app-cli/tests/services/plugins/PluginService.ts index 7bd5b1b9f..a0c386f3f 100644 --- a/packages/app-cli/tests/services/plugins/PluginService.ts +++ b/packages/app-cli/tests/services/plugins/PluginService.ts @@ -1,5 +1,5 @@ import PluginRunner from '../../../app/services/plugins/PluginRunner'; -import PluginService, { PluginSettings } from '@joplin/lib/services/plugins/PluginService'; +import PluginService, { PluginSettings, defaultPluginSetting } from '@joplin/lib/services/plugins/PluginService'; import { ContentScriptType } from '@joplin/lib/services/plugins/api/types'; import MdToHtml from '@joplin/renderer/MdToHtml'; import shim from '@joplin/lib/shim'; @@ -9,6 +9,7 @@ import Note from '@joplin/lib/models/Note'; import Folder from '@joplin/lib/models/Folder'; import { expectNotThrow, setupDatabaseAndSynchronizer, switchClient, expectThrow, createTempDir, supportDir, mockMobilePlatform } from '@joplin/lib/testing/test-utils'; import { newPluginScript } from '../../testUtils'; +import { join } from 'path'; const testPluginDir = `${supportDir}/plugins`; @@ -397,4 +398,78 @@ describe('services_PluginService', () => { expect(newPluginSettings[pluginId1]).toBe(undefined); expect(newPluginSettings[pluginId2]).toBe(undefined); }); + + it('re-running loadAndRunPlugins should reload plugins that have changed but keep unchanged plugins running', async () => { + const testDir = await createTempDir(); + try { + const loadCounterNote = await Note.save({ title: 'Log of plugin loads' }); + const readLoadCounterNote = async () => { + return (await Note.load(loadCounterNote.id)).body; + }; + expect(await readLoadCounterNote()).toBe(''); + + const writePluginScript = async (version: string, id: string) => { + const script = ` + /* joplin-manifest: + { + "id": ${JSON.stringify(id)}, + "manifest_version": 1, + "app_min_version": "1.0.0", + "name": "JS Bundle test", + "version": ${JSON.stringify(version)} + } + */ + + joplin.plugins.register({ + onStart: async function() { + const noteId = ${JSON.stringify(loadCounterNote.id)}; + const pluginId = ${JSON.stringify(id)}; + const note = await joplin.data.get(['notes', noteId], { fields: ['body'] }); + const newBody = note.body + '\\n' + pluginId; + await joplin.data.put(['notes', noteId], null, { body: newBody.trim() }); + }, + }); + `; + await fs.writeFile(join(testDir, `${id}.bundle.js`), script); + }; + + const service = newPluginService(); + const pluginId1 = 'org.joplinapp.testPlugin1'; + await writePluginScript('0.0.1', pluginId1); + const pluginId2 = 'org.joplinapp.testPlugin2'; + await writePluginScript('0.0.1', pluginId2); + + let pluginSettings: PluginSettings = { + [pluginId1]: defaultPluginSetting(), + [pluginId2]: defaultPluginSetting(), + }; + await service.loadAndRunPlugins(testDir, pluginSettings); + + // Plugins should initially load once + expect(service.pluginIds).toHaveLength(2); + expect(service.pluginById(pluginId1).running).toBe(true); + expect(service.pluginById(pluginId2).running).toBe(true); + expect(await readLoadCounterNote()).toBe(`${pluginId1}\n${pluginId2}`); + + // Updating just plugin 1 reload just plugin 1. + await writePluginScript('0.0.2', pluginId1); + await service.loadAndRunPlugins(testDir, pluginSettings); + + expect(service.pluginById(pluginId1).running).toBe(true); + expect(service.pluginById(pluginId2).running).toBe(true); + expect(await readLoadCounterNote()).toBe(`${pluginId1}\n${pluginId2}\n${pluginId1}`); + + // Disabling plugin 1 should not reload plugin 2 + pluginSettings = { ...pluginSettings, [pluginId1]: { ...defaultPluginSetting(), enabled: false } }; + await service.loadAndRunPlugins(testDir, pluginSettings); + + expect(service.pluginById(pluginId1).running).toBe(false); + expect(service.pluginById(pluginId2).running).toBe(true); + expect(await readLoadCounterNote()).toBe(`${pluginId1}\n${pluginId2}\n${pluginId1}`); + + await service.destroy(); + } finally { + await fs.remove(testDir); + } + }); }); diff --git a/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.test.tsx b/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.test.tsx index 05292d8b8..e81f26e00 100644 --- a/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.test.tsx +++ b/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.test.tsx @@ -24,13 +24,12 @@ const shouldShowBasedOnSettingSearchQuery = ()=>true; const PluginStatesWrapper = (props: WrapperProps) => { const styles = configScreenStyles(Setting.THEME_LIGHT); - const [pluginStates, setPluginStates] = useState(() => { - return PluginService.instance().serializePluginSettings(props.initialPluginSettings ?? {}); + const [pluginSettings, setPluginSettings] = useState(() => { + return props.initialPluginSettings ?? {}; }); const updatePluginStates = useCallback((newStates: PluginSettings) => { - const serialized = PluginService.instance().serializePluginSettings(newStates); - setPluginStates(serialized); + setPluginSettings(newStates); }, []); return ( @@ -38,7 +37,7 @@ const PluginStatesWrapper = (props: WrapperProps) => { themeId={Setting.THEME_LIGHT} styles={styles} updatePluginStates={updatePluginStates} - pluginSettings={pluginStates} + pluginSettings={pluginSettings} shouldShowBasedOnSearchQuery={shouldShowBasedOnSettingSearchQuery} /> ); diff --git a/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.tsx b/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.tsx index d2953f3b6..8af0d2d38 100644 --- a/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.tsx +++ b/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.tsx @@ -1,21 +1,23 @@ import * as React from 'react'; -import { useCallback, useState } from 'react'; +import { useCallback, useMemo, useRef, useState } from 'react'; import { ConfigScreenStyles } from '../configScreenStyles'; import { View } from 'react-native'; import { Banner, Button, Text } from 'react-native-paper'; import { _ } from '@joplin/lib/locale'; -import PluginService, { PluginSettings } from '@joplin/lib/services/plugins/PluginService'; +import PluginService, { PluginSettings, SerializedPluginSettings } from '@joplin/lib/services/plugins/PluginService'; import PluginToggle from './PluginToggle'; import SearchPlugins from './SearchPlugins'; import { ItemEvent } from '@joplin/lib/components/shared/config/plugins/types'; import NavService from '@joplin/lib/services/NavService'; import useRepoApi from './utils/useRepoApi'; import RepositoryApi from '@joplin/lib/services/plugins/RepositoryApi'; +import shim from '@joplin/lib/shim'; +import Logger from '@joplin/utils/Logger'; interface Props { themeId: number; styles: ConfigScreenStyles; - pluginSettings: string; + pluginSettings: SerializedPluginSettings; settingsSearchQuery?: string; updatePluginStates: (settingValue: PluginSettings)=> void; @@ -35,6 +37,42 @@ export const getSearchText = () => { return searchText; }; +const logger = Logger.create('PluginStates'); + +// Loaded plugins: All plugins with available manifests. +const useLoadedPluginIds = (pluginSettings: SerializedPluginSettings) => { + const allPluginIds = useMemo(() => { + return Object.keys( + PluginService.instance().unserializePluginSettings(pluginSettings), + ); + }, [pluginSettings]); + + const [pluginReloadCounter, setPluginReloadCounter] = useState(0); + const loadedPluginIds = useMemo(() => { + if (pluginReloadCounter > 0) { + logger.debug(`Not all plugins were loaded in the last render. Re-loading (try ${pluginReloadCounter})`); + } + + const pluginService = PluginService.instance(); + return allPluginIds.filter(id => !!pluginService.plugins[id]); + }, [allPluginIds, pluginReloadCounter]); + const hasLoadingPlugins = loadedPluginIds.length !== allPluginIds.length; + + // Force a re-render if not all plugins have available metadata. This can happen + // if plugins are still loading. + const pluginReloadCounterRef = useRef(0); + pluginReloadCounterRef.current = pluginReloadCounter; + const timeoutRef = useRef(null); + if (hasLoadingPlugins && !timeoutRef.current) { + timeoutRef.current = shim.setTimeout(() => { + timeoutRef.current = null; + setPluginReloadCounter(pluginReloadCounterRef.current + 1); + }, 1000); + } + + return loadedPluginIds; +}; + const PluginStates: React.FC = props => { const [repoApiError, setRepoApiError] = useState(null); const [repoApiLoaded, setRepoApiLoaded] = useState(false); @@ -91,15 +129,17 @@ const PluginStates: React.FC = props => { const installedPluginCards = []; const pluginService = PluginService.instance(); - for (const key in pluginService.plugins) { - const plugin = pluginService.plugins[key]; + + const pluginIds = useLoadedPluginIds(props.pluginSettings); + for (const pluginId of pluginIds) { + const plugin = pluginService.plugins[pluginId]; if (!props.shouldShowBasedOnSearchQuery || props.shouldShowBasedOnSearchQuery(plugin.manifest.name)) { installedPluginCards.push( ; repoApi: RepositoryApi; diff --git a/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginUploadButton.tsx b/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginUploadButton.tsx index af9639f93..988dc7492 100644 --- a/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginUploadButton.tsx +++ b/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginUploadButton.tsx @@ -1,6 +1,6 @@ import { _ } from '@joplin/lib/locale'; -import PluginService, { PluginSettings, defaultPluginSetting } from '@joplin/lib/services/plugins/PluginService'; +import PluginService, { PluginSettings, SerializedPluginSettings, defaultPluginSetting } from '@joplin/lib/services/plugins/PluginService'; import * as React from 'react'; import { useCallback, useState } from 'react'; import { Button } from 'react-native-paper'; @@ -14,7 +14,7 @@ import Setting from '@joplin/lib/models/Setting'; interface Props { updatePluginStates: (settingValue: PluginSettings)=> void; - pluginSettings: string; + pluginSettings: SerializedPluginSettings; } const logger = Logger.create('PluginUploadButton'); diff --git a/packages/app-mobile/components/screens/ConfigScreen/plugins/SearchPlugins.test.tsx b/packages/app-mobile/components/screens/ConfigScreen/plugins/SearchPlugins.test.tsx index 20d28cba5..451d3999e 100644 --- a/packages/app-mobile/components/screens/ConfigScreen/plugins/SearchPlugins.test.tsx +++ b/packages/app-mobile/components/screens/ConfigScreen/plugins/SearchPlugins.test.tsx @@ -7,8 +7,7 @@ import '@testing-library/react-native/extend-expect'; import SearchPlugins from './SearchPlugins'; import Setting from '@joplin/lib/models/Setting'; -import PluginService, { PluginSettings } from '@joplin/lib/services/plugins/PluginService'; -import { useMemo } from 'react'; +import { PluginSettings } from '@joplin/lib/services/plugins/PluginService'; import pluginServiceSetup from './testUtils/pluginServiceSetup'; import newRepoApi from './testUtils/newRepoApi'; @@ -22,14 +21,10 @@ interface WrapperProps { const noOpFunction = ()=>{}; const SearchWrapper = (props: WrapperProps) => { - const serializedPluginSettings = useMemo(() => { - return PluginService.instance().serializePluginSettings(props.pluginSettings ?? {}); - }, [props.pluginSettings]); - return ( void; repoApi: RepositoryApi; diff --git a/packages/app-mobile/plugins/PluginRunner/PluginRunnerWebView.tsx b/packages/app-mobile/plugins/PluginRunner/PluginRunnerWebView.tsx index 1d033c7db..06581c367 100644 --- a/packages/app-mobile/plugins/PluginRunner/PluginRunnerWebView.tsx +++ b/packages/app-mobile/plugins/PluginRunner/PluginRunnerWebView.tsx @@ -8,7 +8,7 @@ import loadPlugins from '../loadPlugins'; import { connect, useStore } from 'react-redux'; import Logger from '@joplin/utils/Logger'; import { View } from 'react-native'; -import PluginService, { PluginSettings } from '@joplin/lib/services/plugins/PluginService'; +import PluginService, { PluginSettings, SerializedPluginSettings } from '@joplin/lib/services/plugins/PluginService'; import { PluginHtmlContents, PluginStates } from '@joplin/lib/services/plugins/reducer'; import useAsyncEffect from '@joplin/lib/hooks/useAsyncEffect'; import PluginDialogManager from './dialogs/PluginDialogManager'; @@ -16,7 +16,7 @@ import { AppState } from '../../utils/types'; const logger = Logger.create('PluginRunnerWebView'); -const usePluginSettings = (serializedPluginSettings: string) => { +const usePluginSettings = (serializedPluginSettings: SerializedPluginSettings) => { return useMemo(() => { const pluginService = PluginService.instance(); return pluginService.unserializePluginSettings(serializedPluginSettings); @@ -41,7 +41,7 @@ const usePlugins = ( interface Props { - serializedPluginSettings: string; + serializedPluginSettings: SerializedPluginSettings; pluginStates: PluginStates; pluginHtmlContents: PluginHtmlContents; themeId: number; diff --git a/packages/app-mobile/plugins/loadPlugins.ts b/packages/app-mobile/plugins/loadPlugins.ts index 4e70ea53e..34aaa18a3 100644 --- a/packages/app-mobile/plugins/loadPlugins.ts +++ b/packages/app-mobile/plugins/loadPlugins.ts @@ -23,10 +23,11 @@ const loadPlugins = async ( ); pluginService.isSafeMode = Setting.value('isSafeMode'); - // Unload any existing plugins (important for React Native's fast refresh) - logger.debug('Unloading plugins...'); - for (const pluginId of pluginService.pluginIds) { - await pluginService.unloadPlugin(pluginId); + for (const pluginId of Object.keys(pluginService.plugins)) { + if (!pluginSettings[pluginId]?.enabled) { + logger.info('Unloading disabled plugin', pluginId); + await pluginService.unloadPlugin(pluginId); + } if (cancel.cancelled) { return; diff --git a/packages/lib/services/plugins/Plugin.ts b/packages/lib/services/plugins/Plugin.ts index bdc6ae5d4..d974c9a6c 100644 --- a/packages/lib/services/plugins/Plugin.ts +++ b/packages/lib/services/plugins/Plugin.ts @@ -43,6 +43,7 @@ export default class Plugin { private dataDir_: string; private dataDirCreated_ = false; private hasErrors_ = false; + private running_ = false; private onUnloadListeners_: OnUnloadListener[] = []; // eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied @@ -87,6 +88,14 @@ export default class Plugin { return this.baseDir_; } + public get running(): boolean { + return this.running_; + } + + public set running(running: boolean) { + this.running_ = running; + } + public async dataDir(): Promise { if (this.dataDirCreated_) return this.dataDir_; diff --git a/packages/lib/services/plugins/PluginService.ts b/packages/lib/services/plugins/PluginService.ts index 45e130497..a6ef8b48d 100644 --- a/packages/lib/services/plugins/PluginService.ts +++ b/packages/lib/services/plugins/PluginService.ts @@ -68,6 +68,8 @@ export interface PluginSettings { [pluginId: string]: PluginSetting; } +export type SerializedPluginSettings = Record>; + interface PluginLoadOptions { devMode: boolean; builtIn: boolean; @@ -158,6 +160,7 @@ export default class PluginService extends BaseService { plugin.onUnload(); await this.runner_.stop(plugin); + plugin.running = false; this.deletePluginAt(pluginId); this.startedPlugins_ = { ...this.startedPlugins_ }; @@ -177,8 +180,7 @@ export default class PluginService extends BaseService { return this.plugins_[id]; } - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - public unserializePluginSettings(settings: any): PluginSettings { + public unserializePluginSettings(settings: SerializedPluginSettings): PluginSettings { const output = { ...settings }; for (const pluginId in output) { @@ -188,7 +190,7 @@ export default class PluginService extends BaseService { }; } - return output; + return output as PluginSettings; } public serializePluginSettings(settings: PluginSettings): string { @@ -410,11 +412,32 @@ export default class PluginService extends BaseService { try { const plugin = await this.loadPluginFromPath(pluginPath); + const enabled = this.pluginEnabled(settings, plugin.id); - // After transforming the plugin path to an ID, multiple plugins might end up with the same ID. For - // example "MyPlugin" and "myplugin" would have the same ID. Technically it's possible to have two - // such folders but to keep things sane we disallow it. - if (this.plugins_[plugin.id]) throw new Error(`There is already a plugin with this ID: ${plugin.id}`); + const existingPlugin = this.plugins_[plugin.id]; + if (existingPlugin) { + const isSamePlugin = existingPlugin.baseDir === plugin.baseDir; + + // On mobile, plugins can reload without restarting the app. If a plugin is currently + // running and hasn't changed, it doesn't need to be reloaded. + if (isSamePlugin) { + const isSameVersion = + existingPlugin.manifest.version === plugin.manifest.version + && existingPlugin.manifest._package_hash === plugin.manifest._package_hash; + if (isSameVersion && existingPlugin.running === enabled) { + logger.debug('Not reloading same-version plugin', plugin.id); + continue; + } else { + logger.info('Reloading plugin with ID', plugin.id); + await this.unloadPlugin(plugin.id); + } + } else { + // After transforming the plugin path to an ID, multiple plugins might end up with the same ID. For + // example "MyPlugin" and "myplugin" would have the same ID. Technically it's possible to have two + // such folders but to keep things sane we disallow it. + throw new Error(`There is already a plugin with this ID: ${plugin.id}`); + } + } // We mark the plugin as built-in even if not enabled (being built-in affects // update UI). @@ -422,7 +445,7 @@ export default class PluginService extends BaseService { this.setPluginAt(plugin.id, plugin); - if (!this.pluginEnabled(settings, plugin.id)) { + if (!enabled) { logger.info(`Not running disabled plugin: "${plugin.id}"`); continue; } @@ -507,6 +530,7 @@ export default class PluginService extends BaseService { plugin.on('started', onStarted); + plugin.running = true; const pluginApi = new Global(this.platformImplementation_, plugin, this.store_); return this.runner_.run(plugin, pluginApi); }