Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Desktop: Fixes #11457: Fix crash on startup if old read-only items are in the trash #11458

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion packages/lib/services/trash/permanentlyDeleteOldItems.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Day, msleep } from '@joplin/utils/time';
import Folder from '../../models/Folder';
import Note from '../../models/Note';
import { setupDatabaseAndSynchronizer, switchClient } from '../../testing/test-utils';
import { setupDatabaseAndSynchronizer, simulateReadOnlyShareEnv, switchClient } from '../../testing/test-utils';
import permanentlyDeleteOldItems from './permanentlyDeleteOldItems';
import Setting from '../../models/Setting';

Expand Down Expand Up @@ -75,4 +75,42 @@ describe('permanentlyDeleteOldItems', () => {
expect(await Folder.count()).toBe(1);
});

it('should not auto-delete read-only items', async () => {
const shareId = 'testShare';

// Simulates a folder having been deleted a long time ago
const longTimeAgo = 1000;

const readOnlyFolder = await Folder.save({
title: 'Read-only folder',
share_id: shareId,
deleted_time: longTimeAgo,
});
const readOnlyNote1 = await Note.save({
title: 'Read-only note',
parent_id: readOnlyFolder.id,
share_id: shareId,
deleted_time: longTimeAgo,
});
const readOnlyNote2 = await Note.save({
title: 'Read-only note 2',
share_id: shareId,
deleted_time: longTimeAgo,
});
const writableNote = await Note.save({
title: 'Editable note',
deleted_time: longTimeAgo,
});

const cleanup = simulateReadOnlyShareEnv(shareId);
await permanentlyDeleteOldItems(Day);

// Should preserve only the read-only items.
expect(await Folder.load(readOnlyFolder.id)).toBeTruthy();
expect(await Note.load(readOnlyNote1.id)).toBeTruthy();
expect(await Note.load(readOnlyNote2.id)).toBeTruthy();
expect(await Note.load(writableNote.id)).toBeFalsy();

cleanup();
});
});
43 changes: 39 additions & 4 deletions packages/lib/services/trash/permanentlyDeleteOldItems.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,44 @@ import Setting from '../../models/Setting';
import Note from '../../models/Note';
import { Day, Hour } from '@joplin/utils/time';
import shim from '../../shim';
import { itemIsReadOnlySync } from '../../models/utils/readOnly';
import BaseItem from '../../models/BaseItem';
import { ModelType } from '../../BaseModel';
import ItemChange from '../../models/ItemChange';

const logger = Logger.create('permanentlyDeleteOldData');

const readOnlyItemsRemoved = async (itemIds: string[], itemType: ModelType) => {
const result = [];
for (const id of itemIds) {
const item = await BaseItem.loadItem(itemType, id);

// Only do the share-related read-only checks. If other checks are done,
// readOnly will always be true because the item is in the trash.
const shareChecksOnly = true;
const readOnly = itemIsReadOnlySync(
itemType,
ItemChange.SOURCE_UNSPECIFIED,
item,
Setting.value('sync.userId'),
BaseItem.syncShareCache,
shareChecksOnly,
);
if (!readOnly) {
result.push(id);
}
}
return result;
};

const itemsToDelete = async (ttl: number|null = null) => {
const result = await Folder.trashItemsOlderThan(ttl);
const folderIds = await readOnlyItemsRemoved(result.folderIds, ModelType.Folder);
const noteIds = await readOnlyItemsRemoved(result.noteIds, ModelType.Note);

return { folderIds, noteIds };
};

const permanentlyDeleteOldItems = async (ttl: number = null) => {
ttl = ttl === null ? Setting.value('trash.ttlDays') * Day : ttl;

Expand All @@ -17,13 +52,13 @@ const permanentlyDeleteOldItems = async (ttl: number = null) => {
return;
}

const result = await Folder.trashItemsOlderThan(ttl);
logger.info('Items to permanently delete:', result);
const toDelete = await itemsToDelete(ttl);
logger.info('Items to permanently delete:', toDelete);

await Note.batchDelete(result.noteIds, { sourceDescription: 'permanentlyDeleteOldItems' });
await Note.batchDelete(toDelete.noteIds, { sourceDescription: 'permanentlyDeleteOldItems' });

// We only auto-delete folders if they are empty.
for (const folderId of result.folderIds) {
for (const folderId of toDelete.folderIds) {
const noteIds = await Folder.noteIds(folderId, { includeDeleted: true });
if (!noteIds.length) {
logger.info(`Deleting empty folder: ${folderId}`);
Expand Down
Loading