Revisions of old notes before the cut off date are not deleted from the sync source

[mrjo118]

Operating system

Android

Joplin version

3.0.8

Desktop version info

No response

Current behaviour

I created a brand new profile in Joplin desktop using a JEX import, synced with OneDrive (after deleting the old Joplin app data there) and then synced with my Android device with a fresh install of the Joplin app. On both devices I set the 'keep note history for' setting to 1 day, with the intention to reduce the number of objects to sync, required to setup a new device in future. Almost a month later, I attempted a new device sync on Joplin desktop, and the total number of synced objects had gone up from the initial value of about 350 to about 850 objects - even though the number of new notes were minimal, and I did not create any new tags since creation of the profile. When I view the note history in Joplin desktop on the new device, it does correctly limit the note history to the last 24 hours. However, when I check inside the SQLite database of the newly setup device, the size of the database file had grown significantly since the original version, and the revisions table contains all the revisions since creating the profile

Expected behaviour

Judging from the code here

joplin/packages/lib/models/Revision.ts

Line 301 in ec36847

public static async deleteOldRevisions(ttl: number) {

I would expect that note revisions in the SQLite database created earlier than 24 hours ago would be cleaned in the database. It's worth noting that since creating the profile on Joplin desktop, almost all note creation and modification has been done on my Android device, rather than on Joplin desktop

Logs

No response

[mrjo118]

I made a further development today. If I sync a new windows device to my sync source, but I leave the profile to have the 'keep note history for' set to the default value of 90 days, then I find that I can view those old revisions in the version history of notes from the UI. I also found that if I load back up the desktop profile I created 2 days ago (with note history set to 1 day), when I checked the SQLite database again, the old revisions had been removed from the table.

So I think there are 2 issues here:

1. On the Android app, the maintenance task for deleting old revisions either does not work, or the maintenance task never fires unless the Joplin app is kept in the foreground for a long enough period. I generally make short changes to notes throughout the day which might take between 30 seconds to 2 minutes
2. When the maintenance task does run (which does work on the desktop version), the removed revisions do not get updated at the sync source, effectively meaning that note history is only ever cleaned up on the local device. This means that if have note history enabled at all, and you sync your notes daily, then regardless of the value of the expiry days you set, the data at the sync source is going to continually grow every time you make changes to your notes. This means note history never expires on the server side, as the clean up of note history is only done on the client side and those changes are not published to the server. This also means that the longer your profile has been in use, the slower new device setup is going the become (assuming you are continually and regularly making changes to your notes)

EDIT: Actually technically I have no way of knowing if the maintenance task works on Android or not, as there's no facility to check the note history on the Android app. So it may be just point 2 that applies, and every day when I edit notes on my phone, I publish to the server any new revisions created that day

[personalizedrefrigerator]

I've written a (failing) test that seems to demonstrate this issue.

Test

diff --git a/packages/lib/services/synchronizer/Synchronizer.revisions.test.ts b/packages/lib/services/synchronizer/Synchronizer.revisions.test.ts
index 1bb6cbaa9..fa152ef16 100644
--- a/packages/lib/services/synchronizer/Synchronizer.revisions.test.ts
+++ b/packages/lib/services/synchronizer/Synchronizer.revisions.test.ts
@@ -181,4 +181,42 @@ describe('Synchronizer.revisions', () => {
 		expect((await Revision.all()).length).toBe(0);
 	}));
 
+	test('should delete old revisions remotely when deleted locally', async () => {
+		Setting.setValue('revisionService.intervalBetweenRevisions', 100);
+		jest.useFakeTimers({ advanceTimers: true });
+
+		const n1 = await Note.save({ title: 'note' });
+		jest.advanceTimersByTime(200);
+
+		await Note.save({ id: n1.id, title: 'note REV0' });
+		jest.advanceTimersByTime(200);
+
+		await revisionService().collectRevisions(); // REV0
+		expect((await Revision.allByType(BaseModel.TYPE_NOTE, n1.id)).length).toBe(1);
+
+		jest.advanceTimersByTime(200);
+
+		await Note.save({ id: n1.id, title: 'note REV1' });
+		await revisionService().collectRevisions(); // REV1
+		expect((await Revision.allByType(BaseModel.TYPE_NOTE, n1.id)).length).toBe(2);
+
+		// Should sync the revisions
+		await synchronizer().start();
+		await switchClient(2);
+
+		await synchronizer().start();
+		expect((await Revision.allByType(BaseModel.TYPE_NOTE, n1.id)).length).toBe(2);
+		await revisionService().deleteOldRevisions(100);
+		
+		expect((await Revision.allByType(BaseModel.TYPE_NOTE, n1.id)).length).toBe(0);
+		await synchronizer().start();
+		expect((await Revision.allByType(BaseModel.TYPE_NOTE, n1.id)).length).toBe(0); // FAILS!!!
+
+		// Syncing a new client should not download the deleted revisions
+		await switchClient(3);
+		await synchronizer().start();
+		expect((await Revision.allByType(BaseModel.TYPE_NOTE, n1.id)).length).toBe(0);
+
+		jest.useRealTimers();
+	});
 });

When it:

1. deletes old revisions locally, then
2. syncs,

the just-deleted revisions seem to download from the sync target.