Skip to content
66 changes: 56 additions & 10 deletions packages/app-desktop/gui/ProfileEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { themeStyle } from '@joplin/lib/theme';
import bridge from '../services/bridge';
import dialogs from './dialogs';
import { Profile, ProfileConfig } from '@joplin/lib/services/profileConfig/types';
import { deleteProfileById, saveProfileConfig } from '@joplin/lib/services/profileConfig';
import { deleteProfileById, isSubProfile, saveProfileConfig } from '@joplin/lib/services/profileConfig';
import Setting from '@joplin/lib/models/Setting';
import shim from '@joplin/lib/shim';
import Logger from '@joplin/utils/Logger';
Expand Down Expand Up @@ -150,18 +150,64 @@ const ProfileEditorComponent: React.FC<Props> = props => {
});
if (!ok) return;

const subProfile = isSubProfile(profile);
const rootDir = Setting.value('rootProfileDir');
const profileDir = `${rootDir}/profile-${profile.id}`;

try {
await shim.fsDriver().remove(profileDir);
logger.info('Deleted profile directory: ', profileDir);
} catch (error) {
logger.error('Error deleting profile directory: ', error);
bridge().showErrorMessageBox(error.message);
// Deleting the default profile must be handled differently. We can't delete the whole directory because it contains other profiles and global settings
if (subProfile) {
const profileDir = `${rootDir}/profile-${profile.id}`;

try {
await shim.fsDriver().remove(profileDir);
logger.info('Deleted profile directory: ', profileDir);
} catch (error) {
logger.error('Error deleting profile directory: ', error);
bridge().showErrorMessageBox(error.message);
}

await saveNewProfileConfig(() => deleteProfileById(profileConfig, profile.id));
} else {
const dirsToDelete = ['cache', 'resources', 'tmp'];
const filesToDelete = ['database.sqlite', 'log.txt', 'keymap-desktop.json'];
Comment thread
mrjo118 marked this conversation as resolved.

// Reset settings for the default profile, but retain global settings
try {
await Setting.resetDefaultProfileSettings();
} catch (error) {
// If the first stage fails, nothing has happened, so throw an error. But if there is a failure in later steps, ignore errors but log them
logger.error('Error deleting the default profile: ', error);
bridge().showErrorMessageBox(error.message);
return;
}

// Delete directories
for (const dir of dirsToDelete) {
const fullPath = `${rootDir}/${dir}`;
try {
if (await shim.fsDriver().exists(fullPath)) {
await shim.fsDriver().remove(fullPath);
logger.info('Deleted directory: ', fullPath);
}
} catch (error) {
logger.error('Error deleting directory: ', fullPath, error);
}
}

// Delete files
for (const file of filesToDelete) {
const fullPath = `${rootDir}/${file}`;
try {
if (await shim.fsDriver().exists(fullPath)) {
await shim.fsDriver().unlink(fullPath);
logger.info('Deleted file: ', fullPath);
}
} catch (error) {
logger.error('Error deleting file: ', fullPath, error);
}
}

bridge().showMessageBox(_('The default profile has been reset.'));
}

await saveNewProfileConfig(() => deleteProfileById(profileConfig, profile.id));
};

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,33 +56,4 @@ describe('deleteProfile', () => {
expect(await pathExists(resourceDir)).toBe(false);
expect(await pathExists(pluginDataDir)).toBe(false);
});

it('should refuse to delete the default profile', async () => {
const config: ProfileConfig = {
version: CurrentProfileVersion,
currentProfileId: 'test',
profiles: [
{
name: 'Testing',
id: DefaultProfileId,
},
{
name: 'Another test',
id: 'test',
},
],
};

try {
await deleteProfile({
profileConfig: config,
toDelete: config.profiles[0],
databaseDriver: new MockDatabaseDriver(),
});

expect('did not throw').toBe('threw');
} catch (error) {
expect(String(error)).toMatch(/The default profile cannot be deleted/);
}
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ import { Profile, ProfileConfig } from '@joplin/lib/services/profileConfig/types
import { getDatabaseName, getPluginDataDir, getResourceDir, saveProfileConfig } from '../../../services/profiles';
import { deleteProfileById, getCurrentProfile, isSubProfile } from '@joplin/lib/services/profileConfig';
import Setting from '@joplin/lib/models/Setting';
import shim from '@joplin/lib/shim';
import shim, { MessageBoxType } from '@joplin/lib/shim';
import Logger from '@joplin/utils/Logger';
import resolvePathWithinDir from '@joplin/lib/utils/resolvePathWithinDir';
import DatabaseDriver from '@joplin/lib/database-driver';
import { _ } from '@joplin/lib/locale';

const logger = Logger.create('deleteProfile');

Expand All @@ -17,14 +18,17 @@ interface DeleteProfileOptions {

const deleteProfile = async (options: DeleteProfileOptions) => {
logger.info('Deleting profile config', options.toDelete.id);
// This step also verifies that the to-be-deleted profile is not the default profile, etc.
const newConfig = deleteProfileById(options.profileConfig, options.toDelete.id);
// Save the profile config early. If the later deletion steps fail, this prevents the user from
// opening a partially-deleted profile:
await saveProfileConfig(newConfig);

if (options.toDelete.id === options.profileConfig.currentProfileId) throw new Error(_('The active profile cannot be deleted. Switch to a different profile and try again.'));
const subProfile = isSubProfile(options.toDelete);
if (!subProfile) throw new Error('Deleting a sub-profile is not supported');

// Deleting the default profile must be handled differently. We can't delete the whole directory because it contains other profiles and global settings
if (subProfile) {
const newConfig = deleteProfileById(options.profileConfig, options.toDelete.id);
// Save the profile config early. If the later deletion steps fail, this prevents the user from
// opening a partially-deleted profile. The default profile does not get deleted from the list,
// but the data will be cleared
await saveProfileConfig(newConfig);
}

// Retrieve and validate both the database name and resources directory
// **before** doing any deletion.
Expand All @@ -42,11 +46,38 @@ const deleteProfile = async (options: DeleteProfileOptions) => {
logger.warn('Failed to delete database: ', error, '. Was the profile initialized?');
}

logger.info('Deleting resources directory', resourcesDir);
await shim.fsDriver().remove(resourcesDir);
if (subProfile) {
logger.info('Deleting resources directory', resourcesDir);
await shim.fsDriver().remove(resourcesDir);
} else {
try {
const items = await shim.fsDriver().readDirStats(resourcesDir);

for (const item of items) {
if (item.isDirectory()) continue;
const fileName = item.path;

if (/^[a-f0-9]{32}\./.test(fileName)) {
const fullPath = `${resourcesDir}/${fileName}`;
try {
await shim.fsDriver().unlink(fullPath);
logger.info('Deleted resource file: ', fullPath);
} catch (error) {
logger.error('Error deleting resource file: ', fullPath, error);
}
}
}
} catch (error) {
logger.error('Error reading resources directory: ', resourcesDir, error);
}
}
Comment thread
mrjo118 marked this conversation as resolved.

logger.info('Deleting plugin data directory', pluginDataDir);
await shim.fsDriver().remove(pluginDataDir);

if (!subProfile) {
await shim.showMessageBox(_('The default profile has been reset.'), { type: MessageBoxType.Info });
}
};

export default deleteProfile;
Expand All @@ -70,7 +101,7 @@ const getTargetResourceDirectory = ({ toDelete: target }: DeleteProfileOptions)
// Add an extra check here to verify that deleting the other profile's resource directory
// doesn't also delete **the active** profile's resource directory. On mobile, the resources
// directory can sometimes contain other profile directories (e.g. in the case of the default profile).
if (resolvePathWithinDir(resourcesDir, Setting.value('resourceDir')) !== null) {
if (isSubProfile(target) && resolvePathWithinDir(resourcesDir, Setting.value('resourceDir')) !== null) {
throw new Error('Refusing to delete a directory that contains the active profile\'s resource directory.');
}
return resourcesDir;
Expand All @@ -79,7 +110,7 @@ const getTargetResourceDirectory = ({ toDelete: target }: DeleteProfileOptions)

const getTargetPluginDataDirectory = ({ toDelete: target }: DeleteProfileOptions) => {
const pluginDataDir = getPluginDataDir(target, isSubProfile(target));
if (resolvePathWithinDir(pluginDataDir, Setting.value('pluginDataDir')) !== null) {
if (isSubProfile(target) && resolvePathWithinDir(pluginDataDir, Setting.value('pluginDataDir')) !== null) {
throw new Error('Refusing to delete a directory that contains the active profile\'s plugin data directory.');
}
return pluginDataDir;
Expand Down
29 changes: 22 additions & 7 deletions packages/lib/models/Setting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1131,13 +1131,7 @@ class Setting extends BaseModel {
return output;
}

public static async saveAll() {
if (Setting.autoSaveEnabled && !this.saveTimeoutId_) return Promise.resolve();

logger.debug('Saving settings...');
shim.clearTimeout(this.saveTimeoutId_);
this.saveTimeoutId_ = null;

private static async getFileValuesAndDbUpdateQueries() {
const keys = this.keys();

const valuesForFile: SettingValues = {};
Expand Down Expand Up @@ -1183,6 +1177,18 @@ class Setting extends BaseModel {
}
}

return { valuesForFile, queries };
}

public static async saveAll() {
if (Setting.autoSaveEnabled && !this.saveTimeoutId_) return Promise.resolve();

logger.debug('Saving settings...');
shim.clearTimeout(this.saveTimeoutId_);
this.saveTimeoutId_ = null;

const { valuesForFile, queries } = await Setting.getFileValuesAndDbUpdateQueries();

await BaseModel.db().transactionExecBatch(queries);

if (this.canUseFileStorage()) {
Expand All @@ -1208,6 +1214,15 @@ class Setting extends BaseModel {
logger.debug('Settings have been saved.');
}

public static async resetDefaultProfileSettings() {
const { valuesForFile } = await Setting.getFileValuesAndDbUpdateQueries();

if (this.canUseFileStorage()) {
const { globalSettings } = splitGlobalAndLocalSettings(valuesForFile);
await this.rootFileHandler.save(globalSettings, { overwrite: true });
}
}

public static scheduleChangeEvent() {
if (this.changeEventTimeoutId_) shim.clearTimeout(this.changeEventTimeoutId_);

Expand Down
28 changes: 20 additions & 8 deletions packages/lib/models/settings/FileHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ const logger = Logger.create('Settings');
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
export type SettingValues = Record<string, any>;

export interface FileHandlerOptions {
overwrite?: boolean;
}

export default class FileHandler {

private filePath_: string;
Expand Down Expand Up @@ -49,15 +53,18 @@ export default class FileHandler {
return result;
}

public async save(values: SettingValues) {
public async save(values: SettingValues, options: FileHandlerOptions = {}) {
values = { ...values };

// Merge with existing settings. This prevents settings stored by disabled or not-yet-loaded
// plugins from being deleted.
for (const key in this.parsedJsonCache_) {
const includesSetting = Object.prototype.hasOwnProperty.call(values, key);
if (!includesSetting) {
values[key] = this.parsedJsonCache_[key];
const overwrite = !!options.overwrite;

if (!overwrite) {
// Merge with existing settings. This prevents settings stored by disabled or not-yet-loaded
// plugins from being deleted.
for (const key in this.parsedJsonCache_) {
const includesSetting = Object.prototype.hasOwnProperty.call(values, key);
if (!includesSetting) {
values[key] = this.parsedJsonCache_[key];
}
}
}

Expand All @@ -70,6 +77,11 @@ export default class FileHandler {

await shim.fsDriver().writeFile(this.filePath_, json, 'utf8');
this.valueJsonCache_ = json;

if (overwrite) {
// Prevent pre-existing settings from being re-instated by subsequent saving of settings
this.parsedJsonCache_ = values;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
mrjo118 marked this conversation as resolved.
}

}
4 changes: 0 additions & 4 deletions packages/lib/services/profileConfig/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { rtrimSlashes } from '../../path-utils';
import shim from '../../shim';
import { CurrentProfileVersion, defaultProfile, defaultProfileConfig, DefaultProfileId, Profile, ProfileConfig } from './types';
import { customAlphabet } from 'nanoid/non-secure';
import { _ } from '../../locale';

// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
export const migrateProfileConfig = (profileConfig: any, toVersion: number): ProfileConfig => {
Expand Down Expand Up @@ -102,9 +101,6 @@ export const createNewProfile = (config: ProfileConfig, profileName: string) =>
};

export const deleteProfileById = (config: ProfileConfig, profileId: string): ProfileConfig => {
if (profileId === DefaultProfileId) throw new Error(_('The default profile cannot be deleted'));
if (profileId === config.currentProfileId) throw new Error(_('The active profile cannot be deleted. Switch to a different profile and try again.'));

const newProfiles = config.profiles.filter(p => p.id !== profileId);
return {
...config,
Expand Down
Loading