Desktop: Fixes #9832: Fix user-installed versions of built-in plugins can't access resources in some cases (#9849)

packages/app-cli/tests/services/plugins/defaultPluginsUtils.ts

@@ -4,8 +4,9 @@ import { checkThrow, setupDatabaseAndSynchronizer, supportDir, switchClient } fr
 import PluginService, { defaultPluginSetting, DefaultPluginsInfo } from '@joplin/lib/services/plugins/PluginService';
 import Setting from '@joplin/lib/models/Setting';
 
+const testDefaultPluginsDir = `${supportDir}/testDefaultPlugins`;
 
-function newPluginService(appVersion = '1.4') {
+function newPluginService(appVersion = '2.4') {
 	const runner = new PluginRunner();
 	const service = new PluginService();
 	service.initialize(
@@ -35,8 +36,7 @@ describe('defaultPluginsUtils', () => {
 		await switchClient(1);
 	});
 
-	it('should load default plugins when nor previously installed', (async () => {
+	it('should load default plugins when not previously installed', (async () => {
-		const testPluginDir = `${supportDir}/testDefaultPlugins`;
 		Setting.setValue('installedDefaultPlugins', []);
 
 		const service = newPluginService('2.1');
@@ -47,7 +47,9 @@ describe('defaultPluginsUtils', () => {
 			expect(pluginSettings[pluginId]).toBeFalsy();
 		}
 
-		const pluginPathsAndNewSettings = await getDefaultPluginPathsAndSettings(testPluginDir, defaultPluginsInfo, pluginSettings);
+		const pluginPathsAndNewSettings = await getDefaultPluginPathsAndSettings(
+			testDefaultPluginsDir, defaultPluginsInfo, pluginSettings, service,
+		);
 
 		for (const pluginId of pluginsId) {
 			expect(
@@ -57,7 +59,6 @@ describe('defaultPluginsUtils', () => {
 	}));
 
 	it('should keep already created default plugins disabled with previous default plugins installed', (async () => {
-		const testPluginDir = `${supportDir}/testDefaultPlugins`;
 		Setting.setValue('installedDefaultPlugins', ['org.joplinapp.plugins.ToggleSidebars']);
 		Setting.setValue('plugins.states', {
 			'org.joplinapp.plugins.ToggleSidebars': { ...defaultPluginSetting(), enabled: false },
@@ -66,7 +67,7 @@ describe('defaultPluginsUtils', () => {
 		const service = newPluginService('2.1');
 
 		const pluginSettings = service.unserializePluginSettings(Setting.value('plugins.states'));
-		const pluginPathsAndNewSettings = await getDefaultPluginPathsAndSettings(testPluginDir, defaultPluginsInfo, pluginSettings);
+		const pluginPathsAndNewSettings = await getDefaultPluginPathsAndSettings(testDefaultPluginsDir, defaultPluginsInfo, pluginSettings, service);
 
 		// Should still be disabled
 		expect(
@@ -202,4 +203,34 @@ describe('defaultPluginsUtils', () => {
 		await service.destroy();
 	});
 
+	// Only returning not-yet-loaded plugins prevents non-default versions of built-in plugins
+	// from being overwritten by PluginService.
+	it('getDefaultPluginPathsAndSettings should return only plugins that haven\'t been loaded', async () => {
+		const service = newPluginService();
+
+		const testPluginId = 'org.joplinapp.plugins.ToggleSidebars';
+		const testPluginPath = `${supportDir}/pluginRepo/plugins/${testPluginId}/plugin.jpl`;
+
+		const pluginSettings = {
+			[testPluginId]: defaultPluginSetting(),
+		};
+
+		await service.loadAndRunPlugins([testPluginPath], pluginSettings, { devMode: false, builtIn: false });
+
+		// Should be running
+		expect(service.isPluginLoaded(testPluginId)).toBe(true);
+
+		const testDefaultPluginsInfo = {
+			[testPluginId]: {},
+			'joplin.plugin.ambrt.backlinksToNote': {},
+		};
+
+		const { pluginPaths } = await getDefaultPluginPathsAndSettings(
+			testDefaultPluginsDir, testDefaultPluginsInfo, pluginSettings, service,
+		);
+
+		// Should only return plugins that aren't loaded.
+		expect(pluginPaths).toHaveLength(1);
+		expect(pluginPaths[0]).toContain('joplin.plugin.ambrt.backlinksToNote');
+	});
 });

packages/lib/services/plugins/PluginService.ts

@@ -109,6 +109,10 @@ export default class PluginService extends BaseService {
 		return enabledPlugins;
 	}
 
+	public isPluginLoaded(pluginId: string) {
+		return !!this.plugins_[pluginId];
+	}
+
 	public get pluginIds(): string[] {
 		return Object.keys(this.plugins_);
 	}

packages/lib/services/plugins/defaultPlugins/defaultPluginsUtils.ts

@@ -11,7 +11,10 @@ const logger = Logger.create('defaultPluginsUtils');
 // Use loadAndRunDefaultPlugins
 // Exported for testing.
 export const getDefaultPluginPathsAndSettings = async (
-	defaultPluginsDir: string, defaultPluginsInfo: DefaultPluginsInfo, pluginSettings: PluginSettings,
+	defaultPluginsDir: string,
+	defaultPluginsInfo: DefaultPluginsInfo,
+	pluginSettings: PluginSettings,
+	pluginService: PluginService,
 ) => {
 	const pluginPaths: string[] = [];
 
@@ -46,6 +49,13 @@ export const getDefaultPluginPathsAndSettings = async (
 			continue;
 		}
 
+		// We skip plugins that are already loaded -- attempting to unpack a different version of a plugin
+		// that has already been loaded causes errors (see #9832).
+		if (pluginService.isPluginLoaded(pluginId)) {
+			logger.info(`Not loading default plugin ${pluginId} -- a plugin with the same ID is already loaded.`);
+			continue;
+		}
+
 		pluginPaths.push(join(defaultPluginsDir, pluginFileName));
 
 		pluginSettings = produce(pluginSettings, (draft: PluginSettings) => {
@@ -69,7 +79,7 @@ export const loadAndRunDefaultPlugins = async (
 	originalPluginSettings: PluginSettings,
 ): Promise<PluginSettings> => {
 	const { pluginPaths, pluginSettings } = await getDefaultPluginPathsAndSettings(
-		defaultPluginsDir, defaultPluginsInfo, originalPluginSettings,
+		defaultPluginsDir, defaultPluginsInfo, originalPluginSettings, service,
 	) ?? { pluginPaths: [], pluginSettings: originalPluginSettings };
 
 	await service.loadAndRunPlugins(pluginPaths, pluginSettings, { builtIn: true, devMode: false });