From b9955f58d3d3185d2b771cb3d1f53d4364706b83 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sun, 16 May 2021 12:46:58 +0200 Subject: [PATCH] Server: Refactor ShareType --- packages/server/src/db.ts | 5 ++--- packages/server/src/models/ItemModel.ts | 2 +- packages/server/src/models/ShareModel.test.ts | 12 ++++++------ packages/server/src/models/ShareModel.ts | 10 +++++----- packages/server/src/models/ShareUserModel.ts | 2 +- packages/server/src/routes/api/share_users.test.ts | 6 +++--- packages/server/src/routes/api/shares.folder.test.ts | 2 +- packages/server/src/routes/api/shares.test.ts | 6 +++--- packages/server/src/routes/api/shares.ts | 2 +- packages/server/src/routes/index/shares.link.test.ts | 10 +++++----- packages/server/src/utils/testing/shareApiUtils.ts | 12 ++++++------ 11 files changed, 34 insertions(+), 35 deletions(-) diff --git a/packages/server/src/db.ts b/packages/server/src/db.ts index dad694f68..185871e85 100644 --- a/packages/server/src/db.ts +++ b/packages/server/src/db.ts @@ -237,9 +237,8 @@ export function changeTypeToString(t: ChangeType): string { } export enum ShareType { - Link = 1, // When a note is shared via a public link - App = 2, // When a note is shared with another user on the same server instance - JoplinRootFolder = 3, + Note = 1, // When a note is shared via a public link + Folder = 3, // When a complete folder is shared with another Joplin Server user } export enum ShareUserStatus { diff --git a/packages/server/src/models/ItemModel.ts b/packages/server/src/models/ItemModel.ts index 0971d3b60..0bcd5e92a 100644 --- a/packages/server/src/models/ItemModel.ts +++ b/packages/server/src/models/ItemModel.ts @@ -416,7 +416,7 @@ export default class ItemModel extends BaseModel { const path = await this.joplinItemPath(jopId); if (!path.length) throw new ApiError(`Cannot retrieve path for item: ${jopId}`, null, 'noPathForItem'); const rootFolderItem = path[path.length - 1]; - const share = await this.models().share().itemShare(ShareType.JoplinRootFolder, rootFolderItem.id); + const share = await this.models().share().itemShare(ShareType.Folder, rootFolderItem.id); if (!share) return null; return { diff --git a/packages/server/src/models/ShareModel.test.ts b/packages/server/src/models/ShareModel.test.ts index 8f48a0e1c..b2360de9e 100644 --- a/packages/server/src/models/ShareModel.test.ts +++ b/packages/server/src/models/ShareModel.test.ts @@ -27,7 +27,7 @@ describe('ShareModel', function() { error = await checkThrowAsync(async () => await models().share().createShare(user.id, 20 as ShareType, item.id)); expect(error instanceof ErrorBadRequest).toBe(true); - error = await checkThrowAsync(async () => await models().share().createShare(user.id, ShareType.Link, 'doesntexist')); + error = await checkThrowAsync(async () => await models().share().createShare(user.id, ShareType.Note, 'doesntexist')); expect(error instanceof ErrorNotFound).toBe(true); }); @@ -49,14 +49,14 @@ describe('ShareModel', function() { }); const folderItem1 = await models().item().loadByJopId(user1.id, '000000000000000000000000000000F1'); - await shareWithUserAndAccept(session1.id, session3.id, user3, ShareType.JoplinRootFolder, folderItem1); + await shareWithUserAndAccept(session1.id, session3.id, user3, ShareType.Folder, folderItem1); const folderItem2 = await models().item().loadByJopId(user2.id, '000000000000000000000000000000F2'); - await shareWithUserAndAccept(session2.id, session1.id, user1, ShareType.JoplinRootFolder, folderItem2); + await shareWithUserAndAccept(session2.id, session1.id, user1, ShareType.Folder, folderItem2); - const shares1 = await models().share().byUserId(user1.id, ShareType.JoplinRootFolder); - const shares2 = await models().share().byUserId(user2.id, ShareType.JoplinRootFolder); - const shares3 = await models().share().byUserId(user3.id, ShareType.JoplinRootFolder); + const shares1 = await models().share().byUserId(user1.id, ShareType.Folder); + const shares2 = await models().share().byUserId(user2.id, ShareType.Folder); + const shares3 = await models().share().byUserId(user3.id, ShareType.Folder); expect(shares1.length).toBe(2); expect(shares1.find(s => s.folder_id === '000000000000000000000000000000F1')).toBeTruthy(); diff --git a/packages/server/src/models/ShareModel.ts b/packages/server/src/models/ShareModel.ts index d640e8b98..dc0c722d7 100644 --- a/packages/server/src/models/ShareModel.ts +++ b/packages/server/src/models/ShareModel.ts @@ -16,7 +16,7 @@ export default class ShareModel extends BaseModel { if (action === AclAction.Create) { if (!await this.models().item().userHasItem(user.id, resource.item_id)) throw new ErrorForbidden('cannot share an item not owned by the user'); - if (resource.type === ShareType.JoplinRootFolder) { + if (resource.type === ShareType.Folder) { const item = await this.models().item().loadByJopId(user.id, resource.folder_id); if (item.jop_parent_id) throw new ErrorForbidden('A shared notebook must be at the root'); } @@ -44,8 +44,8 @@ export default class ShareModel extends BaseModel { } protected async validate(share: Share, options: ValidateOptions = {}): Promise { - if ('type' in share && ![ShareType.Link, ShareType.App, ShareType.JoplinRootFolder].includes(share.type)) throw new ErrorBadRequest(`Invalid share type: ${share.type}`); - if (share.type !== ShareType.Link && await this.itemIsShared(share.type, share.item_id)) throw new ErrorBadRequest('A shared item cannot be shared again'); + if ('type' in share && ![ShareType.Note, ShareType.Folder].includes(share.type)) throw new ErrorBadRequest(`Invalid share type: ${share.type}`); + if (share.type !== ShareType.Note && await this.itemIsShared(share.type, share.item_id)) throw new ErrorBadRequest('A shared item cannot be shared again'); const item = await this.models().item().load(share.item_id); if (!item) throw new ErrorNotFound(`Could not find item: ${share.item_id}`); @@ -299,7 +299,7 @@ export default class ShareModel extends BaseModel { if (share) return share; const shareToSave = { - type: ShareType.JoplinRootFolder, + type: ShareType.Folder, item_id: folderItem.id, owner_id: owner.id, folder_id: folderId, @@ -317,7 +317,7 @@ export default class ShareModel extends BaseModel { if (existingShare) return existingShare; const shareToSave = { - type: ShareType.Link, + type: ShareType.Note, item_id: noteItem.id, owner_id: owner.id, note_id: noteId, diff --git a/packages/server/src/models/ShareUserModel.ts b/packages/server/src/models/ShareUserModel.ts index 87503a8c7..b73ce3b58 100644 --- a/packages/server/src/models/ShareUserModel.ts +++ b/packages/server/src/models/ShareUserModel.ts @@ -128,7 +128,7 @@ export default class ShareUserModel extends BaseModel { public async deleteByShare(share: Share): Promise { // Notes that are shared by link do not have associated ShareUser items, // so there's nothing to do. - if (share.type !== ShareType.JoplinRootFolder) return; + if (share.type !== ShareType.Folder) return; const shareUsers = await this.byShareId(share.id, null); diff --git a/packages/server/src/routes/api/share_users.test.ts b/packages/server/src/routes/api/share_users.test.ts index a2398edb6..e72bb8407 100644 --- a/packages/server/src/routes/api/share_users.test.ts +++ b/packages/server/src/routes/api/share_users.test.ts @@ -29,8 +29,8 @@ describe('share_users', function() { }); const folderItem1 = await models().item().loadByJopId(user1.id, '000000000000000000000000000000F1'); const folderItem2 = await models().item().loadByJopId(user1.id, '000000000000000000000000000000F2'); - const { share: share1 } = await shareWithUserAndAccept(session1.id, session2.id, user2, ShareType.JoplinRootFolder, folderItem1); - const { share: share2 } = await shareWithUserAndAccept(session1.id, session2.id, user2, ShareType.JoplinRootFolder, folderItem2); + const { share: share1 } = await shareWithUserAndAccept(session1.id, session2.id, user2, ShareType.Folder, folderItem1); + const { share: share2 } = await shareWithUserAndAccept(session1.id, session2.id, user2, ShareType.Folder, folderItem2); const shareUsers = await getApi(session2.id, 'share_users'); expect(shareUsers.items.length).toBe(2); @@ -44,7 +44,7 @@ describe('share_users', function() { await createItemTree(user1.id, '', { '000000000000000000000000000000F1': {} }); const folderItem = await models().item().loadByJopId(user1.id, '000000000000000000000000000000F1'); - const { shareUser } = await shareWithUserAndAccept(session1.id, session2.id, user2, ShareType.JoplinRootFolder, folderItem); + const { shareUser } = await shareWithUserAndAccept(session1.id, session2.id, user2, ShareType.Folder, folderItem); // User can modify own UserShare object await patchApi(session2.id, `share_users/${shareUser.id}`, { status: ShareUserStatus.Rejected }); diff --git a/packages/server/src/routes/api/shares.folder.test.ts b/packages/server/src/routes/api/shares.folder.test.ts index 8b2466718..c61efe90f 100644 --- a/packages/server/src/routes/api/shares.folder.test.ts +++ b/packages/server/src/routes/api/shares.folder.test.ts @@ -32,7 +32,7 @@ describe('shares.folder', function() { // Create the file share object // ---------------------------------------------------------------- const share = await postApi(session1.id, 'shares', { - type: ShareType.JoplinRootFolder, + type: ShareType.Folder, folder_id: folderItem.jop_id, }); diff --git a/packages/server/src/routes/api/shares.test.ts b/packages/server/src/routes/api/shares.test.ts index 8befd6a5b..7c75f9e9e 100644 --- a/packages/server/src/routes/api/shares.test.ts +++ b/packages/server/src/routes/api/shares.test.ts @@ -37,7 +37,7 @@ describe('shares', function() { await createItemTree(user1.id, '', tree); const folderItem = await itemModel1.loadByJopId(user1.id, '000000000000000000000000000000F1'); const noteItem2 = await itemModel1.loadByJopId(user1.id, '00000000000000000000000000000002'); - const { share } = await shareWithUserAndAccept(session1.id, session2.id, user2, ShareType.JoplinRootFolder, folderItem); + const { share } = await shareWithUserAndAccept(session1.id, session2.id, user2, ShareType.Folder, folderItem); // Only share with user 3, without accepting it await postApi(session1.id, `shares/${share.id}/users`, { @@ -54,11 +54,11 @@ describe('shares', function() { const share1: Share = shares.items.find(it => it.folder_id === '000000000000000000000000000000F1'); expect(share1).toBeTruthy(); - expect(share1.type).toBe(ShareType.JoplinRootFolder); + expect(share1.type).toBe(ShareType.Folder); const share2: Share = shares.items.find(it => it.note_id === '00000000000000000000000000000002'); expect(share2).toBeTruthy(); - expect(share2.type).toBe(ShareType.Link); + expect(share2.type).toBe(ShareType.Note); const shareUsers = await getApi(session1.id, `shares/${share1.id}/users`); expect(shareUsers.items.length).toBe(2); diff --git a/packages/server/src/routes/api/shares.ts b/packages/server/src/routes/api/shares.ts index 485c5e19d..4d0e4af89 100644 --- a/packages/server/src/routes/api/shares.ts +++ b/packages/server/src/routes/api/shares.ts @@ -92,7 +92,7 @@ router.get('api/shares/:id', async (path: SubPath, ctx: AppContext) => { const shareModel = ctx.models.share(); const share = await shareModel.load(path.id); - if (share && share.type === ShareType.Link) { + if (share && share.type === ShareType.Note) { // No authentication is necessary - anyone who knows the share ID is allowed // to access the file. It is essentially public. return shareModel.toApiOutput(share); diff --git a/packages/server/src/routes/index/shares.link.test.ts b/packages/server/src/routes/index/shares.link.test.ts index b089cdab2..4c7d85a7d 100644 --- a/packages/server/src/routes/index/shares.link.test.ts +++ b/packages/server/src/routes/index/shares.link.test.ts @@ -62,7 +62,7 @@ describe('shares.link', function() { }); const share = await postApi(session.id, 'shares', { - type: ShareType.Link, + type: ShareType.Note, note_id: noteItem.jop_id, }); @@ -82,7 +82,7 @@ describe('shares.link', function() { }); const share = await postApi(session.id, 'shares', { - type: ShareType.Link, + type: ShareType.Note, note_id: noteItem.jop_id, }); @@ -103,7 +103,7 @@ describe('shares.link', function() { await createItem(session.id, 'root:/.resource/96765a68655f4446b3dbad7d41b6566e:', await testImageBuffer()); const share = await postApi(session.id, 'shares', { - type: ShareType.Link, + type: ShareType.Note, note_id: noteItem.jop_id, }); @@ -137,7 +137,7 @@ describe('shares.link', function() { }); const share = await postApi(session.id, 'shares', { - type: ShareType.Link, + type: ShareType.Note, note_id: noteItem.jop_id, }); @@ -151,7 +151,7 @@ describe('shares.link', function() { }); const share = await postApi(session.id, 'shares', { - type: ShareType.Link, + type: ShareType.Note, note_id: noteItem.jop_id, }); diff --git a/packages/server/src/utils/testing/shareApiUtils.ts b/packages/server/src/utils/testing/shareApiUtils.ts index 38bbf3c65..c42eb6ec3 100644 --- a/packages/server/src/utils/testing/shareApiUtils.ts +++ b/packages/server/src/utils/testing/shareApiUtils.ts @@ -16,7 +16,7 @@ export async function createFolderShare(sessionId: string, folderId: string): Pr // const item = await createFolder(sessionId, { id: '00000000 }); return postApi(sessionId, 'shares', { - type: ShareType.JoplinRootFolder, + type: ShareType.Folder, folder_id: folderId, }); } @@ -75,7 +75,7 @@ export async function shareFolderWithUser(sharerSessionId: string, shareeSession }); const share: Share = await postApi(sharerSessionId, 'shares', { - type: ShareType.JoplinRootFolder, + type: ShareType.Folder, folder_id: rootFolderItem.jop_id, }); @@ -113,16 +113,16 @@ export async function shareFolderWithUser(sharerSessionId: string, shareeSession // - User 2 accepts the share // // The result is that user 2 will have a file linked to user 1's file. -export async function shareWithUserAndAccept(sharerSessionId: string, shareeSessionId: string, sharee: User, shareType: ShareType = ShareType.App, item: Item = null): Promise { +export async function shareWithUserAndAccept(sharerSessionId: string, shareeSessionId: string, sharee: User, shareType: ShareType = ShareType.Folder, item: Item = null): Promise { item = item || await createItem(sharerSessionId, 'root:/test.txt:', 'testing share'); let share: Share = null; - if ([ShareType.JoplinRootFolder, ShareType.Link].includes(shareType)) { + if ([ShareType.Folder, ShareType.Note].includes(shareType)) { share = await postApi(sharerSessionId, 'shares', { type: shareType, - note_id: shareType === ShareType.Link ? item.jop_id : undefined, - folder_id: shareType === ShareType.JoplinRootFolder ? item.jop_id : undefined, + note_id: shareType === ShareType.Note ? item.jop_id : undefined, + folder_id: shareType === ShareType.Folder ? item.jop_id : undefined, }); } else { const sharer = await models().session().sessionUser(sharerSessionId);