self-hosted/joplin
1
0
Fork 0
mirror of https://github.com/laurent22/joplin.git synced 2025-10-31 00:07:48 +02:00
Code Issues Releases Activity

All: Fixes #932: Certain attachments were not being automatically deleted

Browse Source
This commit is contained in:
Laurent Cozic
2021-10-05 17:47:38 +01:00
parent f47478f730
commit 5a9bc1297b
4 changed files with 84 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
packages/lib/Synchronizer.ts
View File

@@ -405,9 +405,12 @@ export default class Synchronizer {
return `${Dirnames.Resources}/${resourceId}`;
};
// We index resources and apply the "is_shared" flag before syncing
// because it's going to affect what's sent encrypted, and what's sent
// plain text.
// We index resources before sync mostly to flag any potential orphan
// resource before it is being synced. That way, it can potentially be
// auto-deleted at a later time. Indexing resources is fast so it's fine
// to call it every time here.
//
// https://github.com/laurent22/joplin/issues/932#issuecomment-933736405
try {
if (this.resourceService()) {
logger.info('Indexing resources...');

5
packages/lib/models/BaseItem.ts
View File

@@ -603,6 +603,11 @@ export default class BaseItem extends BaseModel {
throw new Error('Unreachable');
}
public static async itemHasBeenSynced(itemId: string): Promise<boolean> {
const r = await this.db().selectOne('SELECT item_id FROM sync_items WHERE item_id = ?', [itemId]);
return !!r;
}
public static async itemsThatNeedSync(syncTarget: number, limit = 100): Promise<ItemsThatNeedSyncResult> {
// Although we keep the master keys in the database, we no longer sync them
const classNames = this.syncItemClassNames().filter(n => n !== 'MasterKey');

18
packages/lib/models/NoteResource.ts
View File

@@ -1,5 +1,6 @@
import BaseModel from '../BaseModel';
import { SqlQuery } from '../database';
import BaseItem from './BaseItem';
// - If is_associated = 1, note_resources indicates which note_id is currently associated with the given resource_id
// - If is_associated = 0, note_resources indicates which note_id *was* associated with the given resource_id
@@ -104,7 +105,22 @@ export default class NoteResource extends BaseModel {
const queries = [];
for (let i = 0; i < missingResources.length; i++) {
const id = missingResources[i].id;
queries.push({ sql: 'INSERT INTO note_resources (note_id, resource_id, is_associated, last_seen_time) VALUES (?, ?, ?, ?)', params: ['', id, 0, 0] });
// If the resource is not associated with any note, and has never
// been synced, it means it's a local resource that was removed from
// a note (or the note was deleted). In which case, we set a
// "last_seen_time", so that it can be considered an orphan reosurce
// that can be auto-deleted.
//
// https://github.com/laurent22/joplin/issues/932#issuecomment-933736405
const hasBeenSynced = await BaseItem.itemHasBeenSynced(id);
const lastSeenTime = hasBeenSynced ? 0 : Date.now();
queries.push({
sql: 'INSERT INTO note_resources (note_id, resource_id, is_associated, last_seen_time) VALUES (?, ?, ?, ?)',
params: ['', id, 0, lastSeenTime] }
);
}
await this.db().transactionExecBatch(queries);
}

63
packages/lib/services/ResourceService.test.ts
View File

@@ -73,15 +73,17 @@ describe('services/ResourceService', function() {
expect(!!(await Resource.load(resource1.id))).toBe(true);
}));
it('should not delete a resource that has never been associated with any note, because it probably means the resource came via sync, and associated note has not arrived yet', (async () => {
const service = new ResourceService();
await shim.createResourceFromPath(`${supportDir}/photo.jpg`);
// This is now handled below by more correct tests
//
// it('should not delete a resource that has never been associated with any note, because it probably means the resource came via sync, and associated note has not arrived yet', (async () => {
// const service = new ResourceService();
// await shim.createResourceFromPath(`${supportDir}/photo.jpg`);
await service.indexNoteResources();
await service.deleteOrphanResources(0);
// await service.indexNoteResources();
// await service.deleteOrphanResources(0);
expect((await Resource.all()).length).toBe(1);
}));
// expect((await Resource.all()).length).toBe(1);
// }));
it('should not delete resource if it is used in an IMG tag', (async () => {
const service = new ResourceService();
@@ -187,6 +189,53 @@ describe('services/ResourceService', function() {
expect(!!nr.is_associated).toBe(true); // And it should have fixed the situation by re-indexing the note content
}));
it('should delete a resource if it is not associated with any note and has never been synced', (async () => {
// - User creates a note and attaches a resource
// - User deletes the note
// - NoteResource service runs - the resource can be deleted
//
// This is because, since the resource didn't come via sync, it's
// guaranteed that it is orphaned, which means it can be safely deleted.
// See related test below to handle case where the resource actually
// comes via sync.
const note = await Note.save({});
await shim.attachFileToNote(note, `${supportDir}/photo.jpg`);
await Note.delete(note.id);
const resource = (await Resource.all())[0];
await resourceService().indexNoteResources();
await resourceService().deleteOrphanResources(-10);
expect(await Resource.load(resource.id)).toBeFalsy();
}));
it('should NOT delete a resource if it arrived via synced, even if it is not associated with any note', (async () => {
// - C1 creates Resource 1
// - C1 sync
// - C2 sync
// - NoteResource service runs - should find an orphan resource, but not
// delete it because the associated note might come later from U1.
//
// At this point, C1 has the knowledge about Resource 1 so whether it's
// eventually deleted or not will come from there. If it's an orphaned
// resource, it will be deleted on C1 first, then the deletion will be
// synced to other clients.
const note = await Note.save({});
await shim.attachFileToNote(note, `${supportDir}/photo.jpg`);
await Note.delete(note.id);
const resource = (await Resource.all())[0];
await synchronizer().start();
await switchClient(2);
await synchronizer().start();
await resourceService().indexNoteResources();
await resourceService().deleteOrphanResources(0);
expect(await Resource.load(resource.id)).toBeTruthy();
}));
// it('should auto-delete resource even if the associated note was deleted immediately', (async () => {
// // Previoulsy, when a resource was be attached to a note, then the
// // note was immediately deleted, the ResourceService would not have