Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Desktop: Install default plugins on first-app-start #6585

Merged
merged 34 commits into from
Sep 1, 2022
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
bd677c4
initial commit
mak2002 Jun 16, 2022
eb67977
don't load default plugins immediately after installing
mak2002 Jun 16, 2022
302b380
Merge branch 'dev' into install-default-plugins
laurent22 Jun 18, 2022
d8d86f2
fix installed state of default plugins
mak2002 Jun 21, 2022
75d2601
Merge branch 'install-default-plugins' of https://github.com/mak2002/…
mak2002 Jun 21, 2022
4491cf0
organize the code for installing default plugins
mak2002 Jun 23, 2022
84c02c9
improve types of functions
mak2002 Jun 24, 2022
8d61611
add function for default Plugins settings
mak2002 Jun 25, 2022
3c14807
add _built_in field in plugins manifest
mak2002 Jun 25, 2022
191c8a5
added test
mak2002 Jun 28, 2022
32c003b
implement initial settings for plugins
mak2002 Jun 29, 2022
3536393
removed code for mkdir command
mak2002 Jun 29, 2022
4aa26bc
remove unnecessary files
mak2002 Jun 29, 2022
78c2ac7
added tests for initial settings for default plugins
mak2002 Jul 2, 2022
46007c5
remove async keyword from app.ts
mak2002 Jul 2, 2022
ebde62b
Merge branch 'laurent22:dev' into install-default-plugins
mak2002 Jul 6, 2022
4d05817
Merge branch 'laurent22:dev' into install-default-plugins
mak2002 Jul 10, 2022
360de71
Merge branch 'laurent22:dev' into install-default-plugins
mak2002 Jul 13, 2022
25820d3
Merge branch 'dev' into install-default-plugins
mak2002 Jul 22, 2022
b80c487
refactor code
mak2002 Jul 25, 2022
228aa96
small fix
mak2002 Jul 25, 2022
aa3a5fa
Merge branch 'install-default-plugins' of https://github.com/mak2002/…
mak2002 Jul 25, 2022
343c4de
remove old plugins
mak2002 Jul 25, 2022
b68fd07
refactor code
mak2002 Jul 28, 2022
fbc1793
Merge branch 'laurent22:dev' into install-default-plugins
mak2002 Jul 30, 2022
d1db3ec
move default plugins functions into separate folder | update tests
mak2002 Jul 31, 2022
53ddafb
remove test plugins
mak2002 Jul 31, 2022
4cc80d1
small fix
mak2002 Jul 31, 2022
90dfd0d
add comment for checkArrayAndUpdate method
mak2002 Jul 31, 2022
a4fcf8e
make function for initial settings to support profileDir
mak2002 Jul 31, 2022
6ef7c3d
refactor plugins info object
mak2002 Aug 10, 2022
5f6671d
fix import
mak2002 Aug 10, 2022
d90dee1
update import in ConfigScreen
mak2002 Aug 11, 2022
263d33b
Merge branch 'laurent22:dev' into install-default-plugins
mak2002 Aug 30, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -1585,6 +1585,9 @@ packages/lib/services/plugins/api/JoplinWorkspace.js.map
packages/lib/services/plugins/api/types.d.ts
packages/lib/services/plugins/api/types.js
packages/lib/services/plugins/api/types.js.map
packages/lib/services/plugins/defaultPlugins.d.ts
packages/lib/services/plugins/defaultPlugins.js
packages/lib/services/plugins/defaultPlugins.js.map
packages/lib/services/plugins/reducer.d.ts
packages/lib/services/plugins/reducer.js
packages/lib/services/plugins/reducer.js.map
Expand Down
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -1575,6 +1575,9 @@ packages/lib/services/plugins/api/JoplinWorkspace.js.map
packages/lib/services/plugins/api/types.d.ts
packages/lib/services/plugins/api/types.js
packages/lib/services/plugins/api/types.js.map
packages/lib/services/plugins/defaultPlugins.d.ts
packages/lib/services/plugins/defaultPlugins.js
packages/lib/services/plugins/defaultPlugins.js.map
packages/lib/services/plugins/reducer.d.ts
packages/lib/services/plugins/reducer.js
packages/lib/services/plugins/reducer.js.map
Expand Down
Binary file not shown.
Binary file not shown.
55 changes: 54 additions & 1 deletion packages/app-cli/tests/services/plugins/PluginService.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import PluginRunner from '../../../app/services/plugins/PluginRunner';
import PluginService from '@joplin/lib/services/plugins/PluginService';
import PluginService, { InitialSettings } 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';
Expand All @@ -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 } from '@joplin/lib/testing/test-utils';
import { newPluginScript } from '../../testUtils';
import path = require('path');

const testPluginDir = `${supportDir}/plugins`;

Expand Down Expand Up @@ -271,6 +272,17 @@ describe('services_PluginService', function() {
expect(await fs.pathExists(installedPluginPath)).toBe(true);
}));

it('should install default plugins', (async () => {
const service = newPluginService();
const pluginsPath = path.join(__dirname, '..', '/defaultPlugins/');
const pluginSettings = service.unserializePluginSettings(Setting.value('plugins.states'));
await service.installDefaultPlugins(pluginsPath, pluginSettings, service);
const installedPluginPath1 = `${Setting.value('pluginDir')}/io.github.jackgruber.backup.jpl`;
const installedPluginPath2 = `${Setting.value('pluginDir')}/plugin.calebjohn.rich-markdown.jpl`;
expect(await fs.pathExists(installedPluginPath1)).toBe(true);
expect(await fs.pathExists(installedPluginPath2)).toBe(true);
}));

it('should rename the plugin archive to the right name', (async () => {
const tempDir = await createTempDir();
const service = newPluginService();
Expand Down Expand Up @@ -305,4 +317,45 @@ describe('services_PluginService', function() {
expect(JSON.parse(folders[0].title)).toBe(expectedPath);
}));

test('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: "",
type: 2,
section: "backupSection",
public: true,
label: "Backup path",
},
})
},
});`;

const plugin = await service.loadPluginFromJsBundle('', pluginScript);
await service.runPlugin(plugin);

const initialSettings: InitialSettings = {
'io.github.jackgruber.backup': {
'path': '/JoplinBackupTest',
},
};

service.setSettingsForDefaultPlugins(initialSettings);
expect(Setting.value('plugin-io.github.jackgruber.backup.path')).toBe('/JoplinBackupTest');
await service.destroy();
});

});
31 changes: 30 additions & 1 deletion packages/app-desktop/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import checkForUpdates from './checkForUpdates';
import { AppState } from './app.reducer';
import syncDebugLog from '@joplin/lib/services/synchronizer/syncDebugLog';
import eventManager from '@joplin/lib/eventManager';
import path = require('path');
// import { runIntegrationTests } from '@joplin/lib/services/e2ee/ppkTestUtils';

const pluginClasses = [
Expand Down Expand Up @@ -261,7 +262,7 @@ class Application extends BaseApplication {
service.initialize(packageInfo.version, PlatformImplementation.instance(), pluginRunner, this.store());
service.isSafeMode = Setting.value('isSafeMode');

const pluginSettings = service.unserializePluginSettings(Setting.value('plugins.states'));
let pluginSettings = service.unserializePluginSettings(Setting.value('plugins.states'));

{
// Users can add and remove plugins from the config screen at any
Expand All @@ -270,6 +271,7 @@ class Application extends BaseApplication {
// stored in the settings.
const newSettings = service.clearUpdateState(await service.uninstallPlugins(pluginSettings));
Setting.setValue('plugins.states', newSettings);
Setting.setValue('preInstalledDefaultPlugins', newSettings);
CalebJohn marked this conversation as resolved.
Show resolved Hide resolved
}

try {
Expand All @@ -280,6 +282,31 @@ class Application extends BaseApplication {
this.logger().error(`There was an error loading plugins from ${Setting.value('pluginDir')}:`, error);
}

// we are loading default plugins here so as to not disturb user plugins
try {
const pluginsDir = path.join(bridge().buildDir(), 'defaultPlugins');
pluginSettings = await service.installDefaultPlugins(pluginsDir, pluginSettings);

const defaultPluginsPath = await shim.fsDriver().readDirStats(pluginsDir);
const readyToLoadPluginsPath = [];

for (const pluginFolder of defaultPluginsPath) {
// here pluginFolder will be pluginId
// maybe put this.plugins_ in here instead of service.plugins
const newSettings = Setting.value('installedDefaultPlugins');

if (service.plugins[pluginFolder.path] || newSettings.includes(pluginFolder.path)) continue;

readyToLoadPluginsPath.push(path.join(pluginsDir, pluginFolder.path, 'plugin.jpl'));
}

if (readyToLoadPluginsPath.length > 0) {
await service.loadAndRunPlugins(readyToLoadPluginsPath, pluginSettings);
}
} catch (error) {
this.logger().error(`There was an error loading default plugins from ${Setting.value('pluginDir')}:`, error);
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move new functionality into a default Plugins module.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I was thinking of a way to only call function to install default plugins here, I will see what I can do.

try {
if (Setting.value('plugins.devPluginPaths')) {
const paths = Setting.value('plugins.devPluginPaths').split(',').map((p: string) => p.trim());
Expand Down Expand Up @@ -320,6 +347,8 @@ class Application extends BaseApplication {
type: 'STARTUP_PLUGINS_LOADED',
value: true,
});
service.setSettingsForDefaultPlugins(service.initialPluginsSettings, Setting.value('plugins.states'));
// object with plugins id wand if its initilised settings
}
}, 500);
}
Expand Down
Binary file not shown.
Binary file not shown.
Binary file not shown.
17 changes: 17 additions & 0 deletions packages/app-desktop/gui/ConfigScreen/ConfigScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import SyncTargetRegistry from '@joplin/lib/SyncTargetRegistry';
const shared = require('@joplin/lib/components/shared/config-shared.js');
import ClipperConfigScreen from '../ClipperConfigScreen';
import restart from '../../services/restart';
import PluginService from '@joplin/lib/services/plugins/PluginService';
const { KeymapConfigScreen } = require('../KeymapConfig/KeymapConfigScreen');

const settingKeyToControl: any = {
Expand Down Expand Up @@ -60,12 +61,28 @@ class ConfigScreenComponent extends React.Component<any, any> {
this.setState({ settings: this.props.settings });
}

updatePluginsStates(value: any) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use a descriptive parameter name value should be newPluginStates. And please add typing.

if (Object.keys(value).length === 0) return;
const key = 'plugins.states';
const md = Setting.settingMetadata(key);
let newValue = Setting.value('plugins.states');
newValue = {
...newValue, ...value,
};
shared.updateSettingValue(this, key, newValue);

if (md.autoSave) {
shared.scheduleSaveSettings(this);
}
}

CalebJohn marked this conversation as resolved.
Show resolved Hide resolved
componentDidMount() {
if (this.props.defaultSection) {
this.setState({ selectedSectionName: this.props.defaultSection }, () => {
this.switchSection(this.props.defaultSection);
});
}
this.updatePluginsStates(PluginService.instance().setInstalledState());
}

private async handleSettingButton(key: string) {
Expand Down
3 changes: 2 additions & 1 deletion packages/app-desktop/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@
"afterSign": "./tools/notarizeMacApp.js",
"extraResources": [
"build/icons/**",
"build/images/**"
"build/images/**",
"build/defaultPlugins/**"
],
"afterAllArtifactBuild": "./generateSha512.js",
"asar": true,
Expand Down
2 changes: 1 addition & 1 deletion packages/lib/BaseApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,6 @@ export default class BaseApplication {
Setting.setValue('sync.interval', 3600);
}

Setting.setValue('firstStart', 0);
} else {
Setting.applyDefaultMigrations();
}
Expand All @@ -852,6 +851,7 @@ export default class BaseApplication {
Setting.setValue('sync.10.userContentPath', 'http://joplincloud.local:22300');
}

Setting.setValue('firstStart', 0);
// For now always disable fuzzy search due to performance issues:
// https://discourse.joplinapp.org/t/1-1-4-keyboard-locks-up-while-typing/11231/11
// https://discourse.joplinapp.org/t/serious-lagging-when-there-are-tens-of-thousands-of-notes/11215/23
Expand Down
37 changes: 37 additions & 0 deletions packages/lib/models/Setting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1534,6 +1534,14 @@ class Setting extends BaseModel {
storage: SettingStorage.Database,
},

updatedDefaultPluginsInstallStates: {
value: false,
type: SettingItemType.Bool,
public: false,
appTypes: [AppType.Desktop],
storage: SettingStorage.File,
},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this won't cause a dependency loop. Please correct me if I am wrong.


lastSettingDefaultMigration: {
value: -1,
type: SettingItemType.Int,
Expand All @@ -1546,6 +1554,27 @@ class Setting extends BaseModel {
public: false,
},

installedDefaultPlugins: {
value: [],
type: SettingItemType.Array,
public: false,
storage: SettingStorage.File,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be database here

},

setInitialDefaultPluginsSettings: {
value: [],
type: SettingItemType.Array,
public: false,
storage: SettingStorage.File,
},

preInstalledDefaultPlugins: {
value: '',
type: SettingItemType.Object,
public: false,
storage: SettingStorage.File,
},

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove these in favour of using the installedDefaultPlugins field alone.

// 'featureFlag.syncAccurateTimestamps': {
// value: false,
// type: SettingItemType.Bool,
Expand Down Expand Up @@ -1914,6 +1943,14 @@ class Setting extends BaseModel {
return this.setValue(key, !this.value(key));
}

static checkAndUpdate(settingName: string, value: string): boolean {
const settingValue: Array<any> = this.value(settingName);
if (settingValue.includes(value)) return true;
settingValue.push(value);
this.setValue(settingName, settingValue);
return false;
}

static objectValue(settingKey: string, objectKey: string, defaultValue: any = null) {
const o = this.value(settingKey);
if (!o || !(objectKey in o)) return defaultValue;
Expand Down
Loading