Desktop, Mobile, Cli: Allow setting a minimum app version on the sync target (#9778)

packages/app-desktop/gui/MainScreen/MainScreen.tsx

@@ -84,6 +84,7 @@ interface Props {
 	processingShareInvitationResponse: boolean;
 	isResettingLayout: boolean;
 	listRendererId: string;
+	mustUpgradeAppMessage: string;
 }
 
 interface ShareFolderDialogOptions {
@@ -521,10 +522,12 @@ class MainScreenComponent extends React.Component<Props, State> {
 	}
 
 	// eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
-	private renderNotificationMessage(message: string, callForAction: string, callForActionHandler: Function, callForAction2: string = null, callForActionHandler2: Function = null) {
+	private renderNotificationMessage(message: string, callForAction: string = null, callForActionHandler: Function = null, callForAction2: string = null, callForActionHandler2: Function = null) {
 		const theme = themeStyle(this.props.themeId);
 		const urlStyle: any = { color: theme.colorWarnUrl, textDecoration: 'underline' };
 
+		if (!callForAction) return <span>{message}</span>;
+
 		const cfa = (
 			<a href="#" style={urlStyle} onClick={() => callForActionHandler()}>
 				{callForAction}
@@ -671,6 +674,8 @@ class MainScreenComponent extends React.Component<Props, State> {
 				'Install plugin',
 				onViewPluginScreen,
 			);
+		} else if (this.props.mustUpgradeAppMessage) {
+			msg = this.renderNotificationMessage(this.props.mustUpgradeAppMessage);
 		}
 
 		return (
@@ -682,7 +687,18 @@ class MainScreenComponent extends React.Component<Props, State> {
 
 	public messageBoxVisible(props: Props = null) {
 		if (!props) props = this.props;
-		return props.hasDisabledSyncItems || props.showMissingMasterKeyMessage || props.hasMissingSyncCredentials || props.showNeedUpgradingMasterKeyMessage || props.showShouldReencryptMessage || props.hasDisabledEncryptionItems || this.props.shouldUpgradeSyncTarget || props.isSafeMode || this.showShareInvitationNotification(props) || this.props.needApiAuth || this.props.showInstallTemplatesPlugin;
+		return props.hasDisabledSyncItems ||
+			props.showMissingMasterKeyMessage ||
+			props.hasMissingSyncCredentials ||
+			props.showNeedUpgradingMasterKeyMessage ||
+			props.showShouldReencryptMessage ||
+			props.hasDisabledEncryptionItems ||
+			this.props.shouldUpgradeSyncTarget ||
+			props.isSafeMode ||
+			this.showShareInvitationNotification(props) ||
+			this.props.needApiAuth ||
+			this.props.showInstallTemplatesPlugin ||
+			!!this.props.mustUpgradeAppMessage;
 	}
 
 	public registerCommands() {
@@ -921,6 +937,7 @@ const mapStateToProps = (state: AppState) => {
 		showInstallTemplatesPlugin: state.hasLegacyTemplates && !state.pluginService.plugins['joplin.plugin.templates'],
 		isResettingLayout: state.isResettingLayout,
 		listRendererId: state.settings['notes.listRendererId'],
+		mustUpgradeAppMessage: state.mustUpgradeAppMessage,
 	};
 };
 

packages/app-mobile/components/ScreenHeader.tsx

@@ -86,6 +86,7 @@ interface ScreenHeaderProps {
 	hasDisabledEncryptionItems?: boolean;
 	shouldUpgradeSyncTarget?: boolean;
 	showShouldUpgradeSyncTargetMessage?: boolean;
+	mustUpgradeAppMessage: string;
 
 	themeId: number;
 }
@@ -569,6 +570,7 @@ class ScreenHeaderComponent extends PureComponent<ScreenHeaderProps, ScreenHeade
 		if (this.props.showMissingMasterKeyMessage) warningComps.push(this.renderWarningBox('EncryptionConfig', _('Press to set the decryption password.')));
 		if (this.props.hasDisabledSyncItems) warningComps.push(this.renderWarningBox('Status', _('Some items cannot be synchronised. Press for more info.')));
 		if (this.props.shouldUpgradeSyncTarget && this.props.showShouldUpgradeSyncTargetMessage !== false) warningComps.push(this.renderWarningBox('UpgradeSyncTarget', _('The sync target needs to be upgraded. Press this banner to proceed.')));
+		if (this.props.mustUpgradeAppMessage) warningComps.push(this.renderWarningBox('UpgradeApp', this.props.mustUpgradeAppMessage));
 
 		if (this.props.hasDisabledEncryptionItems) {
 			warningComps.push(this.renderWarningBox('Status', _('Some items cannot be decrypted.')));
@@ -668,6 +670,7 @@ const ScreenHeader = connect((state: State) => {
 		showMissingMasterKeyMessage: showMissingMasterKeyMessage(syncInfo, state.notLoadedMasterKeys),
 		hasDisabledSyncItems: state.hasDisabledSyncItems,
 		shouldUpgradeSyncTarget: state.settings['sync.upgradeState'] === Setting.SYNC_UPGRADE_STATE_SHOULD_DO,
+		mustUpgradeAppMessage: state.mustUpgradeAppMessage,
 	};
 })(ScreenHeaderComponent);
 

packages/lib/Synchronizer.ts

@@ -22,7 +22,7 @@ import TaskQueue from './TaskQueue';
 import ItemUploader from './services/synchronizer/ItemUploader';
 import { FileApi, getSupportsDeltaWithItems, PaginatedList, RemoteItem } from './file-api';
 import JoplinDatabase from './JoplinDatabase';
-import { fetchSyncInfo, getActiveMasterKey, localSyncInfo, mergeSyncInfos, saveLocalSyncInfo, setMasterKeyHasBeenUsed, SyncInfo, syncInfoEquals, uploadSyncInfo } from './services/synchronizer/syncInfoUtils';
+import { checkIfCanSync, fetchSyncInfo, getActiveMasterKey, localSyncInfo, mergeSyncInfos, saveLocalSyncInfo, setMasterKeyHasBeenUsed, SyncInfo, syncInfoEquals, uploadSyncInfo } from './services/synchronizer/syncInfoUtils';
 import { getMasterPassword, setupAndDisableEncryption, setupAndEnableEncryption } from './services/e2ee/utils';
 import { generateKeyPair } from './services/e2ee/ppk';
 import syncDebugLog from './services/synchronizer/syncDebugLog';
@@ -115,7 +115,9 @@ export default class Synchronizer {
 	}
 
 	public setLogger(l: Logger) {
+		const previous = this.logger_;
 		this.logger_ = l;
+		return previous;
 	}
 
 	public logger() {
@@ -465,6 +467,9 @@ export default class Synchronizer {
 
 				await this.migrationHandler().checkCanSync(remoteInfo);
 
+				const appVersion = shim.appVersion();
+				if (appVersion !== 'unknown') checkIfCanSync(remoteInfo, appVersion);
+
 				let localInfo = await localSyncInfo();
 
 				logger.info('Sync target local info:', localInfo);
@@ -1055,6 +1060,13 @@ export default class Synchronizer {
 				}
 			} // DELTA STEP
 		} catch (error) {
+			if (error.code === ErrorCode.MustUpgradeApp) {
+				this.dispatch({
+					type: 'MUST_UPGRADE_APP',
+					message: error.message,
+				});
+			}
+
 			if (throwOnError) {
 				errorToThrow = error;
 			} else if (error && ['cannotEncryptEncrypted', 'noActiveMasterKey', 'processingPathTwice', 'failSafe', 'lockError', 'outdatedSyncTarget'].indexOf(error.code) >= 0) {

packages/lib/errors.ts

@@ -4,4 +4,5 @@ export enum ErrorCode {
 	IsReadOnly = 'isReadOnly',
 	NotFound = 'notFound',
 	UnsupportedMimeType = 'unsupportedMimeType',
+	MustUpgradeApp = 'mustUpgradeApp',
 }

packages/lib/file-api-driver-memory.ts

@@ -29,6 +29,7 @@ export default class FileApiDriverMemory {
 	}
 
 	private decodeContent_(content: any) {
+		if (!content) return '';
 		return Buffer.from(content, 'base64').toString('utf-8');
 	}
 

packages/lib/jest.setup.js

@@ -3,10 +3,11 @@ const { shimInit } = require('./shim-init-node.js');
 const sharp = require('sharp');
 const nodeSqlite = require('sqlite3');
 const pdfJs = require('pdfjs-dist');
+const packageInfo = require('./package.json');
 
 require('../../jest.base-setup.js')();
 
-shimInit({ sharp, nodeSqlite, pdfJs });
+shimInit({ sharp, nodeSqlite, pdfJs, appVersion: () => packageInfo.version });
 
 global.afterEach(async () => {
 	await afterEachCleanUp();

packages/lib/reducer.ts

@@ -103,6 +103,7 @@ export interface State {
 	profileConfig: ProfileConfig;
 	noteListRendererIds: string[];
 	noteListLastSortTime: number;
+	mustUpgradeAppMessage: string;
 
 	// Extra reducer keys go here:
 	pluginService: PluginServiceState;
@@ -178,6 +179,7 @@ export const defaultState: State = {
 	profileConfig: null,
 	noteListRendererIds: getListRendererIds(),
 	noteListLastSortTime: 0,
+	mustUpgradeAppMessage: '',
 
 	pluginService: pluginServiceDefaultState,
 	shareService: shareServiceDefaultState,
@@ -1224,6 +1226,10 @@ const reducer = produce((draft: Draft<State> = defaultState, action: any) => {
 			draft.profileConfig = action.value;
 			break;
 
+		case 'MUST_UPGRADE_APP':
+			draft.mustUpgradeAppMessage = action.message;
+			break;
+
 		case 'NOTE_LIST_RENDERER_ADD':
 			{
 				const noteListRendererIds = draft.noteListRendererIds.slice();

packages/lib/services/synchronizer/Synchronizer.basics.test.ts

@@ -1,11 +1,13 @@
 import Setting from '../../models/Setting';
 import { allNotesFolders, remoteNotesAndFolders, localNotesFoldersSameAsRemote } from '../../testing/test-utils-synchronizer';
-import { syncTargetName, afterAllCleanUp, synchronizerStart, setupDatabaseAndSynchronizer, synchronizer, sleep, switchClient, syncTargetId, fileApi } from '../../testing/test-utils';
+import { syncTargetName, afterAllCleanUp, synchronizerStart, setupDatabaseAndSynchronizer, synchronizer, sleep, switchClient, syncTargetId, fileApi, expectThrow } from '../../testing/test-utils';
 import Folder from '../../models/Folder';
 import Note from '../../models/Note';
 import BaseItem from '../../models/BaseItem';
 import WelcomeUtils from '../../WelcomeUtils';
 import { NoteEntity } from '../database/types';
+import { fetchSyncInfo, setAppMinVersion, uploadSyncInfo } from './syncInfoUtils';
+import { ErrorCode } from '../../errors';
 
 describe('Synchronizer.basics', () => {
 
@@ -459,4 +461,40 @@ describe('Synchronizer.basics', () => {
 		expect(remotes.find(r => r.path === `${note.id}.md`)).toBeTruthy();
 	}));
 
+	it('should throw an error if the app version is not compatible with the sync target info', (async () => {
+		await synchronizerStart();
+
+		const remoteInfo = await fetchSyncInfo(synchronizer().api());
+
+		remoteInfo.appMinVersion = '100.0.0';
+		await uploadSyncInfo(synchronizer().api(), remoteInfo);
+
+		await expectThrow(async () => synchronizerStart(1, {
+			throwOnError: true,
+		}), ErrorCode.MustUpgradeApp);
+	}));
+
+	it('should update the remote appMinVersion when synchronising', (async () => {
+		await synchronizerStart();
+
+		const remoteInfoBefore = await fetchSyncInfo(synchronizer().api());
+
+		// Simulates upgrading the client
+		setAppMinVersion('100.0.0');
+		await synchronizerStart();
+
+		// Then after sync, appMinVersion should be the same as that client version
+		const remoteInfoAfter = await fetchSyncInfo(synchronizer().api());
+
+		expect(remoteInfoBefore.appMinVersion).toBe('0.0.0');
+		expect(remoteInfoAfter.appMinVersion).toBe('100.0.0');
+
+		// Now simulates synchronising with an older client version. In that case, it should not be
+		// allowed and the remote info.json should not change.
+		setAppMinVersion('80.0.0');
+		await expectThrow(async () => synchronizerStart(1, { throwOnError: true }), ErrorCode.MustUpgradeApp);
+
+		expect((await fetchSyncInfo(synchronizer().api())).appMinVersion).toBe('100.0.0');
+	}));
+
 });

packages/lib/services/synchronizer/syncInfoUtils.test.ts

@@ -1,6 +1,6 @@
 import { afterAllCleanUp, setupDatabaseAndSynchronizer, logger, switchClient, encryptionService, msleep } from '../../testing/test-utils';
 import MasterKey from '../../models/MasterKey';
-import { localSyncInfo, masterKeyEnabled, mergeSyncInfos, saveLocalSyncInfo, setMasterKeyEnabled, SyncInfo, syncInfoEquals } from './syncInfoUtils';
+import { checkIfCanSync, localSyncInfo, masterKeyEnabled, mergeSyncInfos, saveLocalSyncInfo, setMasterKeyEnabled, SyncInfo, syncInfoEquals } from './syncInfoUtils';
 
 describe('syncInfoUtils', () => {
 
@@ -91,6 +91,22 @@ describe('syncInfoUtils', () => {
 		}
 	});
 
+	it('should merge sync target info and keep the highest appMinVersion', async () => {
+		const syncInfo1 = new SyncInfo();
+		syncInfo1.appMinVersion = '1.0.5';
+		const syncInfo2 = new SyncInfo();
+		syncInfo2.appMinVersion = '1.0.2';
+		expect(mergeSyncInfos(syncInfo1, syncInfo2).appMinVersion).toBe('1.0.5');
+
+		syncInfo1.appMinVersion = '2.1.0';
+		syncInfo2.appMinVersion = '2.2.5';
+		expect(mergeSyncInfos(syncInfo1, syncInfo2).appMinVersion).toBe('2.2.5');
+
+		syncInfo1.appMinVersion = '1.0.0';
+		syncInfo2.appMinVersion = '1.0.0';
+		expect(mergeSyncInfos(syncInfo1, syncInfo2).appMinVersion).toBe('1.0.0');
+	});
+
 	it('should merge sync target info and takes into account usage of master key - 1', async () => {
 		const syncInfo1 = new SyncInfo();
 		syncInfo1.masterKeys = [{
@@ -175,4 +191,21 @@ describe('syncInfoUtils', () => {
 		logger.enabled = true;
 	});
 
+	test.each([
+		['1.0.0', '1.0.4', true],
+		['1.0.0', '0.0.5', false],
+		['1.0.0', '1.0.0', true],
+	])('should check if it can sync', async (appMinVersion, appVersion, expected) => {
+		let succeeded = true;
+		try {
+			const s = new SyncInfo();
+			s.appMinVersion = appMinVersion;
+			checkIfCanSync(s, appVersion);
+		} catch (error) {
+			succeeded = false;
+		}
+
+		expect(succeeded).toBe(expected);
+	});
+
 });

packages/lib/services/synchronizer/syncInfoUtils.ts

@@ -5,6 +5,10 @@ import Setting from '../../models/Setting';
 import { State } from '../../reducer';
 import { PublicPrivateKeyPair } from '../e2ee/ppk';
 import { MasterKeyEntity } from '../e2ee/types';
+import { compareVersions } from 'compare-versions';
+import { _ } from '../../locale';
+import JoplinError from '../../JoplinError';
+import { ErrorCode } from '../../errors';
 const fastDeepEqual = require('fast-deep-equal');
 
 const logger = Logger.create('syncInfoUtils');
@@ -24,6 +28,21 @@ export interface SyncInfoValuePublicPrivateKeyPair {
 	updatedTime: number;
 }
 
+// This should be set to the client version whenever we require all the clients to be at the same
+// version in order to synchronise. One example is when adding support for the trash feature - if an
+// old client that doesn't know about this feature synchronises data with a new client, the notes
+// will no longer be deleted on the old client.
+//
+// Usually this variable should be bumped whenever we add properties to a sync item.
+//
+// `appMinVersion_` should really just be a constant but for testing purposes it can be changed
+// using `setAppMinVersion()`
+let appMinVersion_ = '0.0.0';
+
+export const setAppMinVersion = (v: string) => {
+	appMinVersion_ = v;
+};
+
 export async function migrateLocalSyncInfo(db: JoplinDatabase) {
 	if (Setting.value('syncInfoCache')) return; // Already initialized
 
@@ -96,7 +115,9 @@ const fixSyncInfo = (syncInfo: SyncInfo) => {
 };
 
 export function localSyncInfo(): SyncInfo {
-	return fixSyncInfo(new SyncInfo(Setting.value('syncInfoCache')));
+	const output = new SyncInfo(Setting.value('syncInfoCache'));
+	output.appMinVersion = appMinVersion_;
+	return fixSyncInfo(output);
 }
 
 export function localSyncInfoFromState(state: State): SyncInfo {
@@ -178,6 +199,8 @@ export function mergeSyncInfos(s1: SyncInfo, s2: SyncInfo): SyncInfo {
 		}
 	}
 
+	output.appMinVersion = compareVersions(s1.appMinVersion, s2.appMinVersion) > 0 ? s1.appMinVersion : s2.appMinVersion;
+
 	return output;
 }
 
@@ -192,6 +215,7 @@ export class SyncInfo {
 	private activeMasterKeyId_: SyncInfoValueString;
 	private masterKeys_: MasterKeyEntity[] = [];
 	private ppk_: SyncInfoValuePublicPrivateKeyPair;
+	private appMinVersion_: string = appMinVersion_;
 
 	public constructor(serialized: string = null) {
 		this.e2ee_ = { value: false, updatedTime: 0 };
@@ -208,6 +232,7 @@ export class SyncInfo {
 			activeMasterKeyId: this.activeMasterKeyId_,
 			masterKeys: this.masterKeys,
 			ppk: this.ppk_,
+			appMinVersion: this.appMinVersion,
 		};
 	}
 
@@ -222,6 +247,7 @@ export class SyncInfo {
 		this.activeMasterKeyId_ = 'activeMasterKeyId' in s ? s.activeMasterKeyId : { value: '', updatedTime: 0 };
 		this.masterKeys_ = 'masterKeys' in s ? s.masterKeys : [];
 		this.ppk_ = 'ppk' in s ? s.ppk : { value: null, updatedTime: 0 };
+		this.appMinVersion_ = s.appMinVersion ? s.appMinVersion : '0.00';
 
 		// Migration for master keys that didn't have "hasBeenUsed" property -
 		// in that case we assume they've been used at least once.
@@ -269,6 +295,14 @@ export class SyncInfo {
 		this.e2ee_ = { value: v, updatedTime: Date.now() };
 	}
 
+	public get appMinVersion(): string {
+		return this.appMinVersion_;
+	}
+
+	public set appMinVersion(v: string) {
+		this.appMinVersion_ = v;
+	}
+
 	public get activeMasterKeyId(): string {
 		return this.activeMasterKeyId_.value;
 	}
@@ -388,3 +422,7 @@ export function setPpk(ppk: PublicPrivateKeyPair) {
 export function masterKeyById(id: string) {
 	return localSyncInfo().masterKeys.find(mk => mk.id === id);
 }
+
+export const checkIfCanSync = (s: SyncInfo, appVersion: string) => {
+	if (compareVersions(appVersion, s.appMinVersion) < 0) throw new JoplinError(_('In order to synchronise, please upgrade your application to version %s+', s.appMinVersion), ErrorCode.MustUpgradeApp);
+};

packages/lib/shim-init-node.ts

@@ -93,7 +93,7 @@ interface ShimInitOptions {
 	sharp: any;
 	keytar: any;
 	React: any;
-	appVersion: any;
+	appVersion: ()=> string;
 	electronBridge: any;
 	nodeSqlite: any;
 	pdfJs: typeof pdfJsNamespace;
@@ -673,7 +673,7 @@ function shimInit(options: ShimInitOptions = null) {
 		if (appVersion) return appVersion();
 		// Should not happen but don't throw an error because version number is
 		// used in error messages.
-		return 'unknown-version!';
+		return 'unknown';
 	};
 
 	shim.pathRelativeToCwd = (path) => {

packages/lib/testing/test-utils.ts

@@ -723,7 +723,7 @@ async function checkThrowAsync(asyncFn: Function) {
 }
 
 // eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
-async function expectThrow(asyncFn: Function, errorCode: any = undefined) {
+async function expectThrow(asyncFn: Function, errorCode: any = undefined, errorMessage: string = undefined) {
 	let hasThrown = false;
 	let thrownError = null;
 	try {
@@ -735,6 +735,12 @@ async function expectThrow(asyncFn: Function, errorCode: any = undefined) {
 
 	if (!hasThrown) {
 		expect('not throw').toBe('throw');
+	} else if (errorMessage !== undefined) {
+		if (thrownError.message !== errorMessage) {
+			expect(`error message: ${thrownError.message}`).toBe(`error message: ${errorMessage}`);
+		} else {
+			expect(true).toBe(true);
+		}
 	} else if (thrownError.code !== errorCode) {
 		console.error(thrownError);
 		expect(`error code: ${thrownError.code}`).toBe(`error code: ${errorCode}`);