diff --git a/packages/app-desktop/gui/MainScreen/MainScreen.tsx b/packages/app-desktop/gui/MainScreen/MainScreen.tsx index ed3c79665..a4885a4fb 100644 --- a/packages/app-desktop/gui/MainScreen/MainScreen.tsx +++ b/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 { } // 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 {message}; + const cfa = ( callForActionHandler()}> {callForAction} @@ -671,6 +674,8 @@ class MainScreenComponent extends React.Component { 'Install plugin', onViewPluginScreen, ); + } else if (this.props.mustUpgradeAppMessage) { + msg = this.renderNotificationMessage(this.props.mustUpgradeAppMessage); } return ( @@ -682,7 +687,18 @@ class MainScreenComponent extends React.Component { 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, }; }; diff --git a/packages/app-mobile/components/ScreenHeader.tsx b/packages/app-mobile/components/ScreenHeader.tsx index 39f999af8..a87f214a7 100644 --- a/packages/app-mobile/components/ScreenHeader.tsx +++ b/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 { showMissingMasterKeyMessage: showMissingMasterKeyMessage(syncInfo, state.notLoadedMasterKeys), hasDisabledSyncItems: state.hasDisabledSyncItems, shouldUpgradeSyncTarget: state.settings['sync.upgradeState'] === Setting.SYNC_UPGRADE_STATE_SHOULD_DO, + mustUpgradeAppMessage: state.mustUpgradeAppMessage, }; })(ScreenHeaderComponent); diff --git a/packages/lib/Synchronizer.ts b/packages/lib/Synchronizer.ts index 8c803223a..73704b6d8 100644 --- a/packages/lib/Synchronizer.ts +++ b/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) { diff --git a/packages/lib/errors.ts b/packages/lib/errors.ts index bb1e341a2..e310a472d 100644 --- a/packages/lib/errors.ts +++ b/packages/lib/errors.ts @@ -4,4 +4,5 @@ export enum ErrorCode { IsReadOnly = 'isReadOnly', NotFound = 'notFound', UnsupportedMimeType = 'unsupportedMimeType', + MustUpgradeApp = 'mustUpgradeApp', } diff --git a/packages/lib/file-api-driver-memory.ts b/packages/lib/file-api-driver-memory.ts index 959a37dbf..755aa30a6 100644 --- a/packages/lib/file-api-driver-memory.ts +++ b/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'); } diff --git a/packages/lib/jest.setup.js b/packages/lib/jest.setup.js index 2d1293fdf..84258901c 100644 --- a/packages/lib/jest.setup.js +++ b/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(); diff --git a/packages/lib/reducer.ts b/packages/lib/reducer.ts index d20e5b35f..119e3f967 100644 --- a/packages/lib/reducer.ts +++ b/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 = 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(); diff --git a/packages/lib/services/synchronizer/Synchronizer.basics.test.ts b/packages/lib/services/synchronizer/Synchronizer.basics.test.ts index 40689f2fa..f3c818ac5 100644 --- a/packages/lib/services/synchronizer/Synchronizer.basics.test.ts +++ b/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'); + })); + }); diff --git a/packages/lib/services/synchronizer/syncInfoUtils.test.ts b/packages/lib/services/synchronizer/syncInfoUtils.test.ts index 53984df99..ea796e013 100644 --- a/packages/lib/services/synchronizer/syncInfoUtils.test.ts +++ b/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); + }); + }); diff --git a/packages/lib/services/synchronizer/syncInfoUtils.ts b/packages/lib/services/synchronizer/syncInfoUtils.ts index f42a39e37..b4f565e46 100644 --- a/packages/lib/services/synchronizer/syncInfoUtils.ts +++ b/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); +}; diff --git a/packages/lib/shim-init-node.ts b/packages/lib/shim-init-node.ts index 38a3fb642..493e3cfc5 100644 --- a/packages/lib/shim-init-node.ts +++ b/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) => { diff --git a/packages/lib/testing/test-utils.ts b/packages/lib/testing/test-utils.ts index edc42f3e3..30d26c70b 100644 --- a/packages/lib/testing/test-utils.ts +++ b/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}`);