Desktop: Fixes #8661: Fix note editor blank after syncing an encrypted note with remote changes (#8666)

.eslintignore

@@ -260,6 +260,7 @@ packages/app-desktop/gui/NoteEditor/utils/types.js
 packages/app-desktop/gui/NoteEditor/utils/useDropHandler.js
 packages/app-desktop/gui/NoteEditor/utils/useEffectiveNoteId.js
 packages/app-desktop/gui/NoteEditor/utils/useFolder.js
+packages/app-desktop/gui/NoteEditor/utils/useFormNote.test.js
 packages/app-desktop/gui/NoteEditor/utils/useFormNote.js
 packages/app-desktop/gui/NoteEditor/utils/useMarkupToHtml.js
 packages/app-desktop/gui/NoteEditor/utils/useMessageHandler.js

.gitignore

@@ -246,6 +246,7 @@ packages/app-desktop/gui/NoteEditor/utils/types.js
 packages/app-desktop/gui/NoteEditor/utils/useDropHandler.js
 packages/app-desktop/gui/NoteEditor/utils/useEffectiveNoteId.js
 packages/app-desktop/gui/NoteEditor/utils/useFolder.js
+packages/app-desktop/gui/NoteEditor/utils/useFormNote.test.js
 packages/app-desktop/gui/NoteEditor/utils/useFormNote.js
 packages/app-desktop/gui/NoteEditor/utils/useMarkupToHtml.js
 packages/app-desktop/gui/NoteEditor/utils/useMessageHandler.js

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

@@ -78,6 +78,7 @@ function NoteEditor(props: NoteEditorProps) {
 
 	const { formNote, setFormNote, isNewNote, resourceInfos } = useFormNote({
 		syncStarted: props.syncStarted,
+		decryptionStarted: props.decryptionStarted,
 		noteId: effectiveNoteId,
 		isProvisional: props.isProvisional,
 		titleInputRef: titleInputRef,
@@ -633,6 +634,7 @@ const mapStateToProps = (state: AppState) => {
 		isProvisional: state.provisionalNoteIds.includes(noteId),
 		editorNoteStatuses: state.editorNoteStatuses,
 		syncStarted: state.syncStarted,
+		decryptionStarted: state.decryptionWorker?.state !== 'idle',
 		themeId: state.settings.theme,
 		richTextBannerDismissed: state.settings.richTextBannerDismissed,
 		watchedNoteFiles: state.watchedNoteFiles,

packages/app-desktop/gui/NoteEditor/utils/types.ts

@@ -27,6 +27,7 @@ export interface NoteEditorProps {
 	isProvisional: boolean;
 	editorNoteStatuses: any;
 	syncStarted: boolean;
+	decryptionStarted: boolean;
 	bodyEditor: string;
 	notesParentType: string;
 	selectedNoteTags: any[];

packages/app-desktop/gui/NoteEditor/utils/useFormNote.test.ts

@@ -0,0 +1,73 @@
+import Note from '@joplin/lib/models/Note';
+import { setupDatabaseAndSynchronizer, switchClient } from '@joplin/lib/testing/test-utils';
+import { renderHook } from '@testing-library/react-hooks';
+import useFormNote, { HookDependencies } from './useFormNote';
+
+
+describe('useFormNote', () => {
+	beforeEach(async () => {
+		await setupDatabaseAndSynchronizer(1);
+		await switchClient(1);
+	});
+
+	it('should update note when decryption completes', async () => {
+		const testNote = await Note.save({ title: 'Test Note!' });
+
+		const makeFormNoteProps = (syncStarted: boolean, decryptionStarted: boolean): HookDependencies => {
+			return {
+				syncStarted,
+				decryptionStarted,
+				noteId: testNote.id,
+				isProvisional: false,
+				titleInputRef: null,
+				editorRef: null,
+				onBeforeLoad: ()=>{},
+				onAfterLoad: ()=>{},
+			};
+		};
+
+		const formNote = renderHook(props => useFormNote(props), {
+			initialProps: makeFormNoteProps(true, false),
+		});
+		await formNote.waitFor(() => {
+			expect(formNote.result.current.formNote).toMatchObject({
+				encryption_applied: 0,
+				title: testNote.title,
+			});
+		});
+
+		await Note.save({
+			id: testNote.id,
+			encryption_cipher_text: 'cipher_text',
+			encryption_applied: 1,
+		});
+
+		// Sync starting should cause a re-render
+		formNote.rerender(makeFormNoteProps(false, false));
+
+		await formNote.waitFor(() => {
+			expect(formNote.result.current.formNote).toMatchObject({
+				encryption_applied: 1,
+			});
+		});
+
+
+		formNote.rerender(makeFormNoteProps(false, true));
+
+		await Note.save({
+			id: testNote.id,
+			encryption_applied: 0,
+			title: 'Test Note!',
+		});
+
+		// Ending decryption should also cause a re-render
+		formNote.rerender(makeFormNoteProps(false, false));
+
+		await formNote.waitFor(() => {
+			expect(formNote.result.current.formNote).toMatchObject({
+				encryption_applied: 0,
+				title: 'Test Note!',
+			});
+		});
+	});
+});

packages/app-desktop/gui/NoteEditor/utils/useFormNote.ts

@@ -18,8 +18,9 @@ export interface OnLoadEvent {
 	formNote: FormNote;
 }
 
-interface HookDependencies {
+export interface HookDependencies {
 	syncStarted: boolean;
+	decryptionStarted: boolean;
 	noteId: string;
 	isProvisional: boolean;
 	titleInputRef: any;
@@ -61,15 +62,21 @@ function resourceInfosChanged(a: ResourceInfos, b: ResourceInfos): boolean {
 }
 
 export default function useFormNote(dependencies: HookDependencies) {
-	const { syncStarted, noteId, isProvisional, titleInputRef, editorRef, onBeforeLoad, onAfterLoad } = dependencies;
+	const {
+		syncStarted, decryptionStarted, noteId, isProvisional, titleInputRef, editorRef, onBeforeLoad, onAfterLoad,
+	} = dependencies;
 
 	const [formNote, setFormNote] = useState<FormNote>(defaultFormNote());
-	const [formNoteRefeshScheduled, setFormNoteRefreshScheduled] = useState<boolean>(false);
 	const [isNewNote, setIsNewNote] = useState(false);
 	const prevSyncStarted = usePrevious(syncStarted);
+	const prevDecryptionStarted = usePrevious(decryptionStarted);
 	const previousNoteId = usePrevious(formNote.id);
 	const [resourceInfos, setResourceInfos] = useState<ResourceInfos>({});
 
+	// Increasing the value of this counter cancels any ongoing note refreshes and starts
+	// a new refresh.
+	const [formNoteRefeshScheduled, setFormNoteRefreshScheduled] = useState<number>(0);
+
 	async function initNoteState(n: any) {
 		let originalCss = '';
 
@@ -107,7 +114,7 @@ export default function useFormNote(dependencies: HookDependencies) {
 	}
 
 	useEffect(() => {
-		if (!formNoteRefeshScheduled) return () => {};
+		if (formNoteRefeshScheduled <= 0) return () => {};
 
 		reg.logger().info('Sync has finished and note has never been changed - reloading it');
 
@@ -126,7 +133,7 @@ export default function useFormNote(dependencies: HookDependencies) {
 			}
 
 			await initNoteState(n);
-			setFormNoteRefreshScheduled(false);
+			setFormNoteRefreshScheduled(0);
 		};
 
 		void loadNote();
@@ -136,21 +143,32 @@ export default function useFormNote(dependencies: HookDependencies) {
 		};
 	}, [formNoteRefeshScheduled, noteId]);
 
+	const refreshFormNote = useCallback(() => {
+		// Increase the counter to cancel any ongoing refresh attempts
+		// and start a new one.
+		setFormNoteRefreshScheduled(formNoteRefeshScheduled + 1);
+	}, [formNoteRefeshScheduled]);
+
 	useEffect(() => {
 		// Check that synchronisation has just finished - and
 		// if the note has never been changed, we reload it.
 		// If the note has already been changed, it's a conflict
 		// that's already been handled by the synchronizer.
+		const decryptionJustEnded = prevDecryptionStarted && !decryptionStarted;
+		const syncJustEnded = prevSyncStarted && !syncStarted;
 
-		if (!prevSyncStarted) return;
-		if (syncStarted) return;
+		if (!decryptionJustEnded && !syncJustEnded) return;
 		if (formNote.hasChanged) return;
 
 		// Refresh the form note.
 		// This is kept separate from the above logic so that when prevSyncStarted is changed
 		// from true to false, it doesn't cancel the note from loading.
-		setFormNoteRefreshScheduled(true);
-	}, [prevSyncStarted, syncStarted, formNote.hasChanged]);
+		refreshFormNote();
+	}, [
+		prevSyncStarted, syncStarted,
+		prevDecryptionStarted, decryptionStarted,
+		formNote.hasChanged, refreshFormNote,
+	]);
 
 	useEffect(() => {
 		if (!noteId) {

packages/app-desktop/jest.setup.js

@@ -1,19 +1,14 @@
+/* eslint-disable jest/require-top-level-describe */
 
-const { default: Logger, TargetType } = require('@joplin/utils/Logger');
-const initLib = require('@joplin/lib/initLib').default;
-
-// TODO: Some libraries required by test-utils.js seem to fail to import with the
-// jsdom environment.
-//
-// Thus, require('@joplin/lib/testing/test-utils.js') fails and some setup must be
-// copied.
-
-const logger = new Logger();
-logger.addTarget(TargetType.Console);
-logger.setLevel(Logger.LEVEL_WARN);
-Logger.initializeGlobalLogger(logger);
-initLib(logger);
+const { shimInit } = require('@joplin/lib/shim-init-node');
+const sqlite3 = require('sqlite3');
+const SyncTargetNone = require('@joplin/lib/SyncTargetNone').default;
 
+// Mock the S3 sync target -- the @aws-s3 libraries depend on an old version
+// of uuid that doesn't work with jest without additional configuration.
+jest.doMock('@joplin/lib/SyncTargetAmazonS3', () => {
+	return SyncTargetNone;
+});
 
 // @electron/remote requires electron to be running. Mock it.
 jest.mock('@electron/remote', () => {
@@ -25,3 +20,18 @@ jest.mock('@electron/remote', () => {
 		},
 	};
 });
+
+// Import after mocking problematic libraries
+const { afterEachCleanUp, afterAllCleanUp } = require('@joplin/lib/testing/test-utils.js');
+
+
+shimInit({ nodeSqlite: sqlite3 });
+
+afterEach(async () => {
+	await afterEachCleanUp();
+});
+
+afterAll(async () => {
+	await afterAllCleanUp();
+});
+

packages/lib/testing/test-utils.ts

@@ -34,7 +34,6 @@ const { FileApiDriverLocal } = require('../file-api-driver-local');
 const { FileApiDriverWebDav } = require('../file-api-driver-webdav.js');
 const { FileApiDriverDropbox } = require('../file-api-driver-dropbox.js');
 const { FileApiDriverOneDrive } = require('../file-api-driver-onedrive.js');
-const { FileApiDriverAmazonS3 } = require('../file-api-driver-amazon-s3.js');
 import SyncTargetRegistry from '../SyncTargetRegistry';
 const SyncTargetMemory = require('../SyncTargetMemory.js');
 const SyncTargetFilesystem = require('../SyncTargetFilesystem.js');
@@ -60,7 +59,6 @@ import Synchronizer from '../Synchronizer';
 import SyncTargetNone from '../SyncTargetNone';
 import { setRSA } from '../services/e2ee/ppk';
 const md5 = require('md5');
-const { S3Client } = require('@aws-sdk/client-s3');
 const { Dirnames } = require('../services/synchronizer/utils/types');
 import RSA from '../services/e2ee/RSA.node';
 import { State as ShareState } from '../services/share/reducer';
@@ -627,6 +625,13 @@ async function initFileApi() {
 		const appDir = await api.appDirectory();
 		fileApi = new FileApi(appDir, new FileApiDriverOneDrive(api));
 	} else if (syncTargetId_ === SyncTargetRegistry.nameToId('amazon_s3')) {
+		// (Most of?) the @aws-sdk libraries depend on an old version of uuid
+		// that doesn't work with jest (without converting ES6 exports to CommonJS).
+		//
+		// Require it dynamically so that this doesn't break test environments that
+		// aren't configured to do this conversion.
+		const { FileApiDriverAmazonS3 } = require('../file-api-driver-amazon-s3.js');
+		const { S3Client } = require('@aws-sdk/client-s3');
 
 		// We make sure for S3 tests run in band because tests
 		// share the same directory which will cause locking errors.