From e8bc8f2d631b3c96603e9deed25d5e4d50cf0fe5 Mon Sep 17 00:00:00 2001 From: fengmk2 Date: Tue, 1 Jul 2025 15:04:52 +0800 Subject: [PATCH] feat(server): add comment-attachment storage (#12911) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit close CLOUD-230 #### PR Dependency Tree * **PR #12911** 👈 * **PR #12761** * **PR #12924** * **PR #12925** This tree was auto-generated by [Charcoal](https://github.com/danerwilliams/charcoal) ## Summary by CodeRabbit * **New Features** * Added support for uploading and retrieving comment attachments in workspace documents via a new API endpoint. * Introduced a service for managing comment attachments, including storage, retrieval, deletion, and URL generation. * Implemented localized error messages and improved error handling for missing comment attachments. * **Bug Fixes** * Improved error feedback when comment attachments are not found. * **Tests** * Added comprehensive tests for comment attachment storage, retrieval, deletion, API endpoint behavior, and permission checks. * **Documentation** * Updated GraphQL schema and localization files to include new error types for comment attachments. --- .../e2e/workspace/controller.spec.ts | 118 +++++++++++++++ packages/backend/server/src/base/error/def.ts | 4 + .../server/src/base/error/errors.gen.ts | 9 +- .../__tests__/comment-attachment.spec.ts | 141 ++++++++++++++++++ .../backend/server/src/core/storage/index.ts | 12 +- .../storage/wrappers/comment-attachment.ts | 128 ++++++++++++++++ .../server/src/core/storage/wrappers/index.ts | 1 + .../server/src/core/workspaces/controller.ts | 41 ++++- packages/backend/server/src/schema.gql | 1 + packages/common/graphql/src/schema.ts | 1 + packages/frontend/i18n/src/i18n.gen.ts | 4 + packages/frontend/i18n/src/resources/en.json | 3 +- 12 files changed, 456 insertions(+), 7 deletions(-) create mode 100644 packages/backend/server/src/__tests__/e2e/workspace/controller.spec.ts create mode 100644 packages/backend/server/src/core/storage/__tests__/comment-attachment.spec.ts create mode 100644 packages/backend/server/src/core/storage/wrappers/comment-attachment.ts diff --git a/packages/backend/server/src/__tests__/e2e/workspace/controller.spec.ts b/packages/backend/server/src/__tests__/e2e/workspace/controller.spec.ts new file mode 100644 index 0000000000..ef7602a36c --- /dev/null +++ b/packages/backend/server/src/__tests__/e2e/workspace/controller.spec.ts @@ -0,0 +1,118 @@ +import { randomUUID } from 'node:crypto'; +import { mock } from 'node:test'; + +import { CommentAttachmentStorage } from '../../../core/storage'; +import { Mockers } from '../../mocks'; +import { app, e2e } from '../test'; + +async function createWorkspace() { + const owner = await app.create(Mockers.User); + const workspace = await app.create(Mockers.Workspace, { + owner, + }); + + return { + owner, + workspace, + }; +} + +e2e.afterEach.always(() => { + mock.reset(); +}); + +// #region comment attachment + +e2e( + 'should get comment attachment not found when key is not exists', + async t => { + const { owner, workspace } = await createWorkspace(); + await app.login(owner); + + const docId = randomUUID(); + + const res = await app.GET( + `/api/workspaces/${workspace.id}/docs/${docId}/comment-attachments/not-exists` + ); + + t.is(res.status, 404); + t.is(res.body.message, 'Comment attachment not found.'); + } +); + +e2e( + 'should get comment attachment no permission when user is not member', + async t => { + const { workspace } = await createWorkspace(); + // signup a new user + await app.signup(); + + const docId = randomUUID(); + + const res = await app.GET( + `/api/workspaces/${workspace.id}/docs/${docId}/comment-attachments/some-key` + ); + + t.is(res.status, 403); + t.regex( + res.body.message, + /You do not have permission to perform Doc.Read action on doc / + ); + } +); + +e2e('should get comment attachment body', async t => { + const { owner, workspace } = await createWorkspace(); + await app.login(owner); + + const docId = randomUUID(); + const key = randomUUID(); + const attachment = app.get(CommentAttachmentStorage); + await attachment.put( + workspace.id, + docId, + key, + 'test.txt', + Buffer.from('test') + ); + + const res = await app.GET( + `/api/workspaces/${workspace.id}/docs/${docId}/comment-attachments/${key}` + ); + + t.is(res.status, 200); + t.is(res.headers['content-type'], 'text/plain'); + t.is(res.headers['content-length'], '4'); + t.is(res.headers['cache-control'], 'private, max-age=2592000, immutable'); + t.regex( + res.headers['last-modified'], + /^\w{3}, \d{2} \w{3} \d{4} \d{2}:\d{2}:\d{2} GMT$/ + ); + t.is(res.text, 'test'); +}); + +e2e('should get comment attachment redirect url', async t => { + const { owner, workspace } = await createWorkspace(); + await app.login(owner); + + const docId = randomUUID(); + const key = randomUUID(); + const attachment = app.get(CommentAttachmentStorage); + + mock.method(attachment, 'get', async () => { + return { + body: null, + metadata: null, + redirectUrl: `https://foo.com/${key}`, + }; + }); + + const res = await app.GET( + `/api/workspaces/${workspace.id}/docs/${docId}/comment-attachments/${key}` + ); + + t.is(res.status, 302); + t.is(res.headers['location'], `https://foo.com/${key}`); +}); + +// #endregion diff --git a/packages/backend/server/src/base/error/def.ts b/packages/backend/server/src/base/error/def.ts index 4881467c3c..d31c856a15 100644 --- a/packages/backend/server/src/base/error/def.ts +++ b/packages/backend/server/src/base/error/def.ts @@ -917,4 +917,8 @@ export const USER_FRIENDLY_ERRORS = { type: 'resource_not_found', message: 'Reply not found.', }, + comment_attachment_not_found: { + type: 'resource_not_found', + message: 'Comment attachment not found.', + }, } satisfies Record; diff --git a/packages/backend/server/src/base/error/errors.gen.ts b/packages/backend/server/src/base/error/errors.gen.ts index f7dae9ada3..e8394d76cd 100644 --- a/packages/backend/server/src/base/error/errors.gen.ts +++ b/packages/backend/server/src/base/error/errors.gen.ts @@ -1079,6 +1079,12 @@ export class ReplyNotFound extends UserFriendlyError { super('resource_not_found', 'reply_not_found', message); } } + +export class CommentAttachmentNotFound extends UserFriendlyError { + constructor(message?: string) { + super('resource_not_found', 'comment_attachment_not_found', message); + } +} export enum ErrorNames { INTERNAL_SERVER_ERROR, NETWORK_ERROR, @@ -1216,7 +1222,8 @@ export enum ErrorNames { INVALID_SEARCH_PROVIDER_REQUEST, INVALID_INDEXER_INPUT, COMMENT_NOT_FOUND, - REPLY_NOT_FOUND + REPLY_NOT_FOUND, + COMMENT_ATTACHMENT_NOT_FOUND } registerEnumType(ErrorNames, { name: 'ErrorNames' diff --git a/packages/backend/server/src/core/storage/__tests__/comment-attachment.spec.ts b/packages/backend/server/src/core/storage/__tests__/comment-attachment.spec.ts new file mode 100644 index 0000000000..bec800ae50 --- /dev/null +++ b/packages/backend/server/src/core/storage/__tests__/comment-attachment.spec.ts @@ -0,0 +1,141 @@ +import { randomUUID } from 'node:crypto'; +import { Readable } from 'node:stream'; + +import test from 'ava'; + +import { createModule } from '../../../__tests__/create-module'; +import { Mockers } from '../../../__tests__/mocks'; +import { Models } from '../../../models'; +import { CommentAttachmentStorage, StorageModule } from '..'; + +const module = await createModule({ + imports: [StorageModule], +}); +const storage = module.get(CommentAttachmentStorage); +const models = module.get(Models); + +test.before(async () => { + await storage.onConfigInit(); +}); + +test.after.always(async () => { + await module.close(); +}); + +test('should put comment attachment', async t => { + const workspace = await module.create(Mockers.Workspace); + const docId = randomUUID(); + const key = randomUUID(); + const blob = Buffer.from('test'); + + await storage.put(workspace.id, docId, key, 'test.txt', blob); + + const item = await models.commentAttachment.get(workspace.id, docId, key); + + t.truthy(item); + t.is(item?.workspaceId, workspace.id); + t.is(item?.docId, docId); + t.is(item?.key, key); + t.is(item?.mime, 'text/plain'); + t.is(item?.size, blob.length); + t.is(item?.name, 'test.txt'); +}); + +test('should get comment attachment', async t => { + const workspace = await module.create(Mockers.Workspace); + const docId = randomUUID(); + const key = randomUUID(); + const blob = Buffer.from('test'); + + await storage.put(workspace.id, docId, key, 'test.txt', blob); + + const item = await storage.get(workspace.id, docId, key); + + t.truthy(item); + t.is(item?.metadata?.contentType, 'text/plain'); + t.is(item?.metadata?.contentLength, blob.length); + // body is readable stream + t.truthy(item?.body); + const bytes = await readableToBytes(item?.body as Readable); + t.is(bytes.toString(), 'test'); +}); + +test('should get comment attachment with access url', async t => { + const workspace = await module.create(Mockers.Workspace); + const docId = randomUUID(); + const key = randomUUID(); + const blob = Buffer.from('test'); + + await storage.put(workspace.id, docId, key, 'test.txt', blob); + + const url = storage.getUrl(workspace.id, docId, key); + + t.truthy(url); + t.is( + url, + `http://localhost:3010/api/workspaces/${workspace.id}/docs/${docId}/comment-attachments/${key}` + ); +}); + +test('should delete comment attachment', async t => { + const workspace = await module.create(Mockers.Workspace); + const docId = randomUUID(); + const key = randomUUID(); + const blob = Buffer.from('test'); + + await storage.put(workspace.id, docId, key, 'test.txt', blob); + + await storage.delete(workspace.id, docId, key); + + const item = await models.commentAttachment.get(workspace.id, docId, key); + + t.is(item, null); +}); + +test('should handle comment.attachment.delete event', async t => { + const workspace = await module.create(Mockers.Workspace); + const docId = randomUUID(); + const key = randomUUID(); + const blob = Buffer.from('test'); + + await storage.put(workspace.id, docId, key, 'test.txt', blob); + + await storage.onCommentAttachmentDelete({ + workspaceId: workspace.id, + docId, + key, + }); + + const item = await models.commentAttachment.get(workspace.id, docId, key); + + t.is(item, null); +}); + +test('should handle workspace.deleted event', async t => { + const workspace = await module.create(Mockers.Workspace); + const docId = randomUUID(); + const key1 = randomUUID(); + const key2 = randomUUID(); + const blob1 = Buffer.from('test'); + const blob2 = Buffer.from('test2'); + + await storage.put(workspace.id, docId, key1, 'test.txt', blob1); + await storage.put(workspace.id, docId, key2, 'test.txt', blob2); + + const count = module.event.count('comment.attachment.delete'); + + await storage.onWorkspaceDeleted({ + id: workspace.id, + }); + + t.is(module.event.count('comment.attachment.delete'), count + 2); +}); + +async function readableToBytes(stream: Readable) { + const chunks: Buffer[] = []; + let chunk: Buffer; + for await (chunk of stream) { + chunks.push(chunk); + } + return Buffer.concat(chunks); +} diff --git a/packages/backend/server/src/core/storage/index.ts b/packages/backend/server/src/core/storage/index.ts index 0147e9260b..239f1e44ff 100644 --- a/packages/backend/server/src/core/storage/index.ts +++ b/packages/backend/server/src/core/storage/index.ts @@ -2,12 +2,16 @@ import './config'; import { Module } from '@nestjs/common'; -import { AvatarStorage, WorkspaceBlobStorage } from './wrappers'; +import { + AvatarStorage, + CommentAttachmentStorage, + WorkspaceBlobStorage, +} from './wrappers'; @Module({ - providers: [WorkspaceBlobStorage, AvatarStorage], - exports: [WorkspaceBlobStorage, AvatarStorage], + providers: [WorkspaceBlobStorage, AvatarStorage, CommentAttachmentStorage], + exports: [WorkspaceBlobStorage, AvatarStorage, CommentAttachmentStorage], }) export class StorageModule {} -export { AvatarStorage, WorkspaceBlobStorage }; +export { AvatarStorage, CommentAttachmentStorage, WorkspaceBlobStorage }; diff --git a/packages/backend/server/src/core/storage/wrappers/comment-attachment.ts b/packages/backend/server/src/core/storage/wrappers/comment-attachment.ts new file mode 100644 index 0000000000..49db3eec6e --- /dev/null +++ b/packages/backend/server/src/core/storage/wrappers/comment-attachment.ts @@ -0,0 +1,128 @@ +import { Injectable, Logger } from '@nestjs/common'; + +import { + autoMetadata, + Config, + EventBus, + OnEvent, + type StorageProvider, + StorageProviderFactory, + URLHelper, +} from '../../../base'; +import { Models } from '../../../models'; + +declare global { + interface Events { + 'comment.attachment.delete': { + workspaceId: string; + docId: string; + key: string; + }; + } +} + +@Injectable() +export class CommentAttachmentStorage { + private readonly logger = new Logger(CommentAttachmentStorage.name); + private provider!: StorageProvider; + + get config() { + return this.AFFiNEConfig.storages.blob; + } + + constructor( + private readonly AFFiNEConfig: Config, + private readonly event: EventBus, + private readonly storageFactory: StorageProviderFactory, + private readonly models: Models, + private readonly url: URLHelper + ) {} + + @OnEvent('config.init') + async onConfigInit() { + this.provider = this.storageFactory.create(this.config.storage); + } + + @OnEvent('config.changed') + async onConfigChanged(event: Events['config.changed']) { + if (event.updates.storages?.blob?.storage) { + this.provider = this.storageFactory.create(this.config.storage); + } + } + + private storageKey(workspaceId: string, docId: string, key: string) { + return `comment-attachments/${workspaceId}/${docId}/${key}`; + } + + async put( + workspaceId: string, + docId: string, + key: string, + name: string, + blob: Buffer + ) { + const meta = autoMetadata(blob); + + await this.provider.put( + this.storageKey(workspaceId, docId, key), + blob, + meta + ); + await this.models.commentAttachment.upsert({ + workspaceId, + docId, + key, + name, + mime: meta.contentType ?? 'application/octet-stream', + size: blob.length, + }); + } + + async get( + workspaceId: string, + docId: string, + key: string, + signedUrl?: boolean + ) { + return await this.provider.get( + this.storageKey(workspaceId, docId, key), + signedUrl + ); + } + + async delete(workspaceId: string, docId: string, key: string) { + await this.provider.delete(this.storageKey(workspaceId, docId, key)); + await this.models.commentAttachment.delete(workspaceId, docId, key); + this.logger.log( + `deleted comment attachment ${workspaceId}/${docId}/${key}` + ); + } + + getUrl(workspaceId: string, docId: string, key: string) { + return this.url.link( + `/api/workspaces/${workspaceId}/docs/${docId}/comment-attachments/${key}` + ); + } + + @OnEvent('workspace.deleted') + async onWorkspaceDeleted({ id }: Events['workspace.deleted']) { + const attachments = await this.models.commentAttachment.list(id); + + for (const attachment of attachments) { + this.event.emit('comment.attachment.delete', { + workspaceId: id, + docId: attachment.docId, + key: attachment.key, + }); + } + } + + @OnEvent('comment.attachment.delete') + async onCommentAttachmentDelete({ + workspaceId, + docId, + key, + }: Events['comment.attachment.delete']) { + await this.delete(workspaceId, docId, key); + } +} diff --git a/packages/backend/server/src/core/storage/wrappers/index.ts b/packages/backend/server/src/core/storage/wrappers/index.ts index 250e9a8cc3..d49ec9fa1e 100644 --- a/packages/backend/server/src/core/storage/wrappers/index.ts +++ b/packages/backend/server/src/core/storage/wrappers/index.ts @@ -1,2 +1,3 @@ export { AvatarStorage } from './avatar'; export { WorkspaceBlobStorage } from './blob'; +export { CommentAttachmentStorage } from './comment-attachment'; diff --git a/packages/backend/server/src/core/workspaces/controller.ts b/packages/backend/server/src/core/workspaces/controller.ts index de25226503..9c6c0a944b 100644 --- a/packages/backend/server/src/core/workspaces/controller.ts +++ b/packages/backend/server/src/core/workspaces/controller.ts @@ -4,6 +4,7 @@ import type { Response } from 'express'; import { BlobNotFound, CallMetric, + CommentAttachmentNotFound, DocHistoryNotFound, DocNotFound, InvalidHistoryTimestamp, @@ -13,7 +14,7 @@ import { CurrentUser, Public } from '../auth'; import { PgWorkspaceDocStorageAdapter } from '../doc'; import { DocReader } from '../doc/reader'; import { AccessController } from '../permission'; -import { WorkspaceBlobStorage } from '../storage'; +import { CommentAttachmentStorage, WorkspaceBlobStorage } from '../storage'; import { DocID } from '../utils/doc'; @Controller('/api/workspaces') @@ -21,6 +22,7 @@ export class WorkspacesController { logger = new Logger(WorkspacesController.name); constructor( private readonly storage: WorkspaceBlobStorage, + private readonly commentAttachmentStorage: CommentAttachmentStorage, private readonly ac: AccessController, private readonly workspace: PgWorkspaceDocStorageAdapter, private readonly docReader: DocReader, @@ -180,4 +182,41 @@ export class WorkspacesController { }); } } + + @Get('/:id/docs/:docId/comment-attachments/:key') + @CallMetric('controllers', 'workspace_get_comment_attachment') + async commentAttachment( + @CurrentUser() user: CurrentUser, + @Param('id') workspaceId: string, + @Param('docId') docId: string, + @Param('key') key: string, + @Res() res: Response + ) { + await this.ac.user(user.id).doc(workspaceId, docId).assert('Doc.Read'); + + const { body, metadata, redirectUrl } = + await this.commentAttachmentStorage.get(workspaceId, docId, key); + + if (redirectUrl) { + return res.redirect(redirectUrl); + } + + if (!body) { + throw new CommentAttachmentNotFound(); + } + + // metadata should always exists if body is not null + if (metadata) { + res.setHeader('content-type', metadata.contentType); + res.setHeader('last-modified', metadata.lastModified.toUTCString()); + res.setHeader('content-length', metadata.contentLength); + } else { + this.logger.warn( + `Comment attachment ${workspaceId}/${docId}/${key} has no metadata` + ); + } + + res.setHeader('cache-control', 'private, max-age=2592000, immutable'); + body.pipe(res); + } } diff --git a/packages/backend/server/src/schema.gql b/packages/backend/server/src/schema.gql index d6fa23f098..935fcf44aa 100644 --- a/packages/backend/server/src/schema.gql +++ b/packages/backend/server/src/schema.gql @@ -540,6 +540,7 @@ enum ErrorNames { CAN_NOT_BATCH_GRANT_DOC_OWNER_PERMISSIONS CAN_NOT_REVOKE_YOURSELF CAPTCHA_VERIFICATION_FAILED + COMMENT_ATTACHMENT_NOT_FOUND COMMENT_NOT_FOUND COPILOT_ACTION_TAKEN COPILOT_CONTEXT_FILE_NOT_SUPPORTED diff --git a/packages/common/graphql/src/schema.ts b/packages/common/graphql/src/schema.ts index 0f420393cd..88d5342fc7 100644 --- a/packages/common/graphql/src/schema.ts +++ b/packages/common/graphql/src/schema.ts @@ -709,6 +709,7 @@ export enum ErrorNames { CAN_NOT_BATCH_GRANT_DOC_OWNER_PERMISSIONS = 'CAN_NOT_BATCH_GRANT_DOC_OWNER_PERMISSIONS', CAN_NOT_REVOKE_YOURSELF = 'CAN_NOT_REVOKE_YOURSELF', CAPTCHA_VERIFICATION_FAILED = 'CAPTCHA_VERIFICATION_FAILED', + COMMENT_ATTACHMENT_NOT_FOUND = 'COMMENT_ATTACHMENT_NOT_FOUND', COMMENT_NOT_FOUND = 'COMMENT_NOT_FOUND', COPILOT_ACTION_TAKEN = 'COPILOT_ACTION_TAKEN', COPILOT_CONTEXT_FILE_NOT_SUPPORTED = 'COPILOT_CONTEXT_FILE_NOT_SUPPORTED', diff --git a/packages/frontend/i18n/src/i18n.gen.ts b/packages/frontend/i18n/src/i18n.gen.ts index 45916230ab..d5bedfc89a 100644 --- a/packages/frontend/i18n/src/i18n.gen.ts +++ b/packages/frontend/i18n/src/i18n.gen.ts @@ -8891,6 +8891,10 @@ export function useAFFiNEI18N(): { * `Reply not found.` */ ["error.REPLY_NOT_FOUND"](): string; + /** + * `Comment attachment not found.` + */ + ["error.COMMENT_ATTACHMENT_NOT_FOUND"](): string; } { const { t } = useTranslation(); return useMemo(() => createProxy((key) => t.bind(null, key)), [t]); } function createComponent(i18nKey: string) { return (props) => createElement(Trans, { i18nKey, shouldUnescape: true, ...props }); diff --git a/packages/frontend/i18n/src/resources/en.json b/packages/frontend/i18n/src/resources/en.json index ab7d1b9b52..f47a182c5a 100644 --- a/packages/frontend/i18n/src/resources/en.json +++ b/packages/frontend/i18n/src/resources/en.json @@ -2195,5 +2195,6 @@ "error.INVALID_SEARCH_PROVIDER_REQUEST": "Invalid request argument to search provider: {{reason}}", "error.INVALID_INDEXER_INPUT": "Invalid indexer input: {{reason}}", "error.COMMENT_NOT_FOUND": "Comment not found.", - "error.REPLY_NOT_FOUND": "Reply not found." + "error.REPLY_NOT_FOUND": "Reply not found.", + "error.COMMENT_ATTACHMENT_NOT_FOUND": "Comment attachment not found." }