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

imprv: Upload handler use apiv3 post #8279

Merged
merged 24 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
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
19 changes: 5 additions & 14 deletions apps/app/src/components/PageEditor/PageEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { useRouter } from 'next/router';
import { throttle, debounce } from 'throttle-debounce';

import { useUpdateStateAfterSave, useSaveOrUpdate } from '~/client/services/page-operation';
import { apiGet, apiPostForm } from '~/client/util/apiv1-client';
import { apiv3Get, apiv3PostForm } from '~/client/util/apiv3-client';
import { toastError, toastSuccess } from '~/client/util/toastr';
import { OptionsToSave } from '~/interfaces/page-operation';
import { SocketEventName } from '~/interfaces/websocket';
Expand Down Expand Up @@ -309,10 +309,7 @@ export const PageEditor = React.memo((props: Props): JSX.Element => {
const uploadHandler = useCallback((files: File[]) => {
files.forEach(async(file) => {
try {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const resLimit: any = await apiGet('/attachments.limit', {
fileSize: file.size,
});
const { data: resLimit }: any = await apiv3Get('/attachment/limit', { fileSize: file.size });
jam411 marked this conversation as resolved.
Show resolved Hide resolved

if (!resLimit.isUploadable) {
throw new Error(resLimit.errorMessage);
Expand All @@ -330,7 +327,8 @@ export const PageEditor = React.memo((props: Props): JSX.Element => {
formData.append('page_body', codeMirrorEditor?.getDoc() ?? '');
}

const resAdd: any = await apiPostForm('/attachments.add', formData);
const { data: resAdd }: any = await apiv3PostForm('/attachment/add', formData);

const attachment = resAdd.attachment;
const fileName = attachment.originalName;

Expand All @@ -340,20 +338,13 @@ export const PageEditor = React.memo((props: Props): JSX.Element => {
// modify to "![fileName](url)" syntax
insertText = `!${insertText}`;
}
// TODO: implement
// refs: https://redmine.weseek.co.jp/issues/126528
// editorRef.current.insertText(insertText);

codeMirrorEditor?.insertText(insertText);
}
catch (e) {
logger.error('failed to upload', e);
toastError(e);
}
finally {
// TODO: implement
// refs: https://redmine.weseek.co.jp/issues/126528
// editorRef.current.terminateUploadingState();
}
});

}, [codeMirrorEditor, currentPagePath, pageId]);
Expand Down
230 changes: 230 additions & 0 deletions apps/app/src/server/routes/apiv3/attachment.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,24 @@
import { ErrorV3 } from '@growi/core/dist/models';
import multer from 'multer';

import { SupportedAction } from '~/interfaces/activity';
import { AttachmentType } from '~/server/interfaces/attachment';
import { Attachment } from '~/server/models';
import loggerFactory from '~/utils/logger';

import { generateAddActivityMiddleware } from '../../middlewares/add-activity';
import { apiV3FormValidator } from '../../middlewares/apiv3-form-validator';
import { certifySharedPageAttachmentMiddleware } from '../../middlewares/certify-shared-page-attachment';


const logger = loggerFactory('growi:routes:apiv3:attachment'); // eslint-disable-line no-unused-vars
const express = require('express');

const router = express.Router();
const { query, param } = require('express-validator');

const { serializePageSecurely } = require('../../models/serializers/page-serializer');
const { serializeRevisionSecurely } = require('../../models/serializers/revision-serializer');
const { serializeUserSecurely } = require('../../models/serializers/user-serializer');

/**
Expand All @@ -20,11 +27,74 @@ const { serializeUserSecurely } = require('../../models/serializers/user-seriali
* name: Attachment
*/


/**
* @swagger
*
* components:
* schemas:
* Attachment:
* description: Attachment
* type: object
* properties:
* _id:
* type: string
* description: attachment ID
* example: 5e0734e072560e001761fa67
* __v:
* type: number
* description: attachment version
* example: 0
* fileFormat:
* type: string
* description: file format in MIME
* example: text/plain
* fileName:
* type: string
* description: file name
* example: 601b7c59d43a042c0117e08dd37aad0aimage.txt
* originalName:
* type: string
* description: original file name
* example: file.txt
* creator:
* $ref: '#/components/schemas/User'
* page:
* type: string
* description: page ID attached at
* example: 5e07345972560e001761fa63
* createdAt:
* type: string
* description: date created at
* example: 2010-01-01T00:00:00.000Z
* fileSize:
* type: number
* description: file size
* example: 3494332
* url:
* type: string
* description: attachment URL
* example: http://localhost/files/5e0734e072560e001761fa67
* filePathProxied:
* type: string
* description: file path proxied
* example: "/attachment/5e0734e072560e001761fa67"
* downloadPathProxied:
* type: string
* description: download path proxied
* example: "/download/5e0734e072560e001761fa67"
*/

module.exports = (crowi) => {
const accessTokenParser = require('../../middlewares/access-token-parser')(crowi);
const loginRequired = require('../../middlewares/login-required')(crowi, true);
const Page = crowi.model('Page');
const User = crowi.model('User');
const { attachmentService } = crowi;
const uploads = multer({ dest: `${crowi.tmpDir}uploads` });
const addActivity = generateAddActivityMiddleware(crowi);

const activityEvent = crowi.event('activity');

const validator = {
retrieveAttachment: [
Expand Down Expand Up @@ -95,6 +165,166 @@ module.exports = (crowi) => {
}
});


/**
* @swagger
*
* /attachment/limit:
* get:
* tags: [Attachment]
* operationId: getAttachmentLimit
* summary: /attachment/limit
* description: Get available capacity of uploaded file with GridFS
* parameters:
* - in: query
* name: fileSize
* schema:
* type: number
* description: file size
* example: 23175
* required: true
* responses:
* 200:
* description: Succeeded to get available capacity of uploaded file with GridFS.
* content:
* application/json:
* schema:
* properties:
* isUploadable:
* type: boolean
* description: uploadable
* example: true
* 403:
* $ref: '#/components/responses/403'
* 500:
* $ref: '#/components/responses/500'
*/
/**
* @api {get} /attachment/limit get available capacity of uploaded file with GridFS
* @apiName AddAttachment
* @apiGroup Attachment
*/
router.get('/limit', accessTokenParser, loginRequired, apiV3FormValidator, async(req, res) => {
const { fileUploadService } = crowi;
const fileSize = Number(req.query.fileSize);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Number() で囲っているけれど、validator.isNumeric() で検証しているので不要だと思う。

/attachment/limit を使っている個所は一か所しかなく number 型が入ってくる

accessToken を使ったときに対応しているのか?
ただ、その場合でも isNumeric() により検証できるようになるはず。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

クエリパラメータはテキストで渡るので number 型へのキャストが必要

return res.apiv3(await fileUploadService.checkLimit(fileSize));
jam411 marked this conversation as resolved.
Show resolved Hide resolved
});
jam411 marked this conversation as resolved.
Show resolved Hide resolved

/**
* @swagger
*
* /attachment/add:
* post:
* tags: [Attachment, CrowiCompatibles]
* operationId: addAttachment
* summary: /attachment/add
* description: Add attachment to the page
* requestBody:
* content:
* "multipart/form-data":
* schema:
* properties:
* page_id:
* nullable: true
* type: string
* path:
* nullable: true
* type: string
* file:
* type: string
* format: binary
* description: attachment data
* encoding:
* path:
* contentType: application/x-www-form-urlencoded
* "*\/*":
* schema:
* properties:
* page_id:
* nullable: true
* type: string
* path:
* nullable: true
* type: string
* file:
* type: string
* format: binary
* description: attachment data
* encoding:
* path:
* contentType: application/x-www-form-urlencoded
* responses:
* 200:
* description: Succeeded to add attachment.
* content:
* application/json:
* schema:
* properties:
* page:
* $ref: '#/components/schemas/Page'
* attachment:
* $ref: '#/components/schemas/Attachment'
* url:
* $ref: '#/components/schemas/Attachment/properties/url'
* pageCreated:
* type: boolean
* description: whether the page was created
* example: false
* 403:
* $ref: '#/components/responses/403'
* 500:
* $ref: '#/components/responses/500'
*/
/**
* @api {post} /attachment/add Add attachment to the page
* @apiName AddAttachment
* @apiGroup Attachment
*
* @apiParam {String} page_id
* @apiParam {String} path
* @apiParam {File} file
*/
router.post('/add', uploads.single('file'), accessTokenParser, loginRequired, apiV3FormValidator, addActivity, async(req, res) => {
const pageId = req.body.page_id || null;
const pagePath = req.body.path || null;

// check params
if (req.file == null) {
return res.apiv3Err('File error.');
}
if (pageId == null && pagePath == null) {
return res.apiv3Err('Either page_id or path is required.');
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同じくバリデーションを作ってください。そうすればこのあたりの処理もいらなくなる?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

req 直下のバリデーションが無理そうだったので req.file は残しました。
また、pagePath はあったとしても使わず、pageId == null, pagePath == なにかという状況ではファイルのアップロードができないことからそもそも pagePath を入れるのをやめました。


const file = req.file;

try {
const page = await Page.findById(pageId);

// check the user is accessible
const isAccessible = await Page.isAccessiblePageByViewer(page.id, req.user);
if (!isAccessible) {
return res.apiv3Err(`Forbidden to access to the page '${page.id}'`);
}

const attachment = await attachmentService.createAttachment(file, req.user, pageId, AttachmentType.WIKI_PAGE);

const result = {
page: serializePageSecurely(page),
revision: serializeRevisionSecurely(page.revision),
attachment: attachment.toObject({ virtuals: true }),
};

activityEvent.emit('update', res.locals.activity._id, { action: SupportedAction.ACTION_ATTACHMENT_ADD });

res.apiv3(result);
}
catch (err) {
logger.error(err);
return res.apiv3Err(err.message);
}
});

/**
* @swagger
*
Expand Down
Loading
Loading