self-hosted/joplin
1
0
Fork 0
mirror of https://github.com/laurent22/joplin.git synced 2025-11-23 22:36:32 +02:00
Code Issues Releases Activity

Server: Fixes #13686: Fix items can be incorrectly unshared on conflicting update (#13691)

Browse Source
This commit is contained in:
Henry Heino
2025-11-15 00:58:16 -08:00
committed by GitHub
parent 86a7771d5b
commit 4a0d9220ba
2 changed files with 128 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
packages/server/src/models/ShareModel.ts
View File

@@ -227,12 +227,18 @@ export default class ShareModel extends BaseModel<Share> {
perfTimer.pop();
};
const handleUpdated = async (change: Change, item: Item, share: Share) => {
const previousItem = this.models().change().unserializePreviousItem(change.previous_item);
const previousShareId = previousItem.jop_share_id;
const getPreviousShareId = (change: Change) => {
return this.models().change().unserializePreviousItem(change.previous_item)?.jop_share_id;
};
const handleUpdated = async (change: Change, item: Item, share: Share, nextShareId: Uuid) => {
const previousShareId = getPreviousShareId(change);
const shareId = share ? share.id : '';
if (previousShareId === shareId) return;
const changesShareId = previousShareId !== nextShareId;
if (previousShareId === shareId || !changesShareId) {
return;
}
perfTimer.push('handleUpdated');
@@ -342,6 +348,18 @@ export default class ShareModel extends BaseModel<Share> {
await this.withTransaction(async () => {
perfTimer.push(`Processing ${changes.length} changes`);
const itemToUpdates = new Map<Uuid, Change[]>();
for (const change of changes) {
if (change.type === ChangeType.Update) {
const updates = itemToUpdates.get(change.item_id);
if (updates) {
updates.push(change);
} else {
itemToUpdates.set(change.item_id, [change]);
}
}
}
for (const change of changes) {
const item = items.find(i => i.id === change.item_id);
@@ -355,7 +373,18 @@ export default class ShareModel extends BaseModel<Share> {
}
if (change.type === ChangeType.Update) {
await handleUpdated(change, item, itemShare);
const allUpdates = itemToUpdates.get(item.id);
const changeIndex = allUpdates.indexOf(change);
const nextChange = allUpdates[changeIndex + 1];
let nextShareId;
if (nextChange) {
nextShareId = getPreviousShareId(nextChange);
} else {
nextShareId = item.jop_share_id;
}
await handleUpdated(change, item, itemShare, nextShareId);
}
}

150
packages/server/src/routes/api/shares.folder.test.ts
View File

@@ -1,4 +1,4 @@
import { ChangeType, Share, ShareType, ShareUser, ShareUserStatus } from '../../services/database/types';
import { ChangeType, Session, Share, ShareType, ShareUser, ShareUserStatus } from '../../services/database/types';
import { beforeAllDb, afterAllTests, beforeEachDb, createUserAndSession, models, createNote, createFolder, updateItem, createItemTree, makeNoteSerializedBody, updateNote, expectHttpError, createResource, expectNotThrow } from '../../utils/testing/testUtils';
import { postApi, patchApi, getApi, deleteApi } from '../../utils/testing/apiUtils';
import { PaginatedDeltaChanges } from '../../models/ChangeModel';
@@ -9,6 +9,67 @@ import { resourceBlobPath, serializeJoplinItem, unserializeJoplinItem } from '..
import { PaginatedItems } from '../../models/ItemModel';
import { NoteEntity } from '@joplin/lib/services/database/types';
const createSingleFolderShare = async (session1: Session, session2: Session) => {
const folderItem = await createFolder(session1.id, { title: 'created by sharer' });
// ----------------------------------------------------------------
// Create the file share object
// ----------------------------------------------------------------
const share = await postApi<Share>(session1.id, 'shares', {
type: ShareType.Folder,
folder_id: folderItem.jop_id,
});
// ----------------------------------------------------------------
// Once the share object has been created, the client can add folders
// and notes to it. This is done by setting the share_id property,
// which we simulate here.
// ----------------------------------------------------------------
await models().item().saveForUser(session1.user_id, {
id: folderItem.id,
jop_parent_id: '',
jop_share_id: share.id,
});
// ----------------------------------------------------------------
// Send the share to a user
// ----------------------------------------------------------------
const user2 = await models().user().load(session2.user_id);
let shareUser = await postApi(session1.id, `shares/${share.id}/users`, {
email: user2.email,
}) as ShareUser;
shareUser = await models().shareUser().load(shareUser.id);
expect(shareUser.share_id).toBe(share.id);
expect(shareUser.user_id).toBe(user2.id);
expect(shareUser.status).toBe(ShareUserStatus.Waiting);
// ----------------------------------------------------------------
// On the sharee side, accept the share
// ----------------------------------------------------------------
await patchApi<ShareUser>(session2.id, `share_users/${shareUser.id}`, { status: ShareUserStatus.Accepted });
{
shareUser = await models().shareUser().load(shareUser.id);
expect(shareUser.status).toBe(ShareUserStatus.Accepted);
}
await models().share().updateSharedItems3();
// ----------------------------------------------------------------
// On the sharee side, check that the file is present
// and with the right content.
// ----------------------------------------------------------------
const results = await getApi<PaginatedItems>(session2.id, 'items/root/children');
expect(results.items.length).toBe(1);
expect(results.items[0].name).toBe(folderItem.name);
const itemContent = await getApi<Buffer>(session2.id, `items/root:/${folderItem.name}:/content`);
expect(itemContent.toString().includes('created by sharer')).toBe(true);
return { folderItem, itemContent, share };
};
describe('shares.folder', () => {
beforeAll(async () => {
@@ -24,62 +85,15 @@ describe('shares.folder', () => {
});
test('should share a folder with another user', async () => {
const { user: user1, session: session1 } = await createUserAndSession(1);
const { user: user2, session: session2 } = await createUserAndSession(2);
const folderItem = await createFolder(session1.id, { title: 'created by sharer' });
const { session: session1 } = await createUserAndSession(1);
const { session: session2 } = await createUserAndSession(2);
await createSingleFolderShare(session1, session2);
});
// ----------------------------------------------------------------
// Create the file share object
// ----------------------------------------------------------------
const share = await postApi<Share>(session1.id, 'shares', {
type: ShareType.Folder,
folder_id: folderItem.jop_id,
});
// ----------------------------------------------------------------
// Once the share object has been created, the client can add folders
// and notes to it. This is done by setting the share_id property,
// which we simulate here.
// ----------------------------------------------------------------
await models().item().saveForUser(user1.id, {
id: folderItem.id,
jop_share_id: share.id,
});
// ----------------------------------------------------------------
// Send the share to a user
// ----------------------------------------------------------------
let shareUser = await postApi(session1.id, `shares/${share.id}/users`, {
email: user2.email,
}) as ShareUser;
shareUser = await models().shareUser().load(shareUser.id);
expect(shareUser.share_id).toBe(share.id);
expect(shareUser.user_id).toBe(user2.id);
expect(shareUser.status).toBe(ShareUserStatus.Waiting);
// ----------------------------------------------------------------
// On the sharee side, accept the share
// ----------------------------------------------------------------
await patchApi<ShareUser>(session2.id, `share_users/${shareUser.id}`, { status: ShareUserStatus.Accepted });
{
shareUser = await models().shareUser().load(shareUser.id);
expect(shareUser.status).toBe(ShareUserStatus.Accepted);
}
await models().share().updateSharedItems3();
// ----------------------------------------------------------------
// On the sharee side, check that the file is present
// and with the right content.
// ----------------------------------------------------------------
const results = await getApi<PaginatedItems>(session2.id, 'items/root/children');
expect(results.items.length).toBe(1);
expect(results.items[0].name).toBe(folderItem.name);
const itemContent = await getApi<Buffer>(session2.id, `items/root:/${folderItem.name}:/content`);
expect(itemContent.toString().includes('created by sharer')).toBe(true);
test('sharer should see change if item is changed by sharee', async () => {
const { session: session1 } = await createUserAndSession(1);
const { session: session2 } = await createUserAndSession(2);
const { folderItem, itemContent } = await createSingleFolderShare(session1, session2);
// ----------------------------------------------------------------
// If file is changed by sharee, sharer should see the change too
@@ -97,6 +111,30 @@ describe('shares.folder', () => {
}
});
test('should not delete an item on unshare', async () => {
const { session: session1 } = await createUserAndSession(1);
const { session: session2 } = await createUserAndSession(2);
const { folderItem, share } = await createSingleFolderShare(session1, session2);
await models().item().saveForUser(session2.user_id, {
id: folderItem.id,
jop_parent_id: 'changed',
jop_share_id: share.id,
});
await models().item().saveForUser(session1.user_id, {
id: folderItem.id,
jop_parent_id: 'changed',
jop_share_id: '',
});
await models().share().updateSharedItems3();
const user1HasAccess = !!await models().userItem().byUserAndItemId(session1.user_id, folderItem.id);
const user2HasAccess = !!await models().userItem().byUserAndItemId(session2.user_id, folderItem.id);
expect({ user1HasAccess, user2HasAccess }).toEqual({ user1HasAccess: true, user2HasAccess: false });
});
test('should share a folder and all its children', async () => {
const { session: session1 } = await createUserAndSession(1);
const { session: session2 } = await createUserAndSession(2);