self-hosted/joplin
1
0
Fork 0
mirror of https://github.com/laurent22/joplin.git synced 2024-12-24 10:27:10 +02:00
Code Issues Releases Activity

All: Fixes #5796: Handle duplicate attachments when the parent notebook is shared

Browse Source
This commit is contained in:
Laurent Cozic 2021-11-28 16:46:10 +00:00
parent 0689db48de
commit 605f12552e
4 changed files with 241 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
packages/lib/Synchronizer.ts
View File

@ -429,7 +429,7 @@ export default class Synchronizer {
// Before synchronising make sure all share_id properties are set
// correctly so as to share/unshare the right items.
await Folder.updateAllShareIds();
await Folder.updateAllShareIds(this.resourceService());
const itemUploader = new ItemUploader(this.api(), this.apiCall);

136
packages/lib/models/Folder.sharing.test.ts
View File

@ -1,4 +1,4 @@
import { setupDatabaseAndSynchronizer, switchClient, createFolderTree, supportDir, msleep } from '../testing/test-utils';
import { setupDatabaseAndSynchronizer, switchClient, createFolderTree, supportDir, msleep, resourceService } from '../testing/test-utils';
import Folder from '../models/Folder';
import { allNotesFolders } from '../testing/test-utils-synchronizer';
import Note from '../models/Note';
@ -41,7 +41,7 @@ describe('models/Folder.sharing', function() {
]);
await Folder.save({ id: folder.id, share_id: 'abcd1234' });
await Folder.updateAllShareIds();
await Folder.updateAllShareIds(resourceService());
const allItems = await allNotesFolders();
for (const item of allItems) {
@ -87,7 +87,7 @@ describe('models/Folder.sharing', function() {
await Folder.save({ id: folder1.id, share_id: 'abcd1234' });
await Folder.updateAllShareIds();
await Folder.updateAllShareIds(resourceService());
folder1 = await Folder.loadByTitle('folder 1');
const folder2 = await Folder.loadByTitle('folder 2');
@ -121,7 +121,7 @@ describe('models/Folder.sharing', function() {
await Folder.save({ id: folder1.id, share_id: 'abcd1234' });
await Folder.updateAllShareIds();
await Folder.updateAllShareIds(resourceService());
folder1 = await Folder.loadByTitle('folder 1');
let folder2 = await Folder.loadByTitle('folder 2');
@ -133,7 +133,7 @@ describe('models/Folder.sharing', function() {
// Move the folder outside the shared folder
await Folder.save({ id: folder2.id, parent_id: folder3.id });
await Folder.updateAllShareIds();
await Folder.updateAllShareIds(resourceService());
folder2 = await Folder.loadByTitle('folder 2');
expect(folder2.share_id).toBe('');
@ -141,7 +141,7 @@ describe('models/Folder.sharing', function() {
{
await Folder.save({ id: folder2.id, parent_id: folder1.id });
await Folder.updateAllShareIds();
await Folder.updateAllShareIds(resourceService());
folder2 = await Folder.loadByTitle('folder 2');
expect(folder2.share_id).toBe('abcd1234');
}
@ -180,7 +180,7 @@ describe('models/Folder.sharing', function() {
await Folder.save({ id: folder1.id, share_id: 'abcd1234' });
await Folder.updateAllShareIds();
await Folder.updateAllShareIds(resourceService());
const note1: NoteEntity = await Note.loadByTitle('note 1');
const note2: NoteEntity = await Note.loadByTitle('note 2');
@ -210,7 +210,7 @@ describe('models/Folder.sharing', function() {
]);
await Folder.save({ id: folder1.id, share_id: 'abcd1234' });
await Folder.updateAllShareIds();
await Folder.updateAllShareIds(resourceService());
const note1: NoteEntity = await Note.loadByTitle('note 1');
const folder2: FolderEntity = await Folder.loadByTitle('folder 2');
expect(note1.share_id).toBe('abcd1234');
@ -218,7 +218,7 @@ describe('models/Folder.sharing', function() {
// Move the note outside of the shared folder
await Note.save({ id: note1.id, parent_id: folder2.id });
await Folder.updateAllShareIds();
await Folder.updateAllShareIds(resourceService());
{
const note1: NoteEntity = await Note.loadByTitle('note 1');
@ -228,7 +228,7 @@ describe('models/Folder.sharing', function() {
// Move the note back inside the shared folder
await Note.save({ id: note1.id, parent_id: folder1.id });
await Folder.updateAllShareIds();
await Folder.updateAllShareIds(resourceService());
{
const note1: NoteEntity = await Note.loadByTitle('note 1');
@ -256,7 +256,7 @@ describe('models/Folder.sharing', function() {
]);
await Folder.save({ id: folder1.id, share_id: 'abcd1234' });
await Folder.updateAllShareIds();
await Folder.updateAllShareIds(resourceService());
let note1: NoteEntity = await Note.loadByTitle('note 1');
let note2: NoteEntity = await Note.loadByTitle('note 2');
@ -266,7 +266,7 @@ describe('models/Folder.sharing', function() {
expect(note2.share_id).toBe('abcd1234');
await Note.save({ id: note1.id, parent_id: folder2.id });
await Folder.updateAllShareIds();
await Folder.updateAllShareIds(resourceService());
note1 = await Note.loadByTitle('note 1');
note2 = await Note.loadByTitle('note 2');
@ -296,7 +296,7 @@ describe('models/Folder.sharing', function() {
]);
await Folder.save({ id: folder.id, share_id: 'abcd1234' });
await Folder.updateAllShareIds();
await Folder.updateAllShareIds(resourceService);
const folder2: FolderEntity = await Folder.loadByTitle('folder 2');
const note1: NoteEntity = await Note.loadByTitle('note 1');
@ -312,9 +312,7 @@ describe('models/Folder.sharing', function() {
expect(resource.share_id).toBe('');
}
await Folder.updateAllShareIds();
// await NoteResource.updateResourceShareIds();
await Folder.updateAllShareIds(resourceService);
{
const resource: ResourceEntity = await Resource.load(resourceId);
@ -324,7 +322,7 @@ describe('models/Folder.sharing', function() {
await Note.save({ id: note1.id, parent_id: folder2.id });
await resourceService.indexNoteResources();
await Folder.updateAllShareIds();
await Folder.updateAllShareIds(resourceService);
{
const resource: ResourceEntity = await Resource.load(resourceId);
@ -332,6 +330,108 @@ describe('models/Folder.sharing', function() {
}
});
it('should automatically duplicate resources when they are shared', async () => {
const resourceService = new ResourceService();
const folder1 = await createFolderTree('', [
{
title: 'folder 1', // SHARE 1
children: [
{
title: 'note 1',
},
{
title: 'note 2',
},
],
},
{
title: 'folder 2', // SHARE 2
children: [
{
title: 'note 3',
},
],
},
{
title: 'folder 3', // (not shared)
children: [
{
title: 'note 4',
},
],
},
]);
const folder2: FolderEntity = await Folder.loadByTitle('folder 2');
// await Folder.loadByTitle('folder 3');
let note1: NoteEntity = await Note.loadByTitle('note 1');
let note2: NoteEntity = await Note.loadByTitle('note 2');
let note3: NoteEntity = await Note.loadByTitle('note 3');
let note4: NoteEntity = await Note.loadByTitle('note 4');
await Folder.save({ id: folder1.id, share_id: 'share1' });
await Folder.save({ id: folder2.id, share_id: 'share2' });
note1 = await shim.attachFileToNote(note1, testImagePath);
note2 = await shim.attachFileToNote(note2, testImagePath);
note3 = await Note.save({ id: note3.id, body: note1.body });
note4 = await Note.save({ id: note4.id, body: note1.body });
const userUpdatedTimes: Record<string, number> = {
[note1.id]: note1.user_updated_time,
[note2.id]: note2.user_updated_time,
[note3.id]: note3.user_updated_time,
[note4.id]: note4.user_updated_time,
};
await msleep(1);
// We need to index the resources to populate the note_resources table
await resourceService.indexNoteResources();
await Folder.updateAllShareIds(resourceService);
// BEFORE:
//
// - Note 1 has resource 1 (share1)
// - Note 2 has resource 2 (share1)
// - Note 3 has resource 1 (share2)
// - Note 4 has resource 1 (not shared)
// AFTER:
//
// - Note 1 has resource 1 (share1)
// - Note 2 has resource 2 (share1)
// - Note 3 has resource 3 (share2)
// - Note 4 has resource 4 (not shared)
const resources = await Resource.all();
expect(resources.length).toBe(4);
note1 = await Note.load(note1.id);
note2 = await Note.load(note2.id);
note3 = await Note.load(note3.id);
note4 = await Note.load(note4.id);
console.info(note1.body);
console.info(note2.body);
console.info(note3.body);
console.info(note4.body);
expect(note1.body).not.toBe(note2.body);
expect(note1.body).not.toBe(note3.body);
expect(note1.body).not.toBe(note4.body);
expect(note2.body).not.toBe(note3.body);
expect(note2.body).not.toBe(note4.body);
expect(note3.body).not.toBe(note4.body);
expect(note1.user_updated_time).toBe(userUpdatedTimes[note1.id]);
expect(note2.user_updated_time).toBe(userUpdatedTimes[note2.id]);
expect(note3.user_updated_time).toBe(userUpdatedTimes[note3.id]);
expect(note4.user_updated_time).toBe(userUpdatedTimes[note4.id]);
});
it('should unshare items that are no longer part of an existing share', async () => {
await createFolderTree('', [
{
@ -367,7 +467,7 @@ describe('models/Folder.sharing', function() {
await resourceService.indexNoteResources();
await Folder.updateAllShareIds();
await Folder.updateAllShareIds(resourceService);
await Folder.updateNoLongerSharedItems(['1']);

143
packages/lib/models/Folder.ts
View File

@ -1,4 +1,4 @@
import { FolderEntity, FolderIcon } from '../services/database/types';
import { FolderEntity, FolderIcon, NoteEntity } from '../services/database/types';
import BaseModel, { DeleteOptions } from '../BaseModel';
import time from '../time';
import { _ } from '../locale';
@ -9,6 +9,7 @@ import Resource from './Resource';
import { isRootSharedFolder } from '../services/share/reducer';
import Logger from '../Logger';
import syncDebugLog from '../services/synchronizer/syncDebugLog';
import ResourceService from '../services/ResourceService';
const { substrWithEllipsis } = require('../string-utils.js');
const logger = Logger.create('models/Folder');
@ -368,36 +369,130 @@ export default class Folder extends BaseItem {
}
}
public static async updateResourceShareIds() {
// Find all resources where share_id is different from parent note
// share_id. Then update share_id on all these resources. Essentially it
// makes it match the resource share_id to the note share_id. At the
// same time we also process the is_shared property.
const rows = await this.db().selectAll(`
SELECT r.id, n.share_id, n.is_shared
FROM note_resources nr
LEFT JOIN resources r ON nr.resource_id = r.id
LEFT JOIN notes n ON nr.note_id = n.id
WHERE n.share_id != r.share_id
OR n.is_shared != r.is_shared
`);
public static async updateResourceShareIds(resourceService: ResourceService) {
// Updating the share_id property of the resources is complex because:
//
// The resource association to the note is done indirectly via the
// ResourceService
//
// And a given resource can appear inside multiple notes. However, for
// sharing we make the assumption that a resource can be part of only
// one share (one-to-one relationship because "share_id" is part of the
// "resources" table), which is usually the case. By copying and pasting
// note content from one note to another it's however possible to have
// the same resource in multiple shares (or in a non-shared and a shared
// folder).
//
// So in this function we take this into account - if a shared resource
// is part of multiple notes, we duplicate that resource so that each
// note has its own instance. When such duplication happens, we need to
// resume the process from the start (thus the loop) so that we deal
// with the right note/resource associations.
logger.debug('updateResourceShareIds: resources to update:', rows.length);
for (let i = 0; i < 5; i++) {
// Find all resources where share_id is different from parent note
// share_id. Then update share_id on all these resources. Essentially it
// makes it match the resource share_id to the note share_id. At the
// same time we also process the is_shared property.
for (const row of rows) {
await Resource.save({
id: row.id,
share_id: row.share_id || '',
is_shared: row.is_shared,
updated_time: Date.now(),
}, { autoTimestamp: false });
const rows = await this.db().selectAll(`
SELECT r.id, n.share_id, n.is_shared
FROM note_resources nr
LEFT JOIN resources r ON nr.resource_id = r.id
LEFT JOIN notes n ON nr.note_id = n.id
WHERE (
n.share_id != r.share_id
OR n.is_shared != r.is_shared
) AND nr.is_associated = 1
`);
if (!rows.length) return;
logger.debug('updateResourceShareIds: resources to update:', rows.length);
const resourceIds = rows.map(r => r.id);
interface Row {
resource_id: string;
note_id: string;
share_id: string;
}
// Now we check, for each resource, that it is associated with only
// one note. If it is not, we create duplicate resources so that
// each note has its own separate resource.
const noteResourceAssociations = await this.db().selectAll(`
SELECT resource_id, note_id, notes.share_id
FROM note_resources
LEFT JOIN notes ON notes.id = note_resources.note_id
WHERE resource_id IN ('${resourceIds.join('\',\'')}')
AND is_associated = 1
`) as Row[];
const resourceIdToNotes: Record<string, Row[]> = {};
for (const r of noteResourceAssociations) {
if (!resourceIdToNotes[r.resource_id]) resourceIdToNotes[r.resource_id] = [];
resourceIdToNotes[r.resource_id].push(r);
}
let hasCreatedResources = false;
for (const [resourceId, rows] of Object.entries(resourceIdToNotes)) {
if (rows.length <= 1) continue;
for (let i = 0; i < rows.length - 1; i++) {
const row = rows[i];
const note: NoteEntity = await Note.load(row.note_id);
if (!note) continue; // probably got deleted in the meantime?
const newResource = await Resource.duplicateResource(resourceId);
logger.info(`updateResourceShareIds: Automatically created resource "${newResource.id}" to replace resource "${resourceId}" because it is shared and duplicate across notes:`, row);
const regex = new RegExp(resourceId, 'gi');
const newBody = note.body.replace(regex, newResource.id);
await Note.save({
id: note.id,
body: newBody,
parent_id: note.parent_id,
updated_time: Date.now(),
}, {
autoTimestamp: false,
});
hasCreatedResources = true;
}
}
// If we have created resources, we refresh the note/resource
// associations using ResourceService and we resume the process.
// Normally, if the user didn't create any new notes or resources in
// the meantime, the second loop should find that each shared
// resource is associated with only one note.
if (hasCreatedResources) {
await resourceService.indexNoteResources();
continue;
} else {
// If all is good, we can set the share_id and is_shared
// property of the resource.
for (const row of rows) {
await Resource.save({
id: row.id,
share_id: row.share_id || '',
is_shared: row.is_shared,
updated_time: Date.now(),
}, { autoTimestamp: false });
}
return;
}
}
throw new Error('Failed to update resource share IDs');
}
public static async updateAllShareIds() {
public static async updateAllShareIds(resourceService: ResourceService) {
await this.updateFolderShareIds();
await this.updateNoteShareIds();
await this.updateResourceShareIds();
await this.updateResourceShareIds(resourceService);
}
// Clear the "share_id" property for the items that are associated with a

5
packages/lib/services/share/ShareService.ts
View File

@ -11,6 +11,7 @@ import EncryptionService from '../e2ee/EncryptionService';
import { PublicPrivateKeyPair, mkReencryptFromPasswordToPublicKey, mkReencryptFromPublicKeyToPassword } from '../e2ee/ppk';
import { MasterKeyEntity } from '../e2ee/types';
import { getMasterPassword } from '../e2ee/utils';
import ResourceService from '../ResourceService';
import { addMasterKey, getEncryptionEnabled, localSyncInfo } from '../synchronizer/syncInfoUtils';
import { ShareInvitation, State, stateRootKey, StateShare } from './reducer';
@ -122,7 +123,7 @@ export default class ShareService {
// Note: race condition if the share is created but the app crashes
// before setting share_id on the folder. See unshareFolder() for info.
await Folder.save({ id: folder.id, share_id: share.id });
await Folder.updateAllShareIds();
await Folder.updateAllShareIds(ResourceService.instance());
return share;
}
@ -167,7 +168,7 @@ export default class ShareService {
// It's ok if updateAllShareIds() doesn't run because it's executed on
// each sync too.
await Folder.updateAllShareIds();
await Folder.updateAllShareIds(ResourceService.instance());
}
// This is when a share recipient decides to leave the shared folder.