Merge branch 'dev' into disabled_plugin

packages/lib/services/plugins/PluginService.ts

@@ -287,15 +287,25 @@ export default class PluginService extends BaseService {
 		return this.loadPlugin(baseDir, r.manifestText, r.scriptText, pluginIdIfNotSpecified);
 	}
 
-	public async loadPluginFromPackage(baseDir: string, path: string): Promise<Plugin> {
+	public async loadPluginFromPackage(baseDir: string, path: string, manifestOnly = false): Promise<Plugin> {
 		baseDir = rtrimSlashes(baseDir);
 
 		const fname = filename(path);
-		const hash = await shim.fsDriver().md5File(path);
-
 		const unpackDir = `${Setting.value('cacheDir')}/${fname}`;
 		const manifestFilePath = `${unpackDir}/manifest.json`;
 
+		if (manifestOnly) {
+			// When loading only the manifest (e.g. for disabled plugins), try
+			// to use an already-extracted manifest from cache to avoid the
+			// expensive MD5 hash and tar extraction.
+			const manifest = await this.loadManifestToObject(manifestFilePath);
+			if (manifest) {
+				return this.loadPlugin(unpackDir, JSON.stringify(manifest), '', makePluginId(fname));
+			}
+		}
+
+		const hash = await shim.fsDriver().md5File(path);
+
 		// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
 		let manifest: any = await this.loadManifestToObject(manifestFilePath);
 
@@ -318,7 +328,7 @@ export default class PluginService extends BaseService {
 			await shim.fsDriver().writeFile(manifestFilePath, JSON.stringify(manifest, null, '\t'), 'utf8');
 		}
 
-		return this.loadPluginFromPath(unpackDir);
+		return this.loadPluginFromPath(unpackDir, manifestOnly);
 	}
 
 	// Loads the manifest as a simple object with no validation. Used only
@@ -333,7 +343,7 @@ export default class PluginService extends BaseService {
 		}
 	}
 
-	public async loadPluginFromPath(path: string): Promise<Plugin> {
+	public async loadPluginFromPath(path: string, manifestOnly = false): Promise<Plugin> {
 		path = rtrimSlashes(path);
 
 		const fsDriver = shim.fsDriver();
@@ -341,7 +351,7 @@ export default class PluginService extends BaseService {
 		if (path.toLowerCase().endsWith('.js')) {
 			return this.loadPluginFromJsBundle(dirname(path), await fsDriver.readFile(path), filename(path));
 		} else if (path.toLowerCase().endsWith('.jpl')) {
-			return this.loadPluginFromPackage(dirname(path), path);
+			return this.loadPluginFromPackage(dirname(path), path, manifestOnly);
 		} else {
 			let distPath = path;
 			if (!(await fsDriver.exists(`${distPath}/manifest.json`))) {
@@ -350,8 +360,8 @@ export default class PluginService extends BaseService {
 
 			logger.info(`Loading plugin from ${path}`);
 
-			const scriptText = await fsDriver.readFile(`${distPath}/index.js`);
 			const manifestText = await fsDriver.readFile(`${distPath}/manifest.json`);
+			const scriptText = manifestOnly ? '' : await fsDriver.readFile(`${distPath}/index.js`);
 			const pluginId = makePluginId(filename(path));
 
 			return this.loadPlugin(distPath, manifestText, scriptText, pluginId);
@@ -449,8 +459,16 @@ export default class PluginService extends BaseService {
 			}
 
 			try {
-				const plugin = await this.loadPluginFromPath(pluginPath);
+				// Load only the manifest first to check if the plugin is
+				// enabled before doing the expensive full load.
+				let plugin = await this.loadPluginFromPath(pluginPath, true);
 				const enabled = this.pluginEnabled(settings, plugin.id);
+				if (enabled) {
+					logger.info(`Loading full plugin: ${plugin.id}`);
+					plugin = await this.loadPluginFromPath(pluginPath, false);
+				} else {
+					logger.info(`Loading manifest only for disabled plugin: ${plugin.id}`);
+				}
 
 				const existingPlugin = this.plugins_[plugin.id];
 				if (existingPlugin) {

packages/lib/services/plugins/loadPlugins.test.ts

@@ -1,5 +1,5 @@
 import Setting from '../../models/Setting';
-import PluginService from '../../services/plugins/PluginService';
+import PluginService, { defaultPluginSetting } from '../../services/plugins/PluginService';
 import { setupDatabaseAndSynchronizer, switchClient, withWarningSilenced } from '../../testing/test-utils';
 import loadPlugins, { Props as LoadPluginsProps } from './loadPlugins';
 import MockPluginRunner from './testing/MockPluginRunner';
@@ -8,6 +8,9 @@ import { Action, createStore } from 'redux';
 import MockPlatformImplementation from './testing/MockPlatformImplementation';
 import createTestPlugin from '../../testing/plugins/createTestPlugin';
 import Plugin from './Plugin';
+import { PluginManifest } from './utils/types';
+import { writeFile, mkdirp } from 'fs-extra';
+import { join } from 'path';
 
 const createMockReduxStore = () => {
 	return createStore((state: State = defaultState, action: Action<string>) => {
@@ -136,6 +139,33 @@ describe('loadPlugins', () => {
 		expect([...pluginRunner.runningPluginIds].sort()).toMatchObject(expectedRunningIds);
 	});
 
+	test('should not load the script for disabled plugins', async () => {
+		const createDirPlugin = async (manifest: PluginManifest, enabled: boolean) => {
+			const dir = join(Setting.value('pluginDir'), manifest.id);
+			await mkdirp(dir);
+			await writeFile(join(dir, 'manifest.json'), JSON.stringify(manifest), 'utf-8');
+			await writeFile(join(dir, 'index.js'), 'joplin.plugins.register({ onStart: async function() {} });', 'utf-8');
+			const newStates = {
+				...Setting.value('plugins.states'),
+				[manifest.id]: { ...defaultPluginSetting(), enabled },
+			};
+			Setting.setValue('plugins.states', newStates);
+		};
+
+		const enabledId = 'joplin.test.plugin.enabled';
+		const disabledId = 'joplin.test.plugin.disabled';
+		await createDirPlugin({ ...defaultManifestProperties, id: enabledId, name: 'Enabled' }, true);
+		await createDirPlugin({ ...defaultManifestProperties, id: disabledId, name: 'Disabled' }, false);
+
+		const pluginRunner = new MockPluginRunner();
+		const store = createMockReduxStore();
+		PluginService.instance().initialize('2.3.4', platformImplementation, pluginRunner, store);
+		await PluginService.instance().loadAndRunPlugins(Setting.value('pluginDir'), Setting.value('plugins.states'));
+
+		expect(PluginService.instance().plugins[disabledId].scriptText).toBe('');
+		expect(PluginService.instance().plugins[enabledId].scriptText).not.toBe('');
+	});
+
 	test('should not block allPluginsStarted when a plugin fails to start', async () => {
 		// This tests the fix for https://github.com/laurent22/joplin/issues/12793
 		// When a plugin crashes before calling register(), it should not block