Desktop: Fixes #10381: Fix plugin settings stored in settings.json are lost on startup (#10458)

packages/lib/models/Setting.test.ts

@@ -84,7 +84,7 @@ describe('models/Setting', () => {
 		expect(Setting.value('myCustom')).toBe('123');
 	}));
 
-	it('should not clear old custom settings', (async () => {
+	it.each([SettingStorage.Database, SettingStorage.File])('should not clear old custom settings if not registered immediately', (async (storage) => {
 		// In general the following should work:
 		//
 		// - Plugin register a new setting
@@ -105,6 +105,7 @@ describe('models/Setting', () => {
 			public: true,
 			value: 'default',
 			type: Setting.TYPE_STRING,
+			storage,
 		});
 
 		Setting.setValue('myCustom', '123');
@@ -119,6 +120,7 @@ describe('models/Setting', () => {
 			public: true,
 			value: 'default',
 			type: Setting.TYPE_STRING,
+			storage,
 		});
 
 		await Setting.saveAll();
@@ -126,6 +128,34 @@ describe('models/Setting', () => {
 		expect(Setting.value('myCustom')).toBe('123');
 	}));
 
+	it.each([SettingStorage.Database, SettingStorage.File])('should not clear old custom settings if not registered until restart', async (storage) => {
+		const registerCustom = async () => {
+			await Setting.registerSetting('myCustom', {
+				public: true,
+				value: 'test',
+				type: Setting.TYPE_STRING,
+				storage,
+			});
+		};
+
+		await registerCustom();
+		Setting.setValue('myCustom', 'test2');
+		await Setting.saveAll();
+
+		await Setting.reset();
+		await Setting.load();
+
+		// Change a file setting
+		Setting.setValue('sync.target', 9);
+
+		await Setting.saveAll();
+		await Setting.reset();
+		await Setting.load();
+
+		await registerCustom();
+		expect(Setting.value('myCustom')).toBe('test2');
+	});
+
 	it('should return values with correct type for custom settings', (async () => {
 		await Setting.registerSetting('myCustom', {
 			public: true,
@@ -425,5 +455,4 @@ describe('models/Setting', () => {
 			await Setting.saveAll();
 		}
 	});
-
 });

packages/lib/models/Setting.ts

@@ -2577,6 +2577,10 @@ class Setting extends BaseModel {
 		const keys = this.keys();
 
 		const valuesForFile: SettingValues = {};
+		for (const key of keys) {
+			// undefined => Delete from settings JSON file.
+			valuesForFile[key] = undefined;
+		}
 
 		const queries = [];
 		queries.push(`DELETE FROM settings WHERE key IN ("${keys.join('","')}")`);

packages/lib/models/settings/FileHandler.ts

@@ -11,6 +11,7 @@ export default class FileHandler {
 
 	private filePath_: string;
 	private valueJsonCache_: string = null;
+	private parsedJsonCache_: SettingValues = null;
 
 	public constructor(filePath: string) {
 		this.filePath_ = filePath;
@@ -23,24 +24,43 @@ export default class FileHandler {
 			} else {
 				this.valueJsonCache_ = await shim.fsDriver().readFile(this.filePath_, 'utf8');
 			}
+			this.parsedJsonCache_ = null;
 		}
 
+		if (this.parsedJsonCache_) return this.parsedJsonCache_;
+
+		let result: SettingValues;
 		try {
 			const values = JSON.parse(this.valueJsonCache_);
 			delete values['$id'];
 			delete values['$schema'];
-			return values;
+			result = values;
 		} catch (error) {
 			// Most likely the user entered invalid JSON - in this case we move
 			// the broken file to a new name (otherwise it would be overwritten
 			// by valid JSON and user will lose all their settings).
 			logger.error(`Could not parse JSON file: ${this.filePath_}`, error);
 			await shim.fsDriver().move(this.filePath_, `${this.filePath_}-${Date.now()}-invalid.bak`);
-			return {};
+			result = {};
 		}
+
+		this.parsedJsonCache_ = result;
+
+		return result;
 	}
 
 	public async save(values: SettingValues) {
+		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 json = `${JSON.stringify({
 			'$schema': Setting.schemaUrl,
 			...values,