diff --git a/packages/app-cli/tests/services/plugins/defaultPluginsUtils.ts b/packages/app-cli/tests/services/plugins/defaultPluginsUtils.ts index 8508befc94..b661bc539c 100644 --- a/packages/app-cli/tests/services/plugins/defaultPluginsUtils.ts +++ b/packages/app-cli/tests/services/plugins/defaultPluginsUtils.ts @@ -1,11 +1,9 @@ -import { installDefaultPlugins, getDefaultPluginsInstallState, setSettingsForDefaultPlugins, checkPreInstalledDefaultPlugins } from '@joplin/lib/services/plugins/defaultPlugins/defaultPluginsUtils'; +import { afterDefaultPluginsLoaded, getDefaultPluginPathsAndSettings } from '@joplin/lib/services/plugins/defaultPlugins/defaultPluginsUtils'; import PluginRunner from '../../../app/services/plugins/PluginRunner'; -import { pathExists } from 'fs-extra'; import { checkThrow, setupDatabaseAndSynchronizer, supportDir, switchClient } from '@joplin/lib/testing/test-utils'; -import PluginService, { defaultPluginSetting, DefaultPluginsInfo, PluginSettings } from '@joplin/lib/services/plugins/PluginService'; +import PluginService, { defaultPluginSetting, DefaultPluginsInfo } from '@joplin/lib/services/plugins/PluginService'; import Setting from '@joplin/lib/models/Setting'; -const testPluginDir = `${supportDir}/plugins`; function newPluginService(appVersion = '1.4') { const runner = new PluginRunner(); @@ -27,13 +25,17 @@ function newPluginService(appVersion = '1.4') { describe('defaultPluginsUtils', () => { const pluginsId = ['joplin.plugin.ambrt.backlinksToNote', 'org.joplinapp.plugins.ToggleSidebars']; + const defaultPluginsInfo = { + 'joplin.plugin.ambrt.backlinksToNote': {}, + 'org.joplinapp.plugins.ToggleSidebars': {}, + }; beforeEach(async () => { await setupDatabaseAndSynchronizer(1); await switchClient(1); }); - it('should install default plugins with no previous default plugins installed', (async () => { + it('should load default plugins when nor previously installed', (async () => { const testPluginDir = `${supportDir}/pluginRepo/plugins`; Setting.setValue('installedDefaultPlugins', []); @@ -41,151 +43,69 @@ describe('defaultPluginsUtils', () => { const pluginSettings = service.unserializePluginSettings(Setting.value('plugins.states')); - const newPluginsSettings = await installDefaultPlugins(service, testPluginDir, pluginsId, pluginSettings); + for (const pluginId of pluginsId) { + expect(pluginSettings[pluginId]).toBeFalsy(); + } - const installedPluginPath1 = `${Setting.value('pluginDir')}/${pluginsId[0]}.jpl`; - const installedPluginPath2 = `${Setting.value('pluginDir')}/${pluginsId[1]}.jpl`; - - expect(await pathExists(installedPluginPath1)).toBe(true); - expect(await pathExists(installedPluginPath2)).toBe(true); - - expect(newPluginsSettings[pluginsId[0]]).toMatchObject(defaultPluginSetting()); - expect(newPluginsSettings[pluginsId[1]]).toMatchObject(defaultPluginSetting()); + const pluginPathsAndNewSettings = await getDefaultPluginPathsAndSettings(testPluginDir, defaultPluginsInfo, pluginSettings); + for (const pluginId of pluginsId) { + expect( + pluginPathsAndNewSettings.pluginSettings[pluginId], + ).toMatchObject(defaultPluginSetting()); + } })); - it('should install default plugins with previous default plugins installed', (async () => { - + it('should keep already created default plugins disabled with previous default plugins installed', (async () => { const testPluginDir = `${supportDir}/pluginRepo/plugins`; Setting.setValue('installedDefaultPlugins', ['org.joplinapp.plugins.ToggleSidebars']); + Setting.setValue('plugins.states', { + 'org.joplinapp.plugins.ToggleSidebars': { ...defaultPluginSetting(), enabled: false }, + }); const service = newPluginService('2.1'); const pluginSettings = service.unserializePluginSettings(Setting.value('plugins.states')); + const pluginPathsAndNewSettings = await getDefaultPluginPathsAndSettings(testPluginDir, defaultPluginsInfo, pluginSettings); - const newPluginsSettings = await installDefaultPlugins(service, testPluginDir, pluginsId, pluginSettings); - - const installedPluginPath1 = `${Setting.value('pluginDir')}/${pluginsId[0]}.jpl`; - const installedPluginPath2 = `${Setting.value('pluginDir')}/${pluginsId[1]}.jpl`; - - expect(await pathExists(installedPluginPath1)).toBe(true); - expect(await pathExists(installedPluginPath2)).toBe(false); - - expect(newPluginsSettings[pluginsId[0]]).toMatchObject(defaultPluginSetting()); - expect(newPluginsSettings[pluginsId[1]]).toBeUndefined(); + // Should still be disabled + expect( + pluginPathsAndNewSettings.pluginSettings['org.joplinapp.plugins.ToggleSidebars'].enabled, + ).toBe(false); })); - it('should get default plugins install state', (async () => { - const testCases = [ - { - 'installedDefaultPlugins': [''], - 'loadingPlugins': [`${testPluginDir}/simple`, `${testPluginDir}/jpl_test/org.joplinapp.FirstJplPlugin.jpl`], - 'plugin1DefaultState': defaultPluginSetting(), - 'plugin2DefaultState': defaultPluginSetting(), - 'installedDefaultPlugins1': true, - 'installedDefaultPlugins2': true, - }, - { - 'installedDefaultPlugins': [''], - 'loadingPlugins': [`${testPluginDir}/simple`], - 'plugin1DefaultState': defaultPluginSetting(), - 'plugin2DefaultState': undefined, - 'installedDefaultPlugins1': true, - 'installedDefaultPlugins2': false, - }, - { - 'installedDefaultPlugins': ['org.joplinapp.plugins.Simple'], - 'loadingPlugins': [`${testPluginDir}/simple`, `${testPluginDir}/jpl_test/org.joplinapp.FirstJplPlugin.jpl`], - 'plugin1DefaultState': undefined, - 'plugin2DefaultState': defaultPluginSetting(), - 'installedDefaultPlugins1': true, - 'installedDefaultPlugins2': true, - }, - { - 'installedDefaultPlugins': ['org.joplinapp.plugins.Simple'], - 'loadingPlugins': [`${testPluginDir}/simple`], - 'plugin1DefaultState': undefined, - 'plugin2DefaultState': undefined, - 'installedDefaultPlugins1': true, - 'installedDefaultPlugins2': false, - }, - ]; - - for (const testCase of testCases) { - const service = newPluginService(); - const pluginsId = ['org.joplinapp.plugins.Simple', 'org.joplinapp.FirstJplPlugin']; - - Setting.setValue('installedDefaultPlugins', testCase.installedDefaultPlugins); - await service.loadAndRunPlugins(testCase.loadingPlugins, {}); - - // setting installedDefaultPlugins state - const defaultInstallStates: PluginSettings = getDefaultPluginsInstallState(service, pluginsId); - - expect(defaultInstallStates[pluginsId[0]]).toStrictEqual(testCase.plugin1DefaultState); - expect(defaultInstallStates[pluginsId[1]]).toStrictEqual(testCase.plugin2DefaultState); - - - const installedDefaultPlugins = Setting.value('installedDefaultPlugins'); - expect(installedDefaultPlugins.includes(pluginsId[0])).toBe(testCase.installedDefaultPlugins1); - expect(installedDefaultPlugins.includes(pluginsId[1])).toBe(testCase.installedDefaultPlugins2); - - } - - })); - - it('should check pre-installed default plugins', (async () => { - // with previous pre-installed default plugins - Setting.setValue('installedDefaultPlugins', ['']); - let pluginSettings, installedDefaultPlugins; - - pluginSettings = { [pluginsId[0]]: defaultPluginSetting() }; - checkPreInstalledDefaultPlugins(pluginsId, pluginSettings); - - installedDefaultPlugins = Setting.value('installedDefaultPlugins'); - expect(installedDefaultPlugins.includes(pluginsId[0])).toBe(true); - expect(installedDefaultPlugins.includes(pluginsId[1])).toBe(false); - - - // with no previous pre-installed default plugins - Setting.setValue('installedDefaultPlugins', ['not-a-default-plugin']); - pluginSettings = {}; - checkPreInstalledDefaultPlugins(pluginsId, pluginSettings); - - installedDefaultPlugins = Setting.value('installedDefaultPlugins'); - expect(installedDefaultPlugins.includes(pluginsId[0])).toBe(false); - expect(installedDefaultPlugins.includes(pluginsId[1])).toBe(false); - - })); + const sampleJsBundlePlugin = ` + /* joplin-manifest: + { + "id": "io.github.jackgruber.backup", + "manifest_version": 1, + "app_min_version": "1.4", + "name": "JS Bundle test", + "version": "1.0.0" + } + */ + joplin.plugins.register({ + onStart: async function() { + await joplin.settings.registerSettings({ + path: { + value: "initial-path", + type: 2, + section: "backupSection", + public: true, + label: "Backup path", + }, + }) + }, + });`; it('should set initial settings for default plugins', async () => { const service = newPluginService(); - const pluginScript = ` - /* joplin-manifest: - { - "id": "io.github.jackgruber.backup", - "manifest_version": 1, - "app_min_version": "1.4", - "name": "JS Bundle test", - "version": "1.0.0" - } - */ - joplin.plugins.register({ - onStart: async function() { - await joplin.settings.registerSettings({ - path: { - value: "initial-path", - type: 2, - section: "backupSection", - public: true, - label: "Backup path", - }, - }) - }, - });`; - const plugin = await service.loadPluginFromJsBundle('', pluginScript); + const plugin = await service.loadPluginFromJsBundle('', sampleJsBundlePlugin); + plugin.builtIn = true; await service.runPlugin(plugin); + const runningPlugins = { 'io.github.jackgruber.backup': plugin }; const defaultPluginsInfo: DefaultPluginsInfo = { 'io.github.jackgruber.backup': { @@ -199,46 +119,64 @@ describe('defaultPluginsUtils', () => { // with pre-installed default plugin Setting.setValue('installedDefaultPlugins', ['io.github.jackgruber.backup']); - setSettingsForDefaultPlugins(defaultPluginsInfo); + const pluginSettings = { 'io.github.jackgruber.backup': defaultPluginSetting() }; + + await afterDefaultPluginsLoaded( + runningPlugins, + defaultPluginsInfo, + pluginSettings, + ); expect(Setting.value('plugin-io.github.jackgruber.backup.path')).toBe('initial-path'); - await service.destroy(); // with no pre-installed default plugin Setting.setValue('installedDefaultPlugins', ['']); - setSettingsForDefaultPlugins(defaultPluginsInfo); + await afterDefaultPluginsLoaded( + runningPlugins, + defaultPluginsInfo, + pluginSettings, + ); expect(Setting.value('plugin-io.github.jackgruber.backup.path')).toBe(`${Setting.value('profileDir')}`); await service.destroy(); }); + it('should not overwrite existing settings for a user-installed version of a built-in plugin', async () => { + const service = newPluginService(); + + const plugin = await service.loadPluginFromJsBundle('', sampleJsBundlePlugin); + plugin.builtIn = false; + await service.runPlugin(plugin); + + const defaultPluginsInfo: DefaultPluginsInfo = { + 'io.github.jackgruber.backup': { + settings: { + 'path': 'overwrite?', + }, + }, + }; + + // No pre-installed default plugins + Setting.setValue('installedDefaultPlugins', []); + + // The plugin is running and enabled + const runningPlugins = { 'io.github.jackgruber.backup': plugin }; + const pluginSettings = { 'io.github.jackgruber.backup': defaultPluginSetting() }; + + await afterDefaultPluginsLoaded( + runningPlugins, + defaultPluginsInfo, + pluginSettings, + ); + + // Should not overwrite + expect(Setting.value('plugin-io.github.jackgruber.backup.path')).toBe('initial-path'); + }); + it('should not throw error on missing setting key', async () => { const service = newPluginService(); - const pluginScript = ` - /* joplin-manifest: - { - "id": "io.github.jackgruber.backup", - "manifest_version": 1, - "app_min_version": "1.4", - "name": "JS Bundle test", - "version": "1.0.0" - } - */ - joplin.plugins.register({ - onStart: async function() { - await joplin.settings.registerSettings({ - path: { - value: "initial-path", - type: 2, - section: "backupSection", - public: true, - label: "Backup path", - }, - }) - }, - });`; - - const plugin = await service.loadPluginFromJsBundle('', pluginScript); + const plugin = await service.loadPluginFromJsBundle('', sampleJsBundlePlugin); + plugin.builtIn = true; await service.runPlugin(plugin); const defaultPluginsInfo: DefaultPluginsInfo = { @@ -256,7 +194,10 @@ describe('defaultPluginsUtils', () => { }; Setting.setValue('installedDefaultPlugins', ['']); - expect(checkThrow(() => setSettingsForDefaultPlugins(defaultPluginsInfo))).toBe(false); + const pluginSettings = { 'io.github.jackgruber.backup': defaultPluginSetting() }; + const runningPlugins = { 'io.github.jackgruber.backup': plugin }; + + expect(checkThrow(() => afterDefaultPluginsLoaded(runningPlugins, defaultPluginsInfo, pluginSettings))).toBe(false); expect(Setting.value('plugin-io.github.jackgruber.backup.path')).toBe(`${Setting.value('profileDir')}`); await service.destroy(); }); diff --git a/packages/app-desktop/app.ts b/packages/app-desktop/app.ts index 829a0431ed..267cce4fe9 100644 --- a/packages/app-desktop/app.ts +++ b/packages/app-desktop/app.ts @@ -65,7 +65,7 @@ import { AppState } from './app.reducer'; import syncDebugLog from '@joplin/lib/services/synchronizer/syncDebugLog'; import eventManager, { EventName } from '@joplin/lib/eventManager'; import path = require('path'); -import { checkPreInstalledDefaultPlugins, installDefaultPlugins, setSettingsForDefaultPlugins } from '@joplin/lib/services/plugins/defaultPlugins/defaultPluginsUtils'; +import { afterDefaultPluginsLoaded, loadAndRunDefaultPlugins } from '@joplin/lib/services/plugins/defaultPlugins/defaultPluginsUtils'; import userFetcher, { initializeUserFetcher } from '@joplin/lib/utils/userFetcher'; import { parseNotesParent } from '@joplin/lib/reducer'; import OcrService from '@joplin/lib/services/ocr/OcrService'; @@ -277,7 +277,6 @@ class Application extends BaseApplication { const pluginRunner = new PluginRunner(); service.initialize(packageInfo.version, PlatformImplementation.instance(), pluginRunner, this.store()); service.isSafeMode = Setting.value('isSafeMode'); - const defaultPluginsId = Object.keys(getDefaultPluginsInfo()); let pluginSettings = service.unserializePluginSettings(Setting.value('plugins.states')); { @@ -285,15 +284,11 @@ class Application extends BaseApplication { // time, however we only effectively uninstall the plugin the next // time the app is started. What plugin should be uninstalled is // stored in the settings. - const newSettings = service.clearUpdateState(await service.uninstallPlugins(pluginSettings)); - Setting.setValue('plugins.states', newSettings); + pluginSettings = service.clearUpdateState(await service.uninstallPlugins(pluginSettings)); + Setting.setValue('plugins.states', pluginSettings); } - checkPreInstalledDefaultPlugins(defaultPluginsId, pluginSettings); - try { - const defaultPluginsDir = path.join(bridge().buildDir(), 'defaultPlugins'); - pluginSettings = await installDefaultPlugins(service, defaultPluginsDir, defaultPluginsId, pluginSettings); if (await shim.fsDriver().exists(Setting.value('pluginDir'))) { await service.loadAndRunPlugins(Setting.value('pluginDir'), pluginSettings); } @@ -302,19 +297,31 @@ class Application extends BaseApplication { } try { + const devPluginOptions = { devMode: true, builtIn: false }; + if (Setting.value('plugins.devPluginPaths')) { const paths = Setting.value('plugins.devPluginPaths').split(',').map((p: string) => p.trim()); - await service.loadAndRunPlugins(paths, pluginSettings, true); + await service.loadAndRunPlugins(paths, pluginSettings, devPluginOptions); } // Also load dev plugins that have passed via command line arguments if (Setting.value('startupDevPlugins')) { - await service.loadAndRunPlugins(Setting.value('startupDevPlugins'), pluginSettings, true); + await service.loadAndRunPlugins(Setting.value('startupDevPlugins'), pluginSettings, devPluginOptions); } } catch (error) { this.logger().error(`There was an error loading plugins from ${Setting.value('plugins.devPluginPaths')}:`, error); } + // Load default plugins after loading other plugins -- this allows users + // to override built-in plugins with development versions with the same + // ID. + const defaultPluginsDir = path.join(bridge().buildDir(), 'defaultPlugins'); + try { + pluginSettings = await loadAndRunDefaultPlugins(service, defaultPluginsDir, getDefaultPluginsInfo(), pluginSettings); + } catch (error) { + this.logger().error(`There was an error loading plugins from ${defaultPluginsDir}:`, error); + } + { // Users can potentially delete files from /plugins or even delete // the complete folder. When that happens, we still have the plugin @@ -322,7 +329,7 @@ class Application extends BaseApplication { // out we remove from the state any plugin that has *not* been loaded // above (meaning the file was missing). // https://github.com/laurent22/joplin/issues/5253 - const oldSettings = service.unserializePluginSettings(Setting.value('plugins.states')); + const oldSettings = pluginSettings; const newSettings: PluginSettings = {}; for (const pluginId of Object.keys(oldSettings)) { if (!service.pluginIds.includes(pluginId)) { @@ -332,6 +339,7 @@ class Application extends BaseApplication { newSettings[pluginId] = oldSettings[pluginId]; } Setting.setValue('plugins.states', newSettings); + pluginSettings = newSettings; } this.checkAllPluginStartedIID_ = setInterval(() => { @@ -346,7 +354,7 @@ class Application extends BaseApplication { // tests to wait for plugins to load. ipcRenderer.send('startup-plugins-loaded'); - setSettingsForDefaultPlugins(getDefaultPluginsInfo()); + void afterDefaultPluginsLoaded(service.plugins, getDefaultPluginsInfo(), pluginSettings); } }, 500); } diff --git a/packages/app-desktop/gui/ConfigScreen/ConfigScreen.tsx b/packages/app-desktop/gui/ConfigScreen/ConfigScreen.tsx index 1ba1350ce3..bfb8eb455d 100644 --- a/packages/app-desktop/gui/ConfigScreen/ConfigScreen.tsx +++ b/packages/app-desktop/gui/ConfigScreen/ConfigScreen.tsx @@ -15,9 +15,6 @@ import SyncTargetRegistry from '@joplin/lib/SyncTargetRegistry'; import * as shared from '@joplin/lib/components/shared/config/config-shared.js'; import ClipperConfigScreen from '../ClipperConfigScreen'; import restart from '../../services/restart'; -import PluginService from '@joplin/lib/services/plugins/PluginService'; -import { getDefaultPluginsInstallState, updateDefaultPluginsInstallState } from '@joplin/lib/services/plugins/defaultPlugins/defaultPluginsUtils'; -import getDefaultPluginsInfo from '@joplin/lib/services/plugins/defaultPlugins/desktopDefaultPluginsInfo'; import JoplinCloudConfigScreen from '../JoplinCloudConfigScreen'; import ToggleAdvancedSettingsButton from './controls/ToggleAdvancedSettingsButton'; import shouldShowMissingPasswordWarning from '@joplin/lib/components/shared/config/shouldShowMissingPasswordWarning'; @@ -74,7 +71,6 @@ class ConfigScreenComponent extends React.Component { this.switchSection(this.props.defaultSection); }); } - updateDefaultPluginsInstallState(getDefaultPluginsInstallState(PluginService.instance(), Object.keys(getDefaultPluginsInfo())), this); } private async handleSettingButton(key: string) { diff --git a/packages/app-desktop/gui/ConfigScreen/controls/plugins/PluginBox.tsx b/packages/app-desktop/gui/ConfigScreen/controls/plugins/PluginBox.tsx index bb257d6423..7cdab6379c 100644 --- a/packages/app-desktop/gui/ConfigScreen/controls/plugins/PluginBox.tsx +++ b/packages/app-desktop/gui/ConfigScreen/controls/plugins/PluginBox.tsx @@ -43,6 +43,7 @@ function manifestToItem(manifest: PluginManifest): PluginItem { enabled: true, deleted: false, devMode: false, + builtIn: false, hasBeenUpdated: false, }; } @@ -52,6 +53,7 @@ export interface PluginItem { enabled: boolean; deleted: boolean; devMode: boolean; + builtIn: boolean; hasBeenUpdated: boolean; } @@ -184,7 +186,10 @@ export default function(props: Props) { } function renderDeleteButton() { + // Built-in plugins can only be disabled + if (item.builtIn) return null; if (!props.onDelete) return null; + return