All: Finished service to clean up resources

CliClient/app/app.js

@@ -5,6 +5,7 @@ const { JoplinDatabase } = require('lib/joplin-database.js');
 const { Database } = require('lib/database.js');
 const { FoldersScreenUtils } = require('lib/folders-screen-utils.js');
 const { DatabaseDriverNode } = require('lib/database-driver-node.js');
+const ResourceService = require('lib/services/ResourceService');
 const BaseModel = require('lib/BaseModel.js');
 const Folder = require('lib/models/Folder.js');
 const BaseItem = require('lib/models/BaseItem.js');
@@ -412,6 +413,12 @@ class Application extends BaseApplication {
 
 			const tags = await Tag.allWithNotes();
 
+			const resourceService = new ResourceService();
+			resourceService.maintenance();
+			setInterval(() => {
+				resourceService.maintenance();
+			}, 1000 * 60 * 60 * 4);
+
 			this.dispatch({
 				type: 'TAG_UPDATE_ALL',
 				items: tags,

CliClient/tests/services_ResourceService.js

@@ -0,0 +1,72 @@
+require('app-module-path').addPath(__dirname);
+
+const { time } = require('lib/time-utils.js');
+const { asyncTest, fileContentEqual, setupDatabase, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, objectsEqual, checkThrowAsync } = require('test-utils.js');
+const InteropService = require('lib/services/InteropService.js');
+const Folder = require('lib/models/Folder.js');
+const Note = require('lib/models/Note.js');
+const Tag = require('lib/models/Tag.js');
+const NoteTag = require('lib/models/NoteTag.js');
+const Resource = require('lib/models/Resource.js');
+const ResourceService = require('lib/services/ResourceService.js');
+const fs = require('fs-extra');
+const ArrayUtils = require('lib/ArrayUtils');
+const ObjectUtils = require('lib/ObjectUtils');
+const { shim } = require('lib/shim.js');
+
+process.on('unhandledRejection', (reason, p) => {
+	console.log('Unhandled Rejection at: Promise', p, 'reason:', reason);
+});
+
+jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000;
+
+function exportDir() {
+	return __dirname + '/export';
+}
+
+function fieldsEqual(model1, model2, fieldNames) {
+	for (let i = 0; i < fieldNames.length; i++) {
+		const f = fieldNames[i];
+		expect(model1[f]).toBe(model2[f], 'For key ' + f);
+	}
+}
+
+describe('services_ResourceService', function() {
+
+	beforeEach(async (done) => {
+		await setupDatabaseAndSynchronizer(1);
+		await switchClient(1);
+		done();
+	});
+
+	it('should delete orphaned resources', asyncTest(async () => {
+		const service = new ResourceService();
+
+		let folder1 = await Folder.save({ title: "folder1" });
+		let note1 = await Note.save({ title: 'ma note', parent_id: folder1.id });
+		note1 = await shim.attachFileToNote(note1, __dirname + '/../tests/support/photo.jpg');
+		let resource1 = (await Resource.all())[0];
+		const resourcePath = Resource.fullPath(resource1);
+
+		await service.indexNoteResources();
+		await service.deleteOrphanResources(0);
+
+		expect(!!(await Resource.load(resource1.id))).toBe(true);
+
+		await Note.delete(note1.id);
+		await service.deleteOrphanResources(0);
+
+		expect(!!(await Resource.load(resource1.id))).toBe(true);
+
+		await service.indexNoteResources();
+		await service.deleteOrphanResources(1000 * 60);
+
+		expect(!!(await Resource.load(resource1.id))).toBe(true);
+
+		await service.deleteOrphanResources(0);
+
+		expect(!!(await Resource.load(resource1.id))).toBe(false);
+		expect(await shim.fsDriver().exists(resourcePath)).toBe(false);
+	}));
+
+});

ElectronClient/app/app.js

@@ -22,6 +22,7 @@ const AlarmServiceDriverNode = require('lib/services/AlarmServiceDriverNode');
 const DecryptionWorker = require('lib/services/DecryptionWorker');
 const InteropService = require('lib/services/InteropService');
 const InteropServiceHelper = require('./InteropServiceHelper.js');
+const ResourceService = require('lib/services/ResourceService');
 
 const { bridge } = require('electron').remote.require('./bridge');
 const Menu = bridge().Menu;
@@ -608,6 +609,12 @@ class Application extends BaseApplication {
 			AlarmService.garbageCollect();
 		}, 1000 * 60 * 60);
 
+		const resourceService = new ResourceService();
+		resourceService.maintenance();
+		setInterval(() => {
+			resourceService.maintenance();
+		}, 1000 * 60 * 60 * 4);
+
 		if (Setting.value('env') === 'dev') {
 			AlarmService.updateAllNotifications();
 		} else {

ReactNativeClient/lib/BaseApplication.js

@@ -31,6 +31,7 @@ const SyncTargetNextcloud = require('lib/SyncTargetNextcloud.js');
 const SyncTargetWebDAV = require('lib/SyncTargetWebDAV.js');
 const EncryptionService = require('lib/services/EncryptionService');
 const DecryptionWorker = require('lib/services/DecryptionWorker');
+const BaseService = require('lib/services/BaseService');
 
 SyncTargetRegistry.addClass(SyncTargetFilesystem);
 SyncTargetRegistry.addClass(SyncTargetOneDrive);
@@ -426,6 +427,7 @@ class BaseApplication {
 			setLocale(Setting.value('locale'));
 		}
 
+		BaseService.logger_ = this.logger_;
 		EncryptionService.instance().setLogger(this.logger_);
 		BaseItem.encryptionService_ = EncryptionService.instance();
 		DecryptionWorker.instance().setLogger(this.logger_);

ReactNativeClient/lib/joplin-database.js

@@ -301,7 +301,7 @@ class JoplinDatabase extends Database {
 			if (targetVersion == 10) {
 				const itemChangesTable = `
 					CREATE TABLE item_changes (
-						id INTEGER PRIMARY KEY,
+						id INTEGER PRIMARY KEY AUTOINCREMENT,
 						item_type INT NOT NULL,
 						item_id TEXT NOT NULL,
 						type INT NOT NULL,
@@ -313,7 +313,9 @@ class JoplinDatabase extends Database {
 					CREATE TABLE note_resources (
 						id INTEGER PRIMARY KEY,
 						note_id TEXT NOT NULL,
-						resource_id TEXT NOT NULL
+						resource_id TEXT NOT NULL,
+						is_associated INT NOT NULL,
+						last_seen_time INT NOT NULL
 					);
 				`;
 

ReactNativeClient/lib/models/ItemChange.js

@@ -12,6 +12,10 @@ class ItemChange extends BaseModel {
 	}
 
 	static async add(itemType, itemId, type) {
+		ItemChange.saveCalls_.push(true);
+
+		// Using a mutex so that records can be added to the database in the
+		// background, without making the UI wait.
 		const release = await ItemChange.addChangeMutex_.acquire();
 
 		try {
@@ -21,12 +25,27 @@ class ItemChange extends BaseModel {
 			]);
 		} finally {
 			release();
+			ItemChange.saveCalls_.pop();
 		}
 	}
 
+	// Because item changes are recorded in the background, this function
+	// can be used for synchronous code, in particular when unit testing.
+	static async waitForAllSaved() {
+		return new Promise((resolve, reject) => {
+			const iid = setInterval(() => {
+				if (!ItemChange.saveCalls_.length) {
+					clearInterval(iid);
+					resolve();
+				}
+			}, 100);
+		});
+	}
+
 }
 
 ItemChange.addChangeMutex_ = new Mutex();
+ItemChange.saveCalls_ = [];
 
 ItemChange.TYPE_CREATE = 1;
 ItemChange.TYPE_UPDATE = 2;

ReactNativeClient/lib/models/NoteResource.js

@@ -10,21 +10,39 @@ class NoteResource extends BaseModel {
 		return BaseModel.TYPE_NOTE_RESOURCE;
 	}
 
-	static async associate(noteId, resourceIds) {
-		let queries = [];
-		queries.push({ sql: 'DELETE FROM note_resources WHERE note_id = ?', params: [noteId] });
+	static async setAssociatedResources(noteId, resourceIds) {
+		const existingRows = await this.modelSelectAll('SELECT * FROM note_resources WHERE note_id = ?', [noteId]);
 
-		for (let i = 0; i < resourceIds.length; i++) {
-			queries.push({ sql: 'INSERT INTO note_resources (note_id, resource_id) VALUES (?, ?)', params: [noteId, resourceIds[i]] });
+		const notProcessedResourceIds = resourceIds.slice();
+		const queries = [];
+		for (let i = 0; i < existingRows.length; i++) {
+			const row = existingRows[i];
+			const resourceIndex = resourceIds.indexOf(row.resource_id);
+
+			if (resourceIndex >= 0) {
+				queries.push({ sql: 'UPDATE note_resources SET last_seen_time = ?, is_associated = 1 WHERE id = ?', params: [Date.now(), row.id] });
+				notProcessedResourceIds.splice(notProcessedResourceIds.indexOf(row.resource_id), 1);
+			} else {
+				queries.push({ sql: 'UPDATE note_resources SET is_associated = 0 WHERE id = ?', params: [row.id] });
+			}
+		}
+
+		for (let i = 0; i < notProcessedResourceIds.length; i++) {
+			queries.push({ sql: 'INSERT INTO note_resources (note_id, resource_id, is_associated, last_seen_time) VALUES (?, ?, ?, ?)', params: [noteId, notProcessedResourceIds[i], 1, Date.now()] });
 		}
 
 		await this.db().transactionExecBatch(queries);		
 	}
 
 	static async remove(noteId) {
-		let queries = [];
-		queries.push({ sql: 'DELETE FROM note_resources WHERE note_id = ?', params: [noteId] });
-		await this.db().transactionExecBatch(queries);
+		await this.db().exec({ sql: 'UPDATE note_resources SET is_associated = 0 WHERE note_id = ?', params: [noteId] });
+	}
+
+	static async orphanResources(expiryDelay = null) {
+		if (expiryDelay === null) expiryDelay = 1000 * 60 * 60 * 24;
+		const cutOffTime = Date.now() - expiryDelay;
+		const output = await this.modelSelectAll('SELECT DISTINCT resource_id FROM note_resources WHERE is_associated = 0 AND last_seen_time < ?', [cutOffTime]);
+		return output.map(r => r.resource_id);
 	}
 
 }

ReactNativeClient/lib/services/BaseService.js

@@ -0,0 +1,12 @@
+class BaseService {
+
+	logger() {
+		if (!BaseService.logger_) throw new Error('BaseService.logger_ not set!!');
+		return BaseService.logger_;
+	}
+
+}
+
+BaseService.logger_ = null;
+
+module.exports = BaseService;

ReactNativeClient/lib/services/ResourceService.js

@@ -1,52 +1,80 @@
 const ItemChange = require('lib/models/ItemChange');
 const NoteResource = require('lib/models/NoteResource');
 const Note = require('lib/models/Note');
+const Resource = require('lib/models/Resource');
 const BaseModel = require('lib/BaseModel');
+const BaseService = require('lib/services/BaseService');
 
-class ResourceService {
+class ResourceService extends BaseService {
 
 	async indexNoteResources() {
+		this.logger().info('ResourceService::indexNoteResources: Start');
+
 		let lastId = 0;
-		let lastCreatedTime = 0
+
+		const processedChangeIds = [];
+
+		await ItemChange.waitForAllSaved();
 
 		while (true) {
 			const changes = await ItemChange.modelSelectAll(`
-				SELECT id, item_id, type, created_time
+				SELECT id, item_id, type
 				FROM item_changes
 				WHERE item_type = ?
 				AND id > ?
-				AND created_time >= ?
-				ORDER BY id, created_time ASC
+				ORDER BY id ASC
 				LIMIT 10
-			`, [BaseModel.TYPE_NOTE, lastId, lastCreatedTime]);
+			`, [BaseModel.TYPE_NOTE, lastId]);
 
 			if (!changes.length) break;
 
 			const noteIds = changes.map(a => a.item_id);
-			const changesByNoteId = {};
-			for (let i = 0; i < changes.length; i++) {
-				changesByNoteId[changes[i].item_id] = changes[i];
-			}
-
 			const notes = await Note.modelSelectAll('SELECT id, title, body FROM notes WHERE id IN ("' + noteIds.join('","') + '")');
 
-			for (let i = 0; i < notes.length; i++) {
-				const note = notes[i];
-				const change = changesByNoteId[note.id];
+			const noteById = (noteId) => {
+				for (let i = 0; i < notes.length; i++) {
+					if (notes[i].id === noteId) return notes[i];
+				}
+				throw new Error('Invalid note ID: ' + noteId);
+			}
+
+			for (let i = 0; i < changes.length; i++) {
+				const change = changes[i];
 
 				if (change.type === ItemChange.TYPE_CREATE || change.type === ItemChange.TYPE_UPDATE) {
+					const note = noteById(change.item_id);
 					const resourceIds = Note.linkedResourceIds(note.body);
-					await NoteResource.associate(note.id, resourceIds);
+					await NoteResource.setAssociatedResources(note.id, resourceIds);
 				} else if (change.type === ItemChange.TYPE_DELETE) {
-					await NoteResource.remove(note.id);
+					await NoteResource.remove(change.item_id);
 				} else {
 					throw new Error('Invalid change type: ' + change.type);
 				}
 
 				lastId = change.id;
-				lastCreatedTime = change.created_time;
+
+				processedChangeIds.push(change.id);
 			}
 		}
+
+		if (lastId) {
+			await ItemChange.db().exec('DELETE FROM item_changes WHERE id <= ?', [lastId]);
+		}
+
+		this.logger().info('ResourceService::indexNoteResources: Completed');
+	}
+
+	async deleteOrphanResources(expiryDelay = null) {
+		const resourceIds = await NoteResource.orphanResources(expiryDelay);
+		this.logger().info('ResourceService::deleteOrphanResources:', resourceIds);
+		for (let i = 0; i < resourceIds.length; i++) {
+			await Resource.delete(resourceIds[i]);
+		}
+	}
+
+	async maintenance() {
+		await this.indexNoteResources();
+		await this.deleteOrphanResources();
 	}
 
 }

ReactNativeClient/root.js

@@ -22,6 +22,8 @@ const NoteTag = require('lib/models/NoteTag.js');
 const BaseItem = require('lib/models/BaseItem.js');
 const MasterKey = require('lib/models/MasterKey.js');
 const BaseModel = require('lib/BaseModel.js');
+const BaseService = require('lib/services/BaseService.js');
+const ResourceService = require('lib/services/ResourceService');
 const { JoplinDatabase } = require('lib/joplin-database.js');
 const { Database } = require('lib/database.js');
 const { NotesScreen } = require('lib/components/screens/notes.js');
@@ -318,6 +320,8 @@ async function initialize(dispatch) {
 	reg.setLogger(mainLogger);
 	reg.setShowErrorMessageBoxHandler((message) => { alert(message) });
 
+	BaseService.logger_ = mainLogger;
+
 	reg.logger().info('====================================');
 	reg.logger().info('Starting application ' + Setting.value('appId') + ' (' + Setting.value('env') + ')');
 
@@ -450,6 +454,12 @@ async function initialize(dispatch) {
 		AlarmService.garbageCollect();
 	}, 1000 * 60 * 60);
 
+	const resourceService = new ResourceService();
+	resourceService.maintenance();
+	PoorManIntervals.setInterval(() => {
+		resourceService.maintenance();
+	}, 1000 * 60 * 60 * 4);
+
 	reg.scheduleSync().then(() => {
 		// Wait for the first sync before updating the notifications, since synchronisation
 		// might change the notifications.