Plugins: Use plugin ID as filename

packages/app-cli/tests/services_PluginService.ts

@@ -3,7 +3,9 @@ import PluginService 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';
+import Setting from '@joplin/lib/models/Setting';
 
+const fs = require('fs-extra');
 const { asyncTest, expectNotThrow, setupDatabaseAndSynchronizer, switchClient, expectThrow, createTempDir } = require('./test-utils.js');
 const Note = require('@joplin/lib/models/Note');
 const Folder = require('@joplin/lib/models/Folder');
@@ -260,4 +262,23 @@ describe('services_PluginService', function() {
 		}
 	}));
 
+	it('should install a plugin', asyncTest(async () => {
+		const service = newPluginService();
+		const pluginPath = `${testPluginDir}/jpl_test/org.joplinapp.FirstJplPlugin.jpl`;
+		await service.installPlugin(pluginPath);
+		const installedPluginPath = `${Setting.value('pluginDir')}/org.joplinapp.FirstJplPlugin.jpl`;
+		expect(await fs.existsSync(installedPluginPath)).toBe(true);
+	}));
+
+	it('should rename the plugin archive to the right name', asyncTest(async () => {
+		const tempDir = await createTempDir();
+		const service = newPluginService();
+		const pluginPath = `${testPluginDir}/jpl_test/org.joplinapp.FirstJplPlugin.jpl`;
+		const tempPath = `${tempDir}/something.jpl`;
+		await shim.fsDriver().copy(pluginPath, tempPath);
+		const installedPluginPath = `${Setting.value('pluginDir')}/org.joplinapp.FirstJplPlugin.jpl`;
+		await service.installPlugin(tempPath);
+		expect(await fs.existsSync(installedPluginPath)).toBe(true);
+	}));
+
 });

packages/app-cli/tests/test-utils.js

@@ -216,6 +216,7 @@ async function switchClient(id, options = null) {
 	await Setting.reset();
 	Setting.setConstant('resourceDirName', resourceDirName(id));
 	Setting.setConstant('resourceDir', resourceDir(id));
+	Setting.setConstant('pluginDir', pluginDir(id));
 
 	await loadKeychainServiceAndSettings(options.keychainEnabled ? KeychainServiceDriver : KeychainServiceDriverDummy);
 
@@ -294,6 +295,11 @@ function resourceDir(id = null) {
 	return `${__dirname}/data/${resourceDirName(id)}`;
 }
 
+function pluginDir(id = null) {
+	if (id === null) id = currentClient_;
+	return `${__dirname}/data/plugins-${id}`;
+}
+
 async function setupDatabaseAndSynchronizer(id = null, options = null) {
 	if (id === null) id = currentClient_;
 
@@ -307,6 +313,9 @@ async function setupDatabaseAndSynchronizer(id = null, options = null) {
 	await fs.remove(resourceDir(id));
 	await fs.mkdirp(resourceDir(id), 0o755);
 
+	await fs.remove(pluginDir(id));
+	await fs.mkdirp(pluginDir(id), 0o755);
+
 	if (!synchronizers_[id]) {
 		const SyncTargetClass = SyncTargetRegistry.classById(syncTargetId_);
 		const syncTarget = new SyncTargetClass(db(id));
@@ -727,7 +736,7 @@ class TestApp extends BaseApplication {
 	}
 
 	async profileDir() {
-		return await Setting.value('profileDir');
+		return Setting.value('profileDir');
 	}
 
 	async destroy() {

packages/lib/services/plugins/PluginService.ts

@@ -4,7 +4,7 @@ import Global from './api/Global';
 import BasePluginRunner from './BasePluginRunner';
 import BaseService  from '../BaseService';
 import shim from '../../shim';
-import { filename, dirname, rtrimSlashes, basename } from '../../path-utils';
+import { filename, dirname, rtrimSlashes } from '../../path-utils';
 import Setting from '../../models/Setting';
 import Logger from '../../Logger';
 const compareVersions = require('compare-versions');
@@ -251,6 +251,11 @@ export default class PluginService extends BaseService {
 			plugin.deprecationNotice('1.5', msg);
 		}
 
+		// Sanity check, although at that point the plugin ID should have
+		// been set, either automatically, or because it was defined in the
+		// manifest.
+		if (!plugin.id) throw new Error('Could not load plugin: ID is not set');
+
 		return plugin;
 	}
 
@@ -326,8 +331,15 @@ export default class PluginService extends BaseService {
 	public async installPlugin(jplPath: string): Promise<Plugin> {
 		logger.info(`Installing plugin: "${jplPath}"`);
 
-		const destPath = `${Setting.value('pluginDir')}/${basename(jplPath)}`;
+		// Before moving the plugin to the profile directory, we load it
+		// from where it is now to check that it is valid and to retrieve
+		// the plugin ID.
+		const preloadedPlugin = await this.loadPluginFromPath(jplPath);
+
+		const destPath = `${Setting.value('pluginDir')}/${preloadedPlugin.id}.jpl`;
 		await shim.fsDriver().copy(jplPath, destPath);
+
+		// Now load it from the profile directory
 		const plugin = await this.loadPluginFromPath(destPath);
 		if (!this.plugins_[plugin.id]) this.setPluginAt(plugin.id, plugin);
 		return plugin;

packages/tools/git-changelog.js

@@ -56,6 +56,7 @@ function platformFromTag(tagName) {
 	if (tagName.indexOf('ios') >= 0) return 'ios';
 	if (tagName.indexOf('clipper') === 0) return 'clipper';
 	if (tagName.indexOf('cli') === 0) return 'cli';
+	if (tagName.indexOf('plugin-generator') === 0) return 'plugin-generator';
 	throw new Error(`Could not determine platform from tag: ${tagName}`);
 }