From 5ece18abd0a1026fa742e15a7480010619156051 Mon Sep 17 00:00:00 2001 From: fengmk2 Date: Tue, 7 Aug 2018 14:17:03 +0800 Subject: [PATCH] feat: getFileStream() can accept non file request (#17) ```js const stream = await ctx.getFileStream({ required: false }); if (stream.filename) { console.log('uploaded ' + stream.filename); } else { console.log('file not exists'); } ``` closes https://github.com/eggjs/egg/issues/2178 --- .travis.yml | 2 +- LICENSE | 2 +- README.md | 28 +++++++++++++ app/extend/context.js | 27 ++++++++++--- appveyor.yml | 2 +- package.json | 16 ++++---- .../upload-one-file/app/controller/async.js | 21 ++++++++++ .../apps/upload-one-file/app/router.js | 2 + test/multipart.test.js | 39 +++++++++++++------ 9 files changed, 112 insertions(+), 27 deletions(-) diff --git a/.travis.yml b/.travis.yml index 960bc57..ce21122 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,7 @@ sudo: false language: node_js node_js: - '8' - - '9' + - '10' install: - npm i npminstall && npminstall script: diff --git a/LICENSE b/LICENSE index 5b46802..7295685 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,6 @@ MIT License -Copyright (c) 2017 Alibaba Group Holding Limited and other contributors. +Copyright (c) 2017-present Alibaba Group Holding Limited and other contributors. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal diff --git a/README.md b/README.md index c23f3ae..88bdcd9 100644 --- a/README.md +++ b/README.md @@ -132,6 +132,7 @@ const Controller = require('egg').Controller; module.exports = Class UploadController extends Controller { async upload() { const ctx = this.ctx; + // file not exists will response 400 error const stream = await ctx.getFileStream(); const name = 'egg-multipart-test/' + path.basename(stream.filename); let result; @@ -150,6 +151,33 @@ module.exports = Class UploadController extends Controller { fields: stream.fields, }; } + + async uploadNotRequiredFile() { + const ctx = this.ctx; + // file not required + const stream = await ctx.getFileStream({ requireFile: false }); + let result; + if (stream.filename) { + const name = 'egg-multipart-test/' + path.basename(stream.filename); + try { + // process file or upload to cloud storage + result = await ctx.oss.put(name, stream); + } catch (err) { + // must consume the stream, otherwise browser will be stuck. + await sendToWormhole(stream); + throw err; + } + } else { + // must consume the empty stream + await sendToWormhole(stream); + } + + ctx.body = { + url: result && result.url, + // process form fields by `stream.fields` + fields: stream.fields, + }; + } }; ``` diff --git a/app/extend/context.js b/app/extend/context.js index 9ac241b..8998111 100644 --- a/app/extend/context.js +++ b/app/extend/context.js @@ -1,6 +1,13 @@ 'use strict'; const parse = require('co-busboy'); +const Readable = require('stream').Readable; + +class EmptyStream extends Readable { + _read() { + this.push(null); + } +} module.exports = { /** @@ -28,15 +35,25 @@ module.exports = { * console.log(stream.fields); * ``` * @method Context#getFileStream + * @param {Object} options + * - {Boolean} options.requireFile - required file submit, default is true * @return {ReadStream} stream * @since 1.0.0 */ - async getFileStream() { + async getFileStream(options) { + options = options || {}; const parts = this.multipart({ autoFields: true }); - const stream = await parts(); - // stream not exists, treat as an exception - if (!stream || !stream.filename) { - this.throw(400, 'Can\'t found upload file'); + let stream = await parts(); + + if (options.requireFile !== false) { + // stream not exists, treat as an exception + if (!stream || !stream.filename) { + this.throw(400, 'Can\'t found upload file'); + } + } + + if (!stream) { + stream = new EmptyStream(); } stream.fields = parts.field; stream.once('limit', () => { diff --git a/appveyor.yml b/appveyor.yml index d0aa47e..981e82b 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,7 +1,7 @@ environment: matrix: - nodejs_version: '8' - - nodejs_version: '9' + - nodejs_version: '10' install: - ps: Install-Product node $env:nodejs_version diff --git a/package.json b/package.json index 09101cf..054c2f6 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "app.js" ], "ci": { - "version": "8, 9", + "version": "8, 10", "license": true }, "dependencies": { @@ -48,20 +48,20 @@ "humanize-bytes": "^1.0.1" }, "devDependencies": { - "autod": "^2.10.1", - "egg": "next", - "egg-bin": "^4.3.5", + "autod": "^3.0.1", + "egg": "^2.9.1", + "egg-bin": "^4.8.1", "egg-ci": "^1.8.0", - "egg-mock": "^3.13.1", - "eslint": "^4.10.0", - "eslint-config-egg": "^5.1.1", + "egg-mock": "^3.18.0", + "eslint": "^5.2.0", + "eslint-config-egg": "^7.0.0", "formstream": "^1.1.0", "is-type-of": "^1.0.0", "mkdirp": "^0.5.1", "mz": "^2.7.0", "stream-wormhole": "^1.0.3", "supertest": "^3.0.0", - "urllib": "^2.25.1", + "urllib": "^2.29.1", "webstorm-disable-index": "^1.2.0" } } diff --git a/test/fixtures/apps/upload-one-file/app/controller/async.js b/test/fixtures/apps/upload-one-file/app/controller/async.js index 659e0f3..13dec9c 100644 --- a/test/fixtures/apps/upload-one-file/app/controller/async.js +++ b/test/fixtures/apps/upload-one-file/app/controller/async.js @@ -17,5 +17,26 @@ module.exports = app => { fields: stream.fields, }; } + + async allowEmpty() { + const ctx = this.ctx; + const stream = await ctx.getFileStream({ requireFile: false }); + if (stream.filename) { + const name = 'egg-multipart-test/' + process.version + '-' + Date.now() + '-' + path.basename(stream.filename); + const result = await ctx.oss.put(name, stream); + ctx.body = { + name: result.name, + url: result.url, + status: result.res.status, + fields: stream.fields, + }; + return; + } + + stream.resume(); + ctx.body = { + fields: stream.fields, + }; + } }; }; diff --git a/test/fixtures/apps/upload-one-file/app/router.js b/test/fixtures/apps/upload-one-file/app/router.js index 8222ef0..2f652a3 100644 --- a/test/fixtures/apps/upload-one-file/app/router.js +++ b/test/fixtures/apps/upload-one-file/app/router.js @@ -53,4 +53,6 @@ module.exports = app => { }); app.post('/upload/async', 'async.async'); + + app.post('/upload/allowEmpty', 'async.allowEmpty'); }; diff --git a/test/multipart.test.js b/test/multipart.test.js index 8f28d3a..c1fd143 100644 --- a/test/multipart.test.js +++ b/test/multipart.test.js @@ -420,6 +420,29 @@ describe('test/multipart.test.js', () => { assert(res.data.toString().includes('Can\'t found upload file')); }); + it('should no file upload and only fields', function* () { + const form = formstream(); + form.field('hi', 'ok'); + form.field('hi2', 'ok2'); + + const headers = form.headers(); + const url = host + '/upload/allowEmpty'; + const res = yield urllib.request(url, { + method: 'POST', + headers, + stream: form, + dataType: 'json', + }); + + assert(res.status === 200); + assert.deepEqual(res.data, { + fields: { + hi: 'ok', + hi2: 'ok2', + }, + }); + }); + it('should 400 when no file speicified', function* () { const form = formstream(); form.buffer('file', new Buffer(''), '', 'application/octet-stream'); @@ -480,9 +503,7 @@ describe('test/multipart.test.js', () => { assert(res.status === 413); assert(data.message === 'Request file too large'); - const coreLogPath = path.join(app.baseDir, 'logs/oss/egg-web.log'); - const content = yield fs.readFile(coreLogPath, 'utf8'); - assert(content.includes('nodejs.MultipartFileTooLargeError: Request file too large')); + app.expectLog('nodejs.MultipartFileTooLargeError: Request file too large', 'coreLogger'); }); it('should ignore error when stream not handle error event', function* () { @@ -503,10 +524,8 @@ describe('test/multipart.test.js', () => { assert(res.status === 200); assert(data.url); - const coreLogPath = path.join(app.baseDir, 'logs/oss/common-error.log'); - const content = yield fs.readFile(coreLogPath, 'utf8'); - assert(content.includes('nodejs.MultipartFileTooLargeError: Request file too large')); - assert(content.includes("filename: 'not-handle-error-event.js'")); + app.expectLog('nodejs.MultipartFileTooLargeError: Request file too large', 'errorLogger'); + app.expectLog(/filename: ['"]not-handle-error-event.js['"]/, 'errorLogger'); }); it('should ignore stream next errors after limit event fire', function* () { @@ -527,10 +546,8 @@ describe('test/multipart.test.js', () => { assert(res.status === 200); assert(data.url); - const coreLogPath = path.join(app.baseDir, 'logs/oss/common-error.log'); - const content = yield fs.readFile(coreLogPath, 'utf8'); - assert(content.includes('nodejs.MultipartFileTooLargeError: Request file too large')); - assert(content.includes("filename: 'not-handle-error-event-and-mock-stream-error.js'")); + app.expectLog('nodejs.MultipartFileTooLargeError: Request file too large', 'errorLogger'); + app.expectLog(/filename: ['"]not-handle-error-event-and-mock-stream-error.js['"]/, 'errorLogger'); }); }); });